branch --track: code cleanup and saner handling of local branches

This patch cleans up some complicated code, and replaces it with a
cleaner version, using code from remote.[ch], which got extended a
little in the process.  This also enables us to fix two cases:

The earlier "fix" to setup tracking only when the original ref started
with "refs/remotes" is wrong.  You are absolutely allowed to use a
separate layout for your tracking branches.  The correct fix, of course,
is to set up tracking information only when there is a matching
remote.<nick>.fetch line containing a colon.

Another corner case was not handled properly.  If two remotes write to
the original ref, just warn the user and do not set up tracking.

Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
Signed-off-by: Junio C Hamano <gitster@pobox.com>

diff --git a/Documentation/config.txt b/Documentation/config.txt
index aeece848a542cd72894f4bf3f1bb9f67d18d5c3d..4b67f0adf721d540ddd437254de4aa069f1be0ae 100644 (file)
--- a/Documentation/config.txt
+++ b/Documentation/config.txt
@@ -309,10 +309,7 @@ branch.autosetupmerge::
        so that gitlink:git-pull[1] will appropriately merge from that
        remote branch.  Note that even if this option is not set,
        this behavior can be chosen per-branch using the `--track`
-       and `--no-track` options.  This option can have values
-       'false' (never touch the configuration), 'all' (do this
-       for all branches), or 'true' (do this only when
-       branching from a remote tracking branch), and defaults to 'true'.
+       and `--no-track` options.  This option defaults to false.
 
 branch.<name>.remote::
        When in branch <name>, it tells `git fetch` which remote to fetch.

diff --git a/Documentation/git-checkout.txt b/Documentation/git-checkout.txt
index 82929523c7166f9e5beb08689cef65cfa20c9a8c..818b720b914fa33d3a4b5c7858573bd59f0b0945 100644 (file)
--- a/Documentation/git-checkout.txt
+++ b/Documentation/git-checkout.txt
@@ -52,9 +52,8 @@ OPTIONS
        set up configuration so that git-pull will automatically
        retrieve data from the remote branch.  Set the
        branch.autosetupmerge configuration variable to true if you
-       want git-checkout and git-branch to behave as if
-       '--track' were given when you branch from a remote
-       tracking branch.
+       want git-checkout and git-branch to always behave as if
+       '--track' were given.
 
 --no-track::
        When -b is given and a branch is created off a remote branch,

diff --git a/builtin-branch.c b/builtin-branch.c
index 423c30194c2fecae6c565e2bf36de26fdcfee27e..3f9e7c2def4795328302863d1f6ca8f0c6e79f27 100644 (file)
--- a/builtin-branch.c
+++ b/builtin-branch.c
@@ -10,6 +10,7 @@
 #include "refs.h"
 #include "commit.h"
 #include "builtin.h"
+#include "remote.h"
 
 static const char builtin_branch_usage[] =
   "git-branch [-r] (-d | -D) <branchname> | [--track | --no-track] [-l] [-f] <branchname> [<start-point>] | (-m | -M) [<oldbranch>] <newbranch> | [--color | --no-color] [-r | -a] [-v [--abbrev=<length> | --no-abbrev]]";
@@ -22,7 +23,7 @@ static const char builtin_branch_usage[] =
 static const char *head;
 static unsigned char head_sha1[20];
 
-static int branch_track = 1; /* 0 = none, 1 = remotes, 2 = all */
+static int branch_track = 1;
 
 static int branch_use_color;
 static char branch_colors[][COLOR_MAXLEN] = {
@@ -66,12 +67,8 @@ static int git_branch_config(const char *var, const char *value)
                color_parse(value, var, branch_colors[slot]);
                return 0;
        }
-       if (!strcmp(var, "branch.autosetupmerge")) {
-               if (!strcmp(value, "all"))
-                       branch_track = 2;
-               else
+       if (!strcmp(var, "branch.autosetupmerge"))
                        branch_track = git_config_bool(var, value);
-       }
 
        return git_default_config(var, value);
 }
@@ -328,125 +325,70 @@ static void print_ref_list(int kinds, int detached, int verbose, int abbrev)
        free_ref_list(&ref_list);
 }
 
-static char *config_repo;
-static char *config_remote;
-static const char *start_ref;
+struct tracking {
+       struct refspec spec;
+       char *src;
+       const char *remote;
+       int matches;
+};
 
-static int get_remote_branch_name(const char *value)
+static int find_tracked_branch(struct remote *remote, void *priv)
 {
-       const char *colon;
-       const char *end;
+       struct tracking *tracking = priv;
 
-       if (*value == '+')
-               value++;
-
-       colon = strchr(value, ':');
-       if (!colon)
-               return 0;
-
-       end = value + strlen(value);
-
-       /*
-        * Try an exact match first.  I.e. handle the case where the
-        * value is "$anything:refs/foo/bar/baz" and start_ref is exactly
-        * "refs/foo/bar/baz". Then the name at the remote is $anything.
-        */
-       if (!strcmp(colon + 1, start_ref)) {
-               /* Truncate the value before the colon. */
-               nfasprintf(&config_repo, "%.*s", colon - value, value);
-               return 1;
+       if (!remote_find_tracking(remote, &tracking->spec)) {
+               if (++tracking->matches == 1) {
+                       tracking->src = tracking->spec.src;
+                       tracking->remote = remote->name;
+               } else {
+                       free(tracking->spec.src);
+                       if (tracking->src) {
+                               free(tracking->src);
+                               tracking->src = NULL;
+                       }
+               }
+               tracking->spec.src = NULL;
        }
 
-       /*
-        * Is this a wildcard match?
-        */
-       if ((end - 2 <= value) || end[-2] != '/' || end[-1] != '*' ||
-           (colon - 2 <= value) || colon[-2] != '/' || colon[-1] != '*')
-               return 0;
-
-       /*
-        * Value is "refs/foo/bar/<asterisk>:refs/baz/boa/<asterisk>"
-        * and start_ref begins with "refs/baz/boa/"; the name at the
-        * remote is refs/foo/bar/ with the remaining part of the
-        * start_ref.  The length of the prefix on the RHS is (end -
-        * colon - 2), including the slash immediately before the
-        * asterisk.
-        */
-       if ((strlen(start_ref) < end - colon - 2) ||
-           memcmp(start_ref, colon + 1, end - colon - 2))
-               return 0; /* does not match prefix */
-
-       /* Replace the asterisk with the remote branch name.  */
-       nfasprintf(&config_repo, "%.*s%s",
-                  (colon - 1) - value, value,
-                  start_ref + (end - colon - 2));
-       return 1;
-}
-
-static int get_remote_config(const char *key, const char *value)
-{
-       const char *var;
-       if (prefixcmp(key, "remote."))
-               return 0;
-
-       var = strrchr(key, '.');
-       if (var == key + 6 || strcmp(var, ".fetch"))
-               return 0;
-       /*
-        * Ok, we are looking at key == "remote.$foo.fetch";
-        */
-       if (get_remote_branch_name(value))
-               nfasprintf(&config_remote, "%.*s", var - (key + 7), key + 7);
-
        return 0;
 }
 
-static void set_branch_merge(const char *name, const char *config_remote,
-                            const char *config_repo)
+
+/*
+ * This is called when new_ref is branched off of orig_ref, and tries
+ * to infer the settings for branch.<new_ref>.{remote,merge} from the
+ * config.
+ */
+static int setup_tracking(const char *new_ref, const char *orig_ref)
 {
        char key[1024];
-       if (sizeof(key) <=
-           snprintf(key, sizeof(key), "branch.%s.remote", name))
-               die("what a long branch name you have!");
-       git_config_set(key, config_remote);
-
-       /*
-        * We do not have to check if we have enough space for
-        * the 'merge' key, since it's shorter than the
-        * previous 'remote' key, which we already checked.
-        */
-       snprintf(key, sizeof(key), "branch.%s.merge", name);
-       git_config_set(key, config_repo);
-}
+       struct tracking tracking;
 
-static void set_branch_defaults(const char *name, const char *real_ref)
-{
-       /*
-        * name is the name of new branch under refs/heads;
-        * real_ref is typically refs/remotes/$foo/$bar, where
-        * $foo is the remote name (there typically are no slashes)
-        * and $bar is the branch name we map from the remote
-        * (it could have slashes).
-        */
-       start_ref = real_ref;
-       git_config(get_remote_config);
-       if (!config_repo && !config_remote &&
-           !prefixcmp(real_ref, "refs/heads/")) {
-               set_branch_merge(name, ".", real_ref);
-               printf("Branch %s set up to track local branch %s.\n",
-                      name, real_ref);
-       }
+       if (strlen(new_ref) > 1024 - 7 - 7 - 1)
+               return error("Tracking not set up: name too long: %s",
+                               new_ref);
 
-       if (config_repo && config_remote) {
-               set_branch_merge(name, config_remote, config_repo);
+       memset(&tracking, 0, sizeof(tracking));
+       tracking.spec.dst = (char *)orig_ref;
+       if (for_each_remote(find_tracked_branch, &tracking) ||
+                       !tracking.matches)
+               return 1;
+
+       if (tracking.matches > 1)
+               return error("Not tracking: ambiguous information for ref %s",
+                               orig_ref);
+
+       if (tracking.matches == 1) {
+               sprintf(key, "branch.%s.remote", new_ref);
+               git_config_set(key, tracking.remote ?  tracking.remote : ".");
+               sprintf(key, "branch.%s.merge", new_ref);
+               git_config_set(key, tracking.src);
+               free(tracking.src);
                printf("Branch %s set up to track remote branch %s.\n",
-                      name, real_ref);
+                              new_ref, orig_ref);
        }
 
-       if (config_repo)
-               free(config_repo);
-       if (config_remote)
-               free(config_remote);
+       return 0;
 }
 
 static void create_branch(const char *name, const char *start_name,
@@ -508,10 +450,8 @@ static void create_branch(const char *name, const char *start_name,
        /* When branching off a remote branch, set up so that git-pull
           automatically merges from there.  So far, this is only done for
           remotes registered via .git/config.  */
-       if (real_ref && (track == 2 ||
-                               (track == 1 &&
-                                !prefixcmp(real_ref, "refs/remotes/"))))
-               set_branch_defaults(name, real_ref);
+       if (real_ref && track)
+               setup_tracking(name, real_ref);
 
        if (write_ref_sha1(lock, sha1, msg) < 0)
                die("Failed to write ref: %s.", strerror(errno));
@@ -582,7 +522,7 @@ int cmd_branch(int argc, const char **argv, const char *prefix)
                        break;
                }
                if (!strcmp(arg, "--track")) {
-                       track = 2;
+                       track = 1;
                        continue;
                }
                if (!strcmp(arg, "--no-track")) {

diff --git a/t/t3200-branch.sh b/t/t3200-branch.sh
index a19e961cb3f45b2b1b385d5b9b4f455176ed6d9f..ef1eeb7d8ac349821559dd358470f10ae0531739 100755 (executable)
--- a/t/t3200-branch.sh
+++ b/t/t3200-branch.sh
@@ -148,13 +148,14 @@ test_expect_success 'test tracking setup via config' \
      test $(git config branch.my3.remote) = local &&
      test $(git config branch.my3.merge) = refs/heads/master'
 
-test_expect_success 'autosetupmerge = all' '
+test_expect_success 'avoid ambiguous track' '
        git config branch.autosetupmerge true &&
+       git config remote.ambi1.url = lalala &&
+       git config remote.ambi1.fetch = refs/heads/lalala:refs/heads/master &&
+       git config remote.ambi2.url = lilili &&
+       git config remote.ambi2.fetch = refs/heads/lilili:refs/heads/master &&
        git branch all1 master &&
-       test -z "$(git config branch.all1.merge)" &&
-       git config branch.autosetupmerge all &&
-       git branch all2 master &&
-       test $(git config branch.all2.merge) = refs/heads/master
+       test -z "$(git config branch.all1.merge)"
 '
 
 test_expect_success 'test overriding tracking setup via --no-track' \
@@ -167,10 +168,10 @@ test_expect_success 'test overriding tracking setup via --no-track' \
      ! test "$(git config branch.my2.remote)" = local &&
      ! test "$(git config branch.my2.merge)" = refs/heads/master'
 
-test_expect_success 'test local tracking setup' \
+test_expect_success 'no tracking without .fetch entries' \
     'git branch --track my6 s &&
-     test $(git config branch.my6.remote) = . &&
-     test $(git config branch.my6.merge) = refs/heads/s'
+     test -z "$(git config branch.my6.remote)" &&
+     test -z "$(git config branch.my6.merge)"'
 
 test_expect_success 'test tracking setup via --track but deeper' \
     'git config remote.local.url . &&
@@ -182,8 +183,8 @@ test_expect_success 'test tracking setup via --track but deeper' \
 
 test_expect_success 'test deleting branch deletes branch config' \
     'git branch -d my7 &&
-     test "$(git config branch.my7.remote)" = "" &&
-     test "$(git config branch.my7.merge)" = ""'
+     test -z "$(git config branch.my7.remote)" &&
+     test -z "$(git config branch.my7.merge)"'
 
 test_expect_success 'test deleting branch without config' \
     'git branch my7 s &&