| Message ID | pull.1183.v2.git.1647940686394.gitgitgadget@gmail.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [v2] RFC: tracking branches: add advice to ambiguous refspec error | expand |
On Tue, Mar 22 2022, Tao Klerks via GitGitGadget wrote: > From: Tao Klerks <tao@klerks.biz> > [...] Looking much better! > diff --git a/Documentation/config/advice.txt b/Documentation/config/advice.txt > index 063eec2511d..abfac4f664b 100644 > --- a/Documentation/config/advice.txt > +++ b/Documentation/config/advice.txt > @@ -126,4 +126,8 @@ advice.*:: > Advice shown when either linkgit:git-add[1] or linkgit:git-rm[1] > is asked to update index entries outside the current sparse > checkout. > + ambiguousFetchRefspec:: > + Advice shown when branch tracking relationship setup fails due > + to multiple remotes' refspecs mapping to the same remote > + tracking namespace in the repo. Let's add this in alphabetical section order. It's *somewhat* true of the order now, but for some, but in any case I've got a WIP patch to sort these, so since it's new adding it at the top would save us the churn :) > +struct find_tracked_branch_cb { > + struct tracking *tracking; > + struct strbuf remotes_advice; > +}; Nice, thanks! > 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) { > @@ -28,6 +34,7 @@ static int find_tracked_branch(struct remote *remote, void *priv) > free(tracking->spec.src); > string_list_clear(tracking->srcs, 0); > } > + strbuf_addf(&ftb->remotes_advice, " %s\n", remote->name); Do this as: strbuf_addf([...], _(" %s\n"), [...]); And add a TRANSLATORS comment similar to show_ambiguous_object() in object-name.c. I.e. the alignment needs to be translatable for RTL languages. > +static const char ambiguous_refspec_advice_pre[] = > +N_("\n" > +"There are multiple remotes whose fetch refspecs map to the remote\n" > +"tracking ref";) > +static const char ambiguous_refspec_advice_post[] = > +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.\n"); These were split up before since we incrementally built the message.... Also, is the \n at the beginning/end really needed? > /* > * 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 > @@ -228,12 +247,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)) > return; > > @@ -248,9 +271,18 @@ static void setup_tracking(const char *new_ref, const char *orig_ref, > return; > } > > - 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("%s %s:\n%s\n%s", > + _(ambiguous_refspec_advice_pre), > + orig_ref, > + ftb_cb.remotes_advice.buf, > + _(ambiguous_refspec_advice_post) > + ); ...but now we don't, so this should just be inlined here. I.e. made it one big translatable _() message, the only parameters need to be the orig_ref and the "remote_advice" buf. We're also missing a strbuf_release() here. You can just add it to the "cleanup" omitted from the context: strbuf_release(&ftb_cb.remotes_advice);
diff --git a/Documentation/config/advice.txt b/Documentation/config/advice.txt index 063eec2511d..abfac4f664b 100644 --- a/Documentation/config/advice.txt +++ b/Documentation/config/advice.txt @@ -126,4 +126,8 @@ advice.*:: Advice shown when either linkgit:git-add[1] or linkgit:git-rm[1] is asked to update index entries outside the current sparse checkout. + ambiguousFetchRefspec:: + Advice shown when branch tracking relationship setup fails due + to multiple remotes' refspecs mapping to the same remote + tracking namespace in the repo. -- diff --git a/advice.c b/advice.c index 1dfc91d1767..686612590ec 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 601265fd107..3d68c1a6cb4 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 5d20a2e8484..4a3a77aa338 100644 --- a/branch.c +++ b/branch.c @@ -16,9 +16,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) { @@ -28,6 +34,7 @@ static int find_tracked_branch(struct remote *remote, void *priv) free(tracking->spec.src); string_list_clear(tracking->srcs, 0); } + strbuf_addf(&ftb->remotes_advice, " %s\n", remote->name); tracking->spec.src = NULL; } @@ -217,6 +224,18 @@ static int inherit_tracking(struct tracking *tracking, const char *orig_ref) return 0; } + +static const char ambiguous_refspec_advice_pre[] = +N_("\n" +"There are multiple remotes whose fetch refspecs map to the remote\n" +"tracking ref";) +static const char ambiguous_refspec_advice_post[] = +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.\n"); + /* * 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 @@ -228,12 +247,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)) return; @@ -248,9 +271,18 @@ static void setup_tracking(const char *new_ref, const char *orig_ref, return; } - 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("%s %s:\n%s\n%s", + _(ambiguous_refspec_advice_pre), + orig_ref, + ftb_cb.remotes_advice.buf, + _(ambiguous_refspec_advice_post) + ); + exit(status); + } if (tracking.srcs->nr < 1) string_list_append(tracking.srcs, orig_ref);