Re: [PATCH] hooks: propose repository owner configured hooks

From: Jonathan Tan <jonathantanmy@google.com>
Date: Jun 21, 2021, 19:36 UTC
Message-ID: <20210621193646.1173220-1-jonathantanmy@google.com>
In-Reply-To: <874kghk906.fsf@evledraar.gmail.com>

Show 11 quoted lines

> > Mentioned in [0], the primary motivation for a magic config branch
> > that lived outside of the worktree was "since the configuration is
> > separate from the code base, it allows you to go back in history or to
> > older branches while preserving "improvements" to the hooks
> > configuration e.g. maybe the project has shifted to using a newer
> > version of a linter."
> 
> It also means that your hooks will forever need to be aware of the union
> of all revisions in the project to work properly, or more likely they'll
> simply break on older revisions as e.g. a hook that needs to handle a
> test directory handles just "t", but it used to be called "tests".

Yes, but I think that's better than hooks not working when we're operating on past commits for whatever reason.

> It's also just un-git-y in *requiring* a remote. A .mailmap,
> .gitattributes etc. all work with a repo you find on-disk, why does
> config & hooks need to be different?

Why is a remote required? The purpose of this proposal is for remotes to suggest hooks, but that doesn't mean that the existing hooks mechanism will no longer work.

> How would a user of such a repo suggest changes to a hook? Now it's
> fairly easy for e.g. .gitattributes, you change it, push a branch, ask
> for it to be merged etc.

It depends on the exact implementation, but in my suggestion [1], it is a patch or a PR on a branch.

[1] https://lore.kernel.org/git/cover.1623881977.git.jonathantanmy@google.com/

> If you want the same hook for all revisions ever having some light logic
> in the hook itself to check/cache that (it's executing arbitrary code
> after all) seems like a saner thing for those who have this "magic
> branch" use-case than to make it the default.

If hooks are per-version, this doesn't work for versions before when the hook was introduced.

[skipping discussion about general remote-suggested config]

Show 5 quoted lines

> As noted by brian m. carlson etc. in the side-thread in
> <YGzrfaSC4xd75j2U@camp.crustytoothpaste.net> the danger is that by
> making this a supported feature git becomes the social-engineering
> vector to fool users into trusting a command like that which they
> otherwise might not have.

This danger is being discussed there, and we're trying to balance this danger against the fact that projects need or want functionality like this (as evidenced by the available tools described in the original email).

Show 6 quoted lines

> > This seems like an OK alternative to allow-listing based on remote,
> > but are you expecting users to add a GPG key to their .gitconfig?
> 
> That instead of saying you trust https://github.com/git/git your primary
> means of interaction with this feature would be saying you, as an
> example, trust Junio's GPG key.

I think this feature can be extended to trusting GPG keys later. Do you think that we should move to a model of trusting a key *instead of* (not "in addition to") a URL?

[snip until summary paragraph]

> So if I trust Junio's key and would like this to Just Work it would be
> better if I clone a mirror of git.git that it works, and I don't have to
> maintain a list of all mirrors myself. Trusting based on content and GPG
> keys gives you that, magic URLs don't.

This seems like scope creep to me - perhaps a GPG key is more flexible than a URL, but I don't think we need this flexibility yet (and we can always add it later).