Re: Fetch-hooks

From: Leo Gaspard <leo@gaspard.io>
Date: Feb 10, 2018, 00:37 UTC
Message-ID: <3de8dec0-12c9-56e2-5902-97755f78ab50@gaspard.io>
In-Reply-To: <20180210001317.GA26856@sigill.intra.peff.net>

On 02/10/2018 01:13 AM, Jeff King wrote:

Show 20 quoted lines

> On Sat, Feb 10, 2018 at 12:49:31AM +0100, Leo Gaspard wrote:
>> So the changes that are required are:
>>  * Adding a notification-only post-fetch hook
>>  * For handling tags, there is a need to have a refmap for tags. Maybe
>> adding a remote.my-remote.fetchTags refmap, that would be used when
>> running with --tags, and having it default to “refs/tags/*:refs/tags/*”
>> to keep the current behavior by default?
> 
> 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)

The issue with your solution is that if the user runs 'git fetch --tags', he will get the (potentially compromised) tags directly in his refs/tags/.

> (By the way, the I specifically chose the name "refs/quarantine" instead
> of anything in "refs/remotes" because we'd want to make sure that the
> "git checkout" DWIM behavior cannot accidentally pull from quarantine).

(Indeed, I understood after reading it, and would likely not have thought of it otherwise, thanks!)

Show 10 quoted lines

>> The only remaining issue I can think of is: How do we avoid the issue
>> of the
>> trigger-only-hook-inciting-bad-behavior-by-hook-authors-who-really-want-modification
>> raised in the side-thread that Junio wrote in [1]? Maybe just writing
>> in the documentation that the hook should use a quarantine-like
>> approach if it wants modification would be enough to not have hook
>> authors try to modify the ref in the post-fetch hook?
> 
> I don't have a silver bullet there. Documenting the "right" way at least
> seems like a good first step.

So long as it's not a merge-blocker it's good with me! (but then I'm likely not the one who's going to be pointed at when things go wrong in a hook, so I'm clearly biased on this matter)