[PATCH v3] git-send-email: Respect core.hooksPath setting

From: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
Date: Mar 23, 2021, 17:33 UTC
Message-ID: <patch-1.1-cc0ba73974a-20210323T173032Z-avarab@gmail.com>
In-Reply-To: <20210323162718.1143982-1-robert.foss@linaro.org>

From: Robert Foss <robert.foss@linaro.org>

get-send-email currently makes the assumption that the 'sendemail-validate' hook exists inside of the repository.

Since the introduction of 'core.hooksPath' configuration option in 867ad08a261 (hooks: allow customizing where the hook directory is, 2016-05-04), this is no longer true.

Instead of assuming a hardcoded repo relative path, query git for the actual path of the hooks directory.

Signed-off-by: Robert Foss <robert.foss@linaro.org>
Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
---

Here's a v3 that fixes various issues with Robert's v2. Range-diff & updated patch below.

The advice I had in the v1 feedback about GetHooksPath was bad, just having it be a new accessor is better. It's not like anyone is calling this in a loop.

Range-diff:
1:  56c181cf091 ! 1:  cc0ba73974a git-send-email: Respect core.hooksPath setting
    @@ Commit message
         'sendemail-validate' hook exists inside of the repository.
     
         Since the introduction of 'core.hooksPath' configuration option in
    -    v2.9, this is no longer true.
    +    867ad08a261 (hooks: allow customizing where the hook directory is,
    +    2016-05-04), this is no longer true.
     
         Instead of assuming a hardcoded repo relative path, query
         git for the actual path of the hooks directory.
     
         Signed-off-by: Robert Foss <robert.foss@linaro.org>
    +    Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
     
      ## git-send-email.perl ##
     @@ git-send-email.perl: sub validate_patch {
    @@ git-send-email.perl: sub validate_patch {
      
      	if ($repo) {
     -		my $validate_hook = catfile(catdir($repo->repo_path(), 'hooks'),
    --					    'sendemail-validate');
    -+		my $hook_path = $repo->hooks_path();
    -+		my $validate_hook = catfile($hook_path, 'sendemail-validate');
    ++		my $validate_hook = catfile($repo->hooks_path(),
    + 					    'sendemail-validate');
      		my $hook_error;
      		if (-x $validate_hook) {
    - 			my $target = abs_path($fn);
     
      ## perl/Git.pm ##
    -@@ perl/Git.pm: sub repository {
    - 			$opts{Repository} = abs_path($dir);
    - 		}
    - 
    -+                $opts{HooksPath} = $search->command_oneline('rev-parse', '--git-path', 'hooks');
    -+
    - 		delete $opts{Directory};
    - 	}
    - 
     @@ perl/Git.pm: sub _prompt {
      
      sub repo_path { $_[0]->{opts}->{Repository} }
    @@ perl/Git.pm: sub _prompt {
     +
     +=cut
     +
    -+sub hooks_path { $_[0]->{opts}->{HooksPath} }
    ++sub hooks_path {
    ++	my ($self) = @_;
    ++
    ++	my $dir = $self->command_oneline('rev-parse', '--git-path', 'hooks');
    ++	my $abs = abs_path($dir);
    ++	return $abs;
    ++}
      
      =item wc_path ()
      
    @@ t/t9001-send-email.sh: do
      
      done
      
    -+test_expect_success $PREREQ "--validate respects core.hooksPath path" '
    ++test_expect_success $PREREQ "--validate respects relative core.hooksPath path" '
     +	clean_fake_sendmail &&
    -+	tmp_dir=$(mktemp -d -t ci-XXXXXXXXXX) &&
    -+	printf "#!/bin/sh" >> $tmp_dir/sendemail-validate &&
    -+	printf "return 1" >> $tmp_dir/sendemail-validate &&
    -+	chmod a+x $tmp_dir/sendemail-validate &&
    -+	git -c core.hooksPath=$tmp_dir send-email \
    ++	mkdir my-hooks &&
    ++	test_when_finished "rm my-hooks.ran" &&
    ++	write_script my-hooks/sendemail-validate <<-\EOF &&
    ++	>my-hooks.ran
    ++	exit 1
    ++	EOF
    ++	test_config core.hooksPath "my-hooks" &&
    ++	test_must_fail git send-email \
    ++		--from="Example <nobody@example.com>" \
    ++		--to=nobody@example.com \
    ++		--smtp-server="$(pwd)/fake.sendmail" \
    ++		--validate \
    ++		longline.patch 2>err &&
    ++	test_path_is_file my-hooks.ran &&
    ++	grep "rejected by sendemail-validate" err
    ++'
    ++
    ++test_expect_success $PREREQ "--validate respects absolute core.hooksPath path" '
    ++	test_config core.hooksPath "$(pwd)/my-hooks" &&
    ++	test_when_finished "rm my-hooks.ran" &&
    ++	test_must_fail git send-email \
     +		--from="Example <nobody@example.com>" \
     +		--to=nobody@example.com \
     +		--smtp-server="$(pwd)/fake.sendmail" \
     +		--validate \
    -+		longline.patch \
    -+		2>&1 >/dev/null | \
    -+	grep "rejected by sendemail-validate"
    ++		longline.patch 2>err &&
    ++	test_path_is_file my-hooks.ran &&
    ++	grep "rejected by sendemail-validate" err
     +'
     +
      for enc in 7bit 8bit quoted-printable base64

 git-send-email.perl   |  2 +-
 perl/Git.pm           | 13 +++++++++++++
 t/t9001-send-email.sh | 32 ++++++++++++++++++++++++++++++++
 3 files changed, 46 insertions(+), 1 deletion(-)

diff --git a/git-send-email.perl b/git-send-email.perl
index 1f425c08091..f5bbf1647e3 100755
--- a/git-send-email.perl
+++ b/git-send-email.perl
@@ -1942,7 +1942,7 @@ sub validate_patch {
 	my ($fn, $xfer_encoding) = @_;
 
 	if ($repo) {
-		my $validate_hook = catfile(catdir($repo->repo_path(), 'hooks'),
+		my $validate_hook = catfile($repo->hooks_path(),
 					    'sendemail-validate');
 		my $hook_error;
 		if (-x $validate_hook) {
diff --git a/perl/Git.pm b/perl/Git.pm
index 02eacef0c2a..73ebbf80cc6 100644
--- a/perl/Git.pm
+++ b/perl/Git.pm
@@ -619,6 +619,19 @@ sub _prompt {
 
 sub repo_path { $_[0]->{opts}->{Repository} }
 
+=item hooks_path ()
+
+Return path to the hooks directory. Must be called on a repository instance.
+
+=cut
+
+sub hooks_path {
+	my ($self) = @_;
+
+	my $dir = $self->command_oneline('rev-parse', '--git-path', 'hooks');
+	my $abs = abs_path($dir);
+	return $abs;
+}
 
 =item wc_path ()
 
diff --git a/t/t9001-send-email.sh b/t/t9001-send-email.sh
index 4eee9c3dcb5..1a1caf8f2ed 100755
--- a/t/t9001-send-email.sh
+++ b/t/t9001-send-email.sh
@@ -513,6 +513,38 @@ do
 
 done
 
+test_expect_success $PREREQ "--validate respects relative core.hooksPath path" '
+	clean_fake_sendmail &&
+	mkdir my-hooks &&
+	test_when_finished "rm my-hooks.ran" &&
+	write_script my-hooks/sendemail-validate <<-\EOF &&
+	>my-hooks.ran
+	exit 1
+	EOF
+	test_config core.hooksPath "my-hooks" &&
+	test_must_fail git send-email \
+		--from="Example <nobody@example.com>" \
+		--to=nobody@example.com \
+		--smtp-server="$(pwd)/fake.sendmail" \
+		--validate \
+		longline.patch 2>err &&
+	test_path_is_file my-hooks.ran &&
+	grep "rejected by sendemail-validate" err
+'
+
+test_expect_success $PREREQ "--validate respects absolute core.hooksPath path" '
+	test_config core.hooksPath "$(pwd)/my-hooks" &&
+	test_when_finished "rm my-hooks.ran" &&
+	test_must_fail git send-email \
+		--from="Example <nobody@example.com>" \
+		--to=nobody@example.com \
+		--smtp-server="$(pwd)/fake.sendmail" \
+		--validate \
+		longline.patch 2>err &&
+	test_path_is_file my-hooks.ran &&
+	grep "rejected by sendemail-validate" err
+'
+
 for enc in 7bit 8bit quoted-printable base64
 do
 	test_expect_success $PREREQ "--transfer-encoding=$enc produces correct header" '

-- 
2.31.0.366.ga80606b22c1