From: "Ævar Arnfjörð Bjarmason" <avarab@gmail.com>
To: git@vger.kernel.org
Cc: "Junio C Hamano" <gitster@pobox.com>,
"Johannes Sixt" <j6t@kdbg.org>,
"Johannes Schindelin" <johannes.schindelin@gmx.de>,
"Jonathan Nieder" <jrnieder@gmail.com>,
"Robert Foss" <robert.foss@linaro.org>,
"Ævar Arnfjörð Bjarmason" <avarab@gmail.com>
Subject: [PATCH v2 0/2] send-email: don't needlessly abs_path() the core.hooksPath
Date: Wed, 26 May 2021 13:21:05 +0200 [thread overview]
Message-ID: <cover-0.2-00000000000-20210526T111558Z-avarab@gmail.com> (raw)
In-Reply-To: <xmqq5yz6ehpk.fsf@gitster.g>
On Wed, May 26 2021, Junio C Hamano wrote:
> Ævar Arnfjörð Bjarmason <avarab@gmail.com> writes:
>
>> But I'll leave it to you, if you are convinced and do want to take this
>> 2/2 after all I'll submit another trivial patch on top to remove the new
>> (then unused) repo_path function, which we expect to go away anyway.
>
> Sounds good.
Here that is, now on top of the new "master" which has the
t9001-send-email.sh changes you'd already merged down.
Ævar Arnfjörð Bjarmason (2):
send-email: don't needlessly abs_path() the core.hooksPath
send-email: move "hooks_path" invocation to git-send-email.perl
git-send-email.perl | 3 ++-
perl/Git.pm | 13 -------------
t/t9001-send-email.sh | 7 ++++---
3 files changed, 6 insertions(+), 17 deletions(-)
Range-diff against v1:
1: df3a2b8562d < -: ----------- send-email: fix missing error message regression
2: d097e7b0b81 ! 1: ff01d4619ea send-email: don't needlessly abs_path() the core.hooksPath
@@ Commit message
2021-04-06) when we started emitting the path to the hook, which was
previously only internal to git-send-email.perl.
- I think this change should let us have our cake and eat it too. We now
- emit a relative path for the common case where the hook is in the
- .git/hooks directory, but in the case it's an absolute path (there's
- another test for that, not seen here) we'll prefix it with $(pwd).
+ The just-landed 53753a37d09 (t9001-send-email.sh: fix expected
+ absolute paths on Windows, 2021-05-24) narrowly fixed this issue, but
+ I believe we can do better here. We should not be relying on whatever
+ changes Perl's abs_path() makes to the path "rev-parse --git-path
+ hooks" hands to us. Let's instead trust it, and hand it to Perl's
+ system() in git-send-email.perl. It will handle either a relative or
+ absolute path.
- I hope that unlike the current implementation that $(pwd) v.s. $PWD
- difference won't matter on Windows, since now the absolute path is the
- one we get from rev-parse, not the one that's been passed through
- Perl's Cwd::abs_path().
+ So let's revert most of 53753a37d09 and just have "hooks_path" return
+ what we get from "rev-parse" directly without modification. This has
+ the added benefit of making the error message friendlier in the common
+ case, we'll no longer print an absolute path for repository-local hook
+ errors.
1. http://lore.kernel.org/git/bb30fe2b-cd75-4782-24a6-08bb002a0367@kdbg.org
@@ t/t9001-send-email.sh: test_expect_success $PREREQ "--validate respects relative
test_path_is_file my-hooks.ran &&
cat >expect <<-EOF &&
fatal: longline.patch: rejected by sendemail-validate hook
-- fatal: command '"'"'$(pwd)/my-hooks/sendemail-validate'"'"' died with exit code 1
+- fatal: command '"'"'$PWD/my-hooks/sendemail-validate'"'"' died with exit code 1
+ fatal: command '"'"'my-hooks/sendemail-validate'"'"' died with exit code 1
warning: no patches were sent
EOF
test_cmp expect actual
+ '
+
+ test_expect_success $PREREQ "--validate respects absolute core.hooksPath path" '
+- test_config core.hooksPath "$(pwd)/my-hooks" &&
++ hooks_path="$(pwd)/my-hooks" &&
++ test_config core.hooksPath "$hooks_path" &&
+ test_when_finished "rm my-hooks.ran" &&
+ test_must_fail git send-email \
+ --from="Example <nobody@example.com>" \
+@@ t/t9001-send-email.sh: test_expect_success $PREREQ "--validate respects absolute core.hooksPath path" '
+ test_path_is_file my-hooks.ran &&
+ cat >expect <<-EOF &&
+ fatal: longline.patch: rejected by sendemail-validate hook
+- fatal: command '"'"'$PWD/my-hooks/sendemail-validate'"'"' died with exit code 1
++ fatal: command '"'"'$hooks_path/sendemail-validate'"'"' died with exit code 1
+ warning: no patches were sent
+ EOF
+ test_cmp expect actual
-: ----------- > 2: cda41c36772 send-email: move "hooks_path" invocation to git-send-email.perl
--
2.32.0.rc1.400.g0a5a93401d3
next prev parent reply other threads:[~2021-05-26 11:21 UTC|newest]
Thread overview: 20+ messages / expand[flat|nested] mbox.gz Atom feed top
2021-05-24 19:38 [PATCH] t9001-send-email.sh: fix expected absolute paths on Windows Johannes Sixt
2021-05-24 20:26 ` Jonathan Nieder
2021-05-24 22:15 ` Ævar Arnfjörð Bjarmason
2021-05-24 23:15 ` Ævar Arnfjörð Bjarmason
2021-05-24 23:14 ` [PATCH 0/2] send-email: pre-release fixes for v2.32.0 Ævar Arnfjörð Bjarmason
2021-05-24 23:14 ` [PATCH 1/2] send-email: fix missing error message regression Ævar Arnfjörð Bjarmason
2021-05-24 23:14 ` [PATCH 2/2] send-email: don't needlessly abs_path() the core.hooksPath Ævar Arnfjörð Bjarmason
2021-05-25 1:03 ` Junio C Hamano
2021-05-25 5:57 ` Ævar Arnfjörð Bjarmason
2021-05-25 6:13 ` Ævar Arnfjörð Bjarmason
2021-05-25 6:21 ` Robert Foss
2021-05-25 6:21 ` Junio C Hamano
2021-05-25 12:09 ` Ævar Arnfjörð Bjarmason
2021-05-25 19:28 ` Junio C Hamano
2021-05-26 11:21 ` Ævar Arnfjörð Bjarmason [this message]
2021-05-26 11:21 ` [PATCH v2 1/2] " Ævar Arnfjörð Bjarmason
2021-05-26 11:21 ` [PATCH v2 2/2] send-email: move "hooks_path" invocation to git-send-email.perl Ævar Arnfjörð Bjarmason
2021-05-26 1:22 ` [PATCH 2/2] send-email: don't needlessly abs_path() the core.hooksPath Felipe Contreras
2021-05-25 6:10 ` Robert Foss
2021-06-02 11:40 ` [PATCH] t9001-send-email.sh: fix expected absolute paths on Windows Johannes Schindelin
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=cover-0.2-00000000000-20210526T111558Z-avarab@gmail.com \
--to=avarab@gmail.com \
--cc=git@vger.kernel.org \
--cc=gitster@pobox.com \
--cc=j6t@kdbg.org \
--cc=johannes.schindelin@gmx.de \
--cc=jrnieder@gmail.com \
--cc=robert.foss@linaro.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).