{"thread":{"id":"58790","subject":"[PATCH v3 0/1] Expose header information to git-send-email's sendemail-validate hook","startedAt":"2022-11-11T19:42:37Z","lastAt":"2022-11-15T14:00:43Z","messageCount":3,"participants":["Strawbridge, Michael","Johannes Schindelin"],"isPatch":true,"patchVersion":3,"patchTotal":1},"messages":[{"id":"467157","messageId":"20221111194223.644845-1-michael.strawbridge@amd.com","threadId":"58790","inReplyTo":null,"subject":"[PATCH v3 0/1] Expose header information to git-send-email's sendemail-validate hook","fromName":"Strawbridge, Michael","fromEmail":"michael.strawbridge@amd.com","sentAt":"2022-11-11T19:42:30Z","receivedAt":"2022-11-11T19:42:37Z","isPatch":true,"sender":{"key":"michael.strawbridge@amd.com","avatar":null},"body":"I accidentally missed previous feedback to squash doc edit into main change of git-send-email.\n\nMichael\n\nCc: Luben Tuikov <luben.tuikov@amd.com>\nCc: brian m. carlson <sandals@crustytoothpaste.net>\n\nMichael Strawbridge (1):\n  Expose header information to git-send-email's sendemail-validate hook\n\n Documentation/githooks.txt |  8 +++---\n git-send-email.perl        | 57 +++++++++++++++++++++++++-------------\n 2 files changed, 41 insertions(+), 24 deletions(-)\n\n-- \n2.34.1\n"},{"id":"467158","messageId":"20221111194223.644845-2-michael.strawbridge@amd.com","threadId":"58790","inReplyTo":"20221111194223.644845-1-michael.strawbridge@amd.com","subject":"[PATCH v3 1/1] Expose header information to git-send-email's sendemail-validate hook","fromName":"Strawbridge, Michael","fromEmail":"michael.strawbridge@amd.com","sentAt":"2022-11-11T19:42:35Z","receivedAt":"2022-11-11T19:42:41Z","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.  Docs are\nalso updated.\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: brian m. carlson <sandals@crustytoothpaste.net>\nSigned-off-by: Michael Strawbridge <michael.strawbridge@amd.com>\n---\n Documentation/githooks.txt |  8 +++---\n git-send-email.perl        | 57 +++++++++++++++++++++++++-------------\n 2 files changed, 41 insertions(+), 24 deletions(-)\n\ndiff --git a/Documentation/githooks.txt b/Documentation/githooks.txt\nindex a16e62bc8c..346e536cbe 100644\n--- a/Documentation/githooks.txt\n+++ b/Documentation/githooks.txt\n@@ -583,10 +583,10 @@ 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].  It takes two parameters,\n+the name of a file that holds the patch and the name of a file that holds the\n+SMTP headers.  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 5861e99a6e..3ce5b1aad3 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@@ -1495,16 +1487,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 +1529,22 @@ sub send_message {\n \tif (@xh) {\n \t\t$header .= join(\"\\n\", @xh) . \"\\n\";\n \t}\n+\treturn $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 = unique_email_list(@to);\n+\n+        my $header = gen_header();\n \n \tmy @sendmail_parameters = ('-i', @recipients);\n \tmy $raw_from = $sender;\n@@ -1955,6 +1954,15 @@ sub process_file {\n \t\t}\n \t}\n \n+\n+\tif ($validate) {\n+\t\tforeach my $f (@files) {\n+\t\t\tunless (-p $f) {\n+\t\t\t\tvalidate_patch($f, $target_xfer_encoding);\n+\t\t\t}\n+\t\t}\n+\t}\n+\n \tmy $message_was_sent = send_message();\n \tif ($message_was_sent == -1) {\n \t\tdo_edit($t);\n@@ -2088,11 +2096,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 $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) {\n-- \n2.34.1\n"},{"id":"467306","messageId":"36s0r4s9-n21r-pop9-o7rn-q0qrq487p831@tzk.qr","threadId":"58790","inReplyTo":"20221111194223.644845-2-michael.strawbridge@amd.com","subject":"Re: [PATCH v3 1/1] Expose header information to git-send-email's sendemail-validate hook","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2022-11-15T14:00:22Z","receivedAt":"2022-11-15T14:00:43Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi Michael,\n\nOn Fri, 11 Nov 2022, Strawbridge, Michael wrote:\n\n> To allow further flexibility in the git hook, the smtp header\n> information of the email that git-send-email intends to send, is now\n> passed as a 2nd argument to the sendemail-validate hook.  Docs are\n> also updated.\n>\n> As an example, this can be useful for acting upon keywords in the\n> subject or specific email addresses.\n\nThis patch seems to break t9001:\n\n\tError: failed: t9001.9 Verify commandline\n\tfailure: t9001.9 Verify commandline\n\ttest_cmp expected commandline1\n\n\t+ test_cmp expected commandline1\n\t+ test 2 -ne 2\n\t+ eval diff -u \"$@\"\n\t+ diff -u expected commandline1\n\t--- expected\t2022-11-12 01:49:23.477741140\n\t+0000\n\t+++ commandline1\t2022-11-12 01:49:21.921718804\n\t+0000\n\t@@ -1,4 +1 @@\n\t!nobody@example.com!\n\t-!author@example.com!\n\t-!one@example.com!\n\t-!two@example.com!\n\terror: last command exited with $?=1\n\tnot ok 9 - Verify commandline\n\t#\n\t#\t\ttest_cmp expected\n\t#\t\tcommandline1\n\t#\n\nSee\nhttps://github.com/gitgitgadget/git/actions/runs/3448445848/jobs/5755529504#step:4:1791\nfor details.\n\nIf you need help with debugging this, I would like to point you to the\ninformation I wrote up for the Git for Windows project (but it applies\nhere, too):\nhttps://github.com/git-for-windows/git/wiki/Running-Git's-regression-tests#running-individual-tests\n\nCiao,\nJohannes\n\n>\n> Cc: Luben Tuikov <luben.tuikov@amd.com>\n> Cc: brian m. carlson <sandals@crustytoothpaste.net>\n> Signed-off-by: Michael Strawbridge <michael.strawbridge@amd.com>\n> ---\n>  Documentation/githooks.txt |  8 +++---\n>  git-send-email.perl        | 57 +++++++++++++++++++++++++-------------\n>  2 files changed, 41 insertions(+), 24 deletions(-)\n>\n> diff --git a/Documentation/githooks.txt b/Documentation/githooks.txt\n> index a16e62bc8c..346e536cbe 100644\n> --- a/Documentation/githooks.txt\n> +++ b/Documentation/githooks.txt\n> @@ -583,10 +583,10 @@ 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].  It takes two parameters,\n> +the name of a file that holds the patch and the name of a file that holds the\n> +SMTP headers.  Exiting with a non-zero status causes `git send-email` to abort\n> +before sending any e-mails.\n>\n>  fsmonitor-watchman\n>  ~~~~~~~~~~~~~~~~~~\n> diff --git a/git-send-email.perl b/git-send-email.perl\n> index 5861e99a6e..3ce5b1aad3 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> @@ -1495,16 +1487,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 +1529,22 @@ sub send_message {\n>  \tif (@xh) {\n>  \t\t$header .= join(\"\\n\", @xh) . \"\\n\";\n>  \t}\n> +\treturn $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 = unique_email_list(@to);\n> +\n> +        my $header = gen_header();\n>\n>  \tmy @sendmail_parameters = ('-i', @recipients);\n>  \tmy $raw_from = $sender;\n> @@ -1955,6 +1954,15 @@ sub process_file {\n>  \t\t}\n>  \t}\n>\n> +\n> +\tif ($validate) {\n> +\t\tforeach my $f (@files) {\n> +\t\t\tunless (-p $f) {\n> +\t\t\t\tvalidate_patch($f, $target_xfer_encoding);\n> +\t\t\t}\n> +\t\t}\n> +\t}\n> +\n>  \tmy $message_was_sent = send_message();\n>  \tif ($message_was_sent == -1) {\n>  \t\tdo_edit($t);\n> @@ -2088,11 +2096,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 $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) {\n> --\n> 2.34.1\n>\n>\n"}]}