Merge branch 'tg/git-remote'

The internal API to interact with "remote.*" configuration
variables has been streamlined.

* tg/git-remote:
remote: use remote_is_configured() for add and rename
remote: actually check if remote exits
remote: simplify remote_is_configured()
remote: use parse_config_key

diff --git a/builtin/fetch.c b/builtin/fetch.c
index 55919a9e4fb8e2fd538b6c111bfdacf83acf62ed..48c45ea5e3b77d5e7af339128971f294e6ea9a20 100644 (file)
--- a/builtin/fetch.c
+++ b/builtin/fetch.c
@@ -1022,10 +1022,9 @@ 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;
-               if (!remote_is_configured(name))
+               struct remote *remote = remote_get(name);
+               if (!remote_is_configured(remote))
                        return 0;
-               remote = remote_get(name);
                string_list_append(list, remote->name);
        }
        return 1;

diff --git a/builtin/remote.c b/builtin/remote.c
index 2b2ff9b7d2152ce4ac9314bc8f111c02723703e6..bd57f1b29badc050c74d8ae433cd6444e9dc9114 100644 (file)
--- a/builtin/remote.c
+++ b/builtin/remote.c
@@ -186,10 +186,7 @@ static int add(int argc, const char **argv)
        url = argv[1];
 
        remote = remote_get(name);
-       if (remote && (remote->url_nr > 1 ||
-                       (strcmp(name, remote->url[0]) &&
-                               strcmp(url, remote->url[0])) ||
-                       remote->fetch_refspec_nr))
+       if (remote_is_configured(remote))
                die(_("remote %s already exists."), name);
 
        strbuf_addf(&buf2, "refs/heads/test:refs/remotes/%s/test", name);
@@ -634,14 +631,14 @@ static int mv(int argc, const char **argv)
        rename.remote_branches = &remote_branches;
 
        oldremote = remote_get(rename.old);
-       if (!oldremote)
+       if (!remote_is_configured(oldremote))
                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 (newremote && (newremote->url_nr > 1 || newremote->fetch_refspec_nr))
+       if (remote_is_configured(newremote))
                die(_("remote %s already exists."), rename.new);
 
        strbuf_addf(&buf, "refs/heads/test:refs/remotes/%s/test", rename.new);
@@ -773,7 +770,7 @@ static int rm(int argc, const char **argv)
                usage_with_options(builtin_remote_rm_usage, options);
 
        remote = remote_get(argv[1]);
-       if (!remote)
+       if (!remote_is_configured(remote))
                die(_("No such remote: %s"), argv[1]);
 
        known_remotes.to_delete = remote;
@@ -1441,9 +1438,9 @@ static int set_remote_branches(const char *remotename, const char **branches,
 
        strbuf_addf(&key, "remote.%s.fetch", remotename);
 
-       if (!remote_is_configured(remotename))
-               die(_("No such remote '%s'"), remotename);
        remote = remote_get(remotename);
+       if (!remote_is_configured(remote))
+               die(_("No such remote '%s'"), remotename);
 
        if (!add_mode && remove_all_fetch_refspecs(remotename, key.buf)) {
                strbuf_release(&key);
@@ -1498,9 +1495,9 @@ static int get_url(int argc, const char **argv)
 
        remotename = argv[0];
 
-       if (!remote_is_configured(remotename))
-               die(_("No such remote '%s'"), remotename);
        remote = remote_get(remotename);
+       if (!remote_is_configured(remote))
+               die(_("No such remote '%s'"), remotename);
 
        url_nr = 0;
        if (push_mode) {
@@ -1566,9 +1563,9 @@ static int set_url(int argc, const char **argv)
        if (delete_mode)
                oldurl = newurl;
 
-       if (!remote_is_configured(remotename))
-               die(_("No such remote '%s'"), remotename);
        remote = remote_get(remotename);
+       if (!remote_is_configured(remote))
+               die(_("No such remote '%s'"), remotename);
 
        if (push_mode) {
                strbuf_addf(&name_buf, "remote.%s.pushurl", remotename);

diff --git a/remote.c b/remote.c
index 02e698a5e05a02f8905ec511851eea15e86d746f..f0016811059b60d6d1a683b8d94874e8599329f6 100644 (file)
--- a/remote.c
+++ b/remote.c
@@ -318,93 +318,88 @@ static void read_branches_file(struct remote *remote)
 static int handle_config(const char *key, const char *value, void *cb)
 {
        const char *name;
+       int namelen;
        const char *subkey;
        struct remote *remote;
        struct branch *branch;
-       if (starts_with(key, "branch.")) {
-               name = key + 7;
-               subkey = strrchr(name, '.');
-               if (!subkey)
+       if (parse_config_key(key, "branch", &name, &namelen, &subkey) >= 0) {
+               if (!name)
                        return 0;
-               branch = make_branch(name, subkey - name);
-               if (!strcmp(subkey, ".remote")) {
+               branch = make_branch(name, namelen);
+               if (!strcmp(subkey, "remote")) {
                        return git_config_string(&branch->remote_name, key, value);
-               } else if (!strcmp(subkey, ".pushremote")) {
+               } else if (!strcmp(subkey, "pushremote")) {
                        return git_config_string(&branch->pushremote_name, key, value);
-               } else if (!strcmp(subkey, ".merge")) {
+               } else if (!strcmp(subkey, "merge")) {
                        if (!value)
                                return config_error_nonbool(key);
                        add_merge(branch, xstrdup(value));
                }
                return 0;
        }
-       if (starts_with(key, "url.")) {
+       if (parse_config_key(key, "url", &name, &namelen, &subkey) >= 0) {
                struct rewrite *rewrite;
-               name = key + 4;
-               subkey = strrchr(name, '.');
-               if (!subkey)
+               if (!name)
                        return 0;
-               if (!strcmp(subkey, ".insteadof")) {
-                       rewrite = make_rewrite(&rewrites, name, subkey - name);
+               if (!strcmp(subkey, "insteadof")) {
+                       rewrite = make_rewrite(&rewrites, name, namelen);
                        if (!value)
                                return config_error_nonbool(key);
                        add_instead_of(rewrite, xstrdup(value));
-               } else if (!strcmp(subkey, ".pushinsteadof")) {
-                       rewrite = make_rewrite(&rewrites_push, name, subkey - name);
+               } else if (!strcmp(subkey, "pushinsteadof")) {
+                       rewrite = make_rewrite(&rewrites_push, name, namelen);
                        if (!value)
                                return config_error_nonbool(key);
                        add_instead_of(rewrite, xstrdup(value));
                }
        }
 
-       if (!starts_with(key,  "remote."))
+       if (parse_config_key(key, "remote", &name, &namelen, &subkey) < 0)
                return 0;
-       name = key + 7;
 
        /* Handle remote.* variables */
-       if (!strcmp(name, "pushdefault"))
+       if (!name && !strcmp(subkey, "pushdefault"))
                return git_config_string(&pushremote_name, key, value);
 
+       if (!name)
+               return 0;
        /* Handle remote.<name>.* variables */
        if (*name == '/') {
                warning("Config remote shorthand cannot begin with '/': %s",
                        name);
                return 0;
        }
-       subkey = strrchr(name, '.');
-       if (!subkey)
-               return 0;
-       remote = make_remote(name, subkey - name);
+       remote = make_remote(name, namelen);
        remote->origin = REMOTE_CONFIG;
-       if (!strcmp(subkey, ".mirror"))
+       if (!strcmp(subkey, "mirror"))
                remote->mirror = git_config_bool(key, value);
-       else if (!strcmp(subkey, ".skipdefaultupdate"))
+       else if (!strcmp(subkey, "skipdefaultupdate"))
                remote->skip_default_update = git_config_bool(key, value);
-       else if (!strcmp(subkey, ".skipfetchall"))
+       else if (!strcmp(subkey, "skipfetchall"))
                remote->skip_default_update = git_config_bool(key, value);
-       else if (!strcmp(subkey, ".prune"))
+       else if (!strcmp(subkey, "prune"))
                remote->prune = git_config_bool(key, value);
-       else if (!strcmp(subkey, ".url")) {
+       else if (!strcmp(subkey, "url")) {
                const char *v;
                if (git_config_string(&v, key, value))
                        return -1;
                add_url(remote, v);
-       } else if (!strcmp(subkey, ".pushurl")) {
+       } else if (!strcmp(subkey, "pushurl")) {
                const char *v;
                if (git_config_string(&v, key, value))
                        return -1;
                add_pushurl(remote, v);
-       } else if (!strcmp(subkey, ".push")) {
+       } else if (!strcmp(subkey, "push")) {
                const char *v;
                if (git_config_string(&v, key, value))
                        return -1;
                add_push_refspec(remote, v);
-       } else if (!strcmp(subkey, ".fetch")) {
+       } else if (!strcmp(subkey, "fetch")) {
                const char *v;
                if (git_config_string(&v, key, value))
                        return -1;
                add_fetch_refspec(remote, v);
-       } else if (!strcmp(subkey, ".receivepack")) {
+       } else if (!strcmp(subkey, "receivepack")) {
                const char *v;
                if (git_config_string(&v, key, value))
                        return -1;
@@ -412,7 +407,7 @@ static int handle_config(const char *key, const char *value, void *cb)
                        remote->receivepack = v;
                else
                        error("more than one receivepack given, using the first");
-       } else if (!strcmp(subkey, ".uploadpack")) {
+       } else if (!strcmp(subkey, "uploadpack")) {
                const char *v;
                if (git_config_string(&v, key, value))
                        return -1;
@@ -420,18 +415,18 @@ static int handle_config(const char *key, const char *value, void *cb)
                        remote->uploadpack = v;
                else
                        error("more than one uploadpack given, using the first");
-       } else if (!strcmp(subkey, ".tagopt")) {
+       } else if (!strcmp(subkey, "tagopt")) {
                if (!strcmp(value, "--no-tags"))
                        remote->fetch_tags = -1;
                else if (!strcmp(value, "--tags"))
                        remote->fetch_tags = 2;
-       } else if (!strcmp(subkey, ".proxy")) {
+       } else if (!strcmp(subkey, "proxy")) {
                return git_config_string((const char **)&remote->http_proxy,
                                         key, value);
-       } else if (!strcmp(subkey, ".proxyauthmethod")) {
+       } else if (!strcmp(subkey, "proxyauthmethod")) {
                return git_config_string((const char **)&remote->http_proxy_authmethod,
                                         key, value);
-       } else if (!strcmp(subkey, ".vcs")) {
+       } else if (!strcmp(subkey, "vcs")) {
                return git_config_string(&remote->foreign_vcs, key, value);
        }
        return 0;
@@ -718,18 +713,9 @@ struct remote *pushremote_get(const char *name)
        return remote_get_1(name, pushremote_for_branch);
 }
 
-int remote_is_configured(const char *name)
+int remote_is_configured(struct remote *remote)
 {
-       struct remotes_hash_key lookup;
-       struct hashmap_entry lookup_entry;
-       read_config();
-
-       init_remotes_hash();
-       lookup.str = name;
-       lookup.len = strlen(name);
-       hashmap_entry_init(&lookup_entry, memhash(name, lookup.len));
-
-       return hashmap_get(&remotes_hash, &lookup_entry, &lookup) != NULL;
+       return remote && remote->origin;
 }
 
 int for_each_remote(each_remote_fn fn, void *priv)

diff --git a/remote.h b/remote.h
index 4fd7a0f9b2764d3de935f6387f6a93bfd7ab13b0..c21fd3788c78f28d57e7d76d472dc411986b34d1 100644 (file)
--- a/remote.h
+++ b/remote.h
@@ -5,6 +5,7 @@
 #include "hashmap.h"
 
 enum {
+       REMOTE_UNCONFIGURED = 0,
        REMOTE_CONFIG,
        REMOTE_REMOTES,
        REMOTE_BRANCHES
@@ -59,7 +60,7 @@ struct remote {
 
 struct remote *remote_get(const char *name);
 struct remote *pushremote_get(const char *name);
-int remote_is_configured(const char *name);
+int remote_is_configured(struct remote *remote);
 
 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 0dcc752076a9e42b0cff982d7b0f42c63f85dee2..949725e9bcc5b4d6358ed28ca5675b92f9b7c7fa 100755 (executable)
--- a/t/t5505-remote.sh
+++ b/t/t5505-remote.sh
@@ -144,6 +144,39 @@ test_expect_success 'remove remote protects local branches' '
        )
 '
 
+test_expect_success 'remove errors out early when deleting non-existent branch' '
+       (
+               cd test &&
+               echo "fatal: No such remote: foo" >expect &&
+               test_must_fail git remote rm foo 2>actual &&
+               test_i18ncmp expect actual
+       )
+'
+
+test_expect_success 'rename errors out early when deleting non-existent branch' '
+       (
+               cd test &&
+               echo "fatal: No such remote: foo" >expect &&
+               test_must_fail git remote rename foo bar 2>actual &&
+               test_i18ncmp expect actual
+       )
+'
+
+test_expect_success 'add existing foreign_vcs remote' '
+       test_config remote.foo.vcs bar &&
+       echo "fatal: remote foo already exists." >expect &&
+       test_must_fail git remote add foo bar 2>actual &&
+       test_i18ncmp expect actual
+'
+
+test_expect_success 'add existing foreign_vcs remote' '
+       test_config remote.foo.vcs bar &&
+       test_config remote.bar.vcs bar &&
+       echo "fatal: remote bar already exists." >expect &&
+       test_must_fail git remote rename foo bar 2>actual &&
+       test_i18ncmp expect actual
+'
+
 cat >test/expect <<EOF
 * remote origin
   Fetch URL: $(pwd)/one