remote rename: more carefully determine whether a remote is configured

One of the really nice features of the ~/.gitconfig file is that users can override defaults by their own preferred settings for all of their repositories. One such default that some users like to override is whether the "origin" remote gets auto-pruned or not. The user would simply call git config --global remote.origin.prune true and from now on all "origin" remotes would be pruned automatically when fetching into the local repository. There is just one catch: now Git thinks that the "origin" remote is configured, even if the repository config has no [remote "origin"] section at all, as it does not realize that the "prune" setting was configured globally and that there really is no "origin" remote configured in this repository. That is a problem e.g. when renaming a remote to a new name, when Git may be fooled into thinking that there is already a remote of that new name. Let's fix this by paying more attention to *where* the remote settings came from: if they are configured in the local repository config, we must not overwrite them. If they were configured elsewhere, we cannot overwrite them to begin with, as we only write the repository config. There is only one caller of remote_is_configured() (in `git fetch`) that may want to take remotes into account even if they were configured outside the repository config; all other callers essentially try to prevent the Git command from overwriting settings in the repository config. To accommodate that fact, the remote_is_configured() function now requires a parameter that states whether the caller is interested in all remotes, or only in those that were configured in the repository config. Many thanks to Jeff King whose tireless review helped with settling for nothing less than the current strategy. This fixes https://github.com/git-for-windows/git/issues/888 Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de> Signed-off-by: Junio C Hamano <gitster@pobox.com>

Johannes Schindelin committed Jan 19, 2017 at 22:20 UTC e459b073fb3ab8abe36e6bee5c5d6be1ed3928ae
5 files changed +21 -13
builtin/fetch.c
+1 -1
@@ -1145,7 +1145,7 @@ static int add_remote_or_group(const char *name, struct string_list *list)
1145 git_config(get_remote_group, &g);
1146 if (list->nr == prev_nr) {
1147 struct remote *remote = remote_get(name);
1148 - if (!remote_is_configured(remote))
1148 + if (!remote_is_configured(remote, 0))
1149 return 0;
1150 string_list_append(list, remote->name);
1151 }
builtin/remote.c
+7 -7
@@ -186,7 +186,7 @@ static int add(int argc, const char **argv)
186 url = argv[1];
187
188 remote = remote_get(name);
189 - if (remote_is_configured(remote))
189 + if (remote_is_configured(remote, 1))
190 die(_("remote %s already exists."), name);
191
192 strbuf_addf(&buf2, "refs/heads/test:refs/remotes/%s/test", name);
@@ -618,14 +618,14 @@ static int mv(int argc, const char **argv)
618 rename.remote_branches = &remote_branches;
619
620 oldremote = remote_get(rename.old);
621 - if (!remote_is_configured(oldremote))
621 + if (!remote_is_configured(oldremote, 1))
622 die(_("No such remote: %s"), rename.old);
623
624 if (!strcmp(rename.old, rename.new) && oldremote->origin != REMOTE_CONFIG)
625 return migrate_file(oldremote);
626
627 newremote = remote_get(rename.new);
628 - if (remote_is_configured(newremote))
628 + if (remote_is_configured(newremote, 1))
629 die(_("remote %s already exists."), rename.new);
630
631 strbuf_addf(&buf, "refs/heads/test:refs/remotes/%s/test", rename.new);
@@ -753,7 +753,7 @@ static int rm(int argc, const char **argv)
753 usage_with_options(builtin_remote_rm_usage, options);
754
755 remote = remote_get(argv[1]);
756 - if (!remote_is_configured(remote))
756 + if (!remote_is_configured(remote, 1))
757 die(_("No such remote: %s"), argv[1]);
758
759 known_remotes.to_delete = remote;
@@ -1416,7 +1416,7 @@ static int set_remote_branches(const char *remotename, const char **branches,
1416 strbuf_addf(&key, "remote.%s.fetch", remotename);
1417
1418 remote = remote_get(remotename);
1419 - if (!remote_is_configured(remote))
1419 + if (!remote_is_configured(remote, 1))
1420 die(_("No such remote '%s'"), remotename);
1421
1422 if (!add_mode && remove_all_fetch_refspecs(remotename, key.buf)) {
@@ -1470,7 +1470,7 @@ static int get_url(int argc, const char **argv)
1470 remotename = argv[0];
1471
1472 remote = remote_get(remotename);
1473 - if (!remote_is_configured(remote))
1473 + if (!remote_is_configured(remote, 1))
1474 die(_("No such remote '%s'"), remotename);
1475
1476 url_nr = 0;
@@ -1538,7 +1538,7 @@ static int set_url(int argc, const char **argv)
1538 oldurl = newurl;
1539
1540 remote = remote_get(remotename);
1541 - if (!remote_is_configured(remote))
1541 + if (!remote_is_configured(remote, 1))
1542 die(_("No such remote '%s'"), remotename);
1543
1544 if (push_mode) {
remote.c
+10 -2
@@ -255,6 +255,7 @@ static void read_remotes_file(struct remote *remote)
255
256 if (!f)
257 return;
258 + remote->configured_in_repo = 1;
259 remote->origin = REMOTE_REMOTES;
260 while (strbuf_getline(&buf, f) != EOF) {
261 const char *v;
@@ -289,6 +290,7 @@ static void read_branches_file(struct remote *remote)
290 return;
291 }
292
293 + remote->configured_in_repo = 1;
294 remote->origin = REMOTE_BRANCHES;
295
296 /*
@@ -371,6 +373,8 @@ static int handle_config(const char *key, const char *value, void *cb)
373 }
374 remote = make_remote(name, namelen);
375 remote->origin = REMOTE_CONFIG;
376 + if (current_config_scope() == CONFIG_SCOPE_REPO)
377 + remote->configured_in_repo = 1;
378 if (!strcmp(subkey, "mirror"))
379 remote->mirror = git_config_bool(key, value);
380 else if (!strcmp(subkey, "skipdefaultupdate"))
@@ -714,9 +718,13 @@ struct remote *pushremote_get(const char *name)
718 return remote_get_1(name, pushremote_for_branch);
719 }
720
717 -int remote_is_configured(struct remote *remote)
721 +int remote_is_configured(struct remote *remote, int in_repo)
722 {
719 - return remote && remote->origin;
723 + if (!remote)
724 + return 0;
725 + if (in_repo)
726 + return remote->configured_in_repo;
727 + return !!remote->origin;
728 }
729
730 int for_each_remote(each_remote_fn fn, void *priv)
remote.h
+2 -2
@@ -15,7 +15,7 @@ struct remote {
15 struct hashmap_entry ent; /* must be first */
16
17 const char *name;
18 - int origin;
18 + int origin, configured_in_repo;
19
20 const char *foreign_vcs;
21
@@ -60,7 +60,7 @@ struct remote {
60
61 struct remote *remote_get(const char *name);
62 struct remote *pushremote_get(const char *name);
63 -int remote_is_configured(struct remote *remote);
63 +int remote_is_configured(struct remote *remote, int in_repo);
64
65 typedef int each_remote_fn(struct remote *remote, void *priv);
66 int for_each_remote(each_remote_fn fn, void *priv);
t/t5505-remote.sh
+1 -1
@@ -764,7 +764,7 @@ test_expect_success 'rename a remote with name prefix of other remote' '
764 )
765 '
766
767 -test_expect_failure 'rename succeeds with existing remote.<target>.prune' '
767 +test_expect_success 'rename succeeds with existing remote.<target>.prune' '
768 git clone one four.four &&
769 test_when_finished git config --global --unset remote.upstream.prune &&
770 git config --global remote.upstream.prune true &&