| Message ID | pull.1183.v5.git.1648624810866.gitgitgadget@gmail.com (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | [v5] tracking branches: add advice to ambiguous refspec error | expand |
On Wed, Mar 30 2022, Tao Klerks via GitGitGadget wrote: > From: Tao Klerks <tao@klerks.biz> > + case 2: > + // there are at least two remotes; backfill the first one Nit: I think it's been Junio's preference not to introduce C99 comments, despite other C99 features now being used (and I think it should work in practice as far as portability goes, see https://lore.kernel.org/git/87wnmwpwyf.fsf@evledraar.gmail.com/) > @@ -219,6 +233,7 @@ static int inherit_tracking(struct tracking *tracking, const char *orig_ref) > return 0; > } > > + Stray whitespace? > /* > * Used internally to set the branch.<new_ref>.{remote,merge} config > * settings so that branch 'new_ref' tracks 'orig_ref'. Unlike > @@ -232,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, > + .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 +271,38 @@ 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); This isn't per-se new, but I wonder if while we're at it we shold just quote '%s' here, which we'd usually do. I.e. this message isn't new, but referring again to "ref %s" (and not "ref '%s'") below is. > + 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) { Nit: drop braces, not needed. > + /* > + * 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); > + } > + Per the TRANSLATORS comments in get_short_oid(), it's also good to have one here in front of advice(). E.g.: /* * 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 > + ); Nit: The usual style for multi-line arguments is to "fill" lines until you're at 79 characters, so these last three lines (including the ");") can all go on the "tracking namespaces" line (until they're at 79, then wrap)> > + strbuf_release(&remotes_advice); > + } > + exit(status); > + } Other than the minor nits noted above this version looks good to me, and I think addresses both the translation concerns I brought up, and the "let's not do advice() work if we don't need it" concern by Junio et al. > 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) > > base-commit: abf474a5dd901f28013c52155411a48fd4c09922
On Wed, Mar 30, 2022 at 3:27 PM Ævar Arnfjörð Bjarmason <avarab@gmail.com> wrote: > > > On Wed, Mar 30 2022, Tao Klerks via GitGitGadget wrote: > > Nit: I think it's been Junio's preference not to introduce C99 comments, Argh, my apologies, bad habits. I wonder whether anyone has any tooling to help catch this kind of thing - I'll ask in GitGitGadget, anyway. > > @@ -219,6 +233,7 @@ static int inherit_tracking(struct tracking *tracking, const char *orig_ref) > > return 0; > > } > > > > + > > Stray whitespace? Thx, I will check more carefully next time. > > > /* > > * Used internally to set the branch.<new_ref>.{remote,merge} config > > * settings so that branch 'new_ref' tracks 'orig_ref'. Unlike > > @@ -232,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, > > + .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 +271,38 @@ 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); > > This isn't per-se new, but I wonder if while we're at it we shold just > quote '%s' here, which we'd usually do. I.e. this message isn't new, but > referring again to "ref %s" (and not "ref '%s'") below is. > I will fix the below, but I would default to not changing the above unless someone thinks we should (not sure what the expectations are around changing error messages unnecessarily) > > + 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) { > > Nit: drop braces, not needed. Thx, will do. > > > + /* > > + * 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); > > + } > > + > > Per the TRANSLATORS comments in get_short_oid(), it's also good to have > one here in front of advice(). E.g.: > > /* > * TRANSLATORS: The second argument is a \n-delimited list of > * duplicate refspecs, composed above. > */ > Will do, thx. > > + 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 > > + ); > > Nit: The usual style for multi-line arguments is to "fill" lines until > you're at 79 characters, so these last three lines (including the ");") > can all go on the "tracking namespaces" line (until they're at 79, then > wrap)> > Yep, another oversight, thx. > > + strbuf_release(&remotes_advice); > > + } > > + exit(status); > > + } > > Other than the minor nits noted above this version looks good to me, and > I think addresses both the translation concerns I brought up, and the > "let's not do advice() work if we don't need it" concern by Junio et al. > Awesome, thx!
On Wed, Mar 30, 2022 at 4:23 PM Tao Klerks <tao@klerks.biz> wrote: > > On Wed, Mar 30, 2022 at 3:27 PM Ævar Arnfjörð Bjarmason > <avarab@gmail.com> wrote: > > > > > > On Wed, Mar 30 2022, Tao Klerks via GitGitGadget wrote: > > > > > + if (tracking.matches > 1) { > > > + int status = die_message(_("not tracking: ambiguous information for ref %s"), > > > + orig_ref); > > > > This isn't per-se new, but I wonder if while we're at it we shold just > > quote '%s' here, which we'd usually do. I.e. this message isn't new, but > > referring again to "ref %s" (and not "ref '%s'") below is. > > > > I will fix the below, but I would default to not changing the above > unless someone thinks we should (not sure what the expectations are > around changing error messages unnecessarily) I take this back. I will update both - the change is in a "variable" part of the message anyway, and it's hard to imagine any tool actively/purposefully parsing the ref out of the message and being caught out by the new quotes. Any coordinating tool would already know what ref was being branched / having its tracking remote info updated. > > > > + * 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); > > > + } > > > + > >
On Wed, Mar 30 2022, Tao Klerks wrote: > On Wed, Mar 30, 2022 at 4:23 PM Tao Klerks <tao@klerks.biz> wrote: >> >> On Wed, Mar 30, 2022 at 3:27 PM Ævar Arnfjörð Bjarmason >> <avarab@gmail.com> wrote: >> > >> > >> > On Wed, Mar 30 2022, Tao Klerks via GitGitGadget wrote: >> > >> > > + if (tracking.matches > 1) { >> > > + int status = die_message(_("not tracking: ambiguous information for ref %s"), >> > > + orig_ref); >> > >> > This isn't per-se new, but I wonder if while we're at it we shold just >> > quote '%s' here, which we'd usually do. I.e. this message isn't new, but >> > referring again to "ref %s" (and not "ref '%s'") below is. >> > >> >> I will fix the below, but I would default to not changing the above >> unless someone thinks we should (not sure what the expectations are >> around changing error messages unnecessarily) > > I take this back. I will update both - the change is in a "variable" > part of the message anyway, and it's hard to imagine any tool > actively/purposefully parsing the ref out of the message and being > caught out by the new quotes. Any coordinating tool would already know > what ref was being branched / having its tracking remote info updated. Thanks, I'd be fine either way, it was just a suggestion. Aside from what we do here we don't support third-party tooling that's grepping our specific human-readable output, so changing any such messaging is OK.
Ævar Arnfjörð Bjarmason <avarab@gmail.com> writes: > On Wed, Mar 30 2022, Tao Klerks via GitGitGadget wrote: > >> From: Tao Klerks <tao@klerks.biz> > >> + case 2: >> + // there are at least two remotes; backfill the first one > > Nit: I think it's been Junio's preference not to introduce C99 comments, I often mention the rule to new contributors, simply because it has been that way in our code base, regardless of what my personal preference might be, and sticking to the style will be more consistent. It's not like I am forcing my personal preference on developers. Do not mislead new people into thinking so. It is especially irritating to see ... > despite other C99 features now being used (and I think it should work in > practice as far as portability goes, see > https://lore.kernel.org/git/87wnmwpwyf.fsf@evledraar.gmail.com/) ... a mention like this, when you know that it is not about portability but is about consistency, and also you know I've mentioned more than once on the list if we want to loosen some written CodingGuidelines rules, especially those that tools do not necessarily catch. >> + if (tracking.matches > 1) { >> + int status = die_message(_("not tracking: ambiguous information for ref %s"), >> + orig_ref); > > This isn't per-se new, but I wonder if while we're at it we shold just > quote '%s' here, which we'd usually do. I.e. this message isn't new, but > referring again to "ref %s" (and not "ref '%s'") below is. Good. >> + "To support setting up tracking branches, ensure that\n" >> + "different remotes' fetch refspecs map into different\n" >> + "tracking namespaces."), >> + orig_ref, >> + remotes_advice.buf >> + ); > > Nit: The usual style for multi-line arguments is to "fill" lines until > you're at 79 characters, so these last three lines (including the ");") > can all go on the "tracking namespaces" line (until they're at 79, then > wrap)> I didn't know about the magic "79" number. It makes the resulting source code extremely hard to read, though, while making it easier to grep for specific messages.
On Wed, Mar 30 2022, Junio C Hamano wrote: > Ævar Arnfjörð Bjarmason <avarab@gmail.com> writes: > >> On Wed, Mar 30 2022, Tao Klerks via GitGitGadget wrote: >> >>> From: Tao Klerks <tao@klerks.biz> >> >>> + case 2: >>> + // there are at least two remotes; backfill the first one >> >> Nit: I think it's been Junio's preference not to introduce C99 comments, > > I often mention the rule to new contributors, simply because it has > been that way in our code base, regardless of what my personal > preference might be, and sticking to the style will be more > consistent. It's not like I am forcing my personal preference on > developers. Do not mislead new people into thinking so. It is > especially irritating to see ... > >> despite other C99 features now being used (and I think it should work in >> practice as far as portability goes, see >> https://lore.kernel.org/git/87wnmwpwyf.fsf@evledraar.gmail.com/) > > ... a mention like this, when you know that it is not about > portability but is about consistency, and also you know I've > mentioned more than once on the list if we want to loosen some > written CodingGuidelines rules, especially those that tools do not > necessarily catch. I know, and I didn't mean to imply that you were standing in the way of C99 progress or anything like that. Personally, I much prefer not to have //-style comments introduced. I merely mentioned the portability concern because I thought it helped give a relatively new contributor some context. I.e. that despite any new developments on the C99 front they might discover this particular thing is stylistic and not about portability (although that may also have been a reason in the past). >>> + if (tracking.matches > 1) { >>> + int status = die_message(_("not tracking: ambiguous information for ref %s"), >>> + orig_ref); >> >> This isn't per-se new, but I wonder if while we're at it we shold just >> quote '%s' here, which we'd usually do. I.e. this message isn't new, but >> referring again to "ref %s" (and not "ref '%s'") below is. > > Good. > >>> + "To support setting up tracking branches, ensure that\n" >>> + "different remotes' fetch refspecs map into different\n" >>> + "tracking namespaces."), >>> + orig_ref, >>> + remotes_advice.buf >>> + ); >> >> Nit: The usual style for multi-line arguments is to "fill" lines until >> you're at 79 characters, so these last three lines (including the ");") >> can all go on the "tracking namespaces" line (until they're at 79, then >> wrap)> > > I didn't know about the magic "79" number. It makes the resulting > source code extremely hard to read, though, while making it easier > to grep for specific messages. I'm referring to the "80 characters per line", but omitted the \n, but yeah, I should have just said 80.
Ævar Arnfjörð Bjarmason <avarab@gmail.com> writes: >>>> + "To support setting up tracking branches, ensure that\n" >>>> + "different remotes' fetch refspecs map into different\n" >>>> + "tracking namespaces."), >>>> + orig_ref, >>>> + remotes_advice.buf >>>> + ); >>> >>> Nit: The usual style for multi-line arguments is to "fill" lines until >>> you're at 79 characters, so these last three lines (including the ");") >>> can all go on the "tracking namespaces" line (until they're at 79, then >>> wrap)> >> >> I didn't know about the magic "79" number. It makes the resulting >> source code extremely hard to read, though, while making it easier >> to grep for specific messages. > > I'm referring to the "80 characters per line", but omitted the \n, but > yeah, I should have just said 80. No, what I meant was that you do not want the rule to be to cut *AT* exactly the column whatever random rule specifies, which would result in funny wrapping in the middle of the word, e.g. "To support setting up tracking branches, ensure that diff" "erent remotes' fetch refspecs map into different tracking" " namespaces." and "at 79, then wrap" somehow sounded to me like that. I do not think you meant to imply that (instead, I think you meant to suggest "wrap the line so that the string constant is not longer than 79 columns"), but it risks to be mistaken by new contributors. FWIW, I'd actually prefer to see both the sources to be readable by wrapping to keep the source code line length under certain limit (the current guideline being 70-something), counting the leading indentation, and at the same time keep it possible and easy to grep messages in the source. That requires us to notice when our code has too deeply nested, resulting in overly indented lines, and maintain the readability (refatoring the code may be a way to help in such cases).
On Wed, Mar 30 2022, Junio C Hamano wrote: > Ævar Arnfjörð Bjarmason <avarab@gmail.com> writes: > >>>>> + "To support setting up tracking branches, ensure that\n" >>>>> + "different remotes' fetch refspecs map into different\n" >>>>> + "tracking namespaces."), >>>>> + orig_ref, >>>>> + remotes_advice.buf >>>>> + ); >>>> >>>> Nit: The usual style for multi-line arguments is to "fill" lines until >>>> you're at 79 characters, so these last three lines (including the ");") >>>> can all go on the "tracking namespaces" line (until they're at 79, then >>>> wrap)> >>> >>> I didn't know about the magic "79" number. It makes the resulting >>> source code extremely hard to read, though, while making it easier >>> to grep for specific messages. >> >> I'm referring to the "80 characters per line", but omitted the \n, but >> yeah, I should have just said 80. > > No, what I meant was that you do not want the rule to be to cut *AT* > exactly the column whatever random rule specifies, which would > result in funny wrapping in the middle of the word, e.g. > > "To support setting up tracking branches, ensure that diff" > "erent remotes' fetch refspecs map into different tracking" > " namespaces." > > and "at 79, then wrap" somehow sounded to me like that. I do not > think you meant to imply that (instead, I think you meant to suggest > "wrap the line so that the string constant is not longer than 79 > columns"), but it risks to be mistaken by new contributors. > > FWIW, I'd actually prefer to see both the sources to be readable by > wrapping to keep the source code line length under certain limit > (the current guideline being 70-something), counting the leading > indentation, and at the same time keep it possible and easy to grep > messages in the source. > > That requires us to notice when our code has too deeply nested, > resulting in overly indented lines, and maintain the readability > (refatoring the code may be a way to help in such cases). Yes, I didn't mean to say it was a hard rule. In particular as this code has the strings themselves over 80 characters. It can be good to break that guideline when it doesn't help readability. I just meant that this made sense as a fix-up, in this case: diff --git a/branch.c b/branch.c index 5c28d432103..4ccf5f79e83 100644 --- a/branch.c +++ b/branch.c @@ -282,10 +282,8 @@ static void setup_tracking(const char *new_ref, const char *orig_ref, "\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 - ); + "tracking namespaces."), orig_ref, + ftb_cb.remotes_advice.buf); exit(status); } Which I'd also be tempted to do as: diff --git a/branch.c b/branch.c index 5c28d432103..b9f6fda980b 100644 --- a/branch.c +++ b/branch.c @@ -283,9 +283,7 @@ static void setup_tracking(const char *new_ref, const char *orig_ref, "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 - ); + orig_ref, ftb_cb.remotes_advice.buf); exit(status); } But I find it generally helpful to do it consistently when possible, as when running into merge conflicts it makes things easier.
diff --git a/Documentation/config/advice.txt b/Documentation/config/advice.txt index c40eb09cb7e..343d271c707 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 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 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..e6ab50fff41 100644 --- 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->spec.src); + /* 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; } @@ -219,6 +233,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 +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, + .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 +271,38 @@ 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); + } + + 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)