On Fri, Jul 16, 2021 at 11:13:42AM +0200, Ævar Arnfjörð Bjarmason wrote: > > > On Thu, Jul 15 2021, Emily Shaffer wrote: > > > To enable fine-grained options which apply to a single hook executable, > > and to make it easier for a single executable to be run on multiple hook > > events, teach "hookcmd.<alias>.config". These can be configured as > > follows: > > [...] > > diff --git a/Documentation/config/hook.txt b/Documentation/config/hook.txt > > index a97b980cca..5b35170664 100644 > > --- a/Documentation/config/hook.txt > > +++ b/Documentation/config/hook.txt > > @@ -3,6 +3,11 @@ hook.<command>.command:: > > executable on your device, a oneliner for your shell, or the name of a > > hookcmd. See linkgit:git-hook[1]. > > > > +hookcmd.<name>.command:: > > + A command to execute during a hook for which <name> has been specified > > + as a command. This can be an executable on your device or a oneliner for > > + your shell. See linkgit:git-hook[1]. > > + > > [...] > > +Global config > > +---- > > + [hook "post-commit"] > > + command = "linter" > > + command = "~/typocheck.sh" > > + > > + [hookcmd "linter"] > > + command = "/bin/linter --c" > > +---- > > + > > +Local config > > +---- > > + [hook "prepare-commit-msg"] > > + command = "linter" > > + [hook "post-commit"] > > + command = "python ~/run-test-suite.py" > > +---- > > + > > +With these configs, you'd then run post-commit hooks in this order: > > + > > + /bin/linter --c > > + ~/typocheck.sh > > + python ~/run-test-suite.py > > + .git/hooks/post-commit (if present) > > + > > +and prepare-commit-msg hooks in this order: > > + > > + /bin/linter --c > > + .git/hooks/prepare-commit-msg (if present) > > > > I still have outstanding feedback on the fundamental design > here. I.e. why is this not: > > hook.<name>.event = post-commit > hook.<name>.command = <path> > > See: > > https://lore.kernel.org/git/87mtv8fww3.fsf@xxxxxxxxxxxxxxxxxxx/ > > As noted there I don't see why not, and more complexity results from the > design choice of doing it the way you're proposing. I.e. we can't > discover hooks based on config prefixes, and we end up sticking full FS > paths in config keys. Interesting. My gut says that it would make it harder to neatly write a config with the same hook running at the very beginning of one event and the very end of another, but I'm not sure whether that's a case to optimize for. I would also need twice as many lines to run a script I wrote as a hook - that is, the base case which is probably all most people will need. So with your proposal I *must* name every hook I want to add. Instead of "hook.pre-commit.command = ~/check-for-debug-strings" I need to configure "hook.check-debug-strings.event = pre-commit" and "hook.check-debug-strings.command = ~/check-for-debug-strings". That's a little frustrating, and if I never want to skip that check or move it later or something, it will be extra overhead for no gain for me. I do agree that your approach bans the regrettably awkward "hookcmd.~/check-for-debug-strings.skip = true", but I'm not sure whether or not it's worth it. To help us visualize the change, here's some common and complicated tasks and how they look with either schema (let mine be A and yours be B): #1 - Add a oneliner (not executing a script) A: [hook "post-commit"] command = echo post commit B: [hook "oneliner"] event = post-commit command = echo post commit #2 - Execute a script A: [hook "post-commit"] command = ~/my-post-commit-hook B: [hook "script"] event = post-commit command = ~/my-post-commit-hook #3 - Run a handful of scripts in a specific order on one event A: [hook "post-commit"] command = ~/my-post-commit-1 command = ~/my-post-commit-2 command = ~/my-post-commit-3 B: [hook "script 1"] event = post-commit command = ~/my-post-commit-1 [hook "script 2"] event = post-commit command = ~/my-post-commit-2 [hook "script 3"] event = post-commit command = ~/my-post-commit-3 #4 - Skip a script configured more globally A: (original config) [hook "post-commit"] command = /corp/stuff/corp-post-commit (local config) [hookcmd "/corp/stuff/corp-post-commit"] skip = true OR, (original config) [hookcmd "corp-pc"] command = /corp/stuff/corp-post-commit [hook "post-commit"] command = corp-pc (local config) [hookcmd "corp-pc"] skip = true B: (original config) [hook "corp-pc"] event = post-commit command = /corp/stuff/corp-post-commit (local config) [hook "corp-pc"] skip = true #5 - Execute one script for multiple events A: [hookcmd "reusable-hook"] command = /long/path/reusable-hook [hook "post-commit"] command = reusable-hook [hook "pre-commit"] command = reusable-hook [hook "prepare-commit-msg"] command = reusable-hook B: [hook "reusable-hook"] command = /long/path/reusable-hook event = post-commit event = pre-commit event = prepare-commit-msg #6 - Execute the same script early for one event and late for another event A: (global) [hookcmd "reusable-hook"] command = /long/path/reusable-hook [hook "pre-commit"] command = reusable-hook (local) [hook "post-commit"] command = other command = hooks command = reusable-hook B: (global) [hook "reusable-hook"] command = /long/path/reusable-hook event = pre-commit (local) [hook "other"] event = post-commit command = other [hook "hooks"] event = post-commit command = hooks [hook "reusable-hook"] event = reusable-hook I'm not going to comment on which one I like more yet - I think I will study this for a while and let others weigh in on their preferences too. I tried to order the cases from most common to less common. Please feel free to chime in with more use cases that you think would be handy which I forgot :) - Emily
