tracking branches: add advice to ambiguous refspec error

The error "not tracking: ambiguous information for ref" is raised
when we are evaluating what tracking information to set on a branch,
and find that the ref to be added as tracking branch is mapped
under multiple remotes' fetch refspecs.

This can easily happen when a user copy-pastes a remote definition
in their git config, and forgets to change the tracking path.

Add advice in this situation, explicitly highlighting which remotes
are involved and suggesting how to correct the situation. Also
update a test to explicitly expect that advice.

Signed-off-by: Tao Klerks <tao@klerks.biz>
Signed-off-by: Junio C Hamano <gitster@pobox.com>

diff --git a/Documentation/config/advice.txt b/Documentation/config/advice.txt
index c40eb09cb7e97104d2b5b44b89c4fd1d087f5d33..343d271c70783e308f0b30570bba1c9f6679f787 100644 (file)
--- a/Documentation/config/advice.txt
+++ b/Documentation/config/advice.txt
@@ -4,6 +4,10 @@ advice.*::
        can tell Git that you do not need help by setting these to 'false':
 +
 --
+       ambiguousFetchRefspec::
+               Advice shown when fetch refspec for multiple remotes map to
+               the same remote-tracking branch namespace and causes branch
+               tracking set-up to fail.
        fetchShowForcedUpdates::
                Advice shown when linkgit:git-fetch[1] takes a long time
                to calculate forced updates after ref updates, or to warn

diff --git a/advice.c b/advice.c
index 2e1fd483040bf399e3a689145d6c8dd3dfa277f5..18ac87395197c8cc6470ea2dd39c2ca8820fdcf7 100644 (file)
--- a/advice.c
+++ b/advice.c
@@ -39,6 +39,7 @@ static struct {
        [ADVICE_ADD_EMPTY_PATHSPEC]                     = { "addEmptyPathspec", 1 },
        [ADVICE_ADD_IGNORED_FILE]                       = { "addIgnoredFile", 1 },
        [ADVICE_AM_WORK_DIR]                            = { "amWorkDir", 1 },
+       [ADVICE_AMBIGUOUS_FETCH_REFSPEC]                = { "ambiguousFetchRefspec", 1 },
        [ADVICE_CHECKOUT_AMBIGUOUS_REMOTE_BRANCH_NAME]  = { "checkoutAmbiguousRemoteBranchName", 1 },
        [ADVICE_COMMIT_BEFORE_MERGE]                    = { "commitBeforeMerge", 1 },
        [ADVICE_DETACHED_HEAD]                          = { "detachedHead", 1 },

diff --git a/advice.h b/advice.h
index a3957123a1618d02b1d8b8a3438dbd92a577523a..2d4c94f38ebe45841d305944272ae3e1b87bbbe3 100644 (file)
--- a/advice.h
+++ b/advice.h
@@ -17,6 +17,7 @@ struct string_list;
        ADVICE_ADD_EMPTY_PATHSPEC,
        ADVICE_ADD_IGNORED_FILE,
        ADVICE_AM_WORK_DIR,
+       ADVICE_AMBIGUOUS_FETCH_REFSPEC,
        ADVICE_CHECKOUT_AMBIGUOUS_REMOTE_BRANCH_NAME,
        ADVICE_COMMIT_BEFORE_MERGE,
        ADVICE_DETACHED_HEAD,

diff --git a/branch.c b/branch.c
index 6b31df539a5f72e07bc5efb4592f703ddd70d28c..182f1c5a556052d791ae2b0b0e147397e1bb3062 100644 (file)
--- a/branch.c
+++ b/branch.c
@@ -18,17 +18,31 @@ struct tracking {
        int matches;
 };
 
+struct find_tracked_branch_cb {
+       struct tracking *tracking;
+       struct string_list ambiguous_remotes;
+};
+
 static int find_tracked_branch(struct remote *remote, void *priv)
 {
-       struct tracking *tracking = priv;
+       struct find_tracked_branch_cb *ftb = priv;
+       struct tracking *tracking = ftb->tracking;
 
        if (!remote_find_tracking(remote, &tracking->spec)) {
-               if (++tracking->matches == 1) {
+               switch (++tracking->matches) {
+               case 1:
                        string_list_append(tracking->srcs, tracking->spec.src);
                        tracking->remote = remote->name;
-               } else {
+                       break;
+               case 2:
+                       /* there are at least two remotes; backfill the first one */
+                       string_list_append(&ftb->ambiguous_remotes, tracking->remote);
+                       /* fall through */
+               default:
+                       string_list_append(&ftb->ambiguous_remotes, remote->name);
                        free(tracking->spec.src);
                        string_list_clear(tracking->srcs, 0);
+               break;
                }
                tracking->spec.src = NULL;
        }
@@ -232,12 +246,16 @@ static void setup_tracking(const char *new_ref, const char *orig_ref,
        struct tracking tracking;
        struct string_list tracking_srcs = STRING_LIST_INIT_DUP;
        int config_flags = quiet ? 0 : BRANCH_CONFIG_VERBOSE;
+       struct find_tracked_branch_cb ftb_cb = {
+               .tracking = &tracking,
+               .ambiguous_remotes = STRING_LIST_INIT_DUP,
+       };
 
        memset(&tracking, 0, sizeof(tracking));
        tracking.spec.dst = (char *)orig_ref;
        tracking.srcs = &tracking_srcs;
        if (track != BRANCH_TRACK_INHERIT)
-               for_each_remote(find_tracked_branch, &tracking);
+               for_each_remote(find_tracked_branch, &ftb_cb);
        else if (inherit_tracking(&tracking, orig_ref))
                goto cleanup;
 
@@ -252,9 +270,39 @@ static void setup_tracking(const char *new_ref, const char *orig_ref,
                        goto cleanup;
                }
 
-       if (tracking.matches > 1)
-               die(_("not tracking: ambiguous information for ref %s"),
-                   orig_ref);
+       if (tracking.matches > 1) {
+               int status = die_message(_("not tracking: ambiguous information for ref '%s'"),
+                                           orig_ref);
+               if (advice_enabled(ADVICE_AMBIGUOUS_FETCH_REFSPEC)) {
+                       struct strbuf remotes_advice = STRBUF_INIT;
+                       struct string_list_item *item;
+
+                       for_each_string_list_item(item, &ftb_cb.ambiguous_remotes)
+                               /*
+                                * TRANSLATORS: This is a line listing a remote with duplicate
+                                * refspecs in the advice message below. For RTL languages you'll
+                                * probably want to swap the "%s" and leading "  " space around.
+                                */
+                               strbuf_addf(&remotes_advice, _("  %s\n"), item->string);
+
+                       /*
+                        * TRANSLATORS: The second argument is a \n-delimited list of
+                        * duplicate refspecs, composed above.
+                        */
+                       advise(_("There are multiple remotes whose fetch refspecs map to the remote\n"
+                                "tracking ref '%s':\n"
+                                "%s"
+                                "\n"
+                                "This is typically a configuration error.\n"
+                                "\n"
+                                "To support setting up tracking branches, ensure that\n"
+                                "different remotes' fetch refspecs map into different\n"
+                                "tracking namespaces."), orig_ref,
+                              remotes_advice.buf);
+                       strbuf_release(&remotes_advice);
+               }
+               exit(status);
+       }
 
        if (tracking.srcs->nr < 1)
                string_list_append(tracking.srcs, orig_ref);
@@ -264,6 +312,7 @@ static void setup_tracking(const char *new_ref, const char *orig_ref,
 
 cleanup:
        string_list_clear(&tracking_srcs, 0);
+       string_list_clear(&ftb_cb.ambiguous_remotes, 0);
 }
 
 int read_branch_desc(struct strbuf *buf, const char *branch_name)

diff --git a/t/t3200-branch.sh b/t/t3200-branch.sh
index 7a0ff75ba867c966db326efd55b473d4a3582fdd..e12db5936155453ea86a9b84ce0321e588d955b4 100755 (executable)
--- a/t/t3200-branch.sh
+++ b/t/t3200-branch.sh
@@ -1039,13 +1039,27 @@ test_expect_success 'checkout -b with -l makes reflog when core.logAllRefUpdates
        git rev-parse --verify gamma@{0}
 '
 
-test_expect_success 'avoid ambiguous track' '
+test_expect_success 'avoid ambiguous track and advise' '
        git config branch.autosetupmerge true &&
        git config remote.ambi1.url lalala &&
        git config remote.ambi1.fetch refs/heads/lalala:refs/heads/main &&
        git config remote.ambi2.url lilili &&
        git config remote.ambi2.fetch refs/heads/lilili:refs/heads/main &&
-       test_must_fail git branch all1 main &&
+       cat <<-EOF >expected &&
+       fatal: not tracking: ambiguous information for ref '\''refs/heads/main'\''
+       hint: There are multiple remotes whose fetch refspecs map to the remote
+       hint: tracking ref '\''refs/heads/main'\'':
+       hint:   ambi1
+       hint:   ambi2
+       hint: ''
+       hint: This is typically a configuration error.
+       hint: ''
+       hint: To support setting up tracking branches, ensure that
+       hint: different remotes'\'' fetch refspecs map into different
+       hint: tracking namespaces.
+       EOF
+       test_must_fail git branch all1 main 2>actual &&
+       test_cmp expected actual &&
        test -z "$(git config branch.all1.merge)"
 '