Re: [PATCH v13 0/2] checkout: --track=fetch
- From
Junio C Hamano <gitster@pobox.com>
- Date
- Jun 17, 2026, 19:15 UTC
- Message-ID
- <xmqqmrwtuggb.fsf@gitster.g>
- In-Reply-To
- <pull.2281.v13.git.git.1779565714.gitgitgadget@gmail.com>
"Harald Nordgren via GitGitGadget" <gitgitgadget@gmail.com> writes:
Show 17 quoted lines
> * Create a preparatory commit that exposes find_tracking_remote_for_ref() > and advise_ambiguous_fetch_refspec() from branch.c, so checkout can reuse > the same lookup git branch --track uses. > * Use advise_ambiguous_fetch_refspec() for the "multiple remotes match" > case, so the wording matches git branch --track. > > Harald Nordgren (2): > branch: expose helpers for finding the remote owning a tracking ref > checkout: extend --track with a "fetch" mode to refresh start-point > > Documentation/git-checkout.adoc | 17 +- > Documentation/git-switch.adoc | 5 +- > branch.c | 96 ++++++----- > branch.h | 16 ++ > builtin/checkout.c | 139 +++++++++++++++- > t/t7201-co.sh | 276 ++++++++++++++++++++++++++++++++ > 6 files changed, 498 insertions(+), 51 deletions(-)
I was scanning "What's cooking" and this topic was the oldest one among the ones marked as "Needs review". Nobody seems to have commented on this iteration.
I am still not convinced that it is a good idea to allow "checkout" to go to the network and muck with remote-tracking branches. The remote-tracking branches are meant to give us solid reference points, and such an on-demand update to move them (which by itself is not bad) and then use the updated result without first seeing what it contains (which is the part I disagree with) cannot lead us to a good place. I suspect that the feature encourages a bad workflow to our end-users.
Having said all that, the changes since v12, in response to earlier review comments to avoid duplicating the remote lookup and ambiguity advice logic, look well executed in this round. This also ensures consistent error messages and behavior between 'git branch --track' and 'git checkout --track=fetch'.
IOW, I find the mechanical implementation fairly solid. I am not sure if we are implementing a good thing, though.
One small thing about [1/2];
diff --git a/branch.h b/branch.h index 3dc6e2a0ff..0aafa1673f 100644 --- a/branch.h +++ b/branch.h @@ -1,9 +1,25 @@ #ifndef BRANCH_H #define BRANCH_H +#include "refspec.h" +#include "string-list.h" + struct repository; struct strbuf; +struct tracking { + struct refspec_item spec; + struct string_list *srcs; + const char *remote; + int matches; +}; + +void find_tracking_remote_for_ref(struct tracking *tracking, + struct string_list *ambiguous_remotes); + +void advise_ambiguous_fetch_refspec(const char *dst, + const struct string_list *ambiguous_remotes); + As we are not embedding any "string_list" instance into any of our struct (we only have a pointer to a struct), unlike the way we embed "struct refspec_item" that requires us to include "refspec.h", we do not need to include "string-list.h". Instead, we only need to declare "struct string_list", just like we declare repository and strbuf. diff --git i/branch.h w/branch.h index 0aafa1673f..c2e6725491 100644 --- i/branch.h +++ w/branch.h @@ -2,8 +2,8 @@ #define BRANCH_H #include "refspec.h" -#include "string-list.h" +struct string_list; struct repository; struct strbuf;