On Thu, Mar 11 2021, Emily Shaffer wrote: > By using the new 'git hook run' subcommand to run 'sendemail-validate', > we can reduce the boilerplate needed to run this hook in perl. Using > config-based hooks also allows us to run 'sendemail-validate' hooks that > were configured globally when running 'git send-email' from outside of a > Git directory, alongside other benefits like multihooks and > parallelization. > > Signed-off-by: Emily Shaffer <emilyshaffer@xxxxxxxxxx> > --- > git-send-email.perl | 21 ++++----------------- > t/t9001-send-email.sh | 11 +---------- > 2 files changed, 5 insertions(+), 27 deletions(-) > > diff --git a/git-send-email.perl b/git-send-email.perl > index 1f425c0809..73e1e0b51a 100755 > --- a/git-send-email.perl > +++ b/git-send-email.perl > @@ -1941,23 +1941,10 @@ sub unique_email_list { > sub validate_patch { > my ($fn, $xfer_encoding) = @_; > > - if ($repo) { > - my $validate_hook = catfile(catdir($repo->repo_path(), 'hooks'), > - 'sendemail-validate'); > - my $hook_error; > - if (-x $validate_hook) { > - my $target = abs_path($fn); > - # The hook needs a correct cwd and GIT_DIR. > - my $cwd_save = cwd(); > - chdir($repo->wc_path() or $repo->repo_path()) > - or die("chdir: $!"); > - local $ENV{"GIT_DIR"} = $repo->repo_path(); > - $hook_error = "rejected by sendemail-validate hook" > - if system($validate_hook, $target); > - chdir($cwd_save) or die("chdir: $!"); > - } > - return $hook_error if $hook_error; > - } > + my $target = abs_path($fn); > + return "rejected by sendemail-validate hook" > + if system(("git", "hook", "run", "sendemail-validate", "-a", > + $target)); I see it's just moving code around, but since we're touching this: This conflates the hook exit code with a general failure to invoke it, Perl's system(). Not a big deal in this case, but there's two other existing system() invocations which use the right blurb for it: system('sh', '-c', $editor.' "$@"', $editor, $_); if (($? & 127) || ($? >> 8)) { die(__("the editor exited uncleanly, aborting everything")); } Makes sense to do something similar here for consistency. See "perldoc -f system" for an example. > > # Any long lines will be automatically fixed if we use a suitable transfer > # encoding. > diff --git a/t/t9001-send-email.sh b/t/t9001-send-email.sh > index 4eee9c3dcb..456b471c5c 100755 > --- a/t/t9001-send-email.sh > +++ b/t/t9001-send-email.sh > @@ -2101,16 +2101,7 @@ test_expect_success $PREREQ 'invoke hook' ' > mkdir -p .git/hooks && > > write_script .git/hooks/sendemail-validate <<-\EOF && > - # test that we have the correct environment variable, pwd, and > - # argument > - case "$GIT_DIR" in > - *.git) > - true > - ;; > - *) > - false > - ;; > - esac && > + # test that we have the correct argument This and getting rid of these Perl/Python/whatever special cases is very nice.