[PATCH 9/9] hook: implement hookcmd.<name>.skip

[Emily Shaffer <emilyshaffer@google.com>]

If a user wants a specific repo to skip execution of a hook which is set
at a global or system level, they will be able to do so by specifying
'skip' in their repo config:

~/.gitconfig
  [hook.pre-commit]
    command = skippable-oneliner
    command = skippable-hookcmd

  [hookcmd.skippable-hookcmd]
    command = foo.sh

$GIT_DIR/.git/config
  [hookcmd.skippable-oneliner]
    skip = true
  [hookcmd.skippable-hookcmd]
    skip = true

Later it may make sense to add an option like
"hookcmd.<name>.<hook-event>-skip" - but for simplicity, let's start
with a universal skip setting like this.

Signed-off-by: Emily Shaffer <emilyshaffer@google.com>
---
 Documentation/config/hook.txt |  8 ++++++++
 Documentation/git-hook.txt    | 30 +++++++++++++++++++++++++++++
 hook.c                        | 31 +++++++++++++++++++++---------
 t/t1360-config-based-hooks.sh | 36 +++++++++++++++++++++++++++++++++++
 4 files changed, 96 insertions(+), 9 deletions(-)

diff --git a/Documentation/config/hook.txt b/Documentation/config/hook.txt
index 5b35170664..6b3776f06f 100644
--- a/Documentation/config/hook.txt
+++ b/Documentation/config/hook.txt
@@ -8,6 +8,14 @@ hookcmd.<name>.command::
 	as a command. This can be an executable on your device or a oneliner for
 	your shell. See linkgit:git-hook[1].
 
+hookcmd.<name>.skip::
+	Specify this boolean to remove a command from earlier in the execution
+	order. Useful if you want to make a single repo an exception to hook
+	configured at the system or global scope. If there is no hookcmd
+	specified for the command you want to skip, you can use the value of
+	`hook.<command>.command` as <name> as a shortcut. The "skip" setting
+	must be specified after the "hook.<command>.command" to have an effect.
+
 hook.jobs::
 	Specifies how many hooks can be run simultaneously during parallelized
 	hook execution. If unspecified, defaults to the number of processors on
diff --git a/Documentation/git-hook.txt b/Documentation/git-hook.txt
index 1a4d22fd90..fcd13da4ff 100644
--- a/Documentation/git-hook.txt
+++ b/Documentation/git-hook.txt
@@ -57,6 +57,36 @@ and prepare-commit-msg hooks in this order:
   /bin/linter --c
   .git/hooks/prepare-commit-msg (if present)
 
+If there is a command you wish to run in most cases but have one or two
+exceptional repos where it should be skipped, you can specify
+`hookcmd.<name>.skip`, for example:
+
+System config
+----
+  [hook "post-commit"]
+    command = check-for-secrets
+
+  [hookcmd "check-for-secrets"]
+    command = /bin/secret-checker --aggressive
+----
+
+Local config
+----
+  [hookcmd "check-for-secrets"]
+    skip = true
+  # This works for inlined hook commands, too:
+  [hookcmd "~/typocheck.sh"]
+    skip = true
+----
+
+After these configs are added, and including the earlier example configs, the
+hook list becomes:
+
+post-commit:
+  /bin/linter --c
+  python ~/run-test-suite.py
+  .git/hooks/post-commit (if present)
+
 In general, when instructions suggest adding a script to
 `.git/hooks/<something>`, you can specify it in the config instead by running
 `git config --add hook.<something>.command <path-to-script>` - this way you can
diff --git a/hook.c b/hook.c
index 21904d90f6..5faa1690e4 100644
--- a/hook.c
+++ b/hook.c
@@ -18,6 +18,7 @@ static void free_hook(struct hook *ptr)
  */
 static struct hook * find_hook_by_command(struct list_head *head, const char *command)
 {
+	/* check if the hook is already in the list */
 	struct list_head *pos = NULL, *tmp = NULL;
 	struct hook *found = NULL;
 
@@ -40,7 +41,6 @@ static struct hook * find_hook_by_command(struct list_head *head, const char *co
  */
 static struct hook * append_or_move_hook(struct list_head *head, const char *command)
 {
-	/* check if the hook is already in the list */
 	struct hook *to_add = find_hook_by_command(head, command);
 
 	if (!to_add) {
@@ -175,14 +175,15 @@ static int hook_config_lookup(const char *key, const char *value, void *cb_data)
 	if (!strcmp(key, hook_key)) {
 		const char *command = value;
 		struct strbuf hookcmd_name = STRBUF_INIT;
+		int skip = 0;
 
-
-		if (!command) {
-			strbuf_release(&hookcmd_name);
-			BUG("git_config_get_value overwrote a string it shouldn't have");
-		}
-
-		/* TODO: implement skipping hooks */
+		/*
+		 * Check if we're removing that hook instead. Hookcmds are
+		 * removed by name, and inlined hooks are removed by command
+		 * content.
+		 */
+		strbuf_addf(&hookcmd_name, "hookcmd.%s.skip", command);
+		git_config_get_bool(hookcmd_name.buf, &skip);
 
 		/*
 		 * Check if a hookcmd with that name exists. If it doesn't,
@@ -193,12 +194,24 @@ static int hook_config_lookup(const char *key, const char *value, void *cb_data)
 		strbuf_addf(&hookcmd_name, "hookcmd.%s.command", command);
 		git_config_get_value(hookcmd_name.buf, &command);
 
+		if (!command) {
+			strbuf_release(&hookcmd_name);
+			BUG("git_config_get_value overwrote a string it shouldn't have");
+		}
+
 		/*
 		 * TODO: implement an option-getting callback, e.g.
 		 *   get configs by pattern hookcmd.$value.*
 		 *   for each key+value, do_callback(key, value, cb_data)
 		 */
-		append_or_move_hook(head, command);
+
+		if (skip) {
+			struct hook *to_remove = find_hook_by_command(head, command);
+			if (to_remove)
+				remove_hook(&(to_remove->list));
+		} else {
+			append_or_move_hook(head, command);
+		}
 
 		strbuf_release(&hookcmd_name);
 	}
diff --git a/t/t1360-config-based-hooks.sh b/t/t1360-config-based-hooks.sh
index 50ee824f05..30dc7b6054 100755
--- a/t/t1360-config-based-hooks.sh
+++ b/t/t1360-config-based-hooks.sh
@@ -103,6 +103,42 @@ test_expect_success 'git hook list shows hooks from the hookdir' '
 	test_cmp expected actual
 '
 
+test_expect_success 'git hook list removes skipped hookcmd' '
+	setup_hookcmd &&
+	test_config hookcmd.abc.skip "true" --add &&
+
+	cat >expected <<-EOF &&
+	no commands configured for hook '\''pre-commit'\''
+	EOF
+
+	git hook list pre-commit >actual &&
+	test_cmp expected actual
+'
+
+test_expect_success 'git hook list ignores skip referring to unused hookcmd' '
+	test_config hookcmd.abc.command "/path/abc" --add &&
+	test_config hookcmd.abc.skip "true" --add &&
+
+	cat >expected <<-EOF &&
+	no commands configured for hook '\''pre-commit'\''
+	EOF
+
+	git hook list pre-commit >actual &&
+	test_cmp expected actual
+'
+
+test_expect_success 'git hook list removes skipped inlined hook' '
+	setup_hooks &&
+	test_config hookcmd."$ROOT/path/ghi".skip "true" --add &&
+
+	cat >expected <<-EOF &&
+	$ROOT/path/def
+	EOF
+
+	git hook list pre-commit >actual &&
+	test_cmp expected actual
+'
+
 
 test_expect_success 'inline hook definitions execute oneliners' '
 	test_config hook.pre-commit.command "echo \"Hello World\"" &&
-- 
2.32.0.402.g57bb445576-goog