diff mbox series

[v9,04/37] hook: teach hook.runHookDir

Message ID 20210527000856.695702-5-emilyshaffer@google.com (mailing list archive)
State New, archived
Headers show
Series propose config-based hooks | expand

Commit Message

Emily Shaffer May 27, 2021, 12:08 a.m. UTC
For now, just give a hint about how these hooks will be run in 'git hook
list'. Later on, though, we will pay attention to this enum when running
the hooks.
---

Notes:
    Since v7, tidied up the behavior of the HOOK_UNKNOWN flag and added a test to
    enforce it - now it matches the design doc much better.
    
    Also, thanks Jonathan Tan for pointing out that the commit message made no sense
    and was targeted for a different change. Rewrote the commit message now.
    
    Plus, added HOOK_ERROR flag per Junio and Jonathan Nieder.
    
    Newly split into its own commit since v4, and taking place much sooner.
    
    An unfortunate side effect of adding this support *before* the
    hook.runHookDir support is that the labels on the list are not clear -
    because we aren't yet flagging which hooks are from the hookdir versus
    the config. I suppose we could move the addition of that field to the
    struct hook up to this patch, but it didn't make a lot of sense to me to
    do it just for cosmetic purposes.
    
    Since v7, tidied up the behavior of the HOOK_UNKNOWN flag and added a test to
    enforce it - now it matches the design doc much better.
    
    Also, thanks Jonathan Tan for pointing out that the commit message made no sense
    and was targeted for a different change. Rewrote the commit message now.
    
    Newly split into its own commit since v4, and taking place much sooner.
    
    An unfortunate side effect of adding this support *before* the
    hook.runHookDir support is that the labels on the list are not clear -
    because we aren't yet flagging which hooks are from the hookdir versus
    the config. I suppose we could move the addition of that field to the
    struct hook up to this patch, but it didn't make a lot of sense to me to
    do it just for cosmetic purposes.

 Documentation/config/hook.txt |  5 ++
 builtin/hook.c                | 98 ++++++++++++++++++++++++++++++-----
 hook.c                        | 24 +++++++++
 hook.h                        | 22 ++++++++
 t/t1360-config-based-hooks.sh | 71 +++++++++++++++++++++++++
 5 files changed, 206 insertions(+), 14 deletions(-)
diff mbox series

Patch

diff --git a/Documentation/config/hook.txt b/Documentation/config/hook.txt
index 71449ecbc7..75312754ae 100644
--- a/Documentation/config/hook.txt
+++ b/Documentation/config/hook.txt
@@ -7,3 +7,8 @@  hookcmd.<name>.command::
 	A command to execute during a hook for which <name> has been specified
 	as a command. This can be an executable on your device or a oneliner for
 	your shell. See linkgit:git-hook[1].
+
+hook.runHookDir::
+	Controls how hooks contained in your hookdir are executed. Can be any of
+	"yes", "warn", "interactive", or "no". Defaults to "yes". See
+	linkgit:git-hook[1] and linkgit:git-config[1] "core.hooksPath").
diff --git a/builtin/hook.c b/builtin/hook.c
index e82725f0a6..b1e63a9576 100644
--- a/builtin/hook.c
+++ b/builtin/hook.c
@@ -10,10 +10,13 @@  static const char * const builtin_hook_usage[] = {
 	NULL
 };
 
+static enum hookdir_opt should_run_hookdir;
+
 static int list(int argc, const char **argv, const char *prefix)
 {
 	struct list_head *head, *pos;
 	const char *hookname = NULL;
+	struct strbuf hookdir_annotation = STRBUF_INIT;
 
 	struct option list_options[] = {
 		OPT_END(),
@@ -41,37 +44,104 @@  static int list(int argc, const char **argv, const char *prefix)
 		struct hook *item = list_entry(pos, struct hook, list);
 		item = list_entry(pos, struct hook, list);
 		if (item) {
-			/*
-			 * TRANSLATORS: "<config scope>: <path>". Both fields
-			 * should be left untranslated; config scope matches the
-			 * output of 'git config --show-scope'. Marked for
-			 * translation to provide better RTL support later.
-			 */
-			printf(_("%s: %s\n"),
-			       (item->from_hookdir
-				? "hookdir"
-				: config_scope_name(item->origin)),
-			       item->command.buf);
+			if (item->from_hookdir) {
+				/*
+				 * TRANSLATORS: do not translate 'hookdir' as
+				 * it matches the config setting.
+				 */
+				switch (should_run_hookdir) {
+				case HOOKDIR_NO:
+					printf(_("hookdir: %s (will not run)\n"),
+					       item->command.buf);
+					break;
+				case HOOKDIR_ERROR:
+					printf(_("hookdir: %s (will error and not run)\n"),
+					       item->command.buf);
+					break;
+				case HOOKDIR_INTERACTIVE:
+					printf(_("hookdir: %s (will prompt)\n"),
+					       item->command.buf);
+					break;
+				case HOOKDIR_WARN:
+					printf(_("hookdir: %s (will warn but run)\n"),
+					       item->command.buf);
+					break;
+				case HOOKDIR_YES:
+				/*
+				 * The default behavior should agree with
+				 * hook.c:configured_hookdir_opt(). HOOKDIR_UNKNOWN should just
+				 * do the default behavior.
+				 */
+				case HOOKDIR_UNKNOWN:
+				default:
+					printf(_("hookdir: %s\n"),
+						 item->command.buf);
+					break;
+				}
+			} else {
+				/*
+				 * TRANSLATORS: "<config scope>: <path>". Both fields
+				 * should be left untranslated; config scope matches the
+				 * output of 'git config --show-scope'. Marked for
+				 * translation to provide better RTL support later.
+				 */
+				printf(_("%s: %s\n"),
+					config_scope_name(item->origin),
+					item->command.buf);
+			}
 		}
 	}
 
 	clear_hook_list(head);
+	strbuf_release(&hookdir_annotation);
 
 	return 0;
 }
 
 int cmd_hook(int argc, const char **argv, const char *prefix)
 {
+	const char *run_hookdir = NULL;
+
 	struct option builtin_hook_options[] = {
+		OPT_STRING(0, "run-hookdir", &run_hookdir, N_("option"),
+			   N_("what to do with hooks found in the hookdir")),
 		OPT_END(),
 	};
-	if (argc < 2)
+
+	argc = parse_options(argc, argv, prefix, builtin_hook_options,
+			     builtin_hook_usage, 0);
+
+	/* after the parse, we should have "<command> <hookname> <args...>" */
+	if (argc < 1)
 		usage_with_options(builtin_hook_usage, builtin_hook_options);
 
 	git_config(git_default_config, NULL);
 
-	if (!strcmp(argv[1], "list"))
-		return list(argc - 1, argv + 1, prefix);
+
+	/* argument > config */
+	if (run_hookdir)
+		if (!strcmp(run_hookdir, "no"))
+			should_run_hookdir = HOOKDIR_NO;
+		else if (!strcmp(run_hookdir, "error"))
+			should_run_hookdir = HOOKDIR_ERROR;
+		else if (!strcmp(run_hookdir, "yes"))
+			should_run_hookdir = HOOKDIR_YES;
+		else if (!strcmp(run_hookdir, "warn"))
+			should_run_hookdir = HOOKDIR_WARN;
+		else if (!strcmp(run_hookdir, "interactive"))
+			should_run_hookdir = HOOKDIR_INTERACTIVE;
+		else
+			/*
+			 * TRANSLATORS: leave "yes/warn/interactive/no"
+			 * untranslated; the strings are compared literally.
+			 */
+			die(_("'%s' is not a valid option for --run-hookdir "
+			      "(yes, warn, interactive, no)"), run_hookdir);
+	else
+		should_run_hookdir = configured_hookdir_opt();
+
+	if (!strcmp(argv[0], "list"))
+		return list(argc, argv, prefix);
 
 	usage_with_options(builtin_hook_usage, builtin_hook_options);
 }
diff --git a/hook.c b/hook.c
index b4994fc108..030051cab2 100644
--- a/hook.c
+++ b/hook.c
@@ -102,6 +102,30 @@  static int hook_config_lookup(const char *key, const char *value, void *cb_data)
 	return 0;
 }
 
+enum hookdir_opt configured_hookdir_opt(void)
+{
+	const char *key;
+	if (git_config_get_value("hook.runhookdir", &key))
+		return HOOKDIR_YES; /* by default, just run it. */
+
+	if (!strcmp(key, "no"))
+		return HOOKDIR_NO;
+
+	if (!strcmp(key, "error"))
+		return HOOKDIR_ERROR;
+
+	if (!strcmp(key, "yes"))
+		return HOOKDIR_YES;
+
+	if (!strcmp(key, "warn"))
+		return HOOKDIR_WARN;
+
+	if (!strcmp(key, "interactive"))
+		return HOOKDIR_INTERACTIVE;
+
+	return HOOKDIR_UNKNOWN;
+}
+
 struct list_head* hook_list(const char* hookname)
 {
 	struct strbuf hook_key = STRBUF_INIT;
diff --git a/hook.h b/hook.h
index b6c5480325..7f2b2ee8f2 100644
--- a/hook.h
+++ b/hook.h
@@ -20,6 +20,28 @@  struct hook {
  */
 struct list_head* hook_list(const char *hookname);
 
+enum hookdir_opt
+{
+	HOOKDIR_NO,
+	HOOKDIR_ERROR,
+	HOOKDIR_WARN,
+	HOOKDIR_INTERACTIVE,
+	HOOKDIR_YES,
+	HOOKDIR_UNKNOWN,
+};
+
+/*
+ * Provides the hookdir_opt specified in the config without consulting any
+ * command line arguments.
+ */
+enum hookdir_opt configured_hookdir_opt(void);
+
+/*
+ * Provides the hookdir_opt specified in the config without consulting any
+ * command line arguments.
+ */
+enum hookdir_opt configured_hookdir_opt(void);
+
 /* Free memory associated with a 'struct hook' */
 void free_hook(struct hook *ptr);
 /* Empties the list at 'head', calling 'free_hook()' on each entry */
diff --git a/t/t1360-config-based-hooks.sh b/t/t1360-config-based-hooks.sh
index 0f12af4659..141e6f7590 100755
--- a/t/t1360-config-based-hooks.sh
+++ b/t/t1360-config-based-hooks.sh
@@ -104,4 +104,75 @@  test_expect_success 'git hook list shows hooks from the hookdir' '
 	test_cmp expected actual
 '
 
+test_expect_success 'hook.runHookDir = no is respected by list' '
+	setup_hookdir &&
+
+	test_config hook.runHookDir "no" &&
+
+	cat >expected <<-EOF &&
+	hookdir: $(pwd)/.git/hooks/pre-commit (will not run)
+	EOF
+
+	git hook list pre-commit >actual &&
+	# the hookdir annotation is translated
+	test_cmp expected actual
+'
+
+test_expect_success 'hook.runHookDir = error is respected by list' '
+	setup_hookdir &&
+
+	test_config hook.runHookDir "error" &&
+
+	cat >expected <<-EOF &&
+	hookdir: $(pwd)/.git/hooks/pre-commit (will error and not run)
+	EOF
+
+	git hook list pre-commit >actual &&
+	# the hookdir annotation is translated
+	test_cmp expected actual
+'
+
+test_expect_success 'hook.runHookDir = warn is respected by list' '
+	setup_hookdir &&
+
+	test_config hook.runHookDir "warn" &&
+
+	cat >expected <<-EOF &&
+	hookdir: $(pwd)/.git/hooks/pre-commit (will warn but run)
+	EOF
+
+	git hook list pre-commit >actual &&
+	# the hookdir annotation is translated
+	test_cmp expected actual
+'
+
+
+test_expect_success 'hook.runHookDir = interactive is respected by list' '
+	setup_hookdir &&
+
+	test_config hook.runHookDir "interactive" &&
+
+	cat >expected <<-EOF &&
+	hookdir: $(pwd)/.git/hooks/pre-commit (will prompt)
+	EOF
+
+	git hook list pre-commit >actual &&
+	# the hookdir annotation is translated
+	test_cmp expected actual
+'
+
+test_expect_success 'hook.runHookDir is tolerant to unknown values' '
+	setup_hookdir &&
+
+	test_config hook.runHookDir "junk" &&
+
+	cat >expected <<-EOF &&
+	hookdir: $(pwd)/.git/hooks/pre-commit
+	EOF
+
+	git hook list pre-commit >actual &&
+	# the hookdir annotation is translated
+	test_cmp expected actual
+'
+
 test_done