[PATCH v2 6/7] config: allow configuration of multiple hook error behavior

["brian m. carlson" <sandals@crustytoothpaste.net>]

There are a variety of situations in which a user may want an error
behavior for multiple hooks other than the default. Add a config option,
hook.<name>.errorBehavior to allow users to customize this behavior on a
per-hook basis. Provide options for the default behavior (exiting
early), executing all hooks and succeeding if all hooks succeed, or
executing all hooks and succeeding if any hook succeeds.

Signed-off-by: brian m. carlson <sandals@crustytoothpaste.net>
---
 config.c       |  27 +++++++++++++
 run-command.c  |  42 +++++++++++++++++---
 run-command.h  |   5 +++
 t/lib-hooks.sh | 106 ++++++++++++++++++++++++++++++++++++++++++++++++-
 4 files changed, 173 insertions(+), 7 deletions(-)

diff --git a/config.c b/config.c
index c2846df3f1..9cba4061a9 100644
--- a/config.c
+++ b/config.c
@@ -19,6 +19,7 @@
 #include "utf8.h"
 #include "dir.h"
 #include "color.h"
+#include "run-command.h"
 
 struct config_source {
 	struct config_source *prev;
@@ -1093,6 +1094,29 @@ int git_config_color(char *dest, const char *var, const char *value)
 	return 0;
 }
 
+static int git_default_hook_config(const char *key, const char *value)
+{
+	const char *hook;
+	size_t key_len;
+	uintptr_t behavior;
+
+	key += strlen("hook.");
+	if (strip_suffix(key, ".errorbehavior", &key_len)) {
+		hook = xmemdupz(key, key_len);
+		if (!strcmp(value, "stop-on-first"))
+			behavior = HOOK_ERROR_STOP_ON_FIRST;
+		else if (!strcmp(value, "report-any-error"))
+			behavior = HOOK_ERROR_REPORT_ANY_ERROR;
+		else if (!strcmp(value, "report-any-success"))
+			behavior = HOOK_ERROR_REPORT_ANY_SUCCESS;
+		else
+			die(_("invalid mode for hook %s error behavior: %s"), hook, value);
+		string_list_insert(&hook_error_behavior, hook)->util = (void *)behavior;
+		return 0;
+	}
+	return 0;
+}
+
 static int git_default_core_config(const char *var, const char *value, void *cb)
 {
 	/* This needs a better name */
@@ -1450,6 +1474,9 @@ int git_default_config(const char *var, const char *value, void *cb)
 	    starts_with(var, "committer."))
 		return git_ident_config(var, value, cb);
 
+	if (starts_with(var, "hook."))
+		return git_default_hook_config(var, value);
+
 	if (starts_with(var, "i18n."))
 		return git_default_i18n_config(var, value);
 
diff --git a/run-command.c b/run-command.c
index 191d6f6f7e..70fb19a55b 100644
--- a/run-command.c
+++ b/run-command.c
@@ -1308,6 +1308,8 @@ int async_with_fork(void)
 #endif
 }
 
+struct string_list hook_error_behavior = STRING_LIST_INIT_NODUP;
+
 /*
  * Return 1 if a hook exists at path (which may be modified) using access(2)
  * with check (which should be F_OK or X_OK), 0 otherwise. If strip is true,
@@ -1401,18 +1403,48 @@ int for_each_hook(const char *name,
 		  void *data)
 {
 	struct string_list paths = STRING_LIST_INIT_DUP;
-	int i, ret = 0;
+	int i, hret = 0;
+	uintptr_t behavior = HOOK_ERROR_STOP_ON_FIRST;
+	struct string_list_item *item;
+	/* Use -2 as sentinel because failure to exec is -1. */
+	int ret = -2;
+
+	item = string_list_lookup(&hook_error_behavior, name);
+	if (item)
+		behavior = (uintptr_t)item->util;
 
 	find_hooks(name, &paths);
 	for (i = 0; i < paths.nr; i++) {
 		const char *p = paths.items[i].string;
 
-		ret = handler(name, p, data);
-		if (ret)
-			break;
+		hret = handler(name, p, data);
+		switch (behavior) {
+			case HOOK_ERROR_STOP_ON_FIRST:
+				if (hret) {
+					ret = hret;
+					goto out;
+				}
+				break;
+			case HOOK_ERROR_REPORT_ANY_SUCCESS:
+				if (ret == -2)
+					ret = 1;
+				if (!hret)
+					ret = 0;
+				break;
+			case HOOK_ERROR_REPORT_ANY_ERROR:
+				if (ret == -2)
+					ret = 0;
+				if (hret)
+					ret = hret;
+				break;
+			default:
+				BUG("unknown hook error behavior");
+		}
 	}
-
+out:
 	string_list_clear(&paths, 0);
+	if (ret == -2)
+		return 0;
 	return ret;
 }
 
diff --git a/run-command.h b/run-command.h
index 15974e26d4..879ebb768f 100644
--- a/run-command.h
+++ b/run-command.h
@@ -63,6 +63,11 @@ int finish_command(struct child_process *);
 int finish_command_in_signal(struct child_process *);
 int run_command(struct child_process *);
 
+#define HOOK_ERROR_STOP_ON_FIRST 1
+#define HOOK_ERROR_REPORT_ANY_ERROR 2
+#define HOOK_ERROR_REPORT_ANY_SUCCESS 3
+extern struct string_list hook_error_behavior;
+
 /*
  * Returns the paths to all hook files, or NULL if all hooks are missing or
  * disabled.
diff --git a/t/lib-hooks.sh b/t/lib-hooks.sh
index 721250aea0..c1d7688313 100644
--- a/t/lib-hooks.sh
+++ b/t/lib-hooks.sh
@@ -121,7 +121,7 @@ test_multiple_hooks () {
 		! test -f "$OUTPUTDIR/c"
 	'
 
-	test_expect_success "$hook: multiple hooks, all successful" '
+	test_expect_success "$hook: multiple hooks, all successful by default" '
 		test_when_finished "rm -fr \"$OUTPUTDIR\"" &&
 		rm -f "$HOOK" &&
 		create_multihooks 0 0 0 &&
@@ -131,7 +131,40 @@ test_multiple_hooks () {
 		test -f "$OUTPUTDIR/c"
 	'
 
-	test_expect_success "$hook: hooks after first failure not executed" '
+	test_expect_success "$hook: multiple hooks, all successful with stop-on-first" '
+		test_when_finished "rm -fr \"$OUTPUTDIR\"" &&
+		test_config "hook.$hook.errorbehavior" stop-on-first &&
+		rm -f "$HOOK" &&
+		create_multihooks 0 0 0 &&
+		$cmd content-stop-on-first &&
+		test -f "$OUTPUTDIR/a" &&
+		test -f "$OUTPUTDIR/b" &&
+		test -f "$OUTPUTDIR/c"
+	'
+
+	test_expect_success "$hook: multiple hooks, all successful with report-any-error" '
+		test_when_finished "rm -fr \"$OUTPUTDIR\"" &&
+		test_config "hook.$hook.errorbehavior" report-any-error &&
+		rm -f "$HOOK" &&
+		create_multihooks 0 0 0 &&
+		$cmd content-report-any-error &&
+		test -f "$OUTPUTDIR/a" &&
+		test -f "$OUTPUTDIR/b" &&
+		test -f "$OUTPUTDIR/c"
+	'
+
+	test_expect_success "$hook: multiple hooks, all successful with report-any-success" '
+		test_when_finished "rm -fr \"$OUTPUTDIR\"" &&
+		test_config "hook.$hook.errorbehavior" report-any-success &&
+		rm -f "$HOOK" &&
+		create_multihooks 0 0 0 &&
+		$cmd content-report-any-success &&
+		test -f "$OUTPUTDIR/a" &&
+		test -f "$OUTPUTDIR/b" &&
+		test -f "$OUTPUTDIR/c"
+	'
+
+	test_expect_success "$hook: hooks after first failure not executed by default" '
 		test_when_finished "rm -fr \"$OUTPUTDIR\"" &&
 		create_multihooks 0 1 0 &&
 		$must_fail $cmd more-content &&
@@ -140,6 +173,75 @@ test_multiple_hooks () {
 		! test -f "$OUTPUTDIR/c"
 	'
 
+	test_expect_success "$hook: hooks after first failure not executed with stop-on-first" '
+		test_when_finished "rm -fr \"$OUTPUTDIR\"" &&
+		test_config "hook.$hook.errorbehavior" stop-on-first &&
+		create_multihooks 0 1 0 &&
+		$must_fail $cmd more-content-stop-on-first &&
+		test -f "$OUTPUTDIR/a" &&
+		test -f "$OUTPUTDIR/b" &&
+		! test -f "$OUTPUTDIR/c"
+	'
+
+	test_expect_success "$hook: hooks after first failure executed with report-any-error" '
+		test_when_finished "rm -fr \"$OUTPUTDIR\"" &&
+		test_config "hook.$hook.errorbehavior" report-any-error &&
+		create_multihooks 0 1 0 &&
+		$must_fail $cmd more-content-report-any-error &&
+		test -f "$OUTPUTDIR/a" &&
+		test -f "$OUTPUTDIR/b" &&
+		test -f "$OUTPUTDIR/c"
+	'
+
+	test_expect_success "$hook: hooks after first failure executed with report-any-success" '
+		test_when_finished "rm -fr \"$OUTPUTDIR\"" &&
+		test_config "hook.$hook.errorbehavior" report-any-success &&
+		create_multihooks 0 1 0 &&
+		$cmd more-content-report-any-success &&
+		test -f "$OUTPUTDIR/a" &&
+		test -f "$OUTPUTDIR/b" &&
+		test -f "$OUTPUTDIR/c"
+	'
+
+	test_expect_success "$hook: failing hooks by default" '
+		test_when_finished "rm -fr \"$OUTPUTDIR\"" &&
+		create_multihooks 1 1 1 &&
+		$must_fail $cmd most-content &&
+		test -f "$OUTPUTDIR/a" &&
+		! test -f "$OUTPUTDIR/b" &&
+		! test -f "$OUTPUTDIR/c"
+	'
+
+	test_expect_success "$hook: failing hooks with stop-on-first" '
+		test_when_finished "rm -fr \"$OUTPUTDIR\"" &&
+		test_config "hook.$hook.errorbehavior" stop-on-first &&
+		create_multihooks 1 1 1 &&
+		$must_fail $cmd most-content-stop-on-first &&
+		test -f "$OUTPUTDIR/a" &&
+		! test -f "$OUTPUTDIR/b" &&
+		! test -f "$OUTPUTDIR/c"
+	'
+
+	test_expect_success "$hook: failing hooks with report-any-error" '
+		test_when_finished "rm -fr \"$OUTPUTDIR\"" &&
+		test_config "hook.$hook.errorbehavior" report-any-error &&
+		create_multihooks 1 1 1 &&
+		$must_fail $cmd most-content-report-any-error &&
+		test -f "$OUTPUTDIR/a" &&
+		test -f "$OUTPUTDIR/b" &&
+		test -f "$OUTPUTDIR/c"
+	'
+
+	test_expect_success "$hook: failing hooks with report-any-success" '
+		test_when_finished "rm -fr \"$OUTPUTDIR\"" &&
+		test_config "hook.$hook.errorbehavior" report-any-success &&
+		create_multihooks 1 1 1 &&
+		$must_fail $cmd most-content-report-any-success &&
+		test -f "$OUTPUTDIR/a" &&
+		test -f "$OUTPUTDIR/b" &&
+		test -f "$OUTPUTDIR/c"
+	'
+
 	test_expect_success POSIXPERM "$hook: non-executable hook not executed" '
 		test_when_finished "rm -fr \"$OUTPUTDIR\"" &&
 		create_multihooks 0 1 0 &&