From: Todd Zullinger <tmz@pobox.com>
Date: Fri, 02 Jun 2017 00:22:49 GMT
Subject: Re: [PATCH] send-email: check for repo before invoking hook
Message-ID: <20170602002249.GI25542@zaya.teonanacatl.net>
In-Reply-To: <20170601235055.22621-1-jonathantanmy@google.com>

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)?

-- >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