{"thread":{"id":"59099","subject":"[PATCH v6 0/2] send-email: expose header information to git-send-email's sendemail-validate hook","startedAt":"2023-01-17T01:40:02Z","lastAt":"2023-01-18T20:44:38Z","messageCount":12,"participants":["Strawbridge, Michael","Luben Tuikov","Junio C Hamano","Michael Strawbridge"],"isPatch":true,"patchVersion":6,"patchTotal":2},"messages":[{"id":"470465","messageId":"20230117013932.47570-1-michael.strawbridge@amd.com","threadId":"59099","inReplyTo":null,"subject":"[PATCH v6 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-17T01:39:39Z","receivedAt":"2023-01-17T01:40:02Z","isPatch":true,"sender":{"key":"michael.strawbridge@amd.com","avatar":null},"body":"Thank you for all the great feedback! At your suggestion I improved the\ntest for the header argument and the documentation.\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 | 29 ++++++++++++--\n git-send-email.perl | 80 +++++++++++++++++++++++++-------------\n t/t9001-send-email.sh | 47 +++++++++++++++++++++-\n 3 files changed, 122 insertions(+), 34 deletions(-)\n\n-- \n2.34.1\n"},{"id":"470466","messageId":"20230117013932.47570-2-michael.strawbridge@amd.com","threadId":"59099","inReplyTo":"20230117013932.47570-1-michael.strawbridge@amd.com","subject":"[PATCH v6 1/2] send-email: refactor header generation functions","fromName":"Strawbridge, Michael","fromEmail":"michael.strawbridge@amd.com","sentAt":"2023-01-17T01:39:41Z","receivedAt":"2023-01-17T01:40:04Z","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 more widely available.\n\nCc: Luben Tuikov \nCc: Junio C Hamano \nSigned-off-by: Michael Strawbridge \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..810dd1f1ce 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, 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+\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":"470467","messageId":"20230117013932.47570-3-michael.strawbridge@amd.com","threadId":"59099","inReplyTo":"20230117013932.47570-1-michael.strawbridge@amd.com","subject":"[PATCH v6 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-17T01:39:42Z","receivedAt":"2023-01-17T01:40:08Z","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 that git-send-email intends to send, is now\npassed as a 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 \nCc: Junio C Hamano \nSigned-off-by: Michael Strawbridge \n---\n Documentation/githooks.txt | 29 +++++++++++++++++++----\n git-send-email.perl | 31 +++++++++++++++++--------\n t/t9001-send-email.sh | 47 ++++++++++++++++++++++++++++++++++++--\n 3 files changed, 91 insertions(+), 16 deletions(-)\n\ndiff --git a/Documentation/githooks.txt b/Documentation/githooks.txt\nindex a16e62bc8c..e80f481efd 100644\n--- a/Documentation/githooks.txt\n+++ b/Documentation/githooks.txt\n@@ -583,10 +583,31 @@ 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:\n+1. the name of the file that holds the e-mail to be sent.\n+2. the name of the file that holds the SMTP headers to be used.\n+\n+The SMTP headers will be passed to the hook in the below format.\n+Take notice of the capitalization and multi-line tab structure.\n+\n+ From: Example \n+ To: to@example.com\n+ Cc: cc@example.com,\n+\t A ,\n+\t One ,\n+\t two@example.com\n+ Subject: PATCH-STRING\n+ Date: DATE-STRING\n+ Message-Id: MESSAGE-ID-STRING\n+ X-Mailer: X-MAILER-STRING\n+ Reply-To: Reply \n+ MIME-Version: 1.0\n+ Content-Transfer-Encoding: quoted-printable\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 810dd1f1ce..b2adca515e 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, \"\");\n-\t\t\tmy @cmd_run = (@cmd, $target);\n+\t\t\tmy @cmd_msg = (@cmd, \"\", \"
\");\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..346ff1463e 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 -- '\"'\"' died with exit code 1\n+\tfatal: command '\"'\"'git hook run --ignore-missing sendemail-validate --
'\"'\"' died with exit code 1\n \twarning: no patches were sent\n \tEOF\n \ttest_cmp expect actual\n@@ -559,12 +559,55 @@ 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 -- '\"'\"' died with exit code 1\n+\tfatal: command '\"'\"'git hook run --ignore-missing sendemail-validate --
'\"'\"' 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 \n+To: to@example.com\n+Cc: cc@example.com,\n+\tA ,\n+\tOne ,\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 \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+\tif test -s \"$2\"\n+\tthen\n+\t\tcat \"$2\" >actual\n+\t\texit 1\n+\tfi\n+\tEOF\n+\ttest_config core.hooksPath \"my-hooks\" &&\n+\ttest_must_fail git send-email \\\n+\t\t--dry-run \\\n+\t\t--suppress-cc=sob \\\n+\t\t--from=\"Example \" \\\n+\t\t--reply-to=\"Reply \" \\\n+\t\t--to=to@example.com \\\n+\t\t--cc=cc@example.com \\\n+\t\t--bcc=bcc@example.com \\\n+\t\t--smtp-server=\"$(pwd)/fake.sendmail\" \\\n+\t\t--validate \\\n+\t\tlongline.patch &&\n+\tcat actual | replace_variable_fields \\\n+\t>actual-headers &&\n+\ttest_cmp expected-headers actual-headers\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":"470475","messageId":"045ef69a-607c-7644-9790-0af367109b5e@amd.com","threadId":"59099","inReplyTo":"20230117013932.47570-2-michael.strawbridge@amd.com","subject":"Re: [PATCH v6 1/2] send-email: refactor header generation functions","fromName":"Luben Tuikov","fromEmail":"luben.tuikov@amd.com","sentAt":"2023-01-17T03:38:52Z","receivedAt":"2023-01-17T03:39:03Z","isPatch":true,"sender":{"key":"luben.tuikov@amd.com","avatar":null},"body":"Acked by: Luben Tuikov \n\nRegards,\nLuben\n\nOn 2023-01-16 20:39, Strawbridge, Michael wrote:\n> Split process_file and send_message into easier to use functions.\n> Making SMTP header information more widely available.\n> \n> Cc: Luben Tuikov \n> Cc: Junio C Hamano \n> Signed-off-by: Michael Strawbridge \n> ---\n> git-send-email.perl | 49 ++++++++++++++++++++++++++++-----------------\n> 1 file changed, 31 insertions(+), 18 deletions(-)\n> \n> diff --git a/git-send-email.perl b/git-send-email.perl\n> index 5861e99a6e..810dd1f1ce 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, 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> +\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\n"},{"id":"470477","messageId":"68bf66f2-d9b0-5f29-2e40-7a5b97ea7d79@amd.com","threadId":"59099","inReplyTo":"20230117013932.47570-2-michael.strawbridge@amd.com","subject":"Re: [PATCH v6 1/2] send-email: refactor header generation functions","fromName":"Luben Tuikov","fromEmail":"luben.tuikov@amd.com","sentAt":"2023-01-17T04:13:16Z","receivedAt":"2023-01-17T04:13:28Z","isPatch":true,"sender":{"key":"luben.tuikov@amd.com","avatar":null},"body":"On 2023-01-16 20:39, Strawbridge, Michael wrote:\n> Split process_file and send_message into easier to use functions.\n> Making SMTP header information more widely available.\n\n\"more widely\" --> \"widely\"\n\n> \n> Cc: Luben Tuikov \n> Cc: Junio C Hamano \n> Signed-off-by: Michael Strawbridge \n> ---\n> git-send-email.perl | 49 ++++++++++++++++++++++++++++-----------------\n> 1 file changed, 31 insertions(+), 18 deletions(-)\n> \n> diff --git a/git-send-email.perl b/git-send-email.perl\n> index 5861e99a6e..810dd1f1ce 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, sends it out\n\nPerhaps add an \"and\" as \"... user, and sends it out.\"\n\n> +# Returns 0 if an edit was done and the function should be called again, or 1\n> +# otherwise.\n\n\"otherwise\" is usually used on error. Perhaps we want to say here\n\"or 1 on the email being successfully sent out.\"?\n-- \nRegards,\nLuben\n\n"},{"id":"470479","messageId":"fe862ffd-aec4-932a-73e6-e7ddb0b97ded@amd.com","threadId":"59099","inReplyTo":"20230117013932.47570-3-michael.strawbridge@amd.com","subject":"Re: [PATCH v6 2/2] send-email: expose header information to git-send-email's sendemail-validate hook","fromName":"Luben Tuikov","fromEmail":"luben.tuikov@amd.com","sentAt":"2023-01-17T04:35:37Z","receivedAt":"2023-01-17T04:36:03Z","isPatch":true,"sender":{"key":"luben.tuikov@amd.com","avatar":null},"body":"Hi Michael,\n\nGood work on this. I've a few tiny notes following.\n\nOn 2023-01-16 20:39, Strawbridge, Michael wrote:\n> To allow further flexibility in the git hook, the SMTP header\n\n\"git\" is something different. You want to use the capitalization \"Git\".\n\n> information of the email that git-send-email intends to send, is now\n\n\"that\" --> \"which\".\n\n> passed as a 2nd argument to the sendemail-validate hook.\n\n\"a 2nd argument\" --> \"the 2nd argument\".\n \n> As an example, this can be useful for acting upon keywords in the\n> subject or specific email addresses.\n> \n> Cc: Luben Tuikov \n> Cc: Junio C Hamano \n> Signed-off-by: Michael Strawbridge \n> ---\n> Documentation/githooks.txt | 29 +++++++++++++++++++----\n> git-send-email.perl | 31 +++++++++++++++++--------\n> t/t9001-send-email.sh | 47 ++++++++++++++++++++++++++++++++++++--\n> 3 files changed, 91 insertions(+), 16 deletions(-)\n> \n> diff --git a/Documentation/githooks.txt b/Documentation/githooks.txt\n> index a16e62bc8c..e80f481efd 100644\n> --- a/Documentation/githooks.txt\n> +++ b/Documentation/githooks.txt\n> @@ -583,10 +583,31 @@ 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:\n\n\"It takes two command line arguments. They are,\"\n\n> +1. the name of the file that holds the e-mail to be sent.\n\n\"which holds the contents of the email to be sent.\"\nSentence ends and the next one should be capitalized.\n\n> +2. the name of the file that holds the SMTP headers to be used.\n\n\"The name of the file which holds the SMTP envelope and headers of the email.\"\n\n> +\n> +The SMTP headers will be passed to the hook in the below format.\n> +Take notice of the capitalization and multi-line tab structure.\n\nAlways use present simple tense when describing mechanics of code,\nnot future tense, \"are passed\". Think of when the user is reading\nthis long after the patch went in.\n\nAlso, please use \"the format below.\"\n\n> + From: Example \n> + To: to@example.com\n> + Cc: cc@example.com,\n> +\t A ,\n> +\t One ,\n> +\t two@example.com\n> + Subject: PATCH-STRING\n> + Date: DATE-STRING\n> + Message-Id: MESSAGE-ID-STRING\n> + X-Mailer: X-MAILER-STRING\n> + Reply-To: Reply \n> + MIME-Version: 1.0\n> + Content-Transfer-Encoding: quoted-printable\n\nPerhaps this is too much detail and unnecessary for the generalization\nwe're trying to achieve here?\n\nMaybe the following would suffice?\n\n The SMTP envelope and headers are passed as the 2nd argument to the\n hook, exactly as they are passed to the user's Mail Transport Agent (MTA).\n In effect, the email given to the user's MTA, is the contents of $2 followed\n by the contents of $1.\n-- \nRegards,\nLuben\n\n"},{"id":"470481","messageId":"3a2d4559-fce2-80f3-bafd-5eb8ac1a7eff@amd.com","threadId":"59099","inReplyTo":"20230117013932.47570-3-michael.strawbridge@amd.com","subject":"Re: [PATCH v6 2/2] send-email: expose header information to git-send-email's sendemail-validate hook","fromName":"Luben Tuikov","fromEmail":"luben.tuikov@amd.com","sentAt":"2023-01-17T05:06:35Z","receivedAt":"2023-01-17T05:06:56Z","isPatch":true,"sender":{"key":"luben.tuikov@amd.com","avatar":null},"body":"Hi Michael,\n\nOn 2023-01-16 20:39, Strawbridge, Michael wrote:\n--[cut]--\n> diff --git a/t/t9001-send-email.sh b/t/t9001-send-email.sh\n> index 1130ef21b3..346ff1463e 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 -- '\"'\"' died with exit code 1\n> +\tfatal: command '\"'\"'git hook run --ignore-missing sendemail-validate --
'\"'\"' died with exit code 1\n> \twarning: no patches were sent\n> \tEOF\n> \ttest_cmp expect actual\n> @@ -559,12 +559,55 @@ 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 -- '\"'\"' died with exit code 1\n> +\tfatal: command '\"'\"'git hook run --ignore-missing sendemail-validate --
'\"'\"' 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 \n> +To: to@example.com\n> +Cc: cc@example.com,\n> +\tA ,\n> +\tOne ,\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 \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> +\tif test -s \"$2\"\n> +\tthen\n> +\t\tcat \"$2\" >actual\n> +\t\texit 1\n> +\tfi\n> +\tEOF\n> +\ttest_config core.hooksPath \"my-hooks\" &&\n> +\ttest_must_fail git send-email \\\n> +\t\t--dry-run \\\n> +\t\t--suppress-cc=sob \\\n> +\t\t--from=\"Example \" \\\n> +\t\t--reply-to=\"Reply \" \\\n> +\t\t--to=to@example.com \\\n> +\t\t--cc=cc@example.com \\\n> +\t\t--bcc=bcc@example.com \\\n> +\t\t--smtp-server=\"$(pwd)/fake.sendmail\" \\\n> +\t\t--validate \\\n> +\t\tlongline.patch &&\n> +\tcat actual | replace_variable_fields \\\n> +\t>actual-headers &&\n> +\ttest_cmp expected-headers actual-headers\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\nAs Junio and I discussed in the v5 2/2 patch review, here we may want to\ndo something like this: Add a custom header to the SMTP envelope and then make\nsure that that is present when the hook checks $2.\n\nTo add a custom header, (and this uses real-world data, which is good),\nuse the following:\n\ngit format-patch --stdout --add-header=\"X-test-header: v1.0\" HEAD^..HEAD > /tmp/some-temp-file\n\nThen the hook verifies that \"X-test-header: v1.0\" is present in $2, when git-send-email\nis run with /tmp/some-temp-file.\n-- \nRegards,\nLuben\n\n"},{"id":"470482","messageId":"xmqqbkmxbort.fsf@gitster.g","threadId":"59099","inReplyTo":"3a2d4559-fce2-80f3-bafd-5eb8ac1a7eff@amd.com","subject":"Re: [PATCH v6 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-17T07:31:34Z","receivedAt":"2023-01-17T07:33:48Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Luben Tuikov writes:\n\n>> +test_expect_success $PREREQ \"--validate hook supports header argument\" '\n>> +\twrite_script my-hooks/sendemail-validate <<-\\EOF &&\n>> +\tif test -s \"$2\"\n>> +\tthen\n>> +\t\tcat \"$2\" >actual\n>> +\t\texit 1\n>> +\tfi\n>> +\tEOF\n\nIf \"$2\" is not given, or an empty \"$2\" is given, is that an error?\nI am wondering if the lack of \"else\" clause (and the hook exits with\nsuccess when \"$2\" is an empty file) here is intentional.\n\n>> +\tcat actual | replace_variable_fields \\\n>> +\t>actual-headers &&\n\nDo not cat a single file into a pipe. You can instead redirect out\nof the file to whatever is reading from the pipe. I.e.\n\n\treplace_variable_fields actual-headers &&\n\n>> +\ttest_cmp expected-headers actual-headers\n>> +'\n\nOK. We make sure the presence and the order of the fields in the\noutput just like all the other tests in this file do (which I think\nmay be a bit too much---there is no strong reason to insist that\n\"Subject:\" comes before or after \"Date:\" or is spelled \"Subject:\"\nand not \"subject:\" or \"SUBJECT:\"---but that is a problem shared with\nmany other existing tests in this file and this patch is not making\nit much worse).\n\n>> for enc in 7bit 8bit quoted-printable base64\n>> do\n>> \ttest_expect_success $PREREQ \"--transfer-encoding=$enc produces correct header\" '\n>\n> As Junio and I discussed in the v5 2/2 patch review, here we may want to\n> do something like this: Add a custom header to the SMTP envelope and then make\n> sure that that is present when the hook checks $2.\n\nAdding a custom header test is also fine, but I am OK with what we\nsee above, to verify the headers just the same way as existing\ntests.\n"},{"id":"470591","messageId":"71623e1d-805d-cdc7-d872-224821c1383c@amd.com","threadId":"59099","inReplyTo":"xmqqbkmxbort.fsf@gitster.g","subject":"Re: [PATCH v6 2/2] send-email: expose header information to git-send-email's sendemail-validate hook","fromName":"Luben Tuikov","fromEmail":"luben.tuikov@amd.com","sentAt":"2023-01-18T08:31:39Z","receivedAt":"2023-01-18T09:18:10Z","isPatch":true,"sender":{"key":"luben.tuikov@amd.com","avatar":null},"body":"On 2023-01-17 02:31, Junio C Hamano wrote:\n> Luben Tuikov writes:\n> \n>>> +test_expect_success $PREREQ \"--validate hook supports header argument\" '\n>>> +\twrite_script my-hooks/sendemail-validate <<-\\EOF &&\n>>> +\tif test -s \"$2\"\n>>> +\tthen\n>>> +\t\tcat \"$2\" >actual\n>>> +\t\texit 1\n>>> +\tfi\n>>> +\tEOF\n> \n> If \"$2\" is not given, or an empty \"$2\" is given, is that an error?\n> I am wondering if the lack of \"else\" clause (and the hook exits with\n> success when \"$2\" is an empty file) here is intentional.\n\nI think we'll always have a $2, since it is the SMTP envelope and headers.\n\nFor the rest of the comments, I'll let Michael address them.\n-- \nRegards,\nLuben\n\n"},{"id":"470643","messageId":"xmqqv8l34xkp.fsf@gitster.g","threadId":"59099","inReplyTo":"71623e1d-805d-cdc7-d872-224821c1383c@amd.com","subject":"Re: [PATCH v6 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-18T16:27:50Z","receivedAt":"2023-01-18T16:30:36Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Luben Tuikov writes:\n\n> On 2023-01-17 02:31, Junio C Hamano wrote:\n>> Luben Tuikov writes:\n>> \n>>>> +test_expect_success $PREREQ \"--validate hook supports header argument\" '\n>>>> +\twrite_script my-hooks/sendemail-validate <<-\\EOF &&\n>>>> +\tif test -s \"$2\"\n>>>> +\tthen\n>>>> +\t\tcat \"$2\" >actual\n>>>> +\t\texit 1\n>>>> +\tfi\n>>>> +\tEOF\n>> \n>> If \"$2\" is not given, or an empty \"$2\" is given, is that an error?\n>> I am wondering if the lack of \"else\" clause (and the hook exits with\n>> success when \"$2\" is an empty file) here is intentional.\n>\n> I think we'll always have a $2, since it is the SMTP envelope and headers.\n\nWe write our tests to verify _that_ assumption you have. A future\ndeveloper mistakenly drops the code to append the file to the\ncommand line that invokes the hook, and we want our test to catch\nsuch a mistake.\n\nDo we really feed envelope? E.g. if the --envelope-sender= is\nused, does $2 have the \"From:\" from the header and \"MAIL TO\" from\nthe envelope separately?\n"},{"id":"470648","messageId":"fa9b1371-0a61-147f-637e-cb09f775fe22@amd.com","threadId":"59099","inReplyTo":"xmqqv8l34xkp.fsf@gitster.g","subject":"Re: [PATCH v6 2/2] send-email: expose header information to git-send-email's sendemail-validate hook","fromName":"Luben Tuikov","fromEmail":"luben.tuikov@amd.com","sentAt":"2023-01-18T16:35:19Z","receivedAt":"2023-01-18T16:36:27Z","isPatch":true,"sender":{"key":"luben.tuikov@amd.com","avatar":null},"body":"On 2023-01-18 11:27, Junio C Hamano wrote:\n> Luben Tuikov writes:\n> \n>> On 2023-01-17 02:31, Junio C Hamano wrote:\n>>> Luben Tuikov writes:\n>>>\n>>>>> +test_expect_success $PREREQ \"--validate hook supports header argument\" '\n>>>>> +\twrite_script my-hooks/sendemail-validate <<-\\EOF &&\n>>>>> +\tif test -s \"$2\"\n>>>>> +\tthen\n>>>>> +\t\tcat \"$2\" >actual\n>>>>> +\t\texit 1\n>>>>> +\tfi\n>>>>> +\tEOF\n>>>\n>>> If \"$2\" is not given, or an empty \"$2\" is given, is that an error?\n>>> I am wondering if the lack of \"else\" clause (and the hook exits with\n>>> success when \"$2\" is an empty file) here is intentional.\n>>\n>> I think we'll always have a $2, since it is the SMTP envelope and headers.\n> \n> We write our tests to verify _that_ assumption you have. A future\n> developer mistakenly drops the code to append the file to the\n> command line that invokes the hook, and we want our test to catch\n> such a mistake.\n> \n> Do we really feed envelope? E.g. if the --envelope-sender= is\n> used, does $2 have the \"From:\" from the header and \"MAIL TO\" from\n> the envelope separately?\n\nI'm not sure--I thought we did, but yes, we should _test_ that we indeed\n1) have/get $2, as a non-empty string,\n2) it is a non-empty, readable file,\n3) contains the test header we included in git-format-patch in the test.\n\nThis is what I meant when I wrote \"we'll always have $2 ...\", not having it\nis failure of some kind and yes we should test for it.\n-- \nRegards,\nLuben\n\n"},{"id":"470666","messageId":"bebed448-bb2f-a33d-469e-d6056075ee4e@amd.com","threadId":"59099","inReplyTo":"fa9b1371-0a61-147f-637e-cb09f775fe22@amd.com","subject":"Re: [PATCH v6 2/2] send-email: expose header information to git-send-email's sendemail-validate hook","fromName":"Michael Strawbridge","fromEmail":"michael.strawbridge@amd.com","sentAt":"2023-01-18T20:44:23Z","receivedAt":"2023-01-18T20:44:38Z","isPatch":true,"sender":{"key":"michael.strawbridge@amd.com","avatar":null},"body":"\nOn 2023-01-18 11:35, Luben Tuikov wrote:\n> On 2023-01-18 11:27, Junio C Hamano wrote:\n>> Luben Tuikov writes:\n>>\n>>> On 2023-01-17 02:31, Junio C Hamano wrote:\n>>>> Luben Tuikov writes:\n>>>>\n>>>>>> +test_expect_success $PREREQ \"--validate hook supports header argument\" '\n>>>>>> +\twrite_script my-hooks/sendemail-validate <<-\\EOF &&\n>>>>>> +\tif test -s \"$2\"\n>>>>>> +\tthen\n>>>>>> +\t\tcat \"$2\" >actual\n>>>>>> +\t\texit 1\n>>>>>> +\tfi\n>>>>>> +\tEOF\n>>>> If \"$2\" is not given, or an empty \"$2\" is given, is that an error?\n>>>> I am wondering if the lack of \"else\" clause (and the hook exits with\n>>>> success when \"$2\" is an empty file) here is intentional.\n>>> I think we'll always have a $2, since it is the SMTP envelope and headers.\n>> We write our tests to verify _that_ assumption you have. A future\n>> developer mistakenly drops the code to append the file to the\n>> command line that invokes the hook, and we want our test to catch\n>> such a mistake.\n>>\n>> Do we really feed envelope? E.g. if the --envelope-sender= is\n>> used, does $2 have the \"From:\" from the header and \"MAIL TO\" from\n>> the envelope separately?\n> I'm not sure--I thought we did, but yes, we should _test_ that we indeed\n> 1) have/get $2, as a non-empty string,\n> 2) it is a non-empty, readable file,\n> 3) contains the test header we included in git-format-patch in the test.\n>\n> This is what I meant when I wrote \"we'll always have $2 ...\", not having it\n> is failure of some kind and yes we should test for it.\n\nI've tested using the envelope-sender= and the hook only gets the headers.  I've applied the feedback above in patch set v8 including a test for the 2nd argument.  The new test will fail if either the supplied argument is not a file or the custom header is not found.\n\n"}]}