Re: [RFC PATCH] send-email: allow a custom hook to prevent sending email

From: Junio C Hamano <gitster@pobox.com>
Date: Dec 9, 2016, 22:36 UTC
Message-ID: <xmqqfulwraq2.fsf@gitster.mtv.corp.google.com>
In-Reply-To: <20161209203449.17940-1-sbeller@google.com>

Stefan Beller <sbeller@google.com> writes:

Show 35 quoted lines

> This custom hook could be used to prevent sending out e.g. patches
> with change ids or other information that upstream doesn't like to see
> or is not supposed to see.
>
> Signed-off-by: Stefan Beller <sbeller@google.com>
> ---
>
> My first perl contribution to Git. :)
>
> Marked as RFC to gauge general interest before writing tests and documentation.
>
> Thanks,
> Stefan
>
>  git-send-email.perl | 9 +++++++++
>  1 file changed, 9 insertions(+)
>
> diff --git a/git-send-email.perl b/git-send-email.perl
> index da81be40cb..d3ebf666c3 100755
> --- a/git-send-email.perl
> +++ b/git-send-email.perl
> @@ -815,6 +815,15 @@ if (!$force) {
>  				. "Pass --force if you really want to send.\n";
>  		}
>  	}
> +	my @hook = ( $ENV{GIT_DIR}.'hooks/send-email', $f )
> +	if( -x $hook[0] ) {
> +		unless( system( @hook ) == 0 )
> +		{
> +			die "Refusing to send because the patch\n\t$f\n"
> +				. "was refused by the send-email hook."
> +				. "Pass --force if you really want to send.\n";
> +		}
> +	}
>  }

I doubt that this is the best place to call this hook, because the called hook does not have access to information that may help it make a better decision.

For example, because the hook gets one patchfile at a time, it does not have the entire picture (e.g. "are you sure you want 01/05, 02/05, 04/05 and 05/05 without 03/05?"). For another example, the hook does not have access to the decision git-send-email makes on various "parameters", which are computed based on the contents of the patchfiles and command line arguments at this point in the code. (e.g. @to, @cc, etc. are computed much later, so you cannot say "do not send anythnng outside corp by mistake" with this mechanism).