Re: [PATCH] git-send-email: fix get_maintainer.pl regression

[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.

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