git/list[1] front-page[2] threads[3] people[4] search[5] about
 

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

From
brian m. carlson <sandals@crustytoothpaste.net>
Date
May 14, 2019, 00:23 UTC
Message-ID
<20190514002332.121089-8-sandals@crustytoothpaste.net>
In-Reply-To
<20190514002332.121089-1-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 &&
Previous: brian m. carlsonNext: Duy Nguyen
Message 19 of 34 in “Multiple hook support”
  1. 0/7 Multiple hook supportbrian m. carlson, May 14, 2019
  2. 1/7 run-command: add preliminary support for multiple hooksbrian m. carlson, May 14, 2019
  3. Duy NguyenMay 14, 2019
  4. brian m. carlsonMay 15, 2019
  5. brian m. carlsonMay 29, 2019
  6. Johannes SchindelinMay 14, 2019
  7. brian m. carlsonMay 15, 2019
  8. Johannes SixtMay 16, 2019
  9. Johannes SchindelinMay 17, 2019
  10. 2/7 builtin/receive-pack: add support for multiple hooksbrian m. carlson, May 14, 2019
  11. 3/7 rebase: add support for multiple hooksbrian m. carlson, May 14, 2019
  12. Duy NguyenMay 14, 2019
  13. Johannes SixtMay 14, 2019
  14. brian m. carlsonMay 15, 2019
  15. Duy NguyenMay 16, 2019
  16. 3/7 sequencer: add support for multiple hooksbrian m. carlson, May 14, 2019
  17. 4/7 builtin/worktree: add support for multiple post-checkout hooksbrian m. carlson, May 14, 2019
  18. 5/7 transport: add support for multiple pre-push hooksbrian m. carlson, May 14, 2019
  19. 6/7 config: allow configuration of multiple hook error behaviorbrian m. carlson, May 14, 2019
  20. Duy NguyenMay 14, 2019
  21. brian m. carlsonMay 15, 2019
  22. Jeff KingMay 16, 2019
  23. Jeff KingMay 16, 2019
  24. brian m. carlsonMay 16, 2019
  25. Jeff KingMay 16, 2019
  26. 7/7 docs: document multiple hooksbrian m. carlson, May 14, 2019
  27. Duy NguyenMay 14, 2019
  28. Jonathan NiederMay 14, 2019
  29. brian m. carlsonMay 14, 2019
  30. Jonathan NiederMay 14, 2019
  31. brian m. carlsonMay 16, 2019
  32. Jonathan NiederMay 16, 2019
  33. Jeff KingMay 16, 2019
  34. Duy NguyenMay 14, 2019

Read the whole thread, see it on lore, or plain text.

$ cat FOOTERMessages come from the public archive at lore.kernel.org/git, fetched every hour. The front page is chosen and written each morning by an AI editor and can be wrong; the threads themselves are the record. About and API. For agents: an MCP server at https://gitlist.dev/mcp, and any thread, story or person page as Markdown by adding .md to its URL (or sending Accept: text/markdown). Details in /llms.txt.