Re: should git maintenance prefetch be taught to honor remote.fetch refspec?

From: Derrick Stolee <stolee@gmail.com>
Date: Apr 5, 2021, 13:20 UTC
Message-ID: <e75b1e72-6c9f-d466-ac52-24b324b44b3c@gmail.com>
In-Reply-To: <xmqqft057ijc.fsf@gitster.g>

On 4/4/2021 7:10 PM, Junio C Hamano wrote:

Show 28 quoted lines

> Derrick Stolee <stolee@gmail.com> writes:
>> On 4/2/2021 5:19 PM, Junio C Hamano wrote:
>>> I do not recommend unparsed refspec and textually munging, by the
>>> way.  Doesn't
>>>
>>> 	git fetch master:remotes/origin/master
>>>
>>> first parse to normalize the src/dst sides to turn it into
>>>
>>> 	git fetch refs/heads/master:refs/remotes/origin/master
> 
> I tried to jug my memory in this area a bit by reading the relevant
> code.  For non-wildcard refspec, e.g. with
> 
>     [remote "origin"]
> 	url = ../git.git/
> 	fetch = master:remotes/origin/master
> 	tagopt = --no-tags
> 
> you'd get
> 
>     $ git fetch -v
>     From ../git
>      * [new branch]            master     -> origin/master
>     $ git for-each-ref
>     2e36527f23b7f6ae15e6f21ac3b08bf3fed6ee48 commit	refs/remotes/origin/master
> 
> It all happens inside remote.c::get_fetch_map(), I think.

I see there that some refs are being matched with some expectation of remote refs inside get_remote_ref() (which calls find_ref_by_name_abbrev() as a helper). There does not appear to be any place that modifies the refs directly in general, and instead refs are matched from the refspec using standard short-ref rules.

This is particularly shown in examples like "git push topic" never modifying the push-refspec from having 'src' equal to "topic" and instead the ref machinery discovers that "topic" really means "refs/heads/topic".

I took your advice to not munge raw refpsecs and instead worked directly on the 'dst' of the struct refspec_item. This required adding a method that formats a refspec, which I test carefully. I spent about an hour trying to have parse_refspec() add the appropriate "refs/" prefix, but it becomes difficult when we intend to make a distinction between "refs/heads/" and other sub-areas within "refs/". Finally, I punted on that conversion and made the logic in 'prefetch' extremely obvious:

1. If the refspec's 'dst' starts with "refs/", then replace
   that prefix with "refs/prefetch/".

2. If the refspec's 'dst' does not start with "refs/", then
   concatenate "refs/prefetch/" and 'dst'.

This will keep a roughly-equivalent partition of refs (some might have previously collided that will not any more).

I have posted my patch series [1], so please take a look. It builds up the infrastructure to properly test such a refspec expansion, if we wish to do so.

[1] https://lore.kernel.org/git/pull.924.git.1617627856.gitgitgadget@gmail.com/

Thanks, -Stolee