Re: [PATCH v2] hooks: add sendemail-validate-series

> When sending patch series (with a cover-letter or not)
> sendemail-validate is called with every email/patch file independently
> from the others. When one of the patches depends on a previous one, it
> may not be possible to use this hook in a meaningful way.
>
> Changing sendemail-validate to take all patches as arguments would break
> backward compatibility.
>
> Add a new hook to allow validating patch series instead of patch by
> patch. The patch files are provided in the hook script standard input,
> one per line. Patch file names that contain LF characters are *not*
> validated.
>
> Signed-off-by: Robin Jarry <robin@jarry.cc>
> ---
>
> Notes:
>     v1 -> v2:
>     
>     - Use `git hook run --to-stdin` with a temp file.
>     - Skip validation (with an explicit warning) for patch file names that
>       contain newline characters.
>     - Updated docs.

>  Documentation/git-send-email.txt |  1 +
>  Documentation/githooks.txt       | 19 +++++++++++++++++
>  git-send-email.perl              | 36 ++++++++++++++++++++++++++++++++
>  3 files changed, 56 insertions(+)
>
> diff --git a/Documentation/git-send-email.txt b/Documentation/git-send-email.txt
> index 765b2df8530d..45113b928593 100644
> --- a/Documentation/git-send-email.txt
> +++ b/Documentation/git-send-email.txt
> @@ -438,6 +438,7 @@ have been specified, in which case default to 'compose'.
>  +
>  --
>  		*	Invoke the sendemail-validate hook if present (see linkgit:githooks[5]).
> +		*	Invoke the sendemail-validate-series hook if present (see linkgit:githooks[5]).
>  		*	Warn of patches that contain lines longer than
>  			998 characters unless a suitable transfer encoding
>  			('auto', 'base64', or 'quoted-printable') is used;
> diff --git a/Documentation/githooks.txt b/Documentation/githooks.txt
> index 62908602e7be..0e8573c6c116 100644
> --- a/Documentation/githooks.txt
> +++ b/Documentation/githooks.txt
> @@ -600,6 +600,25 @@ 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.
>  
> +sendemail-validate-series
> +~~~~~~~~~~~~~~~~~~~~~~~~~
> +
> +This hook is invoked by linkgit:git-send-email[1].  It allows performing
> +validation on a complete patch series at once, instead of patch by patch with
> +`sendemail-validate`.
> +
> +`sendemail-validate-series` takes no arguments.  For each e-mail to be sent,
> +it receives on standard input a line of the format:
> +
> +  <patch-file> LF
> +
> +where '<patch-file>' is an absolute path to a file that holds an e-mail to be
> +sent.  Any '<patch-file>' that contains a 'LF' character will *not* be fed to
> +the hook and an explicit warning will be printed instead.
> +
> +If the hook exits with non-zero status, `git send-email` will abort before
> +sending any e-mails.
> +
>  fsmonitor-watchman
>  ~~~~~~~~~~~~~~~~~~
>  
> diff --git a/git-send-email.perl b/git-send-email.perl
> index 07f2a0cbeaad..b29050e14c06 100755
> --- a/git-send-email.perl
> +++ b/git-send-email.perl
> @@ -800,6 +800,7 @@ sub is_format_patch_arg {
>  			validate_patch($f, $target_xfer_encoding);
>  		}
>  	}
> +	validate_patch_series(@files)
>  }
>  
>  if (@files) {
> @@ -2125,6 +2126,41 @@ sub validate_patch {
>  	return;
>  }
>  
> +sub validate_patch_series {
> +	my @files = @_;
> +
> +	unless ($repo) {
> +		return;
> +	}
> +	require File::Temp;
> +	my $tmp = File::Temp->new(
> +		TEMPLATE => "sendemail-series.XXXXXXXX",
> +		UNLINK => 1,
> +	);
> +	for my $fn (@files) {
> +		unless (-p $fn) {
> +			$fn = Cwd::abs_path($fn);

> +			if ($fn =~ /\n/) {
> +				$fn =~ s/\n/'\\n'/g;
> +				printf STDERR __("warning: file name contains '\\n': %s. Skipping validation.\n"), $fn;
> +			} else {
> +				$tmp->print("$fn\n");
> +			}

> +		}
> +	}
> +	my $hook_name = "sendemail-validate-series";
> +	my @cmd = ("git", "hook", "run", "--ignore-missing",
> +		   "--to-stdin", $tmp->filename, $hook_name, "--");
> +	my $hook_error = system_or_msg(\@cmd, undef, "@cmd");
> +	if ($hook_error) {
> +		$hook_error = sprintf(
> +		    __("fatal: series rejected by %s hook\n%s\nwarning: no patches were sent\n"),
> +		    $hook_name, $hook_error);
> +		die $hook_error;
> +	}
> +	return;
> +}
> +
>  sub handle_backup {
>  	my ($last, $lastlen, $file, $known_suffix) = @_;
>  	my ($suffix, $skip);