Re: Fetch-hooks

[Leo Gaspard <leo@gaspard.io>]

On 02/10/2018 01:21 PM, Jeff King wrote:
> On Sat, Feb 10, 2018 at 01:37:20AM +0100, Leo Gaspard wrote:
> 
>>> Yeah, tag-following may be a little tricky, because it usually wants to
>>> write to refs/tags/. One workaround would be to have your config look
>>> like this:
>>>
>>>   [remote "origin"]
>>>   fetch = +refs/heads/*:refs/quarantine/origin/heads/*
>>>   fetch = +refs/tags/*:refs/quarantine/origin/tags/*
>>>   tagOpt = --no-tags
>>>
>>> That's not exactly the same thing, because it would fetch all tags, not
>>> just those that point to the history on the branches. But in most
>>> repositories and workflows the distinction doesn't matter.
>>
>> Hmm... apart from the implementation complexity (of which I have no
>> idea), is there an argument against the idea of adding a
>> remote.<name>.fetchTagsTo refmap similar to remote.<name>.fetch but used
>> every time a tag is fetched? (well, maybe not exactly similar to
>> remote.<name>.fetch because we know the source is going to be
>> refs/tags/*; so just having the part of .fetch past the ':' would be
>> more like what's needed I guess)
> 
> I don't think it would be too hard to implement, and is at least
> logically consistent with the way we handle tags.
> 
> If we were designing from scratch, I do think this would all be helped
> immensely by having more separation of refs fetched from remotes. There
> was a proposal in the v1.8 era to fetch everything for a remote,
> including tags, into "refs/remotes/origin/heads/",
> "refs/remotes/origin/tags/", etc. And then we'd teach the lookup side to
> look for tags in each of the remote-tag namespaces.
> 
> I still think that would be a good direction to go, but it would be a
> big project (which is why the original stalled).

Hmm... would this also drown the remote.<name>.fetch map? Also, I think
this behavior could be emulated with fetch and fetchTagsTo, and it would
look like:
[remote "my-remote"]
    fetch = +refs/heads/*:refs/remotes/my-remote/heads/*
    fetchTagsTo = refs/remotes/my-remote/tags/*
The remaining issue being to teach the lookup side to look for tags in
all the remote-tag namespaces (and the fact it's a breaking change).

That said, actually I just noticed an issue in the “add a
remote.<name>.fetch option to fetch to refs/quarantine then have the
post-fetch hook do the work”: it means if I run `git pull`, then:
 1. The remote references will be pulled to refs/quarantine/...
 2. The post-fetch hook will copy the accepted ones to refs/remotes/...
 3. The `git merge FETCH_HEAD` called by pull will merge FETCH_HEAD into
local branches... and so merge from refs/quarantine.

A solution would be to also update FETCH_HEAD in the post-fetch hook,
but then we're back to the issue raised by Junio after the “*HOWEVER*”
of [1]: the hook writer has to maintain consistency between the “copied”
references and FETCH_HEAD.

So, when thinking about it, I'm back to thinking the proper hook
interface should be the one of the tweak-fetch hook, but its
implementation should make it not go crazy on remote servers. And so
that the implementation should do all this refs/quarantine wizardry
inside git itself.

So basically what I'm getting at at the moment is that git fetch should:
 1. fetch all the references to refs/real-remotes/<name>/{insert here a
copy of the refs/ tree of <name>}
 2. figure out what the “expected” names for these references will by,
by looking at remote.<name>.fetch (and at whether --tags was passed)
 3. for each “expected” name,
     1. if a tweak-fetch hook is present, call it with the
refs/real-remotes/... refname and the “expected end-name” from
remote.<name>.fetch
     2. based on the hook's result, potentially to move the “expected
end-name” to another commit than the one referenced by refs/real-remotes/...
     3. move the “expected” name to the commit referenced in
refs/real-remotes

Which would make the tweak-fetch hook interface simpler (though more
restrictive, but I don't have any real use case for the other change
possibilities) than it is now:
 1. feed the hook with lines of
“refs/real-remotes/my-remote/heads/my-branch
refs/remotes/my-remote/my-branch” (assuming remote.my-remote.fetch is
“normal”) or “refs/real-remotes/my-remote/tags/my-tag refs/tags/my-tag”
(if my-tag is being fetched from my-remote)
 2. read lines of “<refspec> refs/remotes/my-remote/my-branch”, that
will re-point my-branch to <refspec> instead of
refs/real-remotes/my-remote/heads/my-branch. In order to avoid any
issue, the hook is not allowed to pass as second output a reference that
was not passed as second input.

This way, the behavior of the tweak-fetch hook is reasonably preserved
(at least for my use case), and there is no additional load on the
servers thanks to the up-to-date references being stored in
refs/real-remotes/<name>/<refspec>

Does this reasoning make any sense?


[1] https://marc.info/?l=git&m=132478296309094&w=2