git/list[1] front-page[2] threads[3] people[4] search[5] about
 

Re: [PATCH v3] git-send-email: add option to specify sendmail command

From
Gregory Anders <greg@gpanders.com>
Date
May 14, 2021, 14:12 UTC
Message-ID
<YJ6FYDBdRsl3PCcc@gpanders.com>
In-Reply-To
<xmqq7dk27xi2.fsf@gitster.g>
On Fri, 14 May 2021 13:25 +0900, Junio C Hamano wrote:
Show 7 quoted lines
>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.

Show 16 quoted lines
>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).

Show 30 quoted lines
>> +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
Previous: Junio C HamanoNext: Gregory Anders
Message 18 of 19 in “git-send-email: add sendmailCommand option”
  1. git-send-email: add sendmailCommand optionGregory Anders, May 12, 2021
  2. Junio C HamanoMay 12, 2021
  3. Gregory AndersMay 12, 2021
  4. Felipe ContrerasMay 12, 2021
  5. Gregory AndersMay 12, 2021
  6. Felipe ContrerasMay 12, 2021
  7. Gregory AndersMay 12, 2021
  8. Felipe ContrerasMay 12, 2021
  9. Ævar Arnfjörð BjarmasonMay 12, 2021
  10. Gregory AndersMay 12, 2021
  11. git-send-email: add option to specify sendmail commandGregory Anders, May 13, 2021
  12. Junio C HamanoMay 13, 2021
  13. Gregory AndersMay 13, 2021
  14. Junio C HamanoMay 13, 2021
  15. git-send-email: add option to specify sendmail commandGregory Anders, May 13, 2021
  16. Junio C HamanoMay 14, 2021
  17. Junio C HamanoMay 14, 2021
  18. Gregory AndersMay 14, 2021
  19. git-send-email: add option to specify sendmail commandGregory Anders, May 14, 2021

Read the whole thread, see it on lore, or plain text.

$ cat FOOTERMessages come from the public archive at lore.kernel.org/git, fetched every hour. The front page is chosen and written each morning by an AI editor and can be wrong; the threads themselves are the record. About and API. For agents: an MCP server at https://gitlist.dev/mcp, and any thread, story or person page as Markdown by adding .md to its URL (or sending Accept: text/markdown). Details in /llms.txt.