Re: [PATCH] remote: introduce config to set prefetch refs

From: Shubham Kanodia <shubham.kanodia10@gmail.com>
Date: Sep 13, 2024, 06:16 UTC
Message-ID: <CAG=Um+3WSckyZ2P2o2igQr4hbMyMNTDZ_kqjrfdufvL6hUhMjA@mail.gmail.com>
In-Reply-To: <xmqq5xr4r818.fsf@gitster.g>

On Tue, Sep 10, 2024 at 12:03 AM Junio C Hamano <gitster@pobox.com> wrote:

Show 22 quoted lines

>
> Shubham Kanodia <shubham.kanodia10@gmail.com> writes:
>
> > How should we handle the related `remote.<remote-name>.fetch` config?
>
> The get_ref_map() helper is what the rest of the code interacts with
> configured refspec.  remote->fetch is handled there and goes through
> the same filter_prefetch_refspec().
>
> > In an earlier discussion, it was discussed that —
> > `.prefetchref` should override `.fetch` completely (instead of
> > patching it) which makes sense to me.
>
> Maybe it made sense to you back when it was discussed, but after
> seeing the current code (before applying this patch), specifically
> what happens in filter_prefetch_refspec(), it no longer makes much
> sense to me.
>
> Especially it is nonsense to allow .prefetchref to widen the set of
> refs that are being prefetched beyond what is normally fetched, so
> we should look at a design that easily allows such a configuration
> with strong suspicion, I would think.

Ideally, a repository should be able to specify (say):

remote.origin.fetch=+refs/heads/*:refs/remotes/origin/* remote.origin.prefetchref=refs/heads/main

This configuration would maintain the normal behavior for fetches, but only prefetch the main branch. The rationale for this is that the main branch typically serves as the HEAD from which future branches will be forked in an active repository.

Regarding:

> If prefetch_refs contains only positive patterns, then a refspec whose source
> doesn't match any of these patterns is rejected.

Simply rejecting a source refspec pattern in `remote.<remote>.fetch` wouldn't achieve our goal here. Ideally, we'd need to create a subset of it.

What we're looking for is essentially a pattern intersection between the (fetch) `refs/heads/*` and the (prefetchref) `refs/heads/main`, which in this case would result in `refs/heads/main`. However, if I understand correctly, performing such pattern intersections isn't straightforward in the `filter_prefetch_refspec` function (let me know if there' prior art for pattern intersection)

I also believe that allowing negative refs might complicate things without providing a clear use case. For instance, how would we handle the intersection of `fetch` and `prefetchref` if `prefetchref` contained both positive and negative patterns?

Given that both `fetch` and `prefetchref` could have multiple patterns, it might be simpler and more intuitive for users if we adopt an "A wins over B" approach. However, I'm interested in hearing your thoughts on this matter.

Perhaps I should link to the earlier discussion here — Message ID (CAG=Um+1wTbXn_RN+LOCrpZpSNR_QF582PszxNyhz5anVHtBp+w@mail.gmail.com)