{"thread":{"id":"59102","subject":"[PATCH v7 1/2] send-email: refactor header generation functions","startedAt":"2023-01-17T14:29:07Z","lastAt":"2023-01-17T23:32:22Z","messageCount":5,"participants":["Strawbridge, Michael","Junio C Hamano"],"isPatch":true,"patchVersion":7,"patchTotal":2},"messages":[{"id":"470493","messageId":"20230117142706.230404-2-michael.strawbridge@amd.com","threadId":"59102","inReplyTo":"20230117142706.230404-1-michael.strawbridge@amd.com","subject":"[PATCH v7 1/2] send-email: refactor header generation functions","fromName":"Strawbridge, Michael","fromEmail":"michael.strawbridge@amd.com","sentAt":"2023-01-17T14:27:17Z","receivedAt":"2023-01-17T14:29:07Z","isPatch":true,"sender":{"key":"michael.strawbridge@amd.com","avatar":null},"body":"Split process_file and send_message into easier to use functions.\nMaking SMTP header information widely available.\n\nCc: Luben Tuikov <luben.tuikov@amd.com>\nCc: Junio C Hamano <gitster@pobox.com>\nSigned-off-by: Michael Strawbridge <michael.strawbridge@amd.com>\n---\n git-send-email.perl | 49 ++++++++++++++++++++++++++++-----------------\n 1 file changed, 31 insertions(+), 18 deletions(-)\n\ndiff --git a/git-send-email.perl b/git-send-email.perl\nindex 5861e99a6e..42f135a266 100755\n--- a/git-send-email.perl\n+++ b/git-send-email.perl\n@@ -1495,16 +1495,7 @@ sub file_name_is_absolute {\n \treturn File::Spec::Functions::file_name_is_absolute($path);\n }\n \n-# Prepares the email, then asks the user what to do.\n-#\n-# If the user chooses to send the email, it's sent and 1 is returned.\n-# If the user chooses not to send the email, 0 is returned.\n-# If the user decides they want to make further edits, -1 is returned and the\n-# caller is expected to call send_message again after the edits are performed.\n-#\n-# If an error occurs sending the email, this just dies.\n-\n-sub send_message {\n+sub gen_header {\n \tmy @recipients = unique_email_list(@to);\n \t@cc = (grep { my $cc = extract_valid_address_or_die($_);\n \t\t      not grep { $cc eq $_ || $_ =~ /<\\Q${cc}\\E>$/ } @recipients\n@@ -1546,6 +1537,22 @@ sub send_message {\n \tif (@xh) {\n \t\t$header .= join(\"\\n\", @xh) . \"\\n\";\n \t}\n+\tmy $recipients_ref = \\@recipients;\n+\treturn ($recipients_ref, $to, $date, $gitversion, $cc, $ccline, $header);\n+}\n+\n+# Prepares the email, then asks the user what to do.\n+#\n+# If the user chooses to send the email, it's sent and 1 is returned.\n+# If the user chooses not to send the email, 0 is returned.\n+# If the user decides they want to make further edits, -1 is returned and the\n+# caller is expected to call send_message again after the edits are performed.\n+#\n+# If an error occurs sending the email, this just dies.\n+\n+sub send_message {\n+\tmy ($recipients_ref, $to, $date, $gitversion, $cc, $ccline, $header) = gen_header();\n+\tmy @recipients = @$recipients_ref;\n \n \tmy @sendmail_parameters = ('-i', @recipients);\n \tmy $raw_from = $sender;\n@@ -1735,11 +1742,8 @@ sub send_message {\n $references = $initial_in_reply_to || '';\n $message_num = 0;\n \n-# Prepares the email, prompts the user, sends it out\n-# Returns 0 if an edit was done and the function should be called again, or 1\n-# otherwise.\n-sub process_file {\n-\tmy ($t) = @_;\n+sub pre_process_file {\n+\tmy ($t, $quiet) = @_;\n \n \topen my $fh, \"<\", $t or die sprintf(__(\"can't open file %s\"), $t);\n \n@@ -1893,9 +1897,9 @@ sub process_file {\n \t}\n \tclose $fh;\n \n-\tpush @to, recipients_cmd(\"to-cmd\", \"to\", $to_cmd, $t)\n+\tpush @to, recipients_cmd(\"to-cmd\", \"to\", $to_cmd, $t, $quiet)\n \t\tif defined $to_cmd;\n-\tpush @cc, recipients_cmd(\"cc-cmd\", \"cc\", $cc_cmd, $t)\n+\tpush @cc, recipients_cmd(\"cc-cmd\", \"cc\", $cc_cmd, $t, $quiet)\n \t\tif defined $cc_cmd && !$suppress_cc{'cccmd'};\n \n \tif ($broken_encoding{$t} && !$has_content_type) {\n@@ -1954,6 +1958,15 @@ sub process_file {\n \t\t\t@initial_to = @to;\n \t\t}\n \t}\n+}\n+\n+# Prepares the email, prompts the user, and sends it out\n+# Returns 0 if an edit was done and the function should be called again, or 1\n+# on the email being successfully sent out.\n+sub process_file {\n+\tmy ($t) = @_;\n+\n+        pre_process_file($t, $quiet);\n \n \tmy $message_was_sent = send_message();\n \tif ($message_was_sent == -1) {\n@@ -2002,7 +2015,7 @@ sub process_file {\n # Execute a command (e.g. $to_cmd) to get a list of email addresses\n # and return a results array\n sub recipients_cmd {\n-\tmy ($prefix, $what, $cmd, $file) = @_;\n+\tmy ($prefix, $what, $cmd, $file, $quiet) = @_;\n \n \tmy @addresses = ();\n \topen my $fh, \"-|\", \"$cmd \\Q$file\\E\"\n-- \n2.34.1\n"},{"id":"470494","messageId":"20230117142706.230404-3-michael.strawbridge@amd.com","threadId":"59102","inReplyTo":"20230117142706.230404-1-michael.strawbridge@amd.com","subject":"[PATCH v7 2/2] send-email: expose header information to git-send-email's sendemail-validate hook","fromName":"Strawbridge, Michael","fromEmail":"michael.strawbridge@amd.com","sentAt":"2023-01-17T14:27:20Z","receivedAt":"2023-01-17T14:29:10Z","isPatch":true,"sender":{"key":"michael.strawbridge@amd.com","avatar":null},"body":"To allow further flexibility in the Git hook, the SMTP header\ninformation of the email which git-send-email intends to send, is now\npassed as the 2nd argument to the sendemail-validate hook.\n\nAs an example, this can be useful for acting upon keywords in the\nsubject or specific email addresses.\n\nCc: Luben Tuikov <luben.tuikov@amd.com>\nCc: Junio C Hamano <gitster@pobox.com>\nSigned-off-by: Michael Strawbridge <michael.strawbridge@amd.com>\n---\n Documentation/githooks.txt | 28 +++++++++++++++++++++++----\n git-send-email.perl        | 31 ++++++++++++++++++++----------\n t/t9001-send-email.sh      | 39 ++++++++++++++++++++++++++++++++++++--\n 3 files changed, 82 insertions(+), 16 deletions(-)\n\ndiff --git a/Documentation/githooks.txt b/Documentation/githooks.txt\nindex a16e62bc8c..978d599be5 100644\n--- a/Documentation/githooks.txt\n+++ b/Documentation/githooks.txt\n@@ -583,10 +583,30 @@ processed by rebase.\n sendemail-validate\n ~~~~~~~~~~~~~~~~~~\n \n-This hook is invoked by linkgit:git-send-email[1].  It takes a single parameter,\n-the name of the file that holds the e-mail to be sent.  Exiting with a\n-non-zero status causes `git send-email` to abort before sending any\n-e-mails.\n+This hook is invoked by linkgit:git-send-email[1].\n+\n+It takes these command line arguments. They are,\n+1. the name of the file which holds the contents of the email to be sent.\n+2. The name of the file which holds the SMTP envelope and headers of the email.\n+\n+The SMTP envelope and headers are passed in the exact same way as they\n+are passed to the user's Mail Transport Agent (MTA). In effect, the email\n+given to the user's MTA, is the contents of $2 followed by the contents\n+of $1.\n+\n+Below is an example for a few common headers. Take notice of the\n+capitalization and multi-line tab structure.\n+\n+  From: Example <from@example.com>\n+  To: to@example.com\n+  Cc: cc@example.com,\n+\t  A <author@example.com>,\n+\t  One <one@example.com>,\n+\t  two@example.com\n+  Subject: PATCH-STRING\n+\n+Exiting with a non-zero status causes `git send-email` to abort\n+before sending any e-mails.\n \n fsmonitor-watchman\n ~~~~~~~~~~~~~~~~~~\ndiff --git a/git-send-email.perl b/git-send-email.perl\nindex 42f135a266..4f2039284e 100755\n--- a/git-send-email.perl\n+++ b/git-send-email.perl\n@@ -787,14 +787,6 @@ sub is_format_patch_arg {\n \n @files = handle_backup_files(@files);\n \n-if ($validate) {\n-\tforeach my $f (@files) {\n-\t\tunless (-p $f) {\n-\t\t\tvalidate_patch($f, $target_xfer_encoding);\n-\t\t}\n-\t}\n-}\n-\n if (@files) {\n \tunless ($quiet) {\n \t\tprint $_,\"\\n\" for (@files);\n@@ -1738,6 +1730,16 @@ sub send_message {\n \treturn 1;\n }\n \n+if ($validate) {\n+\tforeach my $f (@files) {\n+\t\tunless (-p $f) {\n+\t\t        pre_process_file($f, 1);\n+\n+\t\t\tvalidate_patch($f, $target_xfer_encoding);\n+\t\t}\n+\t}\n+}\n+\n $in_reply_to = $initial_in_reply_to;\n $references = $initial_in_reply_to || '';\n $message_num = 0;\n@@ -2101,11 +2103,20 @@ sub validate_patch {\n \t\t\tchdir($repo->wc_path() or $repo->repo_path())\n \t\t\t\tor die(\"chdir: $!\");\n \t\t\tlocal $ENV{\"GIT_DIR\"} = $repo->repo_path();\n+\n+\t\t\tmy ($recipients_ref, $to, $date, $gitversion, $cc, $ccline, $header) = gen_header();\n+\n+\t\t\trequire File::Temp;\n+\t\t\tmy ($header_filehandle, $header_filename) = File::Temp::tempfile(\n+                            \".gitsendemail.header.XXXXXX\", DIR => $repo->repo_path());\n+\t\t\tprint $header_filehandle $header;\n+\n \t\t\tmy @cmd = (\"git\", \"hook\", \"run\", \"--ignore-missing\",\n \t\t\t\t    $hook_name, \"--\");\n-\t\t\tmy @cmd_msg = (@cmd, \"<patch>\");\n-\t\t\tmy @cmd_run = (@cmd, $target);\n+\t\t\tmy @cmd_msg = (@cmd, \"<patch>\", \"<header>\");\n+\t\t\tmy @cmd_run = (@cmd, $target, $header_filename);\n \t\t\t$hook_error = system_or_msg(\\@cmd_run, undef, \"@cmd_msg\");\n+\t\t\tunlink($header_filehandle);\n \t\t\tchdir($cwd_save) or die(\"chdir: $!\");\n \t\t}\n \t\tif ($hook_error) {\ndiff --git a/t/t9001-send-email.sh b/t/t9001-send-email.sh\nindex 1130ef21b3..c0fcbacdaa 100755\n--- a/t/t9001-send-email.sh\n+++ b/t/t9001-send-email.sh\n@@ -540,7 +540,7 @@ test_expect_success $PREREQ \"--validate respects relative core.hooksPath path\" '\n \ttest_path_is_file my-hooks.ran &&\n \tcat >expect <<-EOF &&\n \tfatal: longline.patch: rejected by sendemail-validate hook\n-\tfatal: command '\"'\"'git hook run --ignore-missing sendemail-validate -- <patch>'\"'\"' died with exit code 1\n+\tfatal: command '\"'\"'git hook run --ignore-missing sendemail-validate -- <patch> <header>'\"'\"' died with exit code 1\n \twarning: no patches were sent\n \tEOF\n \ttest_cmp expect actual\n@@ -559,12 +559,47 @@ test_expect_success $PREREQ \"--validate respects absolute core.hooksPath path\" '\n \ttest_path_is_file my-hooks.ran &&\n \tcat >expect <<-EOF &&\n \tfatal: longline.patch: rejected by sendemail-validate hook\n-\tfatal: command '\"'\"'git hook run --ignore-missing sendemail-validate -- <patch>'\"'\"' died with exit code 1\n+\tfatal: command '\"'\"'git hook run --ignore-missing sendemail-validate -- <patch> <header>'\"'\"' died with exit code 1\n \twarning: no patches were sent\n \tEOF\n \ttest_cmp expect actual\n '\n \n+test_expect_success $PREREQ 'setup expect' \"\n+cat >expected-headers <<\\EOF\n+From: Example <from@example.com>\n+To: to@example.com\n+Cc: cc@example.com,\n+\tA <author@example.com>,\n+\tOne <one@example.com>,\n+\ttwo@example.com\n+Subject: [PATCH 1/1] Second.\n+Date: DATE-STRING\n+Message-Id: MESSAGE-ID-STRING\n+X-Mailer: X-MAILER-STRING\n+Reply-To: Reply <reply@example.com>\n+MIME-Version: 1.0\n+Content-Transfer-Encoding: quoted-printable\n+EOF\n+\"\n+\n+test_expect_success $PREREQ \"--validate hook supports header argument\" '\n+\twrite_script my-hooks/sendemail-validate <<-\\EOF &&\n+\tgrep \"X-test-header: v1.0\" \"$2\"\n+\tEOF\n+\ttest_config core.hooksPath \"my-hooks\" &&\n+\trm -fr outdir &&\n+\tgit format-patch \\\n+\t\t--add-header=\"X-test-header: v1.0\" \\\n+\t\t-n HEAD^1 -o outdir &&\n+\tgit send-email \\\n+\t\t--dry-run \\\n+\t\t--to=nobody@example.com \\\n+\t\t--smtp-server=\"$(pwd)/fake.sendmail\" \\\n+\t\t--validate \\\n+\t\toutdir/000?-*.patch\n+'\n+\n for enc in 7bit 8bit quoted-printable base64\n do\n \ttest_expect_success $PREREQ \"--transfer-encoding=$enc produces correct header\" '\n-- \n2.34.1\n"},{"id":"470495","messageId":"20230117142706.230404-1-michael.strawbridge@amd.com","threadId":"59102","inReplyTo":null,"subject":"[PATCH v7 0/2] send-email: expose header information to git-send-email's sendemail-validate hook","fromName":"Strawbridge, Michael","fromEmail":"michael.strawbridge@amd.com","sentAt":"2023-01-17T14:27:13Z","receivedAt":"2023-01-17T14:29:16Z","isPatch":true,"sender":{"key":"michael.strawbridge@amd.com","avatar":null},"body":"Thank you both for the input on v6.  I believe that I have incorporated\nall of your feedback.  I decided to keep a few examples of headers in\nthe documentation as I believe if I were someone wanting to write a hook\nI would find the example helpful.  It points out unobvious things like\nCc being capitalized as it is and also how multiple lines work.\n\nMichael Strawbridge (2):\n  send-email: refactor header generation functions\n  send-email: expose header information to git-send-email's\n    sendemail-validate hook\n\n Documentation/githooks.txt | 28 +++++++++++--\n git-send-email.perl        | 80 +++++++++++++++++++++++++-------------\n t/t9001-send-email.sh      | 39 ++++++++++++++++++-\n 3 files changed, 113 insertions(+), 34 deletions(-)\n\n-- \n2.34.1\n"},{"id":"470502","messageId":"xmqqedrt9mmt.fsf@gitster.g","threadId":"59102","inReplyTo":"20230117142706.230404-3-michael.strawbridge@amd.com","subject":"Re: [PATCH v7 2/2] send-email: expose header information to git-send-email's sendemail-validate hook","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2023-01-17T16:00:42Z","receivedAt":"2023-01-17T16:00:48Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"\"Strawbridge, Michael\" <Michael.Strawbridge@amd.com> writes:\n\n> +This hook is invoked by linkgit:git-send-email[1].\n> +\n> +It takes these command line arguments. They are,\n> +1. the name of the file which holds the contents of the email to be sent.\n> +2. The name of the file which holds the SMTP envelope and headers of the email.\n\nThe previous iteration said SMTP headers, but now this talks about\nenvelope.  I however did not think we have direct access to any\nenvelope information [*] (i.e. such a feature is necessary if we\nwant to send to one set of recipients by specifying their addresses\nin the envelope, while recording different set of recipients to the\ne-mail headers' To: and Cc: list)?\n\n\tSide note.  We can specify different sender and it gets\n\tpassed as a command line argument \"-f $sender\" to the\n\tsendmail program, which is used in \"MAIL FROM\" SMTP\n\tenvelope.  But I do not think we muck with the list of\n\trecipients that appear in the header when we formulate \"RCPT\n\tTO\".  And I do not see where you give \"MAIL FROM\" value in\n\tthe input to the hook in the patch.\n\nIf we say \"we will give your hook information in the envelope and\nheaders\" to those who know the distinction between the two, they\nwill inevitably say \"that is great. Now how do I tell which in file\n$2 are in the envelope and which are in the header, when some of\nthem are different?\"\n\nWe just hand the message plus the header, and let $sendmail_cmd come\nup with the envelope info using what is in the header, no?  I am not\nsure we want to mention envelope as that would give readers a false\nimpression that we may treat it separately from the headers.\n\nThanks.\n"},{"id":"470561","messageId":"87pmbc3mmx.fsf@amd.com","threadId":"59102","inReplyTo":"xmqqedrt9mmt.fsf@gitster.g","subject":"Re: [PATCH v7 2/2] send-email: expose header information to git-send-email's sendemail-validate hook","fromName":"Strawbridge, Michael","fromEmail":"michael.strawbridge@amd.com","sentAt":"2023-01-17T20:57:11Z","receivedAt":"2023-01-17T23:32:22Z","isPatch":true,"sender":{"key":"michael.strawbridge@amd.com","avatar":null},"body":"\nJunio C Hamano <gitster@pobox.com> writes:\n\n> \"Strawbridge, Michael\" <Michael.Strawbridge@amd.com> writes:\n>\n>> +This hook is invoked by linkgit:git-send-email[1].\n>> +\n>> +It takes these command line arguments. They are,\n>> +1. the name of the file which holds the contents of the email to be sent.\n>> +2. The name of the file which holds the SMTP envelope and headers of the email.\n>\n> The previous iteration said SMTP headers, but now this talks about\n> envelope.  I however did not think we have direct access to any\n> envelope information [*] (i.e. such a feature is necessary if we\n> want to send to one set of recipients by specifying their addresses\n> in the envelope, while recording different set of recipients to the\n> e-mail headers' To: and Cc: list)?\n>\n> \tSide note.  We can specify different sender and it gets\n> \tpassed as a command line argument \"-f $sender\" to the\n> \tsendmail program, which is used in \"MAIL FROM\" SMTP\n> \tenvelope.  But I do not think we muck with the list of\n> \trecipients that appear in the header when we formulate \"RCPT\n> \tTO\".  And I do not see where you give \"MAIL FROM\" value in\n> \tthe input to the hook in the patch.\n>\n> If we say \"we will give your hook information in the envelope and\n> headers\" to those who know the distinction between the two, they\n> will inevitably say \"that is great. Now how do I tell which in file\n> $2 are in the envelope and which are in the header, when some of\n> them are different?\"\n>\n> We just hand the message plus the header, and let $sendmail_cmd come\n> up with the envelope info using what is in the header, no?  I am not\n> sure we want to mention envelope as that would give readers a false\n> impression that we may treat it separately from the headers.\n>\n> Thanks.\n\nThat's fair.  I will remove the mention of envelope."}]}