[RFC] send-email: support validate hook

* [RFC] send-email: support validate hook
@ 2017-05-11 19:37 Jonathan Tan
  2017-05-11 20:39 ` Brandon Williams
                   ` (3 more replies)
  0 siblings, 4 replies; 9+ messages in thread
From: Jonathan Tan @ 2017-05-11 19:37 UTC (permalink / raw)
  To: git; +Cc: Jonathan Tan

Currently, send-email has support for rudimentary e-mail validation.
Allow the user to add support for more validation by providing a
sendemail-validate hook.

Signed-off-by: Jonathan Tan <jonathantanmy@google.com>
---

This is motivated by situations in which we discuss a work in progress
using Gerrit (which requires Change-Id trailers in patches), and then,
forgetting to remove the Change-Id trailers, send them to the Git
mailing list (which does not want such trailers). I can envision such
functionality being useful in other situations, hence this patch
submission.

I'm not very familiar with Perl, and "There Is More Than One Way To Do
It", so advice on Perl style is appreciated.
---
 Documentation/git-send-email.txt |  1 +
 Documentation/githooks.txt       |  8 ++++++++
 git-send-email.perl              | 18 +++++++++++++++++-
 t/t9001-send-email.sh            | 40 ++++++++++++++++++++++++++++++++++++++++
 4 files changed, 66 insertions(+), 1 deletion(-)

diff --git a/Documentation/git-send-email.txt b/Documentation/git-send-email.txt
index 9d66166f6..bb23b02ca 100644
--- a/Documentation/git-send-email.txt
+++ b/Documentation/git-send-email.txt
@@ -377,6 +377,7 @@ have been specified, in which case default to 'compose'.
 	Currently, validation means the following:
 +
 --
+		*	Invoke the sendemail-validate hook if present (see linkgit:githooks[5]).
 		*	Warn of patches that contain lines longer than 998 characters; this
 			is due to SMTP limits as described by http://www.ietf.org/rfc/rfc2821.txt.
 --
diff --git a/Documentation/githooks.txt b/Documentation/githooks.txt
index 706091a56..b2514f4d4 100644
--- a/Documentation/githooks.txt
+++ b/Documentation/githooks.txt
@@ -447,6 +447,14 @@ rebase::
 The commits are guaranteed to be listed in the order that they were
 processed by rebase.
 
+sendemail-validate
+~~~~~~~~~~~~~~~~~~
+
+This hook is invoked by 'git send-email'.  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.
+
 
 GIT
 ---
diff --git a/git-send-email.perl b/git-send-email.perl
index eea0a517f..7de91ca7c 100755
--- a/git-send-email.perl
+++ b/git-send-email.perl
@@ -27,6 +27,7 @@ use Term::ANSIColor;
 use File::Temp qw/ tempdir tempfile /;
 use File::Spec::Functions qw(catfile);
 use Error qw(:try);
+use Cwd qw(abs_path cwd);
 use Git;
 use Git::I18N;
 
@@ -628,9 +629,24 @@ if (@rev_list_opts) {
 @files = handle_backup_files(@files);
 
 if ($validate) {
+	my @hook = ($repo->repo_path().'/hooks/sendemail-validate', '');
+	my $use_hook = -x $hook[0];
+	if ($use_hook) {
+		# The hook needs a correct GIT_DIR.
+		$ENV{"GIT_DIR"} = $repo->repo_path();
+	}
 	foreach my $f (@files) {
 		unless (-p $f) {
-			my $error = validate_patch($f);
+			my $error;
+			if ($use_hook) {
+				$hook[1] = abs_path($f);
+				my $cwd_save = cwd();
+				chdir($repo->wc_path() or $repo->repo_path());
+				$error = "rejected by sendemail-validate hook"
+					unless system(@hook) == 0;
+				chdir($cwd_save);
+			}
+			$error = validate_patch($f) unless $error;
 			$error and die sprintf(__("fatal: %s: %s\nwarning: no patches were sent\n"),
 						  $f, $error);
 		}
diff --git a/t/t9001-send-email.sh b/t/t9001-send-email.sh
index 60a80f60b..f3f238d40 100755
--- a/t/t9001-send-email.sh
+++ b/t/t9001-send-email.sh
@@ -1913,4 +1913,44 @@ test_expect_success $PREREQ 'leading and trailing whitespaces are removed' '
 	test_cmp expected-list actual-list
 '
 
+test_expect_success $PREREQ 'invoke hook' '
+	mkdir -p .git/hooks &&
+
+	write_script .git/hooks/sendemail-validate <<-\EOF &&
+		# test that we have the correct environment variable, pwd, and
+		# argument
+		case "$GIT_DIR" in
+			*.git)
+				true
+				;;
+			*)
+				false
+				;;
+		esac &&
+		test -e 0001-add-master.patch &&
+		grep "add master" "$1"
+	EOF
+
+	mkdir subdir &&
+	(
+		# Test that it works even if we are not at the root of the
+		# working tree
+		cd subdir &&
+		git send-email \
+			--from="Example <nobody@example.com>" \
+			--to=nobody@example.com \
+			--smtp-server="$(pwd)/../fake.sendmail" \
+			../0001-add-master.patch &&
+
+		# Verify error message when a patch is rejected by the hook
+		sed -e "s/add master/x/" ../0001-add-master.patch >../another.patch &&
+		git send-email \
+			--from="Example <nobody@example.com>" \
+			--to=nobody@example.com \
+			--smtp-server="$(pwd)/../fake.sendmail" \
+			../another.patch 2>err
+		test_i18ngrep "rejected by sendemail-validate hook" err
+	)
+'
+
 test_done
-- 
2.13.0.rc2.291.g57267f2277-goog


^ permalink raw reply related	[flat|nested] 9+ messages in thread