From: "Philip Oakley" <philipoakley@iee.org>
To: "Eric Sunshine" <sunshine@sunshineco.com>,
"Alex Bennée" <alex.bennee@linaro.org>
Cc: "Git List" <git@vger.kernel.org>
Subject: Re: [PATCH] git-send-email: fix get_maintainer.pl regression
Date: Tue, 21 Nov 2017 00:07:24 -0000 [thread overview]
Message-ID: <D810DA8B202742B38343EBB44BD54A1D@PhilipOakley> (raw)
In-Reply-To: CAPig+cSbRrGnyDkunMFiFXbWRMAsGyuAL-0FpP1QTtjSUSY2Hg@mail.gmail.com
From: "Eric Sunshine" <sunshine@sunshineco.com>
On Sat, Nov 18, 2017 at 9:54 PM, Eric Sunshine <sunshine@sunshineco.com>
wrote:
> On Thu, Nov 16, 2017 at 10:48 AM, Alex Bennée <alex.bennee@linaro.org>
> wrote:
>> +test_expect_success $PREREQ 'cc trailer with get_maintainer output' '
>> + [...]
>> + git send-email -1 --to=recipient@example.com \
>> + --cc-cmd="$(pwd)/expected-cc-script.sh" \
>> + [...]
>> +'
>> OK I'm afraid I don't fully understand the test harness as this breaks a
>> bunch of other tests. If anyone can offer some pointers on how to fix
>> I'd be grateful.
>
> There are several problems:
> [...]
> * The directory in which the expected-cc-script.sh is created contains
> a space; this is intentional to catch bugs in tests and Git itself. In
> this case, your test is exposing what might be considered a bug in
> git-send-email itself, in which it invokes the --cc-cmd as "/path/with
> space/expected-cc-script.sh", which is interpreted as trying to invoke
> program "/path/with" with argument "space/expected-cc-script.sh". One
> > fix (which you could submit as a preparatory patch, making this a
> > 2-patch series) would be this:
> >
> > --- 8< ---
> > diff --git a/git-send-email.perl b/git-send-email.perl
> > @@ -1724,7 +1724,7 @@ sub recipients_cmd {
> > - open my $fh, "-|", "$cmd \Q$file\E"
> > + open my $fh, "-|", "\Q$cmd\E \Q$file\E"
> > --- 8< ---
> >
> > However, it's possible that might break existing users who rely on
> > --cc-cmd="myscript --option arg" working. It's not clear which
> > behavior is correct.
>
> The more I think about this, the less I consider this a bug in
> git-send-email. As noted, people might legitimately use a complex
> command (--cc-cmd="myscript--option arg"), so changing git-send-email
> to treat cc-cmd as an atomic string seems like a bad idea.
A while back I proposed some documentation updates
https://public-inbox.org/git/1437416790-5792-1-git-send-email-philipoakley@iee.org/
regarding what is (should be) allowed in the cc-cmd etc., and at the time
Junio suggested that possible existing uses of the current code would be
abuses. I didn't pursue it further, but it may be useful guidance here as to
potential real world command lines..
>
> Assuming no changes to git-send-email, to get your test working, you
> could try to figure out how to quote the script's path you're
> specifying with --cc-cmd, however, even easier would be to drop $(pwd)
> altogether. That is, instead of:
>
> --cc-cmd="$(pwd)/expected-cc-script.sh"
>
> just use:
>
> --cc-cmd=./expected-cc-script.sh
next prev parent reply other threads:[~2017-11-21 0:07 UTC|newest]
Thread overview: 27+ messages / expand[flat|nested] mbox.gz Atom feed top
2017-11-16 15:48 [PATCH] git-send-email: fix get_maintainer.pl regression Alex Bennée
2017-11-16 16:46 ` Alex Bennée
2017-11-19 2:54 ` Eric Sunshine
2017-11-20 10:44 ` Alex Bennée
2017-11-20 22:34 ` Eric Sunshine
2017-11-20 18:57 ` Eric Sunshine
2017-11-21 0:07 ` Philip Oakley [this message]
2017-11-21 0:30 ` Eric Sunshine
2017-11-21 0:32 ` Junio C Hamano
2017-11-20 22:14 ` Eric Sunshine
2017-11-21 20:46 ` Alex Bennée
2017-11-21 20:52 ` Thomas Adam
2017-11-22 1:34 ` Junio C Hamano
2017-12-11 17:13 ` Alex Bennée
2017-12-11 17:26 ` Thomas Adam
2017-12-11 19:46 ` Ævar Arnfjörð Bjarmason
2017-12-12 10:30 ` Thomas Adam
2017-12-12 11:49 ` Ævar Arnfjörð Bjarmason
2017-12-12 16:40 ` Alex Bennée
2017-12-12 18:14 ` Ævar Arnfjörð Bjarmason
2017-12-12 19:35 ` Junio C Hamano
2017-12-12 21:25 ` Ævar Arnfjörð Bjarmason
2017-12-12 22:19 ` Junio C Hamano
[not found] ` <b131cc195280498ea3a77a37eff8444e@BPMBX2013-01.univ-lyon1.fr>
2017-11-22 8:22 ` Matthieu Moy
2017-11-22 9:05 ` Alex Bennée
2017-11-22 9:49 ` Thomas Adam
2017-11-22 10:44 ` Junio C Hamano
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=D810DA8B202742B38343EBB44BD54A1D@PhilipOakley \
--to=philipoakley@iee.org \
--cc=alex.bennee@linaro.org \
--cc=git@vger.kernel.org \
--cc=sunshine@sunshineco.com \
/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