Re: [PATCH v8 35/37] git-send-email: use 'git hook run' for 'sendemail-validate'
From: Ævar Arnfjörð Bjarmason <hidden>
Date: 2021-03-12 09:21:54
On Thu, Mar 11 2021, Emily Shaffer wrote:
quoted hunk ↗ jump to hunk
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 <redacted> --- 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.
quoted hunk ↗ jump to hunk
# 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.