All of lore.kernel.org
 help / color / mirror / Atom feed
From: "Ævar Arnfjörð Bjarmason" <avarab@gmail.com>
To: Junio C Hamano <gitster@pobox.com>
Cc: git@vger.kernel.org, Johannes Sixt <j6t@kdbg.org>,
	Johannes Schindelin <johannes.schindelin@gmx.de>,
	Jonathan Nieder <jrnieder@gmail.com>,
	Robert Foss <robert.foss@linaro.org>
Subject: Re: [PATCH 2/2] send-email: don't needlessly abs_path() the core.hooksPath
Date: Tue, 25 May 2021 14:09:35 +0200	[thread overview]
Message-ID: <87v977m2q6.fsf@evledraar.gmail.com> (raw)
In-Reply-To: <xmqq35ubgwpy.fsf@gitster.g>


On Tue, May 25 2021, Junio C Hamano wrote:

> Ævar Arnfjörð Bjarmason <avarab@gmail.com> writes:
>
>> The repo_path() function was introduced in c8243933c74, so it's never
>> been in a release, thus I think it's fine to alter its behavior.
>>
>> The code here doesn't need to concern itself with what needs to be
>> relative to what, you run send-email in some working tree directory (or
>> top-level), and depending on core.hooksPath we'll either return a
>> relative path to the .git/hooks or an absolute one, the system()
>> invocation will accept either.
>
> All of that are convincing, but I'd rather not risk it.  I'll take
> your 1/2 with J6t's fix, both of which are obvious and no brainer,
> but I am willing to take this 2/2 as a post-release clean-up next
> monght, though ;-)

Fair enough. I get that we're in the rc phase, but FWIW I still
disagree.

I do think supporting whatever edge case we're exposing by
round-tripping through abs_path() and its subtly different behavior is
worse than a change to make us rely on the well-understood rev-parse
behavior directly.

Especially since the fix is to something that didn't work to begin with
before the v2.32.0 release is out (core.hooksPath + send-email), but
perhaps that's more of an argument for reverting the topic during RC +
try to have it land in v2.33.0?

And that + what seems to be the prevailing (IMO nonsensical) opinion on
Git.pm's stability promise, meaning that once this new function is out
in a release we'll be stuck supporting it makes me want to change this
pre-release.

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.

  reply	other threads:[~2021-05-25 12:12 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 [this message]
2021-05-25 19:28             ` Junio C Hamano
2021-05-26 11:21               ` [PATCH v2 0/2] " Ævar Arnfjörð Bjarmason
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=87v977m2q6.fsf@evledraar.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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.