On Fri, Sep 13, 2024 at 10:28 PM Junio C Hamano <gitster@xxxxxxxxx> wrote: > > Shubham Kanodia <shubham.kanodia10@xxxxxxxxx> writes: > > > 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. > > Oh, that is 100% agreeable. All I wanted to caution you about was > what should happen when remote.origin.prefetchref in the above is > replaced to something like: > > [remote "origin"] > fetch = +refs/heads/*:refs/remotes/origin/* > prefetchref = refs/notes/* > > That is, if your refspec used for your real fetch (i.e. "git fetch" > without the "--prefetch" option) does not fetch anything from > "refs/notes/" hierarchy, prefetching from the hierarchy does not > help the real fetch. I do not have a strong preference between > marking it as an error and silently ignoring the prefetch but > leaning towards the latter, and that is why my suggestion to > implement this new "prefetchref" as something that extends the > existing filter_prefetch_refspec(), which already filters out > refspec that fetches from the refs/tags/ namespace (and the ones > that do not store by having NULL in the .dst side). > > > 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. > > I used the verb "reject" to mean "filter out", just like a refspec > with left-hand-side that begins with "refs/tags/" is filtered out > in the current filter_prefetch_refspec(). And that is exactly what > we want to achieve our goal here. > > IOW, you would > > * read their ref advertisement, and pick only the ones that have a > matching pattern in the left-hand-side of a remote.$name.fetch > element. With a more recent protocol, remote.$name.fetch may > have already participated in narrowing what they advertise to > begin with, but the end result is the same. > > * give it to filter_prefetch_refspec(). > > * filter_prefetch_refspec() inspects the refspec elements, and > rejects ones with no right-hand-side, and ones with > left-hand-side that begin with refs/tags/. The current code > without your patch already works this way up to this point. > > * We extend the above filtering so that in addition to the two > kinds we currently reject, reject the ones that do not match the > prefetchref criteria. This is what is needed to implement > "prefetchref configuration limits the set of refs that get > prefetched". > > And what you quoted is a beginning of how "prefetchref configuration > limits". It cannot be "add to what filter_prefetch_refspec() did", > like done by the implementation in the patch we are discussing. > > If your configuration were this: > > [remote "origin"] > fetch = +refs/heads/*:refs/remotes/origin/* > > you would want a way to say things like > > (1) I want to prefetch everything I usually fetch > > (2) Among the ones I usually fetch, I only want to prefetch master > and next branches. > > (3) I want to prefetch only refs/heads/jk/* branches, but not > refs/heads/jk/wip/* branches. > > (4) I want to prefetch everything I usually fetch, except for > refs/heads/wip/* branches. > > The case (1) is the simplest. You will leave .prefetchref empty. > > For the case (2), you would write something like > > [remote "origin"] > prefetchref = refs/heads/master > prefetchref = refs/heads/next > > So, when your prefetchref has all positive patterns, after the > existing conditional in filter_prefetch_refspec() passes a refspec > whose right-hand-side (i.e., .dst) is not NULL and whose > left-hand-side (i.e., .src) does not begin with "refs/tags/", you > further inspect and make sure it matches one of these prefetchref > patterns. In example (2), if they advertised master, next, and seen > branches, refs/heads/seen would be filtered out because it matches > neither of the two patterns, so we would end up prefetching master > and next branches. > > For the case (3), you would want to say something like > > [remote "origin"] > prefetchref = refs/heads/jk/* > prefetchref = !refs/heads/jk/wip/* > > Now your prefetchref has some negative pattern. When filtering what > the existing conditional in filter_prefetch_refspec() passed, you'd > inspect the refspec element and see if it matches any of the > positive patterns, and also if it does not match any of the negative > ones. refs/heads/next does not match any positive ones and gets > rejected. refs/heads/jk/main does match the positive pattern > 'refs/heads/jk/*', and it does not match the negative pattern > 'refs/heads/jk/wip/*', so it passes and will get prefetched. > > For the case (4), you would write something like > > [remote "origin"] > prefetchref = !refs/heads/wip/* > > There is no positive pattern, so if you blindly apply the rule you > used for (3) above, everything will get rejected, which is not what > you want. refs/heads/main does not match any positive patterns > (because there are no positive patterns given), but it does not > match any negative ones, so it passes and will get prefetched. > > The condition to implement the above four cases (which I think > covers all the cases we care about, but I won't guarantee it is > exhaustive---you'd need to sanity check) would be > > - If there is 1 or more positive prefetchref patterns, the refspec > element must match one of them to be considered for the next > rule. Otherwise, it will not be prefetched. > > - If the refspec element matches any of negative prefetchref > patterns, it will not be prefetched. > If we're trying to determine if a pattern (remote.<remote>.prefetchref) is a subset of another or not (remote.<remote>.fetch) (to not accidentally expand the scope beyond `fetch`), we'd need a function that does that pattern-to-pattern. Are you aware of any existing functions that do so? I see `wildmatch`, but that is only used for pattern to full-text comparisons.
