Re: [RFC] Hook management via 'git hooks' command

On 2019-11-18 at 22:38:19, Emily Shaffer wrote:

On Sat, Nov 16, 2019 at 05:45:01AM +0000, brian m. carlson wrote:

quoted

On 2019-11-16 at 01:11:25, Emily Shaffer wrote:

quoted

Here's my suggestion.

 - Let configs handle the hook list and ordering, via 'hook.{hookname}' which
   can be specified more than once.
   - global config: hook.update = /path/to/firsthook
     user config: hook.update = /path/to/secondhook
     worktree config: hook.update = -/path/to/firsthook #eliminate the firsthook
      call
   - global config: hook.update = /path/to/firsthook
     repo config: hook.update = /path/to/secondhook
     repo config: hook.update = ^/path/to/firsthook #move firsthook execution
       after secondhook for this project

I'd like to hear more about how we handle multiple hooks that are
repo-specific and don't live in the PATH.  This is a common situation
for existing tools that handle multiple hooks, and it's one I think we
should support.

I guess I'm confused about where PATH comes into play. Do you mean that
the hook being run relies on PATH to be set appropriately? I had
envisioned absolute paths in the config.

In past discussions, there's been an assumption that hooks in the config
will be found in PATH if they're not specified explicitly, and I assumed
(apparently incorrectly) that the same would be true here.

I do expect folks are going to want to use non-absolute paths, though.
If I'm invoking the git binary in a hook, I don't care whether it exists
in /usr/bin, /usr/local/bin, ~/bin, or somewhere else entirely.  That's
my shell's problem to figure out.

It's also common for folks to use something like "bundle exec" in a hook
to run a linter that's installed by the local package manager, and in
order to do that, you have to honor PATH to find the package manager's
binary.  That could be in a variety of places, and it could end up
changing dynamically in a session due to a tool like RVM.

quoted

Perhaps we add a "git hook execute" subcommand that executes scripts
from the hook directory.

Can you give an example of when you'd use it? I'm not understanding your
concern and I think an example use case would help me see what you mean.

Sure.  Currently, if I have pre-push hook, it lives in
.git/hooks/pre-push.  Now, I want to have multiple hooks for that which
are specific to my repo.  Maybe I've stuffed them into
.git/hooks/pre-push.d/hook1 and .git/hooks/pre-push.d/hook2, since
that's where my previous hook management system put them, but I now want
to use those same hooks with the config style and drop the old tool.

I'd like to use "git hook execute pre-push.d/hook1" and "git hook
execute pre-push.d/hook2" to automatically find the right hooks and
invoke them.  Similarly, I could use "git hook execute pre-push" to
execute the old pre-push hook.

I suppose if we continue to keep the existing behavior of changing the
directory and we pass the config options to the shell, then we could
just write "$(git config core.hooksPath || echo
.git/hooks)/pre-push.d/hook1" instead, which, while ugly, gets the job
done.  Then we wouldn't need such a command.

quoted

I think this addresses most of the concerns that I had about ordering.
It is still a little suboptimal that we're relying on the ordering of
the config file, since it makes things equivalent to numbered files in
.d directories hard.

Hm, I suppose I don't see why, if the final ordering is determined by
the .git/config (or future replacement for that). Can you explain what
you mean? I want to understand where you're coming from.

One of the benefits to using numbered files in a .d directory is that
you can explicitly control ordering of operations.  For example, maybe I
have a per-repo pre-push hook that performs some checks and rejects a
push if something is off.  I also have a pre-push hook for Git LFS that
pushes the Git LFS objects to the remote server if Git LFS is in use.

In this case, I'd always want my sanity-check hook to run first, and so
I'd number it first.  This is fine if both are per-repo, but if the LFS
hook is global, then it's in the wrong order and my LFS objects are
pushed even though my sanity check failed.

quoted

Possibly as an alternative to the ^ syntax, we could make the hook value
be of the form "key program", where key is a sort key (e.g., a number)
and program is the program to run.  We pick normal config file ordering
if the keys are identical.  Then if the system config wants to have a
hook that always runs at the end, it can do so easily.

Interesting. This way if you decide after you've set up all your configs
just so that you really want something to run at the end of the update
event, you can change one place, not n=number of Git repos. (I do still
want to be able to say "don't run that global hook in this project"
though.)

Exactly.  A global or per-user commit-msg hook may want to see the final
message before approving or rejecting it, and that wouldn't be possible
without some sort of ordering.

I strongly agree that we should still allow removing higher-level hooks.

quoted

In addition, we should be sure that attempting to remove a hook which
doesn't exist isn't an error, since a user might want to set their
~/.gitconfig file to always unset a global hook that may or may not
exist.

I'd be comfortable with a warning when exiting 'git hook edit' mode and
silence when actually running the hook list.

Yeah, that's what I'm going for.
-- 
brian m. carlson: Houston, Texas, US
OpenPGP: https://keybase.io/bk2204