Re: [PATCH] send-email: check for repo before invoking hook
- From
Jacob Keller <jacob.keller@gmail.com>
- Date
- Jun 2, 2017, 00:49 UTC
- Message-ID
- <CA+P7+xrb-a_UczPkdPSgZN7jW0zBU6v8yJ7Q20maNAROzyKBzA@mail.gmail.com>
- In-Reply-To
- <20170602002249.GI25542@zaya.teonanacatl.net>
On Thu, Jun 1, 2017 at 5:22 PM, Todd Zullinger <tmz@pobox.com> wrote:
Show 41 quoted lines
> Hi Jonathan,
>
> Jonathan Tan wrote:
>>
>> 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)?
>This approach makes more sense to me, but either should fix the bug. The first approach might be more resilient against future changes though...?
Thanks, Jake
Show 25 quoted lines
> -- >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
>