Re: [PATCH] send-email: export patch counters in validate environment

[Phillip Wood <phillip.wood123@gmail.com>]

Hi Robin

On 11/04/2023 12:47, Robin Jarry wrote:
> 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. A hook that
> wants to check some property of the whole series needs to know which
> patch is the final one.
> 
> Expose the current and total number of patches to the hook via the
> GIT_SENDEMAIL_PATCH_COUNTER and GIT_SENDEMAIL_PATCH_TOTAL environment
> variables so that both incremental and global validation is possible.
> 
> Sharing any other state between successive invocations of the validate
> hook must be done via external means. For example, by storing it in
> a GIT_DIR/SENDEMAIL_VALIDATE file.
> 
> Suggested-by: Phillip Wood <phillip.wood123@gmail.com>
> Signed-off-by: Robin Jarry <robin@jarry.cc>
> ---
> 
> Notes:
>      Follow up on:
>      https://lore.kernel.org/git/9b8d6cc4-741a-5081-d5de-df0972efec37@gmail.com/
>      
>      As suggested by Phillip, this is a less intrusive change which allows
>      validating whole series. Let me know what you think.

This is certainly less intrusive, if it does what you need and is 
efficient enough for your needs then I'd be inclined to go with this 
approach.

>   Documentation/githooks.txt | 10 ++++++++++
>   git-send-email.perl        |  7 +++++++
>   2 files changed, 17 insertions(+)
> 
> diff --git a/Documentation/githooks.txt b/Documentation/githooks.txt
> index 62908602e7be..55f00e0f6f8c 100644
> --- a/Documentation/githooks.txt
> +++ b/Documentation/githooks.txt
> @@ -600,6 +600,16 @@ 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.
>   
> +The following environment variables are set when executing the hook.
> +
> +`GIT_SENDEMAIL_PATCH_COUNTER`::
> +	A 1-based counter incremented by one for every file.
> +
> +`GIT_SENDEMAIL_PATCH_TOTAL`::
> +	The total number of files.
> +
> +These variables can be used to validate patch series.
> +
>   fsmonitor-watchman
>   ~~~~~~~~~~~~~~~~~~
>   
> diff --git a/git-send-email.perl b/git-send-email.perl
> index 07f2a0cbeaad..e962d5e15983 100755
> --- a/git-send-email.perl
> +++ b/git-send-email.perl
> @@ -795,10 +795,17 @@ sub is_format_patch_arg {
>   @files = handle_backup_files(@files);
>   
>   if ($validate) {
> +	my $num = 1;
> +	my $num_patches = @files;
>   	foreach my $f (@files) {
>   		unless (-p $f) {
> +			$ENV{GIT_SENDEMAIL_PATCH_COUNTER} = "$num";
> +			$ENV{GIT_SENDEMAIL_PATCH_TOTAL} = "$num_patches";

We only need to set this once outside the loop

>   			validate_patch($f, $target_xfer_encoding);
> +			delete $ENV{GIT_SENDEMAIL_PATCH_COUNTER};
> +			delete $ENV{GIT_SENDEMAIL_PATCH_TOTAL};

Do we really need to clear these? Certainly not in each iteration of the 
loop I would think.

Best Wishes

Phillip
>   		}
> +		$num += 1;
>   	}
>   }
>