{"thread":{"id":"55372","subject":"[PATCH v2] git-send-email: Respect core.hooksPath setting","startedAt":"2021-03-23T16:28:29Z","lastAt":"2021-03-24T01:18:32Z","messageCount":5,"participants":["Robert Foss","Ævar Arnfjörð Bjarmason","Junio C Hamano"],"isPatch":true,"patchVersion":2,"patchTotal":null},"messages":[{"id":"420041","messageId":"20210323162718.1143982-1-robert.foss@linaro.org","threadId":"55372","inReplyTo":null,"subject":"[PATCH v2] git-send-email: Respect core.hooksPath setting","fromName":"Robert Foss","fromEmail":"robert.foss@linaro.org","sentAt":"2021-03-23T16:27:18Z","receivedAt":"2021-03-23T16:28:29Z","isPatch":true,"sender":{"key":"robert.foss@linaro.org","avatar":"https://avatars.githubusercontent.com/u/523260?v=4"},"body":"get-send-email currently makes the assumption that the\n'sendemail-validate' hook exists inside of the repository.\n\nSince the introduction of 'core.hooksPath' configuration option in\nv2.9, this is no longer true.\n\nInstead of assuming a hardcoded repo relative path, query\ngit for the actual path of the hooks directory.\n\nSigned-off-by: Robert Foss <robert.foss@linaro.org>\n---\n\nChanges since v1:\n - Ævar: Add unit test\n - Ævar: Add support for getting the hooks_path() from Git perl module\n - Ævar: Use new hooks_path() instread of issuing git rev-parse in\n         git-send-email.perl\n\n\nNote: The test currently leaves a file in a mktemp directory\nin /tmp.\n\n git-send-email.perl   |  4 ++--\n perl/Git.pm           |  9 +++++++++\n t/t9001-send-email.sh | 16 ++++++++++++++++\n 3 files changed, 27 insertions(+), 2 deletions(-)\n\ndiff --git a/git-send-email.perl b/git-send-email.perl\nindex 1f425c0809..c3dd825322 100755\n--- a/git-send-email.perl\n+++ b/git-send-email.perl\n@@ -1942,8 +1942,8 @@ sub validate_patch {\n \tmy ($fn, $xfer_encoding) = @_;\n \n \tif ($repo) {\n-\t\tmy $validate_hook = catfile(catdir($repo->repo_path(), 'hooks'),\n-\t\t\t\t\t    'sendemail-validate');\n+\t\tmy $hook_path = $repo->hooks_path();\n+\t\tmy $validate_hook = catfile($hook_path, 'sendemail-validate');\n \t\tmy $hook_error;\n \t\tif (-x $validate_hook) {\n \t\t\tmy $target = abs_path($fn);\ndiff --git a/perl/Git.pm b/perl/Git.pm\nindex 02eacef0c2..ac1fabff28 100644\n--- a/perl/Git.pm\n+++ b/perl/Git.pm\n@@ -226,6 +226,8 @@ sub repository {\n \t\t\t$opts{Repository} = abs_path($dir);\n \t\t}\n \n+                $opts{HooksPath} = $search->command_oneline('rev-parse', '--git-path', 'hooks');\n+\n \t\tdelete $opts{Directory};\n \t}\n \n@@ -619,6 +621,13 @@ sub _prompt {\n \n sub repo_path { $_[0]->{opts}->{Repository} }\n \n+=item hooks_path ()\n+\n+Return path to the hooks directory. Must be called on a repository instance.\n+\n+=cut\n+\n+sub hooks_path { $_[0]->{opts}->{HooksPath} }\n \n =item wc_path ()\n \ndiff --git a/t/t9001-send-email.sh b/t/t9001-send-email.sh\nindex 4eee9c3dcb..73b3bc1ce6 100755\n--- a/t/t9001-send-email.sh\n+++ b/t/t9001-send-email.sh\n@@ -513,6 +513,22 @@ do\n \n done\n \n+test_expect_success $PREREQ \"--validate respects core.hooksPath setting\" '\n+\tclean_fake_sendmail &&\n+\ttmp_dir=$(mktemp -d -t ci-XXXXXXXXXX) &&\n+\tprintf \"#!/bin/sh\" >> $tmp_dir/sendemail-validate &&\n+\tprintf \"return 1\" >> $tmp_dir/sendemail-validate &&\n+\tchmod a+x $tmp_dir/sendemail-validate &&\n+\tgit -c core.hooksPath=$tmp_dir send-email \\\n+\t\t--from=\"Example <nobody@example.com>\" \\\n+\t\t--to=nobody@example.com \\\n+\t\t--smtp-server=\"$(pwd)/fake.sendmail\" \\\n+\t\t--validate \\\n+\t\tlongline.patch \\\n+\t\t2>&1 >/dev/null | \\\n+\tgrep \"rejected by sendemail-validate\"\n+'\n+\n for enc in 7bit 8bit quoted-printable base64\n do\n \ttest_expect_success $PREREQ \"--transfer-encoding=$enc produces correct header\" '\n-- \n2.31.0.30.g398dba342d.dirty\n\n"},{"id":"420050","messageId":"patch-1.1-cc0ba73974a-20210323T173032Z-avarab@gmail.com","threadId":"55372","inReplyTo":"20210323162718.1143982-1-robert.foss@linaro.org","subject":"[PATCH v3] git-send-email: Respect core.hooksPath setting","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2021-03-23T17:33:27Z","receivedAt":"2021-03-23T17:34:18Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"From: Robert Foss <robert.foss@linaro.org>\n\nget-send-email currently makes the assumption that the\n'sendemail-validate' hook exists inside of the repository.\n\nSince the introduction of 'core.hooksPath' configuration option in\n867ad08a261 (hooks: allow customizing where the hook directory is,\n2016-05-04), this is no longer true.\n\nInstead of assuming a hardcoded repo relative path, query\ngit for the actual path of the hooks directory.\n\nSigned-off-by: Robert Foss <robert.foss@linaro.org>\nSigned-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>\n---\n\nHere's a v3 that fixes various issues with Robert's v2. Range-diff &\nupdated patch below.\n\nThe advice I had in the v1 feedback about GetHooksPath was bad, just\nhaving it be a new accessor is better. It's not like anyone is calling\nthis in a loop.\n\nRange-diff:\n1:  56c181cf091 ! 1:  cc0ba73974a git-send-email: Respect core.hooksPath setting\n    @@ Commit message\n         'sendemail-validate' hook exists inside of the repository.\n     \n         Since the introduction of 'core.hooksPath' configuration option in\n    -    v2.9, this is no longer true.\n    +    867ad08a261 (hooks: allow customizing where the hook directory is,\n    +    2016-05-04), this is no longer true.\n     \n         Instead of assuming a hardcoded repo relative path, query\n         git for the actual path of the hooks directory.\n     \n         Signed-off-by: Robert Foss <robert.foss@linaro.org>\n    +    Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>\n     \n      ## git-send-email.perl ##\n     @@ git-send-email.perl: sub validate_patch {\n    @@ git-send-email.perl: sub validate_patch {\n      \n      \tif ($repo) {\n     -\t\tmy $validate_hook = catfile(catdir($repo->repo_path(), 'hooks'),\n    --\t\t\t\t\t    'sendemail-validate');\n    -+\t\tmy $hook_path = $repo->hooks_path();\n    -+\t\tmy $validate_hook = catfile($hook_path, 'sendemail-validate');\n    ++\t\tmy $validate_hook = catfile($repo->hooks_path(),\n    + \t\t\t\t\t    'sendemail-validate');\n      \t\tmy $hook_error;\n      \t\tif (-x $validate_hook) {\n    - \t\t\tmy $target = abs_path($fn);\n     \n      ## perl/Git.pm ##\n    -@@ perl/Git.pm: sub repository {\n    - \t\t\t$opts{Repository} = abs_path($dir);\n    - \t\t}\n    - \n    -+                $opts{HooksPath} = $search->command_oneline('rev-parse', '--git-path', 'hooks');\n    -+\n    - \t\tdelete $opts{Directory};\n    - \t}\n    - \n     @@ perl/Git.pm: sub _prompt {\n      \n      sub repo_path { $_[0]->{opts}->{Repository} }\n    @@ perl/Git.pm: sub _prompt {\n     +\n     +=cut\n     +\n    -+sub hooks_path { $_[0]->{opts}->{HooksPath} }\n    ++sub hooks_path {\n    ++\tmy ($self) = @_;\n    ++\n    ++\tmy $dir = $self->command_oneline('rev-parse', '--git-path', 'hooks');\n    ++\tmy $abs = abs_path($dir);\n    ++\treturn $abs;\n    ++}\n      \n      =item wc_path ()\n      \n    @@ t/t9001-send-email.sh: do\n      \n      done\n      \n    -+test_expect_success $PREREQ \"--validate respects core.hooksPath path\" '\n    ++test_expect_success $PREREQ \"--validate respects relative core.hooksPath path\" '\n     +\tclean_fake_sendmail &&\n    -+\ttmp_dir=$(mktemp -d -t ci-XXXXXXXXXX) &&\n    -+\tprintf \"#!/bin/sh\" >> $tmp_dir/sendemail-validate &&\n    -+\tprintf \"return 1\" >> $tmp_dir/sendemail-validate &&\n    -+\tchmod a+x $tmp_dir/sendemail-validate &&\n    -+\tgit -c core.hooksPath=$tmp_dir send-email \\\n    ++\tmkdir my-hooks &&\n    ++\ttest_when_finished \"rm my-hooks.ran\" &&\n    ++\twrite_script my-hooks/sendemail-validate <<-\\EOF &&\n    ++\t>my-hooks.ran\n    ++\texit 1\n    ++\tEOF\n    ++\ttest_config core.hooksPath \"my-hooks\" &&\n    ++\ttest_must_fail git send-email \\\n    ++\t\t--from=\"Example <nobody@example.com>\" \\\n    ++\t\t--to=nobody@example.com \\\n    ++\t\t--smtp-server=\"$(pwd)/fake.sendmail\" \\\n    ++\t\t--validate \\\n    ++\t\tlongline.patch 2>err &&\n    ++\ttest_path_is_file my-hooks.ran &&\n    ++\tgrep \"rejected by sendemail-validate\" err\n    ++'\n    ++\n    ++test_expect_success $PREREQ \"--validate respects absolute core.hooksPath path\" '\n    ++\ttest_config core.hooksPath \"$(pwd)/my-hooks\" &&\n    ++\ttest_when_finished \"rm my-hooks.ran\" &&\n    ++\ttest_must_fail git send-email \\\n     +\t\t--from=\"Example <nobody@example.com>\" \\\n     +\t\t--to=nobody@example.com \\\n     +\t\t--smtp-server=\"$(pwd)/fake.sendmail\" \\\n     +\t\t--validate \\\n    -+\t\tlongline.patch \\\n    -+\t\t2>&1 >/dev/null | \\\n    -+\tgrep \"rejected by sendemail-validate\"\n    ++\t\tlongline.patch 2>err &&\n    ++\ttest_path_is_file my-hooks.ran &&\n    ++\tgrep \"rejected by sendemail-validate\" err\n     +'\n     +\n      for enc in 7bit 8bit quoted-printable base64\n\n git-send-email.perl   |  2 +-\n perl/Git.pm           | 13 +++++++++++++\n t/t9001-send-email.sh | 32 ++++++++++++++++++++++++++++++++\n 3 files changed, 46 insertions(+), 1 deletion(-)\n\ndiff --git a/git-send-email.perl b/git-send-email.perl\nindex 1f425c08091..f5bbf1647e3 100755\n--- a/git-send-email.perl\n+++ b/git-send-email.perl\n@@ -1942,7 +1942,7 @@ sub validate_patch {\n \tmy ($fn, $xfer_encoding) = @_;\n \n \tif ($repo) {\n-\t\tmy $validate_hook = catfile(catdir($repo->repo_path(), 'hooks'),\n+\t\tmy $validate_hook = catfile($repo->hooks_path(),\n \t\t\t\t\t    'sendemail-validate');\n \t\tmy $hook_error;\n \t\tif (-x $validate_hook) {\ndiff --git a/perl/Git.pm b/perl/Git.pm\nindex 02eacef0c2a..73ebbf80cc6 100644\n--- a/perl/Git.pm\n+++ b/perl/Git.pm\n@@ -619,6 +619,19 @@ sub _prompt {\n \n sub repo_path { $_[0]->{opts}->{Repository} }\n \n+=item hooks_path ()\n+\n+Return path to the hooks directory. Must be called on a repository instance.\n+\n+=cut\n+\n+sub hooks_path {\n+\tmy ($self) = @_;\n+\n+\tmy $dir = $self->command_oneline('rev-parse', '--git-path', 'hooks');\n+\tmy $abs = abs_path($dir);\n+\treturn $abs;\n+}\n \n =item wc_path ()\n \ndiff --git a/t/t9001-send-email.sh b/t/t9001-send-email.sh\nindex 4eee9c3dcb5..1a1caf8f2ed 100755\n--- a/t/t9001-send-email.sh\n+++ b/t/t9001-send-email.sh\n@@ -513,6 +513,38 @@ do\n \n done\n \n+test_expect_success $PREREQ \"--validate respects relative core.hooksPath path\" '\n+\tclean_fake_sendmail &&\n+\tmkdir my-hooks &&\n+\ttest_when_finished \"rm my-hooks.ran\" &&\n+\twrite_script my-hooks/sendemail-validate <<-\\EOF &&\n+\t>my-hooks.ran\n+\texit 1\n+\tEOF\n+\ttest_config core.hooksPath \"my-hooks\" &&\n+\ttest_must_fail git send-email \\\n+\t\t--from=\"Example <nobody@example.com>\" \\\n+\t\t--to=nobody@example.com \\\n+\t\t--smtp-server=\"$(pwd)/fake.sendmail\" \\\n+\t\t--validate \\\n+\t\tlongline.patch 2>err &&\n+\ttest_path_is_file my-hooks.ran &&\n+\tgrep \"rejected by sendemail-validate\" err\n+'\n+\n+test_expect_success $PREREQ \"--validate respects absolute core.hooksPath path\" '\n+\ttest_config core.hooksPath \"$(pwd)/my-hooks\" &&\n+\ttest_when_finished \"rm my-hooks.ran\" &&\n+\ttest_must_fail git send-email \\\n+\t\t--from=\"Example <nobody@example.com>\" \\\n+\t\t--to=nobody@example.com \\\n+\t\t--smtp-server=\"$(pwd)/fake.sendmail\" \\\n+\t\t--validate \\\n+\t\tlongline.patch 2>err &&\n+\ttest_path_is_file my-hooks.ran &&\n+\tgrep \"rejected by sendemail-validate\" err\n+'\n+\n for enc in 7bit 8bit quoted-printable base64\n do\n \ttest_expect_success $PREREQ \"--transfer-encoding=$enc produces correct header\" '\n-- \n2.31.0.366.ga80606b22c1\n\n"},{"id":"420075","messageId":"xmqqeeg51ojs.fsf@gitster.g","threadId":"55372","inReplyTo":"patch-1.1-cc0ba73974a-20210323T173032Z-avarab@gmail.com","subject":"Re: [PATCH v3] git-send-email: Respect core.hooksPath setting","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2021-03-23T22:40:55Z","receivedAt":"2021-03-23T22:41:35Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Ævar Arnfjörð Bjarmason  <avarab@gmail.com> writes:\n\n> From: Robert Foss <robert.foss@linaro.org>\n>\n> get-send-email currently makes the assumption that the\n> 'sendemail-validate' hook exists inside of the repository.\n>\n> Since the introduction of 'core.hooksPath' configuration option in\n> 867ad08a261 (hooks: allow customizing where the hook directory is,\n> 2016-05-04), this is no longer true.\n>\n> Instead of assuming a hardcoded repo relative path, query\n> git for the actual path of the hooks directory.\n>\n> Signed-off-by: Robert Foss <robert.foss@linaro.org>\n> Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>\n> ---\n>\n> Here's a v3 that fixes various issues with Robert's v2. Range-diff &\n> updated patch below.\n>\n> The advice I had in the v1 feedback about GetHooksPath was bad, just\n> having it be a new accessor is better. It's not like anyone is calling\n> this in a loop.\n\nHow urgent is this \"fix\".  I am wondering if Emily's \"git hook\"\nautomatically fix this for us when it comes.\n"},{"id":"420078","messageId":"87zgytpehg.fsf@evledraar.gmail.com","threadId":"55372","inReplyTo":"xmqqeeg51ojs.fsf@gitster.g","subject":"Re: [PATCH v3] git-send-email: Respect core.hooksPath setting","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2021-03-24T00:44:27Z","receivedAt":"2021-03-24T00:47:46Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"\nOn Tue, Mar 23 2021, Junio C Hamano wrote:\n\n> Ævar Arnfjörð Bjarmason  <avarab@gmail.com> writes:\n>\n>> From: Robert Foss <robert.foss@linaro.org>\n>>\n>> get-send-email currently makes the assumption that the\n>> 'sendemail-validate' hook exists inside of the repository.\n>>\n>> Since the introduction of 'core.hooksPath' configuration option in\n>> 867ad08a261 (hooks: allow customizing where the hook directory is,\n>> 2016-05-04), this is no longer true.\n>>\n>> Instead of assuming a hardcoded repo relative path, query\n>> git for the actual path of the hooks directory.\n>>\n>> Signed-off-by: Robert Foss <robert.foss@linaro.org>\n>> Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>\n>> ---\n>>\n>> Here's a v3 that fixes various issues with Robert's v2. Range-diff &\n>> updated patch below.\n>>\n>> The advice I had in the v1 feedback about GetHooksPath was bad, just\n>> having it be a new accessor is better. It's not like anyone is calling\n>> this in a loop.\n>\n> How urgent is this \"fix\".  I am wondering if Emily's \"git hook\"\n> automatically fix this for us when it comes.\n\nWe've had iterations of that topic for almost a year now (since 2019\ncounting RFC discussions).\n\nWhile I'd like to see it land I'm skeptical of parts of that approach[1]\nand expect we'll have more re-rolls of it, and in any case the conflict\nin send-email[2] will be trivial to resolve. So I think it makes sense\nto queue up this narrow fix and not have this wait on the larger topic.\n\n1. https://lore.kernel.org/git/87mtv8fww3.fsf@evledraar.gmail.com/\n2. https://lore.kernel.org/git/20210311021037.3001235-36-emilyshaffer@google.com/\n"},{"id":"420082","messageId":"xmqq7dlx1hav.fsf@gitster.g","threadId":"55372","inReplyTo":"87zgytpehg.fsf@evledraar.gmail.com","subject":"Re: [PATCH v3] git-send-email: Respect core.hooksPath setting","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2021-03-24T01:17:28Z","receivedAt":"2021-03-24T01:18:32Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Ævar Arnfjörð Bjarmason <avarab@gmail.com> writes:\n\n> While I'd like to see it land I'm skeptical of parts of that approach[1]\n> and expect we'll have more re-rolls of it, and in any case the conflict\n> in send-email[2] will be trivial to resolve.\n> So I think it makes sense\n> to queue up this narrow fix and not have this wait on the larger topic.\n\nWell, that is what I am doing for now, queue up this narrow change,\nwhile resolving the conflict to use Emily's topic.  Which means that\nthe new code this topic adds is never exercised while the other topic\nis still cooking in tree.\n\n> 1. https://lore.kernel.org/git/87mtv8fww3.fsf@evledraar.gmail.com/\n> 2. https://lore.kernel.org/git/20210311021037.3001235-36-emilyshaffer@google.com/\n"}]}