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
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.
We probably would not care about the first difference, but it is
easy enough to guard against, I think. Perhaps with this patch?
-- >8 --
parsing remotes: forbid "remote./foo.variable" and fix segfault
Historically we did not pay attention to a remote shorthand
defined in the config file whose name starts with a slash
(because we always took such a string as a literal localfile
URL), but the new organization of the parsing code makes config
always take precedence. It does not make much sense to define
such a remote shorthand, so protect ourselves against it.
Also, the code forgot that a config variable could be spelled
without a value to denote a boolean set to true, in which case
the config parser passes a NULL in value parameter, and run
xstrdup() on it without checking. Fix this.
Signed-off-by: Junio C Hamano <junkio@xxxxxxx>
---
remote.c | 17 +++++++++++++++++
1 files changed, 17 insertions(+), 0 deletions(-)
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);
remote = make_remote(name, subkey - name);
+ if (!value) {
+ /* if we ever have a boolean variable, e.g. "remote.*.disabled"
+ * [remote "frotz"]
+ * disabled
+ * is a valid way to set it to true; we get NULL in value so
+ * we need to handle it here.
+ *
+ * if (!strcmp(subkey, ".disabled")) {
+ * val = git_config_bool(key, value);
+ * return 0;
+ * } else
+ *
+ */
+ return error("Config with no value for remote %s", name);
+ }
if (!strcmp(subkey, ".url")) {
add_uri(remote, xstrdup(value));
} else if (!strcmp(subkey, ".push")) {
-
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
