Re: [PATCH v13 0/6] fetch: avoid fetching every branch of a new remote in a shallow repo

[Phillip Wood <phillip.wood123@gmail.com>]

On 11/11/2026 17:37, Junio C Hamano wrote:
> Phillip Wood <phillip.wood123@gmail.com> writes:
>> On 11/11/2026 08:31, Harald Nordgren wrote:
>>> Hi, could I get some (hopefully final) reviews here?
>>
>> The diffstat for this series is
>>
>> 12 files changed, 593 insertions(+), 33 deletions(-)
>>
>> which will take a considerable amount of time to review properly. Yet
>> the cover letter which should explain the problem being solved, how it
>> is solved (i.e. the design and any trade-offs that entails, alternatives
>> that were discarded etc.) and anything notable in the implementation is just
>>
>> Avoid fetching every branch of a new remote in a shallow repo.
>>
>> I've no idea from that how these patches relate to the discussion on
>> separating "which refs to fetch" from "where to store those refs" or
>> anything else. If you want people to volunteer their time to review your
>> patches you need to make a effort to explain what those patches do.
> 
> I would say that it is a fair comment.
> 
> I understand what the first three wants to do.
> 
>   1. We have had for a long time "git fetch --refmap=from:to remote refs..."
>      given on the command line to decide how refs fetched are stored
>      separately from what refs are fetched.  This allows a configuration
>      to allow you to pretend that you give it all the time.
> 
>   2. Preliminary refactoring of a helper that will gain second
>      callsite in the next step.
> 
>   3. This allows "git fetch --refmap=from:to remote" that does not
>      specify what to fetch (contrast it with 1. above) to default to
>      fetch refs that our local branches build on.  The net effect
>      with configured remote.<remote>.fetchRefmap is that "git fetch
>      remote" would only fetch and store selected refs from them, only
>      the ones that we "care about" (aka "build on").

Thanks for the summary. I'm still not keen on patch 3 as I said in [1] I 
think it is a mistake to use the same variable for both "what to fetch" 
and "where to store it". After patch 1 we have the 
remote.<name>.fetchRefmap for "where to store it" and can use use 
remote.<name>.fetch for "what to fetch" (though I am concerned about the 
doubling up that happens when "fetchRefmap" is set and "fetch" is a 
refspec). We don't need a special case for when there are no refspecs on 
the command line - we can use the default values of those config 
settings instead and the user can configure the behavior that they want.

> I do not have strong opinion on the other 3 patches.  4. and 5. may
> help ergonomics by allowing to set up refmap from the command line
> option, but from my point of view it looks more like icing on the
> cake.

I think having a way to set up fetching a limited set of branches is 
worthwhile, but it does not need to be part of the same series.

> When we are working with only a subset of their branches, the first
> thing that comes to my mind is a "matching push" that does not need
> any fancy @{push} support.  Because of that, I am not interested in
> 6. very much.

I've not looked at what that patch does

> In any case, this topic one is keeping a topic by another author
> waiting to settle, so we have stronger incentive to see it move past
> the finish line.

What topic is that? - I can't see anything marked as depending on this 
in the latest "what's cooking" email. Would merging the first part of 
this series unblock the other one?

Thanks

Phillip