Looks good to me.
Reviewed-by: Luben Tuikov <luben.tuikov@xxxxxxx>
Regards,
Luben
On 2023-01-06 16:51, Strawbridge, Michael wrote:
> 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. A new t9001
> test was added to test this 2nd arg and docs are also updated.
>
> As an example, this can be useful for acting upon keywords in the
> subject or specific email addresses.
>
> Cc: Luben Tuikov <luben.tuikov@xxxxxxx>
> Cc: brian m. carlson <sandals@xxxxxxxxxxxxxxxxxxxx>
> Signed-off-by: Michael Strawbridge <michael.strawbridge@xxxxxxx>
> ---
> Documentation/githooks.txt | 8 +++---
> git-send-email.perl | 55 +++++++++++++++++++++++++-------------
> t/t9001-send-email.sh | 25 +++++++++++++++++
> 3 files changed, 65 insertions(+), 23 deletions(-)
>
> diff --git a/Documentation/githooks.txt b/Documentation/githooks.txt
> index a16e62bc8c..346e536cbe 100644
> --- a/Documentation/githooks.txt
> +++ b/Documentation/githooks.txt
> @@ -583,10 +583,10 @@ 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 two parameters,
> +the name of a file that holds the patch and the name of a file that holds the
> +SMTP headers. 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 5861e99a6e..5a626a4238 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);
> @@ -1495,16 +1487,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 +1529,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;
> @@ -1955,6 +1954,15 @@ sub process_file {
> }
> }
>
> +
> + if ($validate) {
> + foreach my $f (@files) {
> + unless (-p $f) {
> + validate_patch($f, $target_xfer_encoding);
> + }
> + }
> + }
> +
> my $message_was_sent = send_message();
> if ($message_was_sent == -1) {
> do_edit($t);
> @@ -2088,11 +2096,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_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..11e68f9c18 100755
> --- a/t/t9001-send-email.sh
> +++ b/t/t9001-send-email.sh
> @@ -565,6 +565,31 @@ test_expect_success $PREREQ "--validate respects absolute core.hooksPath path" '
> 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@xxxxxxxxxxx>" \
> + --to=nobody@xxxxxxxxxxx \
> + --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>'"'"' died with exit code 1
> + warning: no patches were sent
> + EOF
> + test_cmp expect actual
> +'
> +
> for enc in 7bit 8bit quoted-printable base64
> do
> test_expect_success $PREREQ "--transfer-encoding=$enc produces correct header" '
