Re: [PATCH] send-email: check for repo before invoking hook
- From
Todd Zullinger <tmz@pobox.com>
- Date
- Jun 2, 2017, 00:22 UTC
- Message-ID
- <20170602002249.GI25542@zaya.teonanacatl.net>
- In-Reply-To
- <20170601235055.22621-1-jonathantanmy@google.com>
Hi Jonathan,
Jonathan Tan wrote:
Show 46 quoted lines
> Thanks for the notification. Here's a patch to fix that.
> ---
> git-send-email.perl | 32 +++++++++++++++++---------------
> t/t9001-send-email.sh | 8 ++++++++
> 2 files changed, 25 insertions(+), 15 deletions(-)
>
> diff --git a/git-send-email.perl b/git-send-email.perl
> index f0417f64e..94c54dc5a 100755
> --- a/git-send-email.perl
> +++ b/git-send-email.perl
> @@ -1755,21 +1755,23 @@ sub unique_email_list {
> sub validate_patch {
> my $fn = shift;
>
> - 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;
> + 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;
> + }Would it be worth doing the $repo test when defining $validate_hook to avoid a layer of indentation? Something like this (with whatever proper wrapping/indentation is used for perl, if I have that wildly incorrect, of course)?
-- >8 --
diff --git i/git-send-email.perl w/git-send-email.perl index f0417f64e7..e78a0a728a 100755 --- i/git-send-email.perl +++ w/git-send-email.perl @@ -1755,8 +1755,9 @@ sub unique_email_list { sub validate_patch { my $fn = shift; - my $validate_hook = catfile(catdir($repo->repo_path(), 'hooks'), - 'sendemail-validate'); + my $validate_hook = $repo ? + catfile(catdir($repo->repo_path(), 'hooks'), + 'sendemail-validate') : ''; my $hook_error; if (-x $validate_hook) { my $target = abs_path($fn);
--
Todd
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
I have to decide between two equally frightening options. If I wanted
to do that, I'd vote.
-- Duckman