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>

diff --git a/builtin/fetch.c b/builtin/fetch.c
index cd7e3cefe62b98786097de9a6a456589060c7ebd..44de68564ac4d526b51fa4cac732e57e63eb69ad 100644 (file)
--- a/builtin/fetch.c
+++ b/builtin/fetch.c
@@ -1145,7 +1145,7 @@ static int add_remote_or_group(const char *name, struct string_list *list)
        git_config(get_remote_group, &g);
        if (list->nr == prev_nr) {
                struct remote *remote = remote_get(name);
        git_config(get_remote_group, &g);
        if (list->nr == prev_nr) {
                struct remote *remote = remote_get(name);

-               if (!remote_is_configured(remote))
+               if (!remote_is_configured(remote, 0))

                        return 0;
                string_list_append(list, remote->name);
        }
                        return 0;
                string_list_append(list, remote->name);
        }

diff --git a/builtin/remote.c b/builtin/remote.c
index 9f6a6b3a9cea036d9a58c52f7d17c2b043a18f33..be1b62683aad76c2657af946bc212cbfdf2a2f21 100644 (file)
--- a/builtin/remote.c
+++ b/builtin/remote.c
@@ -186,7 +186,7 @@ static int add(int argc, const char **argv)
        url = argv[1];
 
        remote = remote_get(name);
        url = argv[1];
 
        remote = remote_get(name);

-       if (remote_is_configured(remote))
+       if (remote_is_configured(remote, 1))

                die(_("remote %s already exists."), name);
 
        strbuf_addf(&buf2, "refs/heads/test:refs/remotes/%s/test", name);
                die(_("remote %s already exists."), name);
 
        strbuf_addf(&buf2, "refs/heads/test:refs/remotes/%s/test", name);


@@ -618,14 +618,14 @@ static int mv(int argc, const char **argv)
        rename.remote_branches = &remote_branches;
 
        oldremote = remote_get(rename.old);
        rename.remote_branches = &remote_branches;
 
        oldremote = remote_get(rename.old);

-       if (!remote_is_configured(oldremote))
+       if (!remote_is_configured(oldremote, 1))

                die(_("No such remote: %s"), rename.old);
 
        if (!strcmp(rename.old, rename.new) && oldremote->origin != REMOTE_CONFIG)
                return migrate_file(oldremote);
 
        newremote = remote_get(rename.new);
                die(_("No such remote: %s"), rename.old);
 
        if (!strcmp(rename.old, rename.new) && oldremote->origin != REMOTE_CONFIG)
                return migrate_file(oldremote);
 
        newremote = remote_get(rename.new);

-       if (remote_is_configured(newremote))
+       if (remote_is_configured(newremote, 1))

                die(_("remote %s already exists."), rename.new);
 
        strbuf_addf(&buf, "refs/heads/test:refs/remotes/%s/test", rename.new);
                die(_("remote %s already exists."), rename.new);
 
        strbuf_addf(&buf, "refs/heads/test:refs/remotes/%s/test", rename.new);


@@ -753,7 +753,7 @@ static int rm(int argc, const char **argv)
                usage_with_options(builtin_remote_rm_usage, options);
 
        remote = remote_get(argv[1]);
                usage_with_options(builtin_remote_rm_usage, options);
 
        remote = remote_get(argv[1]);

-       if (!remote_is_configured(remote))
+       if (!remote_is_configured(remote, 1))

                die(_("No such remote: %s"), argv[1]);
 
        known_remotes.to_delete = remote;
                die(_("No such remote: %s"), argv[1]);
 
        known_remotes.to_delete = remote;


@@ -1416,7 +1416,7 @@ static int set_remote_branches(const char *remotename, const char **branches,
        strbuf_addf(&key, "remote.%s.fetch", remotename);
 
        remote = remote_get(remotename);
        strbuf_addf(&key, "remote.%s.fetch", remotename);
 
        remote = remote_get(remotename);

-       if (!remote_is_configured(remote))
+       if (!remote_is_configured(remote, 1))

                die(_("No such remote '%s'"), remotename);
 
        if (!add_mode && remove_all_fetch_refspecs(remotename, key.buf)) {
                die(_("No such remote '%s'"), remotename);
 
        if (!add_mode && remove_all_fetch_refspecs(remotename, key.buf)) {


@@ -1470,7 +1470,7 @@ static int get_url(int argc, const char **argv)
        remotename = argv[0];
 
        remote = remote_get(remotename);
        remotename = argv[0];
 
        remote = remote_get(remotename);

-       if (!remote_is_configured(remote))
+       if (!remote_is_configured(remote, 1))

                die(_("No such remote '%s'"), remotename);
 
        url_nr = 0;
                die(_("No such remote '%s'"), remotename);
 
        url_nr = 0;


@@ -1538,7 +1538,7 @@ static int set_url(int argc, const char **argv)
                oldurl = newurl;
 
        remote = remote_get(remotename);
                oldurl = newurl;
 
        remote = remote_get(remotename);

-       if (!remote_is_configured(remote))
+       if (!remote_is_configured(remote, 1))

                die(_("No such remote '%s'"), remotename);
 
        if (push_mode) {
                die(_("No such remote '%s'"), remotename);
 
        if (push_mode) {

diff --git a/remote.c b/remote.c
index ad6c5424edab2ae15ac17bfcf12ac4ee93b5aa3f..8524135de4286a079519068be17f0fe6fe7632f9 100644 (file)
--- a/remote.c
+++ b/remote.c
@@ -255,6 +255,7 @@ static void read_remotes_file(struct remote *remote)
 
        if (!f)
                return;
 
        if (!f)
                return;

+       remote->configured_in_repo = 1;

        remote->origin = REMOTE_REMOTES;
        while (strbuf_getline(&buf, f) != EOF) {
                const char *v;
        remote->origin = REMOTE_REMOTES;
        while (strbuf_getline(&buf, f) != EOF) {
                const char *v;


@@ -289,6 +290,7 @@ static void read_branches_file(struct remote *remote)
                return;
        }
 
                return;
        }
 

+       remote->configured_in_repo = 1;

        remote->origin = REMOTE_BRANCHES;
 
        /*
        remote->origin = REMOTE_BRANCHES;
 
        /*


@@ -371,6 +373,8 @@ static int handle_config(const char *key, const char *value, void *cb)
        }
        remote = make_remote(name, namelen);
        remote->origin = REMOTE_CONFIG;
        }
        remote = make_remote(name, namelen);
        remote->origin = REMOTE_CONFIG;

+       if (current_config_scope() == CONFIG_SCOPE_REPO)
+               remote->configured_in_repo = 1;

        if (!strcmp(subkey, "mirror"))
                remote->mirror = git_config_bool(key, value);
        else if (!strcmp(subkey, "skipdefaultupdate"))
        if (!strcmp(subkey, "mirror"))
                remote->mirror = git_config_bool(key, value);
        else if (!strcmp(subkey, "skipdefaultupdate"))


@@ -714,9 +718,13 @@ struct remote *pushremote_get(const char *name)
        return remote_get_1(name, pushremote_for_branch);
 }
 
        return remote_get_1(name, pushremote_for_branch);
 }
 

-int remote_is_configured(struct remote *remote)
+int remote_is_configured(struct remote *remote, int in_repo)

 {
 {

-       return remote && remote->origin;
+       if (!remote)
+               return 0;
+       if (in_repo)
+               return remote->configured_in_repo;
+       return !!remote->origin;

 }
 
 int for_each_remote(each_remote_fn fn, void *priv)
 }
 
 int for_each_remote(each_remote_fn fn, void *priv)

diff --git a/remote.h b/remote.h
index 924881169d9f6c5b9b09e2434d964f62e0e28d09..a5bbbe0ef965c5e62be50e8ee0c03794e393cc8c 100644 (file)
--- a/remote.h
+++ b/remote.h
@@ -15,7 +15,7 @@ struct remote {
        struct hashmap_entry ent;  /* must be first */
 
        const char *name;
        struct hashmap_entry ent;  /* must be first */
 
        const char *name;

-       int origin;
+       int origin, configured_in_repo;

 
        const char *foreign_vcs;
 
 
        const char *foreign_vcs;
 


@@ -60,7 +60,7 @@ struct remote {
 
 struct remote *remote_get(const char *name);
 struct remote *pushremote_get(const char *name);
 
 struct remote *remote_get(const char *name);
 struct remote *pushremote_get(const char *name);

-int remote_is_configured(struct remote *remote);
+int remote_is_configured(struct remote *remote, int in_repo);

 
 typedef int each_remote_fn(struct remote *remote, void *priv);
 int for_each_remote(each_remote_fn fn, void *priv);
 
 typedef int each_remote_fn(struct remote *remote, void *priv);
 int for_each_remote(each_remote_fn fn, void *priv);

diff --git a/t/t5505-remote.sh b/t/t5505-remote.sh
index 2c03f44c85991ce57f59b671f5d3e3ce1025a1bf..ba46e86de0a7285bf88ff5fe05e378c8bfd256f9 100755 (executable)
--- a/t/t5505-remote.sh
+++ b/t/t5505-remote.sh
@@ -764,7 +764,7 @@ test_expect_success 'rename a remote with name prefix of other remote' '
        )
 '
 
        )
 '
 

-test_expect_failure 'rename succeeds with existing remote.<target>.prune' '
+test_expect_success 'rename succeeds with existing remote.<target>.prune' '

        git clone one four.four &&
        test_when_finished git config --global --unset remote.upstream.prune &&
        git config --global remote.upstream.prune true &&
        git clone one four.four &&
        test_when_finished git config --global --unset remote.upstream.prune &&
        git config --global remote.upstream.prune true &&