On Sat, 12 May 2007, Junio C Hamano wrote:
> Daniel Barkalow <barkalow@xxxxxxxxxxxx> writes:
>
> > This series is the same as the previous version, except that it matches
> > the current behavior of builtin-push with respect to treating names as
> > literal URIs.
>
> Thanks.
>
> diff --git a/remote.c b/remote.c
> index 32a0acf..1dd2e77 100644
> --- a/remote.c
> +++ b/remote.c
> @@ -189,12 +189,14 @@ struct remote *remote_get(const char *name)
> if (!name)
> name = default_remote_name;
> ret = make_remote(name, 0);
> - if (*name == '/')
> - add_uri(ret, name);
> - if (!ret->uri)
> - read_remotes_file(ret);
> + if (name[0] != '/') {
> + if (!ret->uri)
> + read_remotes_file(ret);
> + if (!ret->uri)
> + read_branches_file(ret);
> + }
> if (!ret->uri)
> - read_branches_file(ret);
> + add_uri(ret, name);
> if (!ret->uri)
> return NULL;
> return ret;
>
> This is more similar to the original from builtin-push.c than
> your previous round, but it is still not identical.
>
> The differences should not matter in real life, but I think we
> need to make it clear what the differences are to warn users.
> Here is my reading of the change (please correct me).
>
> Earlier.
>
> - A name that does not begin with a slash could be a remote
> shorthand. Check remotes, config and branches in this order
> and stop once a match is found.
>
> - Otherwise use the name as a literal URI.
>
> This patch.
>
> - Config always wins.
>
> - A name that does not begin with a slash could be found in
> remotes or branches; check them in this order.
>
> - Otherwise use it as is.
>
> Theoretically people _could_ have had a config like
>
> [remote "/pub"]
> url = blah
>
> but it would never have matched. This ``broken'' config file
> suddenly start to interfere when somebody does:
>
> $ git push /pub
This is true, and I missed it before. Feel free to mention it in the
appropriate commit message.
> Also people may have had a remotes and config of the same name,
> and currently what is defined in config is ignored, but with the
> new code, config takes precedence. Which is unarguably good,
> but still a change I should remember to write down in the
> release notes, hence prefer to have it clearly described in the
> commit log message.
This is in the message for [1/3] already, along with the other change: it
will use the current branch's remote, if there is one, instead of "origin"
if no repository is given on the command line.
I was only claiming here (incorrectly, it turns out) that the configured
remote vs. literal URI behavior is the same with this series.
> We probably would not care about the first difference, but it is
> easy enough to guard against, I think. Perhaps with this patch?
>
> -- >8 --
> diff --git a/remote.c b/remote.c
> index 1dd2e77..05df196 100644
> --- a/remote.c
> +++ b/remote.c
> @@ -149,7 +149,24 @@ static int handle_config(const char *key, const char *value)
> subkey = strrchr(name, '.');
> if (!subkey)
> return error("Config with no key for remote %s", name);
> + if (*subkey == '/')
> + return error("Config remote shorthand cannot begin with '/': %s", name);
Maybe just return? If we change the behavior to give an error in this
situation, we might as well make the config file actually take effect
instead.
-Daniel
*This .sig left intentionally blank*
-
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
