[PATCH RFC aiaiai 08/11] aiaiai: change dispatcher hook to email hook

Thu Mar 27 11:40:06 PDT 2014

This patch modifies the current dispatcher hook to the new model of hook
(reading hook contents from stdout of hook). In addition, it also moves
the hook into aiaiai-email-test-patchset instead of putting it in the
dispatcher-helper. This helps simplify the hook process overall.

Signed-off-by: Jacob Keller <jacob.e.keller at intel.com>
---
 doc/email/configuration.txt          | 73 ++++++++++++++++++++----------------
 doc/email/example-aiaiai.cfg         | 10 ++---
 email/aiaiai-email-dispatcher-helper |  7 ----
 email/aiaiai-email-sh-functions      |  7 ++--
 email/aiaiai-email-test-patchset     | 58 +++++++++++++++++++++++++---
 5 files changed, 101 insertions(+), 54 deletions(-)

diff --git a/doc/email/configuration.txt b/doc/email/configuration.txt
index 9056e0c0fb74..3ba16a48ff4d 100644
--- a/doc/email/configuration.txt
+++ b/doc/email/configuration.txt
@@ -160,42 +160,49 @@ specified for each project.
     patch-submitter visible mirror of the local path used internally. If not
     supplied, aiaiai will display the path instead.
 
-2.4 Aiaiai Hooks
+2.4 Hooks
+~~~~~~~~~
+
+The [hooks] section contains settings for various points where a hook can be
+run to customize the behavior of aiaiai. Generally, hooks should output
+mbox-style headers, which the aiaiai program will parse for required
+information. Only certain headers are supported, outlined below.
+
+A hook is expected to return zero for successful runs, 127 for patch rejection,
+and any other non zero value at run-time error. In addition, for a zero return
+status, the output should be a list of headers outlined below, (one per line)
+which will be parsed by aiaiai for customizing behavior. For 127 exit status,
+the stdout will be used for creating a subsequent response email, and it should
+contain information explaining to the user why the hook rejected the patch. For
+other non-zero exit status, the output is ignored.
+
+Hooks are passed the configuration file, and the mbox file for processing and
+therefor have full access to the configuration file. Discretion should be used
+if adding custom variables to the configuration file such that the hook won't
+have a namespace collission with regular aiaiai configuration options.
+
+The only currently supported hook is the email hook configured by setting
+hooks.email to the path of an executable script.
+
+2.4.1 Email Hook
 ~~~~~~~~~~~~~~~~
 
-The [hooks] section can be used to specify scripts which are called by various
-aiaiai tools, and can be used to customize the behavior of aiaiai. This is
-generally done by modifying the mbox to include custom headers, which are
-detailed below.
-
-The two following headers are generally inserted by aiaiai-email-lda and most
-likely don't need to be inserted by a hook, but are included here for
-completeness.
-
-* X-Aiaiai-Cover-Letter-Subject
-    This is the subject of the 0/n email if one is given when sending a patch
-    series.
-* X-Aiaiai-Cover-Letter-Message-Id
-    This is the message Id of the 0/n email if one was given when sending a
-    patch series.
-
-The above two settings are used by aiaiai-email-test-patchset so that it can
-formulate a reply to the 0/n email instead of replying to the 1/n email of a
-patch series. This helps indicate that the entire patch series was validated,
-instead of appearing as though only the first patch was tested.
-
-The following headers will be used if available, and may be helpful for hooks
-to set them, in order to help customize aiaiai.
+This hook is run from within aiaiai-email-test-patchset, and is run just prior
+to determining the project. This hook can be used to customize the behavior and
+settings of aiaiai-email-test-patchset. Primarily it can be used to implement
+any kind of patch-rejection scheme desired, as well as use the following
+headers to customize various aspects of aiaiai.
 
 * X-Aiaiai-Project
-    This header tells aiaiai which project should be used. It overrides the
-    default +project from the email address, and could be set in a scenario
-    where the project is not specified by address.
+    This header indicates to email-test-patchset which project the patch series
+    belongs to. This overrides the default +prj format from the cfg_ownmail
+    address. This can be used to implement any elaborate scheme desired for
+    determining the project, instead of relying on users mailing patches to the
+    correct email address.
 
-2.4.1 Dispatcher Hook
-~~~~~~~~~~~~~~~~~~~~~
+* X-Aiaiai-Commit
+    This header indicates what commit to test against instead of the default
+    refspec supplied by the project. It can be used to implement some form of
+    patch-version detection, or other setup. It likely should be used in tandem
+    with the X-Aiaiai-Project setting.
 
-The dispatcher hook is called by aiaiai-email-dispatcher-helper, and receives
-the mbox file as its first argument, and the cfgfile as its second argument. It
-should modify the mbox file inplace, to include any custom headers specified
-above.
diff --git a/doc/email/example-aiaiai.cfg b/doc/email/example-aiaiai.cfg
index f9c7e0fbdc67..1a2a48935fda 100644
--- a/doc/email/example-aiaiai.cfg
+++ b/doc/email/example-aiaiai.cfg
@@ -45,11 +45,11 @@
 	built_preamble = I have tested your changes
 
 [hooks]
-	# A hook called by aiaiai-email-dispatcher-helper, which should insert
-	# custom headers into the mbox file. Specific documentation on this is
-	# provided in the doc/email section, including expected argument
-	# format, as well as what headers are recognized by other tools.
-	dispatcher =
+	# A hook called by aiaiai-email-test-patchset, which should output a
+	# zero exit status as well as the supported custom headers. If the hook
+	# wants to reject a patch, exit with return code 127. Any other return
+	# code indicates an internal error in the hook.
+	email =
 
 # These options are probably not useful, but may help debug issues with aiaiai
 [debug]
diff --git a/email/aiaiai-email-dispatcher-helper b/email/aiaiai-email-dispatcher-helper
index 9627182326fc..9558b7799452 100755
--- a/email/aiaiai-email-dispatcher-helper
+++ b/email/aiaiai-email-dispatcher-helper
@@ -96,13 +96,6 @@ parse_config "$cfgfile"
 
 tmpdir="$(mktemp -dt "$PROG.XXXX")"
 
-# Run the dispatcher hook
-if [ -n "$cfg_dispatch_hook" ]; then
-	hook="$(readlink -fv -- "$cfg_dispatch_hook")"
-	verbose "Running \"$hook\" on \"$mbox\""
-	$hook "$mbox" "$cfgfile"
-fi
-
 verbose "Validating \"$mbox\", queue directory is: $queuedir"
 verbose "Configuration file: $cfgfile"
 
diff --git a/email/aiaiai-email-sh-functions b/email/aiaiai-email-sh-functions
index 4e9f34936aaf..28b049de44e9 100644
--- a/email/aiaiai-email-sh-functions
+++ b/email/aiaiai-email-sh-functions
@@ -148,7 +148,8 @@ parse_config_supported_projects()
 #
 # cfg_ownname, cfg_ownmail, cfg_adminname, cfg_adminmail, cfg_workdir,
 # cfg_max_validators, cfg_jobs, cfg_preamble, cfg_signature,
-# cfg_built_preamble, cfg_disable_notifications, cfg_preserve_files
+# cfg_built_preamble, cfg_disable_notifications, cfg_preserve_files,
+# and cfg_email_hook
 #
 # Additionally, the following variables are set:
 #  o cfg_ownmail_local - the local portion of the ownmail address
@@ -171,8 +172,8 @@ parse_config()
 	ini_config_get_or_die cfg_signature      "$cfgfile" "global" "signature"
 	ini_config_get_or_die cfg_built_preamble "$cfgfile" "global" "built_preamble"
 
-	# Hooks which can be used to enable custom behavior
-	cfg_dispatch_hook="$(ini_config_get "$cfgfile" "hooks" "dispatcher")"
+	# Get the location of email hook(s)
+	cfg_email_hook="$(ini_config_get "$cfgfile" "hooks" "email")"
 
 	# Debug options
 	cfg_disable_notifications="$(ini_config_get "$cfgfile" "debug" "disable_notifications")"
diff --git a/email/aiaiai-email-test-patchset b/email/aiaiai-email-test-patchset
index 57a08538a103..da07eebc8372 100755
--- a/email/aiaiai-email-test-patchset
+++ b/email/aiaiai-email-test-patchset
@@ -122,6 +122,23 @@ EOF
 	exit 0
 }
 
+# This function is called when a hook has requested discarding a patch, and
+# should be passed the main body content from the hook output as stdin.
+error_hook_rejected_patch()
+{
+	send_email <<EOF
+$(cat)
+
+List of projects $cfg_ownname supports:
+
+$(list_projects)
+
+Please, contact "$cfg_adminname" <$cfg_adminemail>
+if you have any questions.
+EOF
+	exit 0
+}
+
 # This function is called when the patch submitter specifies a non-existing
 # project. It sends a sensible reply back, without carbon-copying anyone else.
 error_project_not_found()
@@ -254,11 +271,8 @@ mkdir $verbose -p -- "$cfg_workdir" >&2
 tmpdir="$(mktemp --tmpdir="$cfg_workdir" -dt "$PROG.XXXX")"
 mv $verbose -- "$mbox" "$tmpdir/mbox" >&2
 mbox="$tmpdir/mbox"
-
-# Find out the project name
-prj="$(fetch_header "X-Aiaiai-Project" < "$mbox")"
-[ -n "$prj" ] || prj="$(fetch_project_name "$to" "$cfg_ownmail")"
-verbose "Project \"$prj\""
+hookoutput="$tmpdir/hook"
+touch -- "$hookoutput"
 
 # Replies will refer the first patch of the patch-set under test
 reply_subj="$subj"
@@ -268,6 +282,34 @@ reply_to="$from"
 # And do not Cc anyone thus far
 reply_cc=
 
+# Run the aiaiai email hook. Output is stored in $hookoutput and we can parse
+# this via fetch_header similar to how we parse the mbox.
+hookfile="$(readlink -ev -- $cfg_email_hook)"
+if [ -f "$hookfile" ] && [ -x "$hookfile" ]; then
+	# Hook points to an executable file, so we run it
+	verbose "Executing \"$hookfile\""
+	if ! $hookfile $cfgfile $mbox > $hookoutput; then
+		# Save return immediately, so we don't lose it!
+		hookret=$?
+
+		# Error code 127 is an expected output of the hook, and
+		# indicates that we should reject this patch. The reply email
+		# will be sent to the user, and the hook is expected to have
+		# outputted the rejection indication. As a precaution, the
+		# rejection email will include a list of projects supported.
+		if [ "$hookref" -eq "127" ]; then
+			error_hook_rejected_patch < $hookoutput
+		else
+			error_internal_error_occurred
+		fi
+	fi
+fi
+
+# Find out the project name
+prj="$(fetch_header "X-Aiaiai-Project" < "$hookoutput")"
+[ -n "$prj" ] || prj="$(fetch_project_name "$to" "$cfg_ownmail")"
+verbose "Project \"$prj\""
+
 # Reject the e-mail if the project has not been specified
 if [ -z "$prj" ]; then
 	error_no_project_specified
@@ -307,11 +349,15 @@ if [ "$pcfg_accept_notify" = "1" ]; then
 	send_accepted_email
 fi
 
+# Use the supplied commit from the hook or default to branch head
+commit="$(fetch_header "X-Aiaiai-Commit" < "$hookoutput")"
+[ -n "$commit" ] || commit="$pcfg_branch"
+
 # Test the path (or patch-set)
 verbose "Test configs \"$pcfg_configs\" branch \"$pcfg_branch\" of \"$pcfg_path\""
 aiaiai-test-patchset $verbose ${cfg_preserve_files:+--preserve} \
 	${pcfg_targets:+--targets "$pcfg_targets"} $bisectability $sparse $smatch $cppcheck $coccinelle \
-	-i "$mbox" -j "$cfg_jobs" -c "$pcfg_branch" -w "$tmpdir" \
+	-i "$mbox" -j "$cfg_jobs" -c "$commit" -w "$tmpdir" \
 	${pcfg_defconfigdir:+-C "$pcfg_defconfigdir"} \
 	${pcfg_unwanted_keywords:+-K "$pcfg_unwanted_keywords"} \
 	${pcfg_kmake_opts:+-M "$pcfg_kmake_opts"} -- \
-- 
1.8.3.1


