On Tue, Oct 12, 2021 at 03:30:25PM +0200, Ævar Arnfjörð Bjarmason wrote:
>
> This series is part 2 of an incremental restart of the greater
> "config-based hooks" topic by Emily Shaffer that I've been helping
> with. See [1] for part 1, and [2] (search for "mark2") for a summary
> of parts to come.
>
> This goes on top of the "ab/config-based-hooks-1" topic that's already
> in "next" and marked for graduation to "master".
>
> In this topic we build upon the skeleton hook.[ch] library and build
> infrastructure added in part 1 and add a hook running library and new
> "git hook" command.
>
> The new "git hook" command is (for now) only for the benefit of
> in-tree non-C programs that need to run hooks, and this series
> converts git-send-email and git-p4 to use it.
>
> At the end of the series we remove
> "run_hook_{le,ve}()" from run-command.c, as we've migrated all its
> callers.
>
> What we don't do is convert any of the more complex in-tree hooks that
> require input on stdin such as "pre-receive" and the like, that's in
> later parts once this lands.
>
> This series is approximately patch 6-20/36 of the previous 36 patch
> ab/config-based-hooks-base topic. A range-diff to that v5[3] is
> included below.
>
> The changes since that v5 are rather trivial, they are:
>
> * Formatting changes to reduce the diff to parts that come after this
> (which Emily & I were juggling at the time), and re-flowing some
> overly long lines.
>
> * The new test is now marked for SANITIZE=leak testing, with the new
> test facility I've added recently (and which just landed in
> "master"). They all pass, the new API doesn't leak.
>
> * I rewrote some of the git-send-email.perl code to avoid
> de-duplication and hardcoding (just using intermediate variables).
>
> Things to focus on reviewing:
>
> * This should all be pretty solid and well tested, but the git-p4.py
> part in particular I've never tested for real (not having access to
> p4), and think Emily hasn't either[4].
Correct. I seem to remember wanting to give this a test and not having access
to real perforce to try it against.
>
> The relevant patch looks trivially correct to me[5], and I've tried it
> out in the Python REPL. But if any of the CC'd people active in
> git-p4.py development could give it some end-to-end testing that
> would be much appreciated.
>
> * Both Python and Perl code now calls the in-between "git hook run"
> command rather than calling hooks directly. Will this behave
> differently due to any special behavior running via a git built-in
> adds?
>
> I vaguely recall a third-party "git-foo" program breaking in the
> past when invoked as "git foo" but not "git-foo", due to git
> squatting on SIGINT, but none of that should be relevant here
> (we're not starting a pager etc.).
I would be very surprised, assuming that we invoke 'git hook' in the same way
that those Python and Perl scripts invoke other 'git whatever' commands...
That said, once we invoke a user hook, all bets are off as far as "we're not
starting a pager or anything". A determined user could find a reason to invoke
a pager, editor, whatever during a hook. I'm curious to know more about this
remembered breakage.
Last time I looked at the range-diff; this time I'll look at the individual
patches, instead.
Note: I looked at a bunch of these offline, so sorry for the sudden
ingress of mails today ;)
- Emily
