git@vger.kernel.org mailing list mirror (one of many)
 help / color / mirror / code / Atom feed
* [PATCH v5 0/2] send-email: expose header information to git-send-email's sendemail-validate hook
@ 2023-01-10 21:16 Strawbridge, Michael
  2023-01-17  1:49 ` Strawbridge, Michael
  0 siblings, 1 reply; 6+ messages in thread
From: Strawbridge, Michael @ 2023-01-10 21:16 UTC (permalink / raw)
  To: git@vger.kernel.org; +Cc: Strawbridge, Michael

Hi Junio,

I very much appreciate the feedback and believe I have changed things to match.

To 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.

Michael Strawbridge (2):
  send-email: refactor header generation functions
  send-email: expose header information to git-send-email's
    sendemail-validate hook

 Documentation/githooks.txt | 17 ++++++--
 git-send-email.perl        | 80 +++++++++++++++++++++++++-------------
 t/t9001-send-email.sh      | 29 +++++++++++++-
 3 files changed, 92 insertions(+), 34 deletions(-)

-- 
2.34.1

^ permalink raw reply	[flat|nested] 6+ messages in thread

* [PATCH v5 0/2] send-email: expose header information to git-send-email's sendemail-validate hook
@ 2023-01-17  1:37 Strawbridge, Michael
  2023-01-17  1:37 ` [PATCH v5 1/2] send-email: refactor header generation functions Strawbridge, Michael
                   ` (2 more replies)
  0 siblings, 3 replies; 6+ messages in thread
From: Strawbridge, Michael @ 2023-01-17  1:37 UTC (permalink / raw)
  To: git@vger.kernel.org; +Cc: Strawbridge, Michael

Hi Junio,

I very much appreciate the feedback and believe I have changed things to match.

To 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.

Michael Strawbridge (2):
  send-email: refactor header generation functions
  send-email: expose header information to git-send-email's
    sendemail-validate hook

 Documentation/githooks.txt | 17 ++++++--
 git-send-email.perl        | 80 +++++++++++++++++++++++++-------------
 t/t9001-send-email.sh      | 29 +++++++++++++-
 3 files changed, 92 insertions(+), 34 deletions(-)

-- 
2.34.1

^ permalink raw reply	[flat|nested] 6+ messages in thread

* [PATCH v5 1/2] send-email: refactor header generation functions
  2023-01-17  1:37 [PATCH v5 0/2] send-email: expose header information to git-send-email's sendemail-validate hook Strawbridge, Michael
@ 2023-01-17  1:37 ` Strawbridge, Michael
  2023-01-17  1:37 ` [PATCH v5 2/2] send-email: expose header information to git-send-email's sendemail-validate hook Strawbridge, Michael
  2023-01-17  1:37 ` [PATCH v6 0/2] " Strawbridge, Michael
  2 siblings, 0 replies; 6+ messages in thread
From: Strawbridge, Michael @ 2023-01-17  1:37 UTC (permalink / raw)
  To: git@vger.kernel.org; +Cc: Strawbridge, Michael, Tuikov, Luben, Junio C Hamano

Split process_file and send_message into easier to use functions.
Making SMTP header information more widely available.

Cc: Luben Tuikov <luben.tuikov@amd.com>
Cc: Junio C Hamano <gitster@pobox.com>
Signed-off-by: Michael Strawbridge <michael.strawbridge@amd.com>
---
 git-send-email.perl | 49 ++++++++++++++++++++++++++++-----------------
 1 file changed, 31 insertions(+), 18 deletions(-)

diff --git a/git-send-email.perl b/git-send-email.perl
index 5861e99a6e..810dd1f1ce 100755
--- a/git-send-email.perl
+++ b/git-send-email.perl
@@ -1495,16 +1495,7 @@ sub file_name_is_absolute {
 	return File::Spec::Functions::file_name_is_absolute($path);
 }
 
-# Prepares the email, then asks the user what to do.
-#
-# If the user chooses to send the email, it's sent and 1 is returned.
-# If the user chooses not to send the email, 0 is returned.
-# If the user decides they want to make further edits, -1 is returned and the
-# caller is expected to call send_message again after the edits are performed.
-#
-# If an error occurs sending the email, this just dies.
-
-sub send_message {
+sub gen_header {
 	my @recipients = unique_email_list(@to);
 	@cc = (grep { my $cc = extract_valid_address_or_die($_);
 		      not grep { $cc eq $_ || $_ =~ /<\Q${cc}\E>$/ } @recipients
@@ -1546,6 +1537,22 @@ sub send_message {
 	if (@xh) {
 		$header .= join("\n", @xh) . "\n";
 	}
+	my $recipients_ref = \@recipients;
+	return ($recipients_ref, $to, $date, $gitversion, $cc, $ccline, $header);
+}
+
+# Prepares the email, then asks the user what to do.
+#
+# If the user chooses to send the email, it's sent and 1 is returned.
+# If the user chooses not to send the email, 0 is returned.
+# If the user decides they want to make further edits, -1 is returned and the
+# caller is expected to call send_message again after the edits are performed.
+#
+# If an error occurs sending the email, this just dies.
+
+sub send_message {
+	my ($recipients_ref, $to, $date, $gitversion, $cc, $ccline, $header) = gen_header();
+	my @recipients = @$recipients_ref;
 
 	my @sendmail_parameters = ('-i', @recipients);
 	my $raw_from = $sender;
@@ -1735,11 +1742,8 @@ sub send_message {
 $references = $initial_in_reply_to || '';
 $message_num = 0;
 
-# Prepares the email, prompts the user, sends it out
-# Returns 0 if an edit was done and the function should be called again, or 1
-# otherwise.
-sub process_file {
-	my ($t) = @_;
+sub pre_process_file {
+	my ($t, $quiet) = @_;
 
 	open my $fh, "<", $t or die sprintf(__("can't open file %s"), $t);
 
@@ -1893,9 +1897,9 @@ sub process_file {
 	}
 	close $fh;
 
-	push @to, recipients_cmd("to-cmd", "to", $to_cmd, $t)
+	push @to, recipients_cmd("to-cmd", "to", $to_cmd, $t, $quiet)
 		if defined $to_cmd;
-	push @cc, recipients_cmd("cc-cmd", "cc", $cc_cmd, $t)
+	push @cc, recipients_cmd("cc-cmd", "cc", $cc_cmd, $t, $quiet)
 		if defined $cc_cmd && !$suppress_cc{'cccmd'};
 
 	if ($broken_encoding{$t} && !$has_content_type) {
@@ -1954,6 +1958,15 @@ sub process_file {
 			@initial_to = @to;
 		}
 	}
+}
+
+# Prepares the email, prompts the user, sends it out
+# Returns 0 if an edit was done and the function should be called again, or 1
+# otherwise.
+sub process_file {
+	my ($t) = @_;
+
+        pre_process_file($t, $quiet);
 
 	my $message_was_sent = send_message();
 	if ($message_was_sent == -1) {
@@ -2002,7 +2015,7 @@ sub process_file {
 # Execute a command (e.g. $to_cmd) to get a list of email addresses
 # and return a results array
 sub recipients_cmd {
-	my ($prefix, $what, $cmd, $file) = @_;
+	my ($prefix, $what, $cmd, $file, $quiet) = @_;
 
 	my @addresses = ();
 	open my $fh, "-|", "$cmd \Q$file\E"
-- 
2.34.1

^ permalink raw reply related	[flat|nested] 6+ messages in thread

* [PATCH v5 2/2] send-email: expose header information to git-send-email's sendemail-validate hook
  2023-01-17  1:37 [PATCH v5 0/2] send-email: expose header information to git-send-email's sendemail-validate hook Strawbridge, Michael
  2023-01-17  1:37 ` [PATCH v5 1/2] send-email: refactor header generation functions Strawbridge, Michael
@ 2023-01-17  1:37 ` Strawbridge, Michael
  2023-01-17  1:37 ` [PATCH v6 0/2] " Strawbridge, Michael
  2 siblings, 0 replies; 6+ messages in thread
From: Strawbridge, Michael @ 2023-01-17  1:37 UTC (permalink / raw)
  To: git@vger.kernel.org; +Cc: Strawbridge, Michael, Tuikov, Luben, Junio C Hamano

To allow further flexibility in the git hook, the SMTP header
information of the email that git-send-email intends to send, is now
passed as a 2nd argument to the sendemail-validate hook.

As an example, this can be useful for acting upon keywords in the
subject or specific email addresses.

Cc: Luben Tuikov <luben.tuikov@amd.com>
Cc: Junio C Hamano <gitster@pobox.com>
Signed-off-by: Michael Strawbridge <michael.strawbridge@amd.com>
---
 Documentation/githooks.txt | 17 +++++++++++++----
 git-send-email.perl        | 31 +++++++++++++++++++++----------
 t/t9001-send-email.sh      | 29 +++++++++++++++++++++++++++--
 3 files changed, 61 insertions(+), 16 deletions(-)

diff --git a/Documentation/githooks.txt b/Documentation/githooks.txt
index a16e62bc8c..2b5c6640cc 100644
--- a/Documentation/githooks.txt
+++ b/Documentation/githooks.txt
@@ -583,10 +583,19 @@ processed by rebase.
 sendemail-validate
 ~~~~~~~~~~~~~~~~~~
 
-This hook is invoked by linkgit:git-send-email[1].  It takes a single parameter,
-the name of the file that holds the e-mail to be sent.  Exiting with a
-non-zero status causes `git send-email` to abort before sending any
-e-mails.
+This hook is invoked by linkgit:git-send-email[1].
+
+It takes these command line arguments:
+1. the name of the file that holds the e-mail to be sent.
+2. the name of the file that holds the SMTP headers to be used.
+
+The hook doesn't need to support multiple header names (for example 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.
+
+Exiting with a non-zero status causes `git send-email` to abort
+before sending any e-mails.
 
 fsmonitor-watchman
 ~~~~~~~~~~~~~~~~~~
diff --git a/git-send-email.perl b/git-send-email.perl
index 810dd1f1ce..b2adca515e 100755
--- a/git-send-email.perl
+++ b/git-send-email.perl
@@ -787,14 +787,6 @@ sub is_format_patch_arg {
 
 @files = handle_backup_files(@files);
 
-if ($validate) {
-	foreach my $f (@files) {
-		unless (-p $f) {
-			validate_patch($f, $target_xfer_encoding);
-		}
-	}
-}
-
 if (@files) {
 	unless ($quiet) {
 		print $_,"\n" for (@files);
@@ -1738,6 +1730,16 @@ sub send_message {
 	return 1;
 }
 
+if ($validate) {
+	foreach my $f (@files) {
+		unless (-p $f) {
+		        pre_process_file($f, 1);
+
+			validate_patch($f, $target_xfer_encoding);
+		}
+	}
+}
+
 $in_reply_to = $initial_in_reply_to;
 $references = $initial_in_reply_to || '';
 $message_num = 0;
@@ -2101,11 +2103,20 @@ sub validate_patch {
 			chdir($repo->wc_path() or $repo->repo_path())
 				or die("chdir: $!");
 			local $ENV{"GIT_DIR"} = $repo->repo_path();
+
+			my ($recipients_ref, $to, $date, $gitversion, $cc, $ccline, $header) = gen_header();
+
+			require File::Temp;
+			my ($header_filehandle, $header_filename) = File::Temp::tempfile(
+                            ".gitsendemail.header.XXXXXX", DIR => $repo->repo_path());
+			print $header_filehandle $header;
+
 			my @cmd = ("git", "hook", "run", "--ignore-missing",
 				    $hook_name, "--");
-			my @cmd_msg = (@cmd, "<patch>");
-			my @cmd_run = (@cmd, $target);
+			my @cmd_msg = (@cmd, "<patch>", "<header>");
+			my @cmd_run = (@cmd, $target, $header_filename);
 			$hook_error = system_or_msg(\@cmd_run, undef, "@cmd_msg");
+			unlink($header_filehandle);
 			chdir($cwd_save) or die("chdir: $!");
 		}
 		if ($hook_error) {
diff --git a/t/t9001-send-email.sh b/t/t9001-send-email.sh
index 1130ef21b3..f02b1eba16 100755
--- a/t/t9001-send-email.sh
+++ b/t/t9001-send-email.sh
@@ -540,7 +540,7 @@ test_expect_success $PREREQ "--validate respects relative core.hooksPath path" '
 	test_path_is_file my-hooks.ran &&
 	cat >expect <<-EOF &&
 	fatal: longline.patch: rejected by sendemail-validate hook
-	fatal: command '"'"'git hook run --ignore-missing sendemail-validate -- <patch>'"'"' died with exit code 1
+	fatal: command '"'"'git hook run --ignore-missing sendemail-validate -- <patch> <header>'"'"' died with exit code 1
 	warning: no patches were sent
 	EOF
 	test_cmp expect actual
@@ -559,7 +559,32 @@ test_expect_success $PREREQ "--validate respects absolute core.hooksPath path" '
 	test_path_is_file my-hooks.ran &&
 	cat >expect <<-EOF &&
 	fatal: longline.patch: rejected by sendemail-validate hook
-	fatal: command '"'"'git hook run --ignore-missing sendemail-validate -- <patch>'"'"' died with exit code 1
+	fatal: command '"'"'git hook run --ignore-missing sendemail-validate -- <patch> <header>'"'"' died with exit code 1
+	warning: no patches were sent
+	EOF
+	test_cmp expect actual
+'
+
+test_expect_success $PREREQ "--validate hook supports header argument" '
+	test_when_finished "rm my-hooks.ran" &&
+	write_script my-hooks/sendemail-validate <<-\EOF &&
+	filesize=$(stat -c%s "$2")
+	if [ "$filesize" != "0" ]; then
+	>my-hooks.ran
+	fi
+	exit 1
+	EOF
+	test_config core.hooksPath "my-hooks" &&
+	test_must_fail git send-email \
+		--from="Example <nobody@example.com>" \
+		--to=nobody@example.com \
+		--smtp-server="$(pwd)/fake.sendmail" \
+		--validate \
+		longline.patch 2>actual &&
+	test_path_is_file my-hooks.ran &&
+	cat >expect <<-EOF &&
+	fatal: longline.patch: rejected by sendemail-validate hook
+	fatal: command '"'"'git hook run --ignore-missing sendemail-validate -- <patch> <header>'"'"' died with exit code 1
 	warning: no patches were sent
 	EOF
 	test_cmp expect actual
-- 
2.34.1

^ permalink raw reply related	[flat|nested] 6+ messages in thread

* [PATCH v6 0/2] send-email: expose header information to git-send-email's sendemail-validate hook
  2023-01-17  1:37 [PATCH v5 0/2] send-email: expose header information to git-send-email's sendemail-validate hook Strawbridge, Michael
  2023-01-17  1:37 ` [PATCH v5 1/2] send-email: refactor header generation functions Strawbridge, Michael
  2023-01-17  1:37 ` [PATCH v5 2/2] send-email: expose header information to git-send-email's sendemail-validate hook Strawbridge, Michael
@ 2023-01-17  1:37 ` Strawbridge, Michael
  2 siblings, 0 replies; 6+ messages in thread
From: Strawbridge, Michael @ 2023-01-17  1:37 UTC (permalink / raw)
  To: git@vger.kernel.org; +Cc: Strawbridge, Michael

Thank you for all the great feedback!  At your suggestion I improved the
test for the header argument and the documentation.

Michael Strawbridge (2):
  send-email: refactor header generation functions
  send-email: expose header information to git-send-email's
    sendemail-validate hook

 Documentation/githooks.txt | 29 ++++++++++++--
 git-send-email.perl        | 80 +++++++++++++++++++++++++-------------
 t/t9001-send-email.sh      | 47 +++++++++++++++++++++-
 3 files changed, 122 insertions(+), 34 deletions(-)

-- 
2.34.1

^ permalink raw reply	[flat|nested] 6+ messages in thread

* [PATCH v5 0/2] send-email: expose header information to git-send-email's sendemail-validate hook
  2023-01-10 21:16 [PATCH v5 " Strawbridge, Michael
@ 2023-01-17  1:49 ` Strawbridge, Michael
  0 siblings, 0 replies; 6+ messages in thread
From: Strawbridge, Michael @ 2023-01-17  1:49 UTC (permalink / raw)
  To: git@vger.kernel.org; +Cc: Strawbridge, Michael

Please ignore this thread.

Michael Strawbridge

-- 
2.34.1

^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2023-01-17  1:49 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2023-01-17  1:37 [PATCH v5 0/2] send-email: expose header information to git-send-email's sendemail-validate hook Strawbridge, Michael
2023-01-17  1:37 ` [PATCH v5 1/2] send-email: refactor header generation functions Strawbridge, Michael
2023-01-17  1:37 ` [PATCH v5 2/2] send-email: expose header information to git-send-email's sendemail-validate hook Strawbridge, Michael
2023-01-17  1:37 ` [PATCH v6 0/2] " Strawbridge, Michael
  -- strict thread matches above, loose matches on Subject: below --
2023-01-10 21:16 [PATCH v5 " Strawbridge, Michael
2023-01-17  1:49 ` Strawbridge, Michael

Code repositories for project(s) associated with this public inbox

	https://80x24.org/mirrors/git.git

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).