Re: [PATCH v8 2/2] send-email: expose header information to git-send-email's sendemail-validate hook
- From
Ævar Arnfjörð Bjarmason <avarab@gmail.com>
- Date
- Jan 18, 2023, 23:12 UTC
- Message-ID
- <230119.868rhzwi36.gmgdl@evledraar.gmail.com>
- In-Reply-To
- <20230118163203.488652-3-michael.strawbridge@amd.com>
On Wed, Jan 18 2023, Michael Strawbridge wrote:
Show 10 quoted lines
> To allow further flexibility in the Git hook, the SMTP header > information of the email which git-send-email intends to send, is now > passed as the 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. > > As a consequence of needing all the header data, validation has been > moved later in the sequence to right before sending the emails instead > of at the beginning.
Ah, I see. I tested this (i.e. moving it back to the previous behavior) and you did this change because you don't have the $sender variable yet.
I tried this quickly on top, which seems to work, i.e. now we do this in
the same order as before, but we just move the $sender code earlier:
diff --git a/git-send-email.perl b/git-send-email.perl
index d123dfd33d5..7e7681116bb 100755
--- a/git-send-email.perl
+++ b/git-send-email.perl
@@ -787,6 +787,28 @@ sub is_format_patch_arg {
@files = handle_backup_files(@files);
+if (defined $sender) {
+ $sender =~ s/^\s+|\s+$//g;
+ ($sender) = expand_aliases($sender);
+} else {
+ $sender = $repoauthor->() || $repocommitter->() || '';
+}
+
+# $sender could be an already sanitized address
+# (e.g. sendemail.from could be manually sanitized by user).
+# But it's a no-op to run sanitize_address on an already sanitized address.
+$sender = sanitize_address($sender);
+
+if ($validate) {
+ foreach my $f (@files) {
+ unless (-p $f) {
+ pre_process_file($f, 1);
+
+ validate_patch($f, $target_xfer_encoding);
+ }
+ }
+}
+
if (@files) {
unless ($quiet) {
print $_,"\n" for (@files);
@@ -1035,18 +1057,6 @@ sub file_declares_8bit_cte {
}
}
-if (defined $sender) {
- $sender =~ s/^\s+|\s+$//g;
- ($sender) = expand_aliases($sender);
-} else {
- $sender = $repoauthor->() || $repocommitter->() || '';
-}
-
-# $sender could be an already sanitized address
-# (e.g. sendemail.from could be manually sanitized by user).
-# But it's a no-op to run sanitize_address on an already sanitized address.
-$sender = sanitize_address($sender);
-
my $to_whom = __("To whom should the emails be sent (if anyone)?");
my $prompting = 0;
if (!@initial_to && !defined $to_cmd) {
@@ -1120,16 +1130,6 @@ sub expand_one_alias {
$time = time - scalar $#files;
-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;All tests pass with that, which is less good than it sounds, because shouldn't your tests be checking whether we have this non--quiet print-out of the files as expected before or after the validation hook runs?
Show 17 quoted lines
> +
> + 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: $!");I'm still curious about the "stdin" question I asked in the last round.