From: Gregory Anders <greg@gpanders.com>
To: Junio C Hamano <gitster@pobox.com>
Cc: git@vger.kernel.org
Subject: Re: [PATCH v3] git-send-email: add option to specify sendmail command
Date: Fri, 14 May 2021 08:12:48 -0600 [thread overview]
Message-ID: <YJ6FYDBdRsl3PCcc@gpanders.com> (raw)
In-Reply-To: <xmqq7dk27xi2.fsf@gitster.g>
On Fri, 14 May 2021 13:25 +0900, Junio C Hamano wrote:
>Are you talking about the use of $sm that is local to the debug
>output? I think leaving $sendmail_cmd intact by using a separate
>variable is the right choice. Isn't the problem you observed a
>consequence of send_message() getting called once for each message,
>so assigning to $sendmail_cmd in the function for the first
>invocation of the function would change its value for the second
>invocation?
Yes that is right. That makes sense. I didn't realize the subprocess was
called twice, though that is such an obvious explanation I don't know
why I didn't think of it.
>Also, if we have been using
>
> --smtp-server=$(pwd)/fake.sendmail
>
>we cannot expect to use the same value like this:
>
> --sendmail-cmd=$(pwd)/fake.sendmail
>
>because we deliberately add a space in the $(pwd) by choosing the
>name of the test directory to be "trash directory.something". We'd
>need to do something like
>
> --sendmail-cmd='$(pwd)/fake.sendmail'
>
>so that the shell sees '$(pwd)/fake.sendmail' literally and runs pwd
>to find out what the path to the program is, I would think.
Indeed, in prior versions of this patch I had replaced the uses of
`--smtp-server` in the test suite with `--sendmail-cmd` which included
those extra quotes (I reverted back to using --smtp-server after
feedback from other reviewers in lieu of simply adding new test cases
for --sendmail-cmd).
>> +test_expect_success $PREREQ 'test using arguments with
>> --sendmail-cmd' '
>> + clean_fake_sendmail &&
>> + git send-email \
>> + --from="Example <nobody@example.com>" \
>> + --to=nobody@example.com \
>> + --sendmail-cmd="\"$(pwd)/fake.sendmail\" -f nobody@example.com" \
>> + HEAD^ &&
>> + test_path_is_file commandline1
>> +'
>
>Hmph, if $(pwd) has a double quote character in it, this may not
>work as expected, as the shell that is expanding the command line
>arguments for "git send-email" would see $(pwd), expand it and our
>program will see
>
> "/path/with/d"quote/git/t/trash directory.9001/fake.sendmail" -f nobody@e.c
>
>as the value of --sendmail-cmd, which would not interpolate well,
>no?
>
>We want the shell that eats the command line of 'git send-email' to see
>
> --sendmail-cmd='$(pwd)/fake.sendmail'\" -f nobody@example.com"
>
>and because this is inside a sq pair, it would become
>
> --sendmail-cmd='\''$(pwd)/fake.sendmail'\''\" -f nobody@example.com"
>
>after we replace each sq with '\'', or something like that, perhaps?
I'll take a look at this (as well as your followup email) and send a new
version.
Thanks,
Greg
next prev parent reply other threads:[~2021-05-14 14:12 UTC|newest]
Thread overview: 19+ messages / expand[flat|nested] mbox.gz Atom feed top
2021-05-12 3:30 [PATCH] git-send-email: add sendmailCommand option Gregory Anders
2021-05-12 4:19 ` Junio C Hamano
2021-05-12 13:03 ` Gregory Anders
2021-05-12 7:57 ` Felipe Contreras
2021-05-12 13:12 ` Gregory Anders
2021-05-12 17:21 ` Felipe Contreras
2021-05-12 18:06 ` Gregory Anders
2021-05-12 19:32 ` Felipe Contreras
2021-05-12 9:04 ` Ævar Arnfjörð Bjarmason
2021-05-12 13:18 ` Gregory Anders
2021-05-13 2:32 ` [PATCH v2] git-send-email: add option to specify sendmail command Gregory Anders
2021-05-13 3:58 ` Junio C Hamano
2021-05-13 13:31 ` Gregory Anders
2021-05-13 21:21 ` Junio C Hamano
2021-05-13 15:23 ` [PATCH v3] " Gregory Anders
2021-05-14 4:25 ` Junio C Hamano
2021-05-14 5:16 ` Junio C Hamano
2021-05-14 14:12 ` Gregory Anders [this message]
2021-05-14 15:15 ` [PATCH v4] " Gregory Anders
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=YJ6FYDBdRsl3PCcc@gpanders.com \
--to=greg@gpanders.com \
--cc=git@vger.kernel.org \
--cc=gitster@pobox.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 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.