Historically, hooks are declared by placing an executable into
$GIT_DIR/hooks/$HOOKNAME (or $HOOKDIR/$HOOKNAME). Although hooks taken
from the config are more featureful than hooks placed in the $HOOKDIR,
those hooks should not stop working for users who already have them.
When we add hooks from $HOOKDIR to the list of all hooks to run, to
support paths with spaces in them, quote legacy hook paths.
Signed-off-by: Emily Shaffer <emilyshaffer@xxxxxxxxxx>
---
Notes:
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 | 70 +++++++++++++++++++++++++++++++----
hook.c | 36 ++++++++++++++++++
hook.h | 16 ++++++++
t/t1360-config-based-hooks.sh | 62 +++++++++++++++++++++++++++++++
5 files changed, 182 insertions(+), 7 deletions(-)
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 4d36de52f8..16324d4195 100644
--- a/builtin/hook.c
+++ b/builtin/hook.c
@@ -11,11 +11,14 @@ 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;
struct hook *item;
struct strbuf hookname = STRBUF_INIT;
+ struct strbuf hookdir_annotation = STRBUF_INIT;
struct option list_options[] = {
OPT_END(),
@@ -40,12 +43,39 @@ static int list(int argc, const char **argv, const char *prefix)
return 0;
}
+ switch (should_run_hookdir) {
+ case hookdir_no:
+ strbuf_addstr(&hookdir_annotation, _(" (will not run)"));
+ break;
+ case hookdir_interactive:
+ strbuf_addstr(&hookdir_annotation, _(" (will prompt)"));
+ break;
+ case hookdir_warn:
+ case hookdir_unknown:
+ strbuf_addstr(&hookdir_annotation, _(" (will warn)"));
+ break;
+ case hookdir_yes:
+ /*
+ * The default behavior should agree with
+ * hook.c:configured_hookdir_opt().
+ */
+ default:
+ break;
+ }
+
list_for_each(pos, head) {
item = list_entry(pos, struct hook, list);
- if (item)
- printf("%s: %s\n",
- config_scope_name(item->origin),
- item->command.buf);
+ if (item) {
+ /* Don't translate 'hookdir' - it matches the config */
+ printf("%s: %s%s\n",
+ (item->from_hookdir
+ ? "hookdir"
+ : config_scope_name(item->origin)),
+ item->command.buf,
+ (item->from_hookdir
+ ? hookdir_annotation.buf
+ : ""));
+ }
}
clear_hook_list(head);
@@ -56,14 +86,40 @@ static int list(int argc, const char **argv, const char *prefix)
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);
- 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, "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
+ 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 937dc768c8..340e5a35c8 100644
--- a/hook.c
+++ b/hook.c
@@ -2,6 +2,7 @@
#include "hook.h"
#include "config.h"
+#include "run-command.h"
void free_hook(struct hook *ptr)
{
@@ -34,6 +35,7 @@ static void append_or_move_hook(struct list_head *head, const char *command)
to_add = xmalloc(sizeof(struct hook));
strbuf_init(&to_add->command, 0);
strbuf_addstr(&to_add->command, command);
+ to_add->from_hookdir = 0;
}
/* re-set the scope so we show where an override was specified */
@@ -95,11 +97,33 @@ 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, "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 struct strbuf* hookname)
{
struct strbuf hook_key = STRBUF_INIT;
struct list_head *hook_head = xmalloc(sizeof(struct list_head));
struct hook_config_cb cb_data = { &hook_key, hook_head };
+ const char *legacy_hook_path = NULL;
INIT_LIST_HEAD(hook_head);
@@ -110,6 +134,18 @@ struct list_head* hook_list(const struct strbuf* hookname)
git_config(hook_config_lookup, (void*)&cb_data);
+ if (have_git_dir())
+ legacy_hook_path = find_hook(hookname->buf);
+
+ /* Unconditionally add legacy hook, but annotate it. */
+ if (legacy_hook_path) {
+ struct hook *legacy_hook;
+
+ append_or_move_hook(hook_head, absolute_path(legacy_hook_path));
+ legacy_hook = list_entry(hook_head->prev, struct hook, list);
+ legacy_hook->from_hookdir = 1;
+ }
+
strbuf_release(&hook_key);
return hook_head;
}
diff --git a/hook.h b/hook.h
index 8ffc4f14b6..ca45d388d3 100644
--- a/hook.h
+++ b/hook.h
@@ -12,6 +12,7 @@ struct hook
enum config_scope origin;
/* The literal command to run. */
struct strbuf command;
+ int from_hookdir;
};
/*
@@ -20,6 +21,21 @@ struct hook
*/
struct list_head* hook_list(const struct strbuf *hookname);
+enum hookdir_opt
+{
+ hookdir_no,
+ 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);
+
/* 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 6e4a3e763f..91127a50a4 100755
--- a/t/t1360-config-based-hooks.sh
+++ b/t/t1360-config-based-hooks.sh
@@ -23,6 +23,14 @@ setup_hookcmd () {
test_config_global hookcmd.abc.command "/path/abc" --add
}
+setup_hookdir () {
+ mkdir .git/hooks
+ write_script .git/hooks/pre-commit <<-EOF
+ echo \"Legacy Hook\"
+ EOF
+ test_when_finished rm -rf .git/hooks
+}
+
test_expect_success 'git hook rejects commands without a mode' '
test_must_fail git hook pre-commit
'
@@ -85,4 +93,58 @@ test_expect_success 'git hook list reorders on duplicate commands' '
test_cmp expected actual
'
+test_expect_success 'git hook list shows hooks from the hookdir' '
+ setup_hookdir &&
+
+ cat >expected <<-EOF &&
+ hookdir: $(pwd)/.git/hooks/pre-commit
+ EOF
+
+ git hook list pre-commit >actual &&
+ 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_i18ncmp 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)
+ EOF
+
+ git hook list pre-commit >actual &&
+ # the hookdir annotation is translated
+ test_i18ncmp 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_i18ncmp expected actual
+'
+
test_done
--
2.28.0.rc0.142.g3c755180ce-goog
