{"thread":{"id":"55366","subject":"[PATCH v1] git-send-email: Respect core.hooksPath setting","startedAt":"2021-03-22T16:20:56Z","lastAt":"2021-03-22T21:14:21Z","messageCount":3,"participants":["Robert Foss","Ævar Arnfjörð Bjarmason"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"419956","messageId":"20210322162008.468779-1-robert.foss@linaro.org","threadId":"55366","inReplyTo":null,"subject":"[PATCH v1] git-send-email: Respect core.hooksPath setting","fromName":"Robert Foss","fromEmail":"robert.foss@linaro.org","sentAt":"2021-03-22T16:20:08Z","receivedAt":"2021-03-22T16:20:56Z","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\n\nThis patch does not include a test for this bug fix.\nThis is entirely due to me not being able to think up a way\nto test this. So I'm very much open to suggestions.\n\n\n git-send-email.perl | 5 +++--\n 1 file changed, 3 insertions(+), 2 deletions(-)\n\ndiff --git a/git-send-email.perl b/git-send-email.perl\nindex 1f425c0809..3934dceb70 100755\n--- a/git-send-email.perl\n+++ b/git-send-email.perl\n@@ -1942,8 +1942,9 @@ 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->command('rev-parse', '--git-path', 'hooks');\n+\t\tchomp($hook_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);\n-- \n2.27.0\n\n"},{"id":"419958","messageId":"875z1jqgpq.fsf@evledraar.gmail.com","threadId":"55366","inReplyTo":"20210322162008.468779-1-robert.foss@linaro.org","subject":"Re: [PATCH v1] git-send-email: Respect core.hooksPath setting","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2021-03-22T16:46:25Z","receivedAt":"2021-03-22T16:47:19Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"\nOn Mon, Mar 22 2021, Robert Foss wrote:\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> v2.9, 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> ---\n>\n>\n> This patch does not include a test for this bug fix.\n> This is entirely due to me not being able to think up a way\n> to test this. So I'm very much open to suggestions.\n\nThere's an \"invoke hook\" test in t9001-send-email.sh which should be\neasy to tweak (or mostly copy/pasted to another test) to run the same\nway once the hook is moved from .git/hooks to somedir/ and -c\ncore.hooksPath=somedir is set.\n\n>  git-send-email.perl | 5 +++--\n>  1 file changed, 3 insertions(+), 2 deletions(-)\n>\n> diff --git a/git-send-email.perl b/git-send-email.perl\n> index 1f425c0809..3934dceb70 100755\n> --- a/git-send-email.perl\n> +++ b/git-send-email.perl\n> @@ -1942,8 +1942,9 @@ 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->command('rev-parse', '--git-path', 'hooks');\n> +\t\tchomp($hook_path);\n> +\t\tmy $validate_hook = catfile($hook_path, 'sendemail-validate');\n\nThis looks like it work, small nits:\n\n1. This would be better in perl/Git.pm, it already has various accessors\n   etc. for these rev-parse'd values. You could just pass a a new\n   GetHooksPath => 1 to Git->repository() and if so populate this, then\n   call that as $repo->git_path_hooks.\n\n2. FWIW it's more idiomatic in perl to just do : chomp(my $x = y()); not\n   my $x = y(); chomp $x. The chomp operator works in-place, but once\n   you'd use the helpers in Git.pm for this they'd do all that for you.\n\n>  \t\tmy $hook_error;\n>  \t\tif (-x $validate_hook) {\n>  \t\t\tmy $target = abs_path($fn);\n\n"},{"id":"419974","messageId":"CAG3jFysGE-Wq2ReXmNa2VGPOWMW_MDn++Xv_WvAYW9EiQjAbdA@mail.gmail.com","threadId":"55366","inReplyTo":"875z1jqgpq.fsf@evledraar.gmail.com","subject":"Re: [PATCH v1] git-send-email: Respect core.hooksPath setting","fromName":"Robert Foss","fromEmail":"robert.foss@linaro.org","sentAt":"2021-03-22T21:13:05Z","receivedAt":"2021-03-22T21:14:21Z","isPatch":true,"sender":{"key":"robert.foss@linaro.org","avatar":"https://avatars.githubusercontent.com/u/523260?v=4"},"body":"Hey Ævar,\n\nThank you for the quick feedback.\n\nOn Mon, 22 Mar 2021 at 17:46, Ævar Arnfjörð Bjarmason <avarab@gmail.com> wrote:\n>\n>\n> On Mon, Mar 22 2021, Robert Foss wrote:\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> > v2.9, 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> > ---\n> >\n> >\n> > This patch does not include a test for this bug fix.\n> > This is entirely due to me not being able to think up a way\n> > to test this. So I'm very much open to suggestions.\n>\n> There's an \"invoke hook\" test in t9001-send-email.sh which should be\n> easy to tweak (or mostly copy/pasted to another test) to run the same\n> way once the hook is moved from .git/hooks to somedir/ and -c\n> core.hooksPath=somedir is set.\n\nAck\n\n\n>\n> >  git-send-email.perl | 5 +++--\n> >  1 file changed, 3 insertions(+), 2 deletions(-)\n> >\n> > diff --git a/git-send-email.perl b/git-send-email.perl\n> > index 1f425c0809..3934dceb70 100755\n> > --- a/git-send-email.perl\n> > +++ b/git-send-email.perl\n> > @@ -1942,8 +1942,9 @@ sub validate_patch {\n> >       my ($fn, $xfer_encoding) = @_;\n> >\n> >       if ($repo) {\n> > -             my $validate_hook = catfile(catdir($repo->repo_path(), 'hooks'),\n> > -                                         'sendemail-validate');\n> > +             my $hook_path = $repo->command('rev-parse', '--git-path', 'hooks');\n> > +             chomp($hook_path);\n> > +             my $validate_hook = catfile($hook_path, 'sendemail-validate');\n>\n> This looks like it work, small nits:\n>\n> 1. This would be better in perl/Git.pm, it already has various accessors\n>    etc. for these rev-parse'd values. You could just pass a a new\n>    GetHooksPath => 1 to Git->repository() and if so populate this, then\n>    call that as $repo->git_path_hooks.\n\nI reworked this, but being a level 1 perl coder I don't quite grok the\n\"GetHooksPath => 1 to Git->repository()\" part of your suggestion.\n\nhttps://github.com/robertfoss/git/commit/9388d1f66b8d182f0dcc869f627736596af382da\n\nThis is what I came up with.\n\n>\n> 2. FWIW it's more idiomatic in perl to just do : chomp(my $x = y()); not\n>    my $x = y(); chomp $x. The chomp operator works in-place, but once\n>    you'd use the helpers in Git.pm for this they'd do all that for you.\n>\n> >               my $hook_error;\n> >               if (-x $validate_hook) {\n> >                       my $target = abs_path($fn);\n>\n\nAck\n"}]}