Re: Fetch-hooks

From: Leo Gaspard <leo@gaspard.io>
Date: Feb 14, 2018, 02:02 UTC
Message-ID: <96dd7fb3-849b-8de6-7c3a-cd6bde9da432@gaspard.io>
In-Reply-To: <20180214013520.GA25188@sigill.intra.peff.net>

On 02/14/2018 02:35 AM, Jeff King wrote:

> On Sat, Feb 10, 2018 at 07:36:47PM +0100, Leo Gaspard wrote:
>> [...]
> I think there may have been some more concrete proposals after that, but
> that's what I was able to dig up quickly.

Thanks! Though it looks way above my knowledge of git internals as well as time available, it looks like a great project, and I hope it someday succeeds!

Show 27 quoted lines

>> 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.
> 
> Good point. You can't munge FETCH_HEAD by playing with refspecs.
> 
> I am starting to come around to the idea that "pre-fetch" might be the
> best way to do what you want. Not to rewrite refs, but perhaps to simply
> reject them. In the same way that we allow pre-receive to reject pushed
> refs (both are, after all, the final check on admitting new history into
> the repository, just in opposite directions).
> 
>> 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 does anybody actually want to be able to adjust the refs as they pass
> through? It really sounds like you just want to be able to reject or not
> reject the fetch. And that rejecting would be the uncommon case, so it's
> OK to just abort the whole operation and expect the user to figure it
> out.

This completely fits my use case (modulo the fact that it's more similar to the `update` hook than to `pre-receive` I think, as verifying the signature requires the objects to already have been downloaded), indeed, though I'm not sure it would have fit Joey's (based on my understanding, adding a merge was what was originally asked for).

Actually, I'm wondering if the existing semantics of `update` could not be reused for the `pre-fetch`. Would it make sense to just call `update` during a fetch in the same way as during a receive-pack? That said it likely makes this a breaking change, though it's maybe unlikely that a repository is used both for fetching and for receive-pack'ing, it could happen.

So the proposal as I understand it would currently be adding a `fetch-update` hook that does exactly the same thing as `update` but for `fetch`. This solves my use case in a nice way, though it likely doesn't solve Joey's original one (which has been taken care of by a wrapper since then).

What do you all think about it?