diff mbox series

[v4] tracking branches: add advice to ambiguous refspec error

Message ID pull.1183.v4.git.1648553209157.gitgitgadget@gmail.com (mailing list archive)
State Superseded
Headers show
Series [v4] tracking branches: add advice to ambiguous refspec error | expand

Commit Message

Tao Klerks March 29, 2022, 11:26 a.m. UTC
From: Tao Klerks <tao@klerks.biz>

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.

Signed-off-by: Tao Klerks <tao@klerks.biz>
---
    tracking branches: add advice to ambiguous refspec error
    
    I believe this third version incorporates all Ævar's suggestions, and
    might be usable. Removed "RFC" prefix.

Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-1183%2FTaoK%2Fadvise-ambiguous-tracking-refspec-v4
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-1183/TaoK/advise-ambiguous-tracking-refspec-v4
Pull-Request: https://github.com/gitgitgadget/git/pull/1183

Range-diff vs v3:

 1:  22ffe81ac26 = 1:  ac6c782f566 tracking branches: add advice to ambiguous refspec error


 Documentation/config/advice.txt |  4 +++
 advice.c                        |  1 +
 advice.h                        |  1 +
 branch.c                        | 44 +++++++++++++++++++++++++++++----
 4 files changed, 45 insertions(+), 5 deletions(-)


base-commit: abf474a5dd901f28013c52155411a48fd4c09922

Comments

Tao Klerks March 29, 2022, 11:31 a.m. UTC | #1
On Tue, Mar 29, 2022 at 1:26 PM Tao Klerks via GitGitGadget
<gitgitgadget@gmail.com> wrote:
>
>     I believe this third version incorporates all Ævar's suggestions, and
>     might be usable. Removed "RFC" prefix.

Argh, sorry about the broken cover letter. This *fourth* version
attempts to address Junio's concerns over unnecessary extra
error-preparation work being done in the very-vastly-more-common
"normal" case.
Junio C Hamano March 29, 2022, 3:49 p.m. UTC | #2
"Tao Klerks via GitGitGadget" <gitgitgadget@gmail.com> writes:

> From: Tao Klerks <tao@klerks.biz>
>
> 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.
>
> Signed-off-by: Tao Klerks <tao@klerks.biz>
> ---
>     tracking branches: add advice to ambiguous refspec error
>     
>     I believe this third version incorporates all Ævar's suggestions, and
>     might be usable. Removed "RFC" prefix.
>
> Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-1183%2FTaoK%2Fadvise-ambiguous-tracking-refspec-v4
> Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-1183/TaoK/advise-ambiguous-tracking-refspec-v4
> Pull-Request: https://github.com/gitgitgadget/git/pull/1183
>
> Range-diff vs v3:
>
>  1:  22ffe81ac26 = 1:  ac6c782f566 tracking branches: add advice to ambiguous refspec error
>
>
>  Documentation/config/advice.txt |  4 +++
>  advice.c                        |  1 +
>  advice.h                        |  1 +
>  branch.c                        | 44 +++++++++++++++++++++++++++++----
>  4 files changed, 45 insertions(+), 5 deletions(-)

Sent a wrong version?

The patch text seems to be identical to that of v3 message archived at
https://lore.kernel.org/git/pull.1183.v3.git.1648450268285.gitgitgadget@gmail.com/
Tao Klerks March 30, 2022, 4:17 a.m. UTC | #3
On Tue, Mar 29, 2022 at 5:49 PM Junio C Hamano <gitster@pobox.com> wrote:
[...]
>
> Sent a wrong version?
[...]

Yes indeed. My apologies once again. V5 coming as soon as the test run
completes.
diff mbox series

Patch

diff --git a/Documentation/config/advice.txt b/Documentation/config/advice.txt
index c40eb09cb7e..90f7dbd03aa 100644
--- 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 branch tracking relationship setup fails due
+		to multiple remotes' refspecs mapping to the same remote
+		tracking namespace in the repo.
 	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 2e1fd483040..18ac8739519 100644
--- 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 a3957123a16..2d4c94f38eb 100644
--- 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 6b31df539a5..5c28d432103 100644
--- a/branch.c
+++ b/branch.c
@@ -18,9 +18,15 @@  struct tracking {
 	int matches;
 };
 
+struct find_tracked_branch_cb {
+	struct tracking *tracking;
+	struct strbuf remotes_advice;
+};
+
 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) {
@@ -30,6 +36,13 @@  static int find_tracked_branch(struct remote *remote, void *priv)
 			free(tracking->spec.src);
 			string_list_clear(tracking->srcs, 0);
 		}
+		/*
+		 * TRANSLATORS: This is a line listing a remote with duplicate
+		 * refspecs, to be later included in advice message
+		 * ambiguousFetchRefspec. For RTL languages you'll probably want
+		 * to swap the "%s" and leading "  " space around.
+		 */
+		strbuf_addf(&ftb->remotes_advice, _("  %s\n"), remote->name);
 		tracking->spec.src = NULL;
 	}
 
@@ -219,6 +232,7 @@  static int inherit_tracking(struct tracking *tracking, const char *orig_ref)
 	return 0;
 }
 
+
 /*
  * Used internally to set the branch.<new_ref>.{remote,merge} config
  * settings so that branch 'new_ref' tracks 'orig_ref'. Unlike
@@ -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,
+		.remotes_advice = STRBUF_INIT,
+	};
 
 	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,24 @@  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))
+			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,
+			       ftb_cb.remotes_advice.buf
+			       );
+		exit(status);
+	}
 
 	if (tracking.srcs->nr < 1)
 		string_list_append(tracking.srcs, orig_ref);
@@ -263,6 +296,7 @@  static void setup_tracking(const char *new_ref, const char *orig_ref,
 		exit(-1);
 
 cleanup:
+	strbuf_release(&ftb_cb.remotes_advice);
 	string_list_clear(&tracking_srcs, 0);
 }