{"thread":{"id":"59114","subject":"[PATCH v8 0/2] send-email: expose header information to git-send-email's sendemail-validate hook","startedAt":"2023-01-18T16:35:21Z","lastAt":"2023-01-19T01:05:55Z","messageCount":5,"participants":["Michael Strawbridge","Ævar Arnfjörð Bjarmason"],"isPatch":true,"patchVersion":8,"patchTotal":2},"messages":[{"id":"470645","messageId":"20230118163203.488652-1-michael.strawbridge@amd.com","threadId":"59114","inReplyTo":null,"subject":"[PATCH v8 0/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-18T16:32:01Z","receivedAt":"2023-01-18T16:35:21Z","isPatch":true,"sender":{"key":"michael.strawbridge@amd.com","avatar":null},"body":"This patch version addresses:\n- extra test for number of arguments in the header t9001 test\n- removed use of envelope in the documentation\n- re-arranged the validation loop and surrounding code to be closer to\n the other code not in functions\n- made a note in the commit message that the moving of validation later\n can have some difference in stdout compared to before but actual email\nfunctionality should be ok\n\nHopefully this looks better.\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 | 27 +++++++--\n git-send-email.perl | 109 ++++++++++++++++++++++---------------\n t/t9001-send-email.sh | 27 ++++++++-\n 3 files changed, 114 insertions(+), 49 deletions(-)\n\n-- \n2.34.1\n\n"},{"id":"470646","messageId":"20230118163203.488652-2-michael.strawbridge@amd.com","threadId":"59114","inReplyTo":"20230118163203.488652-1-michael.strawbridge@amd.com","subject":"[PATCH v8 1/2] send-email: refactor header generation functions","fromName":"Michael Strawbridge","fromEmail":"michael.strawbridge@amd.com","sentAt":"2023-01-18T16:32:02Z","receivedAt":"2023-01-18T16:35:48Z","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 \nCc: Junio C Hamano \nCc: Ævar Arnfjörð Bjarmason \nAcked-by: Luben Tuikov \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..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\n"},{"id":"470647","messageId":"20230118163203.488652-3-michael.strawbridge@amd.com","threadId":"59114","inReplyTo":"20230118163203.488652-1-michael.strawbridge@amd.com","subject":"[PATCH v8 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-18T16:32:03Z","receivedAt":"2023-01-18T16:35:51Z","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\nAs a consequence of needing all the header data, validation has been\nmoved later in the sequence to right before sending the emails instead\nof at the beginning.\n\nCc: Luben Tuikov \nCc: Junio C Hamano \nCc: Ævar Arnfjörð Bjarmason \nAcked-by: Luben Tuikov \nSigned-off-by: Michael Strawbridge \n---\n Documentation/githooks.txt | 27 ++++++++++++++---\n git-send-email.perl | 60 ++++++++++++++++++++++----------------\n t/t9001-send-email.sh | 27 +++++++++++++++--\n 3 files changed, 83 insertions(+), 31 deletions(-)\n\ndiff --git a/Documentation/githooks.txt b/Documentation/githooks.txt\nindex a16e62bc8c..0decbfc92d 100644\n--- a/Documentation/githooks.txt\n+++ b/Documentation/githooks.txt\n@@ -583,10 +583,29 @@ 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 headers of the email.\n+\n+The SMTP headers are passed in the exact same way as they are passed to the\n+user's Mail Transport Agent (MTA). In effect, the email given to the user's\n+MTA, is the contents of $2 followed by the contents 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 \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+\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..d123dfd33d 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@@ -1124,6 +1116,30 @@ sub expand_one_alias {\n our ($message_id, %mail, $subject, $in_reply_to, $references, $message,\n \t$needs_confirm, $message_num, $ask_default);\n \n+my ($message_id_stamp, $message_id_serial);\n+\n+$time = time - scalar $#files;\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+\n+foreach my $t (@files) {\n+\twhile (!process_file($t)) {\n+\t\t# user edited the file\n+\t}\n+}\n+\n sub extract_valid_address {\n \tmy $address = shift;\n \tmy $local_part_regexp = qr/[^<>\"\\s@]+/;\n@@ -1189,7 +1205,6 @@ sub validate_address_list {\n \n # We'll setup a template for the message id, using the \"from\" address:\n \n-my ($message_id_stamp, $message_id_serial);\n sub make_message_id {\n \tmy $uniq;\n \tif (!defined $message_id_stamp) {\n@@ -1214,10 +1229,6 @@ sub make_message_id {\n \t#print \"new message id = $message_id\\n\"; # Was useful for debugging\n }\n \n-\n-\n-$time = time - scalar $#files;\n-\n sub unquote_rfc2047 {\n \tlocal ($_) = @_;\n \tmy $charset;\n@@ -1738,10 +1749,6 @@ sub send_message {\n \treturn 1;\n }\n \n-$in_reply_to = $initial_in_reply_to;\n-$references = $initial_in_reply_to || '';\n-$message_num = 0;\n-\n sub pre_process_file {\n \tmy ($t, $quiet) = @_;\n \n@@ -2006,12 +2013,6 @@ sub process_file {\n \treturn 1;\n }\n \n-foreach my $t (@files) {\n-\twhile (!process_file($t)) {\n-\t\t# user edited the file\n-\t}\n-}\n-\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@@ -2101,11 +2102,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..8a5c111a24 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,35 @@ 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 \"--validate hook supports header argument\" '\n+\twrite_script my-hooks/sendemail-validate <<-\\EOF &&\n+ if test \"$#\" -ge 2\n+\tthen\n+\t\tgrep \"X-test-header: v1.0\" \"$2\"\n+\telse\n+\t\techo \"No header arg passed\"\n+\t\texit 1\n+\tfi\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\n"},{"id":"470681","messageId":"230119.868rhzwi36.gmgdl@evledraar.gmail.com","threadId":"59114","inReplyTo":"20230118163203.488652-3-michael.strawbridge@amd.com","subject":"Re: [PATCH v8 2/2] send-email: expose header information to git-send-email's sendemail-validate hook","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2023-01-18T23:12:25Z","receivedAt":"2023-01-18T23:15:03Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"\nOn Wed, Jan 18 2023, Michael Strawbridge wrote:\n\n> To allow further flexibility in the Git hook, the SMTP header\n> information of the email which git-send-email intends to send, is now\n> passed as the 2nd argument to the sendemail-validate hook.\n>\n> As an example, this can be useful for acting upon keywords in the\n> subject or specific email addresses.\n>\n> As a consequence of needing all the header data, validation has been\n> moved later in the sequence to right before sending the emails instead\n> of at the beginning.\n\nAh, I see. I tested this (i.e. moving it back to the previous behavior)\nand you did this change because you don't have the $sender variable yet.\n\nI tried this quickly on top, which seems to work, i.e. now we do this in\nthe same order as before, but we just move the $sender code earlier:\n\t\n\tdiff --git a/git-send-email.perl b/git-send-email.perl\n\tindex d123dfd33d5..7e7681116bb 100755\n\t--- a/git-send-email.perl\n\t+++ b/git-send-email.perl\n\t@@ -787,6 +787,28 @@ sub is_format_patch_arg {\n\t \n\t @files = handle_backup_files(@files);\n\t \n\t+if (defined $sender) {\n\t+\t$sender =~ s/^\\s+|\\s+$//g;\n\t+\t($sender) = expand_aliases($sender);\n\t+} else {\n\t+\t$sender = $repoauthor->() || $repocommitter->() || '';\n\t+}\n\t+\n\t+# $sender could be an already sanitized address\n\t+# (e.g. sendemail.from could be manually sanitized by user).\n\t+# But it's a no-op to run sanitize_address on an already sanitized address.\n\t+$sender = sanitize_address($sender);\n\t+\n\t+if ($validate) {\n\t+\tforeach my $f (@files) {\n\t+\t\tunless (-p $f) {\n\t+\t\t pre_process_file($f, 1);\n\t+\n\t+\t\t\tvalidate_patch($f, $target_xfer_encoding);\n\t+\t\t}\n\t+\t}\n\t+}\n\t+\n\t if (@files) {\n\t \tunless ($quiet) {\n\t \t\tprint $_,\"\\n\" for (@files);\n\t@@ -1035,18 +1057,6 @@ sub file_declares_8bit_cte {\n\t \t}\n\t }\n\t \n\t-if (defined $sender) {\n\t-\t$sender =~ s/^\\s+|\\s+$//g;\n\t-\t($sender) = expand_aliases($sender);\n\t-} else {\n\t-\t$sender = $repoauthor->() || $repocommitter->() || '';\n\t-}\n\t-\n\t-# $sender could be an already sanitized address\n\t-# (e.g. sendemail.from could be manually sanitized by user).\n\t-# But it's a no-op to run sanitize_address on an already sanitized address.\n\t-$sender = sanitize_address($sender);\n\t-\n\t my $to_whom = __(\"To whom should the emails be sent (if anyone)?\");\n\t my $prompting = 0;\n\t if (!@initial_to && !defined $to_cmd) {\n\t@@ -1120,16 +1130,6 @@ sub expand_one_alias {\n\t \n\t $time = time - scalar $#files;\n\t \n\t-if ($validate) {\n\t-\tforeach my $f (@files) {\n\t-\t\tunless (-p $f) {\n\t-\t\t pre_process_file($f, 1);\n\t-\n\t-\t\t\tvalidate_patch($f, $target_xfer_encoding);\n\t-\t\t}\n\t-\t}\n\t-}\n\t-\n\t $in_reply_to = $initial_in_reply_to;\n\t $references = $initial_in_reply_to || '';\n\t $message_num = 0;\n\nAll tests pass with that, which is less good than it sounds, because\nshouldn't your tests be checking whether we have this non--quiet\nprint-out of the files as expected before or after the validation hook\nruns?\n\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\nI'm still curious about the \"stdin\" question I asked in the last round.\n"},{"id":"470692","messageId":"5c3f8309-8dfd-9888-3c7f-1ca32b502e3e@amd.com","threadId":"59114","inReplyTo":"230119.868rhzwi36.gmgdl@evledraar.gmail.com","subject":"Re: [PATCH v8 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-19T01:05:45Z","receivedAt":"2023-01-19T01:05:55Z","isPatch":true,"sender":{"key":"michael.strawbridge@amd.com","avatar":null},"body":"\nOn 2023-01-18 18:12, Ævar Arnfjörð Bjarmason wrote:\n> On Wed, Jan 18 2023, Michael Strawbridge wrote:\n>\n>> To allow further flexibility in the Git hook, the SMTP header\n>> information of the email which git-send-email intends to send, is now\n>> passed as the 2nd argument to the sendemail-validate hook.\n>>\n>> As an example, this can be useful for acting upon keywords in the\n>> subject or specific email addresses.\n>>\n>> As a consequence of needing all the header data, validation has been\n>> moved later in the sequence to right before sending the emails instead\n>> of at the beginning.\n> Ah, I see. I tested this (i.e. moving it back to the previous behavior)\n> and you did this change because you don't have the $sender variable yet.\n>\n> I tried this quickly on top, which seems to work, i.e. now we do this in\n> the same order as before, but we just move the $sender code earlier:\n> \t\n> \tdiff --git a/git-send-email.perl b/git-send-email.perl\n> \tindex d123dfd33d5..7e7681116bb 100755\n> \t--- a/git-send-email.perl\n> \t+++ b/git-send-email.perl\n> \t@@ -787,6 +787,28 @@ sub is_format_patch_arg {\n> \t \n> \t @files = handle_backup_files(@files);\n> \t \n> \t+if (defined $sender) {\n> \t+\t$sender =~ s/^\\s+|\\s+$//g;\n> \t+\t($sender) = expand_aliases($sender);\n> \t+} else {\n> \t+\t$sender = $repoauthor->() || $repocommitter->() || '';\n> \t+}\n> \t+\n> \t+# $sender could be an already sanitized address\n> \t+# (e.g. sendemail.from could be manually sanitized by user).\n> \t+# But it's a no-op to run sanitize_address on an already sanitized address.\n> \t+$sender = sanitize_address($sender);\n> \t+\n> \t+if ($validate) {\n> \t+\tforeach my $f (@files) {\n> \t+\t\tunless (-p $f) {\n> \t+\t\t pre_process_file($f, 1);\n> \t+\n> \t+\t\t\tvalidate_patch($f, $target_xfer_encoding);\n> \t+\t\t}\n> \t+\t}\n> \t+}\n> \t+\n> \t if (@files) {\n> \t \tunless ($quiet) {\n> \t \t\tprint $_,\"\\n\" for (@files);\n> \t@@ -1035,18 +1057,6 @@ sub file_declares_8bit_cte {\n> \t \t}\n> \t }\n> \t \n> \t-if (defined $sender) {\n> \t-\t$sender =~ s/^\\s+|\\s+$//g;\n> \t-\t($sender) = expand_aliases($sender);\n> \t-} else {\n> \t-\t$sender = $repoauthor->() || $repocommitter->() || '';\n> \t-}\n> \t-\n> \t-# $sender could be an already sanitized address\n> \t-# (e.g. sendemail.from could be manually sanitized by user).\n> \t-# But it's a no-op to run sanitize_address on an already sanitized address.\n> \t-$sender = sanitize_address($sender);\n> \t-\n> \t my $to_whom = __(\"To whom should the emails be sent (if anyone)?\");\n> \t my $prompting = 0;\n> \t if (!@initial_to && !defined $to_cmd) {\n> \t@@ -1120,16 +1130,6 @@ sub expand_one_alias {\n> \t \n> \t $time = time - scalar $#files;\n> \t \n> \t-if ($validate) {\n> \t-\tforeach my $f (@files) {\n> \t-\t\tunless (-p $f) {\n> \t-\t\t pre_process_file($f, 1);\n> \t-\n> \t-\t\t\tvalidate_patch($f, $target_xfer_encoding);\n> \t-\t\t}\n> \t-\t}\n> \t-}\n> \t-\n> \t $in_reply_to = $initial_in_reply_to;\n> \t $references = $initial_in_reply_to || '';\n> \t $message_num = 0;\n>\n> All tests pass with that, which is less good than it sounds, because\n> shouldn't your tests be checking whether we have this non--quiet\n> print-out of the files as expected before or after the validation hook\n> runs?\n\nThank you very much!  That idea to move sender earlier actually\nsimplifies this a lot.  I had to move a piece relating to the date field\nearlier as well but after doing that I think the patch gives the same\noutput.\n\nI think checking the timing of stdout in tests is not easy.  I'm a bit\nunsure we would want to test such specific stdout behaviour in the\ntests.  I have focused on the functional output (eg header output). \nLuckily with the above, my change will no longer introduce any stdout\ndifferences anymore.\n\nI will wait a little before pushing out a v9 in case there is more feedback.\n\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> I'm still curious about the \"stdin\" question I asked in the last round.\n\nFor the stdin question, are you referring to the git hook run question? \nI know there have been a lot of parallel threads so you may have missed\nmy reply\n(https://public-inbox.org/git/20230117215811.78313-1-michael.strawbridge@amd.com/).\n\nHere's what I responded with:\n\nI was trying to follow the convention that the original hook was using.\nI'm not against changing this if the out of tree patches you speak of\nare going to be rolled in soon. However, I'd prefer not to delay this\npatch if these other patches are far off. Thanks.\n\n"}]}