{"thread":{"id":"59098","subject":"[PATCH v5 0/2] send-email: expose header information to git-send-email's sendemail-validate hook","startedAt":"2023-01-17T01:37:33Z","lastAt":"2023-01-17T01:38:32Z","messageCount":4,"participants":["Strawbridge, Michael"],"isPatch":true,"patchVersion":5,"patchTotal":2},"messages":[{"id":"470461","messageId":"20230117013709.47054-1-michael.strawbridge@amd.com","threadId":"59098","inReplyTo":null,"subject":"[PATCH v5 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:37:22Z","receivedAt":"2023-01-17T01:37:33Z","isPatch":true,"sender":{"key":"michael.strawbridge@amd.com","avatar":null},"body":"Hi Junio,\n\nI very much appreciate the feedback and believe I have changed things to match.\n\nTo answer your earlier question, the hook doesn't need to support multiple header capitalizations (ie. only Cc is passed). However, it does need to understand that lines beginning with whitespace belong to the previous header.  The header information follows the same format as the confirmation given at the end of send-email.\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 | 17 ++++++--\n git-send-email.perl        | 80 +++++++++++++++++++++++++-------------\n t/t9001-send-email.sh      | 29 +++++++++++++-\n 3 files changed, 92 insertions(+), 34 deletions(-)\n\n-- \n2.34.1\n"},{"id":"470462","messageId":"20230117013709.47054-2-michael.strawbridge@amd.com","threadId":"59098","inReplyTo":"20230117013709.47054-1-michael.strawbridge@amd.com","subject":"[PATCH v5 1/2] send-email: refactor header generation functions","fromName":"Strawbridge, Michael","fromEmail":"michael.strawbridge@amd.com","sentAt":"2023-01-17T01:37:34Z","receivedAt":"2023-01-17T01:38:10Z","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 <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..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":"470463","messageId":"20230117013709.47054-3-michael.strawbridge@amd.com","threadId":"59098","inReplyTo":"20230117013709.47054-1-michael.strawbridge@amd.com","subject":"[PATCH v5 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:37:38Z","receivedAt":"2023-01-17T01:38:20Z","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 <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 | 17 +++++++++++++----\n git-send-email.perl        | 31 +++++++++++++++++++++----------\n t/t9001-send-email.sh      | 29 +++++++++++++++++++++++++++--\n 3 files changed, 61 insertions(+), 16 deletions(-)\n\ndiff --git a/Documentation/githooks.txt b/Documentation/githooks.txt\nindex a16e62bc8c..2b5c6640cc 100644\n--- a/Documentation/githooks.txt\n+++ b/Documentation/githooks.txt\n@@ -583,10 +583,19 @@ 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 hook doesn't need to support multiple header names (for example only Cc\n+is passed). However, it does need to understand that lines beginning with\n+whitespace belong to the previous header.  The header information follows\n+the same format as the confirmation given at the end of send-email.\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, \"<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..f02b1eba16 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,7 +559,32 @@ 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 \"--validate hook supports header argument\" '\n+\ttest_when_finished \"rm my-hooks.ran\" &&\n+\twrite_script my-hooks/sendemail-validate <<-\\EOF &&\n+\tfilesize=$(stat -c%s \"$2\")\n+\tif [ \"$filesize\" != \"0\" ]; then\n+\t>my-hooks.ran\n+\tfi\n+\texit 1\n+\tEOF\n+\ttest_config core.hooksPath \"my-hooks\" &&\n+\ttest_must_fail git send-email \\\n+\t\t--from=\"Example <nobody@example.com>\" \\\n+\t\t--to=nobody@example.com \\\n+\t\t--smtp-server=\"$(pwd)/fake.sendmail\" \\\n+\t\t--validate \\\n+\t\tlongline.patch 2>actual &&\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> <header>'\"'\"' died with exit code 1\n \twarning: no patches were sent\n \tEOF\n \ttest_cmp expect actual\n-- \n2.34.1\n"},{"id":"470464","messageId":"20230117013709.47054-4-michael.strawbridge@amd.com","threadId":"59098","inReplyTo":"20230117013709.47054-1-michael.strawbridge@amd.com","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:37:44Z","receivedAt":"2023-01-17T01:38:32Z","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"}]}