Git development
 help / color / mirror / Atom feed
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


  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