{"thread":{"id":"54421","subject":"[PATCH v5 6/8] parse-options: parse into strvec","startedAt":"2020-10-15T01:54:39Z","lastAt":"2020-10-21T23:37:57Z","messageCount":12,"participants":["Emily Shaffer","Ævar Arnfjörð Bjarmason","Junio C Hamano"],"isPatch":true,"patchVersion":5,"patchTotal":8},"messages":[{"id":"407562","messageId":"20201014232447.3050579-4-emilyshaffer@google.com","threadId":"54421","inReplyTo":"20201014232447.3050579-1-emilyshaffer@google.com","subject":"[PATCH v5 3/8] hook: add list command","fromName":"Emily Shaffer","fromEmail":"emilyshaffer@google.com","sentAt":"2020-10-14T23:24:42Z","receivedAt":"2020-10-15T01:54:39Z","isPatch":true,"sender":{"key":"nasamuffin@google.com","avatar":"https://avatars.githubusercontent.com/u/1606826?v=4"},"body":"Teach 'git hook list <hookname>', which checks the known configs in\norder to create an ordered list of hooks to run on a given hook event.\n\nMultiple commands can be specified for a given hook by providing\nmultiple \"hook.<hookname>.command = <path-to-hook>\" lines. Hooks will be\nrun in config order. If more properties need to be set on a given hook\nin the future, commands can also be specified by providing\n\"hook.<hookname>.command = <hookcmd-name>\", as well as a \"[hookcmd\n<hookcmd-name>]\" subsection; at minimum, this subsection must contain a\n\"hookcmd.<hookcmd-name>.command = <path-to-hook>\" line.\n\nFor example:\n\n  $ git config --list | grep ^hook\n  hook.pre-commit.command=baz\n  hook.pre-commit.command=~/bar.sh\n  hookcmd.baz.command=~/baz/from/hookcmd.sh\n\n  $ git hook list pre-commit\n  global: ~/baz/from/hookcmd.sh\n  local: ~/bar.sh\n\nSigned-off-by: Emily Shaffer <emilyshaffer@google.com>\n---\n\nNotes:\n    Since v4, more work on the documentation. Also a slight change to the\n    output format (space instead of tab).\n\n Documentation/config/hook.txt |   9 +++\n Documentation/git-hook.txt    |  59 ++++++++++++++++-\n Makefile                      |   1 +\n builtin/hook.c                |  56 +++++++++++++++--\n hook.c                        | 115 ++++++++++++++++++++++++++++++++++\n hook.h                        |  26 ++++++++\n t/t1360-config-based-hooks.sh |  81 +++++++++++++++++++++++-\n 7 files changed, 338 insertions(+), 9 deletions(-)\n create mode 100644 Documentation/config/hook.txt\n create mode 100644 hook.c\n create mode 100644 hook.h\n\ndiff --git a/Documentation/config/hook.txt b/Documentation/config/hook.txt\nnew file mode 100644\nindex 0000000000..71449ecbc7\n--- /dev/null\n+++ b/Documentation/config/hook.txt\n@@ -0,0 +1,9 @@\n+hook.<command>.command::\n+\tA command to execute during the <command> hook event. This can be an\n+\texecutable on your device, a oneliner for your shell, or the name of a\n+\thookcmd. See linkgit:git-hook[1].\n+\n+hookcmd.<name>.command::\n+\tA command to execute during a hook for which <name> has been specified\n+\tas a command. This can be an executable on your device or a oneliner for\n+\tyour shell. See linkgit:git-hook[1].\ndiff --git a/Documentation/git-hook.txt b/Documentation/git-hook.txt\nindex 9eeab0009d..f19875ed68 100644\n--- a/Documentation/git-hook.txt\n+++ b/Documentation/git-hook.txt\n@@ -8,12 +8,65 @@ git-hook - Manage configured hooks\n SYNOPSIS\n --------\n [verse]\n-'git hook'\n+'git hook' list <hook-name>\n \n DESCRIPTION\n -----------\n-A placeholder command. Later, you will be able to list, add, and modify hooks\n-with this command.\n+You can list configured hooks with this command. Later, you will be able to run,\n+add, and modify hooks with this command.\n+\n+This command parses the default configuration files for sections `hook` and\n+`hookcmd`. `hook` is used to describe the commands which will be run during a\n+particular hook event; commands are run in the order Git encounters them during\n+the configuration parse (see linkgit:git-config[1]). `hookcmd` is used to\n+describe attributes of a specific command. If additional attributes don't need\n+to be specified, a command to run can be specified directly in the `hook`\n+section; if a `hookcmd` by that name isn't found, Git will attempt to run the\n+provided value directly. For example:\n+\n+Global config\n+----\n+  [hook \"post-commit\"]\n+    command = \"linter\"\n+    command = \"~/typocheck.sh\"\n+\n+  [hookcmd \"linter\"]\n+    command = \"/bin/linter --c\"\n+----\n+\n+Local config\n+----\n+  [hook \"prepare-commit-msg\"]\n+    command = \"linter\"\n+  [hook \"post-commit\"]\n+    command = \"python ~/run-test-suite.py\"\n+----\n+\n+With these configs, you'd then see:\n+\n+----\n+$ git hook list \"post-commit\"\n+global: /bin/linter --c\n+global: ~/typocheck.sh\n+local: python ~/run-test-suite.py\n+\n+$ git hook list \"prepare-commit-msg\"\n+local: /bin/linter --c\n+----\n+\n+COMMANDS\n+--------\n+\n+list `<hook-name>`::\n+\n+List the hooks which have been configured for `<hook-name>`. Hooks appear\n+in the order they should be run, and print the config scope where the relevant\n+`hook.<hook-name>.command` was specified, not the `hookcmd` (if applicable).\n+This output is human-readable and the format is subject to change over time.\n+\n+CONFIGURATION\n+-------------\n+include::config/hook.txt[]\n \n GIT\n ---\ndiff --git a/Makefile b/Makefile\nindex 9152f6d7c8..5cd1486e42 100644\n--- a/Makefile\n+++ b/Makefile\n@@ -902,6 +902,7 @@ LIB_OBJS += grep.o\n LIB_OBJS += hashmap.o\n LIB_OBJS += help.o\n LIB_OBJS += hex.o\n+LIB_OBJS += hook.o\n LIB_OBJS += ident.o\n LIB_OBJS += json-writer.o\n LIB_OBJS += kwset.o\ndiff --git a/builtin/hook.c b/builtin/hook.c\nindex b2bbc84d4d..4d36de52f8 100644\n--- a/builtin/hook.c\n+++ b/builtin/hook.c\n@@ -1,21 +1,69 @@\n #include \"cache.h\"\n \n #include \"builtin.h\"\n+#include \"config.h\"\n+#include \"hook.h\"\n #include \"parse-options.h\"\n+#include \"strbuf.h\"\n \n static const char * const builtin_hook_usage[] = {\n-\tN_(\"git hook\"),\n+\tN_(\"git hook list <hookname>\"),\n \tNULL\n };\n \n-int cmd_hook(int argc, const char **argv, const char *prefix)\n+static int list(int argc, const char **argv, const char *prefix)\n {\n-\tstruct option builtin_hook_options[] = {\n+\tstruct list_head *head, *pos;\n+\tstruct hook *item;\n+\tstruct strbuf hookname = STRBUF_INIT;\n+\n+\tstruct option list_options[] = {\n \t\tOPT_END(),\n \t};\n \n-\targc = parse_options(argc, argv, prefix, builtin_hook_options,\n+\targc = parse_options(argc, argv, prefix, list_options,\n \t\t\t     builtin_hook_usage, 0);\n \n+\tif (argc < 1) {\n+\t\tusage_msg_opt(_(\"You must specify a hook event name to list.\"),\n+\t\t\t      builtin_hook_usage, list_options);\n+\t}\n+\n+\tstrbuf_addstr(&hookname, argv[0]);\n+\n+\thead = hook_list(&hookname);\n+\n+\tif (list_empty(head)) {\n+\t\tprintf(_(\"no commands configured for hook '%s'\\n\"),\n+\t\t       hookname.buf);\n+\t\tstrbuf_release(&hookname);\n+\t\treturn 0;\n+\t}\n+\n+\tlist_for_each(pos, head) {\n+\t\titem = list_entry(pos, struct hook, list);\n+\t\tif (item)\n+\t\t\tprintf(\"%s: %s\\n\",\n+\t\t\t       config_scope_name(item->origin),\n+\t\t\t       item->command.buf);\n+\t}\n+\n+\tclear_hook_list(head);\n+\tstrbuf_release(&hookname);\n+\n \treturn 0;\n }\n+\n+int cmd_hook(int argc, const char **argv, const char *prefix)\n+{\n+\tstruct option builtin_hook_options[] = {\n+\t\tOPT_END(),\n+\t};\n+\tif (argc < 2)\n+\t\tusage_with_options(builtin_hook_usage, builtin_hook_options);\n+\n+\tif (!strcmp(argv[1], \"list\"))\n+\t\treturn list(argc - 1, argv + 1, prefix);\n+\n+\tusage_with_options(builtin_hook_usage, builtin_hook_options);\n+}\ndiff --git a/hook.c b/hook.c\nnew file mode 100644\nindex 0000000000..937dc768c8\n--- /dev/null\n+++ b/hook.c\n@@ -0,0 +1,115 @@\n+#include \"cache.h\"\n+\n+#include \"hook.h\"\n+#include \"config.h\"\n+\n+void free_hook(struct hook *ptr)\n+{\n+\tif (ptr) {\n+\t\tstrbuf_release(&ptr->command);\n+\t\tfree(ptr);\n+\t}\n+}\n+\n+static void append_or_move_hook(struct list_head *head, const char *command)\n+{\n+\tstruct list_head *pos = NULL, *tmp = NULL;\n+\tstruct hook *to_add = NULL;\n+\n+\t/*\n+\t * remove the prior entry with this command; we'll replace it at the\n+\t * end.\n+\t */\n+\tlist_for_each_safe(pos, tmp, head) {\n+\t\tstruct hook *it = list_entry(pos, struct hook, list);\n+\t\tif (!strcmp(it->command.buf, command)) {\n+\t\t    list_del(pos);\n+\t\t    /* we'll simply move the hook to the end */\n+\t\t    to_add = it;\n+\t\t}\n+\t}\n+\n+\tif (!to_add) {\n+\t\t/* adding a new hook, not moving an old one */\n+\t\tto_add = xmalloc(sizeof(struct hook));\n+\t\tstrbuf_init(&to_add->command, 0);\n+\t\tstrbuf_addstr(&to_add->command, command);\n+\t}\n+\n+\t/* re-set the scope so we show where an override was specified */\n+\tto_add->origin = current_config_scope();\n+\n+\tlist_add_tail(&to_add->list, pos);\n+}\n+\n+static void remove_hook(struct list_head *to_remove)\n+{\n+\tstruct hook *hook_to_remove = list_entry(to_remove, struct hook, list);\n+\tlist_del(to_remove);\n+\tfree_hook(hook_to_remove);\n+}\n+\n+void clear_hook_list(struct list_head *head)\n+{\n+\tstruct list_head *pos, *tmp;\n+\tlist_for_each_safe(pos, tmp, head)\n+\t\tremove_hook(pos);\n+}\n+\n+struct hook_config_cb\n+{\n+\tstruct strbuf *hookname;\n+\tstruct list_head *list;\n+};\n+\n+static int hook_config_lookup(const char *key, const char *value, void *cb_data)\n+{\n+\tstruct hook_config_cb *data = cb_data;\n+\tconst char *hook_key = data->hookname->buf;\n+\tstruct list_head *head = data->list;\n+\n+\tif (!strcmp(key, hook_key)) {\n+\t\tconst char *command = value;\n+\t\tstruct strbuf hookcmd_name = STRBUF_INIT;\n+\n+\t\t/* Check if a hookcmd with that name exists. */\n+\t\tstrbuf_addf(&hookcmd_name, \"hookcmd.%s.command\", command);\n+\t\tgit_config_get_value(hookcmd_name.buf, &command);\n+\n+\t\tif (!command) {\n+\t\t\tstrbuf_release(&hookcmd_name);\n+\t\t\tBUG(\"git_config_get_value overwrote a string it shouldn't have\");\n+\t\t}\n+\n+\t\t/*\n+\t\t * TODO: implement an option-getting callback, e.g.\n+\t\t *   get configs by pattern hookcmd.$value.*\n+\t\t *   for each key+value, do_callback(key, value, cb_data)\n+\t\t */\n+\n+\t\tappend_or_move_hook(head, command);\n+\n+\t\tstrbuf_release(&hookcmd_name);\n+\t}\n+\n+\treturn 0;\n+}\n+\n+struct list_head* hook_list(const struct strbuf* hookname)\n+{\n+\tstruct strbuf hook_key = STRBUF_INIT;\n+\tstruct list_head *hook_head = xmalloc(sizeof(struct list_head));\n+\tstruct hook_config_cb cb_data = { &hook_key, hook_head };\n+\n+\tINIT_LIST_HEAD(hook_head);\n+\n+\tif (!hookname)\n+\t\treturn NULL;\n+\n+\tstrbuf_addf(&hook_key, \"hook.%s.command\", hookname->buf);\n+\n+\tgit_config(hook_config_lookup, (void*)&cb_data);\n+\n+\tstrbuf_release(&hook_key);\n+\treturn hook_head;\n+}\ndiff --git a/hook.h b/hook.h\nnew file mode 100644\nindex 0000000000..8ffc4f14b6\n--- /dev/null\n+++ b/hook.h\n@@ -0,0 +1,26 @@\n+#include \"config.h\"\n+#include \"list.h\"\n+#include \"strbuf.h\"\n+\n+struct hook\n+{\n+\tstruct list_head list;\n+\t/*\n+\t * Config file which holds the hook.*.command definition.\n+\t * (This has nothing to do with the hookcmd.<name>.* configs.)\n+\t */\n+\tenum config_scope origin;\n+\t/* The literal command to run. */\n+\tstruct strbuf command;\n+};\n+\n+/*\n+ * Provides a linked list of 'struct hook' detailing commands which should run\n+ * in response to the 'hookname' event, in execution order.\n+ */\n+struct list_head* hook_list(const struct strbuf *hookname);\n+\n+/* Free memory associated with a 'struct hook' */\n+void free_hook(struct hook *ptr);\n+/* Empties the list at 'head', calling 'free_hook()' on each entry */\n+void clear_hook_list(struct list_head *head);\ndiff --git a/t/t1360-config-based-hooks.sh b/t/t1360-config-based-hooks.sh\nindex 34b0df5216..6e4a3e763f 100755\n--- a/t/t1360-config-based-hooks.sh\n+++ b/t/t1360-config-based-hooks.sh\n@@ -4,8 +4,85 @@ test_description='config-managed multihooks, including git-hook command'\n \n . ./test-lib.sh\n \n-test_expect_success 'git hook command does not crash' '\n-\tgit hook\n+ROOT=\n+if test_have_prereq MINGW\n+then\n+\t# In Git for Windows, Unix-like paths work only in shell scripts;\n+\t# `git.exe`, however, will prefix them with the pseudo root directory\n+\t# (of the Unix shell). Let's accommodate for that.\n+\tROOT=\"$(cd / && pwd)\"\n+fi\n+\n+setup_hooks () {\n+\ttest_config hook.pre-commit.command \"/path/ghi\" --add\n+\ttest_config_global hook.pre-commit.command \"/path/def\" --add\n+}\n+\n+setup_hookcmd () {\n+\ttest_config hook.pre-commit.command \"abc\" --add\n+\ttest_config_global hookcmd.abc.command \"/path/abc\" --add\n+}\n+\n+test_expect_success 'git hook rejects commands without a mode' '\n+\ttest_must_fail git hook pre-commit\n+'\n+\n+\n+test_expect_success 'git hook rejects commands without a hookname' '\n+\ttest_must_fail git hook list\n+'\n+\n+test_expect_success 'git hook runs outside of a repo' '\n+\tsetup_hooks &&\n+\n+\tcat >expected <<-EOF &&\n+\tglobal: $ROOT/path/def\n+\tEOF\n+\n+\tnongit git config --list --global &&\n+\n+\tnongit git hook list pre-commit >actual &&\n+\ttest_cmp expected actual\n+'\n+\n+test_expect_success 'git hook list orders by config order' '\n+\tsetup_hooks &&\n+\n+\tcat >expected <<-EOF &&\n+\tglobal: $ROOT/path/def\n+\tlocal: $ROOT/path/ghi\n+\tEOF\n+\n+\tgit hook list pre-commit >actual &&\n+\ttest_cmp expected actual\n+'\n+\n+test_expect_success 'git hook list dereferences a hookcmd' '\n+\tsetup_hooks &&\n+\tsetup_hookcmd &&\n+\n+\tcat >expected <<-EOF &&\n+\tglobal: $ROOT/path/def\n+\tlocal: $ROOT/path/ghi\n+\tlocal: $ROOT/path/abc\n+\tEOF\n+\n+\tgit hook list pre-commit >actual &&\n+\ttest_cmp expected actual\n+'\n+\n+test_expect_success 'git hook list reorders on duplicate commands' '\n+\tsetup_hooks &&\n+\n+\ttest_config hook.pre-commit.command \"/path/def\" --add &&\n+\n+\tcat >expected <<-EOF &&\n+\tlocal: $ROOT/path/ghi\n+\tlocal: $ROOT/path/def\n+\tEOF\n+\n+\tgit hook list pre-commit >actual &&\n+\ttest_cmp expected actual\n '\n \n test_done\n-- \n2.28.0.rc0.142.g3c755180ce-goog\n\n"},{"id":"407561","messageId":"20201014232447.3050579-7-emilyshaffer@google.com","threadId":"54421","inReplyTo":"20201014232447.3050579-1-emilyshaffer@google.com","subject":"[PATCH v5 6/8] parse-options: parse into strvec","fromName":"Emily Shaffer","fromEmail":"emilyshaffer@google.com","sentAt":"2020-10-14T23:24:45Z","receivedAt":"2020-10-15T01:54:40Z","isPatch":true,"sender":{"key":"nasamuffin@google.com","avatar":"https://avatars.githubusercontent.com/u/1606826?v=4"},"body":"parse-options already knows how to read into a string_list, and it knows\nhow to read into an strvec as a passthrough (that is, including the\nargument as well as its value). string_list and strvec serve similar\npurposes but are somewhat painful to convert between; so, let's teach\nparse-options to read values of string arguments directly into an\nstrvec without preserving the argument name.\n\nThis is useful if collecting generic arguments to pass through to\nanother command, for example, 'git hook run --arg \"--quiet\" --arg\n\"--format=pretty\" some-hook'. The resulting strvec would contain\n{ \"--quiet\", \"--format=pretty\" }.\n\nThe implementation is based on that of OPT_STRING_LIST.\n\nSigned-off-by: Emily Shaffer <emilyshaffer@google.com>\n---\n\nNotes:\n    Since v4, fixed one or two more places where I missed the argv_array->strvec\n    rename.\n\n Documentation/technical/api-parse-options.txt |  5 +++++\n parse-options-cb.c                            | 16 ++++++++++++++++\n parse-options.h                               |  4 ++++\n 3 files changed, 25 insertions(+)\n\ndiff --git a/Documentation/technical/api-parse-options.txt b/Documentation/technical/api-parse-options.txt\nindex 5a60bbfa7f..679bd98629 100644\n--- a/Documentation/technical/api-parse-options.txt\n+++ b/Documentation/technical/api-parse-options.txt\n@@ -173,6 +173,11 @@ There are some macros to easily define options:\n \tThe string argument is stored as an element in `string_list`.\n \tUse of `--no-option` will clear the list of preceding values.\n \n+`OPT_STRVEC(short, long, &struct strvec, arg_str, description)`::\n+\tIntroduce an option with a string argument.\n+\tThe string argument is stored as an element in `strvec`.\n+\tUse of `--no-option` will clear the list of preceding values.\n+\n `OPT_INTEGER(short, long, &int_var, description)`::\n \tIntroduce an option with integer argument.\n \tThe integer is put into `int_var`.\ndiff --git a/parse-options-cb.c b/parse-options-cb.c\nindex 4542d4d3f9..c2451dfb1b 100644\n--- a/parse-options-cb.c\n+++ b/parse-options-cb.c\n@@ -207,6 +207,22 @@ int parse_opt_string_list(const struct option *opt, const char *arg, int unset)\n \treturn 0;\n }\n \n+int parse_opt_strvec(const struct option *opt, const char *arg, int unset)\n+{\n+\tstruct strvec *v = opt->value;\n+\n+\tif (unset) {\n+\t\tstrvec_clear(v);\n+\t\treturn 0;\n+\t}\n+\n+\tif (!arg)\n+\t\treturn -1;\n+\n+\tstrvec_push(v, arg);\n+\treturn 0;\n+}\n+\n int parse_opt_noop_cb(const struct option *opt, const char *arg, int unset)\n {\n \treturn 0;\ndiff --git a/parse-options.h b/parse-options.h\nindex 7030d8f3da..75cc8c7c96 100644\n--- a/parse-options.h\n+++ b/parse-options.h\n@@ -177,6 +177,9 @@ struct option {\n #define OPT_STRING_LIST(s, l, v, a, h) \\\n \t\t\t\t    { OPTION_CALLBACK, (s), (l), (v), (a), \\\n \t\t\t\t      (h), 0, &parse_opt_string_list }\n+#define OPT_STRVEC(s, l, v, a, h) \\\n+\t\t\t\t    { OPTION_CALLBACK, (s), (l), (v), (a), \\\n+\t\t\t\t      (h), 0, &parse_opt_strvec }\n #define OPT_UYN(s, l, v, h)         { OPTION_CALLBACK, (s), (l), (v), NULL, \\\n \t\t\t\t      (h), PARSE_OPT_NOARG, &parse_opt_tertiary }\n #define OPT_EXPIRY_DATE(s, l, v, h) \\\n@@ -296,6 +299,7 @@ int parse_opt_commits(const struct option *, const char *, int);\n int parse_opt_commit(const struct option *, const char *, int);\n int parse_opt_tertiary(const struct option *, const char *, int);\n int parse_opt_string_list(const struct option *, const char *, int);\n+int parse_opt_strvec(const struct option *, const char *, int);\n int parse_opt_noop_cb(const struct option *, const char *, int);\n enum parse_opt_result parse_opt_unknown_cb(struct parse_opt_ctx_t *ctx,\n \t\t\t\t\t   const struct option *,\n-- \n2.28.0.rc0.142.g3c755180ce-goog\n\n"},{"id":"407563","messageId":"20201014232447.3050579-9-emilyshaffer@google.com","threadId":"54421","inReplyTo":"20201014232447.3050579-1-emilyshaffer@google.com","subject":"[PATCH v5 8/8] hook: replace find_hook() with hook_exists()","fromName":"Emily Shaffer","fromEmail":"emilyshaffer@google.com","sentAt":"2020-10-14T23:24:47Z","receivedAt":"2020-10-15T01:54:40Z","isPatch":true,"sender":{"key":"nasamuffin@google.com","avatar":"https://avatars.githubusercontent.com/u/1606826?v=4"},"body":"Add a helper to easily determine whether any hooks exist for a given\nhook event.\n\nMany callers want to check whether some state could be modified by a\nhook; that check should include the config-based hooks as well. Optimize\nby checking the config directly. Since commands which execute hooks\nmight want to take args to replace 'hook.runHookDir', let\n'hook_exists()' mirror the behavior of 'hook.runHookDir'.\n\nSigned-off-by: Emily Shaffer <emilyshaffer@google.com>\n---\n\nNotes:\n    Since v4, a little more nuance when deciding whether a hookdir hook can happen.\n\n hook.c | 14 ++++++++++++++\n hook.h |  9 +++++++++\n 2 files changed, 23 insertions(+)\n\ndiff --git a/hook.c b/hook.c\nindex 1494a32c1a..e3d289d0e9 100644\n--- a/hook.c\n+++ b/hook.c\n@@ -218,6 +218,20 @@ struct list_head* hook_list(const struct strbuf* hookname)\n \treturn hook_head;\n }\n \n+int hook_exists(const char *hookname, enum hookdir_opt should_run_hookdir)\n+{\n+\tconst char *value = NULL; /* throwaway */\n+\tstruct strbuf hook_key = STRBUF_INIT;\n+\n+\tint could_run_hookdir = (should_run_hookdir == hookdir_interactive ||\n+\t\t\t\tshould_run_hookdir == hookdir_warn ||\n+\t\t\t\tshould_run_hookdir == hookdir_yes)\n+\t\t\t\t&& !!find_hook(hookname);\n+\n+\tstrbuf_addf(&hook_key, \"hook.%s.command\", hookname);\n+\n+\treturn (!git_config_get_value(hook_key.buf, &value)) || could_run_hookdir;\n+}\n \n int run_hooks(const char *const *env, const char *hookname,\n \t      const struct strvec *args, enum hookdir_opt run_hookdir)\ndiff --git a/hook.h b/hook.h\nindex 6eb1dc99c4..bf8ea3ee11 100644\n--- a/hook.h\n+++ b/hook.h\n@@ -36,6 +36,15 @@ enum hookdir_opt\n  * command line arguments.\n  */\n enum hookdir_opt configured_hookdir_opt(void);\n+\n+/*\n+ * Returns 1 if any hooks are specified in the config or if a hook exists in the\n+ * hookdir. Typically, invoke hook_exsts() like:\n+ *   hook_exists(hookname, configured_hookdir_opt());\n+ * Like with run_hooks, if you take a --run-hookdir flag, reflect that\n+ * user-specified behavior here instead.\n+ */\n+int hook_exists(const char *hookname, enum hookdir_opt should_run_hookdir);\n /*\n  * Runs all hooks associated to the 'hookname' event in order. Each hook will be\n  * passed 'env' and 'args'.\n-- \n2.28.0.rc0.142.g3c755180ce-goog\n\n"},{"id":"407564","messageId":"20201014232447.3050579-2-emilyshaffer@google.com","threadId":"54421","inReplyTo":"20201014232447.3050579-1-emilyshaffer@google.com","subject":"[PATCH v5 1/8] doc: propose hooks managed by the config","fromName":"Emily Shaffer","fromEmail":"emilyshaffer@google.com","sentAt":"2020-10-14T23:24:40Z","receivedAt":"2020-10-15T01:54:45Z","isPatch":true,"sender":{"key":"nasamuffin@google.com","avatar":"https://avatars.githubusercontent.com/u/1606826?v=4"},"body":"Begin a design document for config-based hooks, managed via git-hook.\nFocus on an overview of the implementation and motivation for design\ndecisions. Briefly discuss the alternatives considered before this\npoint. Also, attempt to redefine terms to fit into a multihook world.\n\nSigned-off-by: Emily Shaffer <emilyshaffer@google.com>\n---\n\nNotes:\n    Since v4, addressed comments from Jonathan Tan about wording.\n\n Documentation/Makefile                        |   1 +\n .../technical/config-based-hooks.txt          | 367 ++++++++++++++++++\n 2 files changed, 368 insertions(+)\n create mode 100644 Documentation/technical/config-based-hooks.txt\n\ndiff --git a/Documentation/Makefile b/Documentation/Makefile\nindex 80d1908a44..58d6b3acbe 100644\n--- a/Documentation/Makefile\n+++ b/Documentation/Makefile\n@@ -81,6 +81,7 @@ SP_ARTICLES += $(API_DOCS)\n TECH_DOCS += MyFirstContribution\n TECH_DOCS += MyFirstObjectWalk\n TECH_DOCS += SubmittingPatches\n+TECH_DOCS += technical/config-based-hooks\n TECH_DOCS += technical/hash-function-transition\n TECH_DOCS += technical/http-protocol\n TECH_DOCS += technical/index-format\ndiff --git a/Documentation/technical/config-based-hooks.txt b/Documentation/technical/config-based-hooks.txt\nnew file mode 100644\nindex 0000000000..dac391f505\n--- /dev/null\n+++ b/Documentation/technical/config-based-hooks.txt\n@@ -0,0 +1,367 @@\n+Configuration-based hook management\n+===================================\n+:sectanchors:\n+\n+[[motivation]]\n+== Motivation\n+\n+Replace the .git/hook/hookname path as the only source of hooks to execute;\n+allow users to define hooks using config files, in a way which is friendly to\n+users with multiple repos which have similar needs.\n+\n+Redefine \"hook\" as an event rather than a single script, allowing users to\n+perform unrelated actions on a single event.\n+\n+Take a step closer to safety when copying zipped Git repositories from untrusted\n+users by making it more apparent to users which scripts will be run during\n+normal Git operations.\n+\n+Make it easier for users to discover Git's hook feature and automate their\n+workflows.\n+\n+[[user-interfaces]]\n+== User interfaces\n+\n+[[config-schema]]\n+=== Config schema\n+\n+Hooks can be introduced by editing the configuration manually. There are two new\n+sections added, `hook` and `hookcmd`.\n+\n+[[config-schema-hook]]\n+==== `hook`\n+\n+Primarily contains subsections for each hook event. The order of variables in\n+these subsections defines the hook command execution order; hook commands can be\n+specified by setting the value directly to the command if no additional\n+configuration is needed, or by setting the value as the name of a `hookcmd`. If\n+Git does not find a `hookcmd` whose subsection matches the value of the given\n+command string, Git will try to execute the string directly. Hooks are executed\n+by passing the resolved command string to the shell. In the future, hook event\n+subsections could also contain per-hook-event settings; see\n+<<per-hook-event-settings,the section in Future Work>> for more details.\n+\n+Also contains top-level hook execution settings, for example, `hook.runHookDir`\n+or `hook.disableAll`. (These settings are described more in\n+<<library,Library>>.)\n+\n+----\n+[hook \"pre-commit\"]\n+  command = perl-linter\n+  command = /usr/bin/git-secrets --pre-commit\n+\n+[hook \"pre-applypatch\"]\n+  command = perl-linter\n+  # for illustration purposes; error behavior isn't planned yet\n+  error = ignore\n+\n+[hook]\n+  runHookDir = interactive\n+----\n+\n+[[config-schema-hookcmd]]\n+==== `hookcmd`\n+\n+Defines a hook command and its attributes, which will be used when a hook event\n+occurs. Unqualified attributes are assumed to apply to this hook during all hook\n+events, but event-specific attributes can also be supplied. The example runs\n+`/usr/bin/lint-it --language=perl <args passed by Git>`, but for repos which\n+include this config, the hook command will be skipped for all events to which\n+it's normally subscribed _except_ `pre-commit`.\n+\n+----\n+[hookcmd \"perl-linter\"]\n+  command = /usr/bin/lint-it --language=perl\n+  skip = true\n+  # for illustration purposes; below hasn't been defined yet\n+  pre-commit-skip = false\n+----\n+\n+[[command-line-api]]\n+=== Command-line API\n+\n+Users should be able to view, reorder, and create hook commands via the command\n+line. External tools should be able to view a list of hooks in the correct order\n+to run.\n+\n+*`git hook list <hook-event>`*\n+\n+*`git hook list (--system|--global|--local|--worktree)`*\n+\n+*`git hook edit <hook-event>`*\n+\n+*`git hook add <hook-command> <hook-event> <options...>`*\n+\n+[[hook-editor]]\n+=== Hook editor\n+\n+The tool which is presented by `git hook edit <hook-command>`. Ideally, this\n+tool should be easier to use than manually editing the config, and then produce\n+a concise config afterwards. It may take a form similar to `git rebase\n+--interactive`.\n+\n+[[implementation]]\n+== Implementation\n+\n+[[library]]\n+=== Library\n+\n+`hook.c` and `hook.h` are responsible for interacting with the config files. In\n+the case when the code generating a hook event doesn't have special concerns\n+about how to run the hooks, the hook library will provide a basic API to call\n+all hooks in config order with an `strvec` provided by the code which\n+generates the hook event:\n+\n+*`int run_hooks(const char *hookname, struct strvec *args)`*\n+\n+This call includes the hook command provided by `run-command.h:find_hook()`;\n+eventually, this legacy hook will be gated by a config `hook.runHookDir`. The\n+config is checked against a number of cases:\n+\n+- \"no\": the legacy hook will not be run\n+- \"interactive\": Git will prompt the user before running the legacy hook\n+- \"warn\": Git will print a warning to stderr before running the legacy hook\n+- \"yes\" (default): Git will silently run the legacy hook\n+\n+In case this list is expanded in the future, if a value for `hook.runHookDir` is\n+given which Git does not recognize, Git should discard that config entry. For\n+example, if \"warn\" was specified at system level and \"junk\" was specified at\n+global level, Git would resolve the value to \"warn\"; if the only time the config\n+was set was to \"junk\", Git would use the default value of \"yes\".\n+\n+If the caller wants to do something more complicated, the hook library can also\n+provide a callback API:\n+\n+*`int for_each_hookcmd(const char *hookname, hookcmd_function *cb)`*\n+\n+Finally, to facilitate the builtin, the library will also provide the following\n+APIs to interact with the config:\n+\n+----\n+int set_hook_commands(const char *hookname, struct string_list *commands,\n+\tenum config_scope scope);\n+int set_hookcmd(const char *hookcmd, struct hookcmd options);\n+\n+int list_hook_commands(const char *hookname, struct string_list *commands);\n+int list_hooks_in_scope(enum config_scope scope, struct string_list *commands);\n+----\n+\n+`struct hookcmd` is expected to grow in size over time as more functionality is\n+added to hooks; so that other parts of the code don't need to understand the\n+config schema, `struct hookcmd` should contain logical values instead of string\n+pairs.\n+\n+----\n+struct hookcmd {\n+  const char *name;\n+  const char *command;\n+\n+  /* for illustration only; not planned at present */\n+  int parallelizable;\n+  const char *hookcmd_before;\n+  const char *hookcmd_after;\n+  enum recovery_action on_fail;\n+}\n+----\n+\n+[[builtin]]\n+=== Builtin\n+\n+`builtin/hook.c` is responsible for providing the frontend. It's responsible for\n+formatting user-provided data and then calling the library API to set the\n+configs as appropriate. The builtin frontend is not responsible for calling the\n+config directly, so that other areas of Git can rely on the hook library to\n+understand the most recent config schema for hooks.\n+\n+[[migration]]\n+=== Migration path\n+\n+[[stage-0]]\n+==== Stage 0\n+\n+Hooks are called by running `run-command.h:find_hook()` with the hookname and\n+executing the result. The hook library and builtin do not exist. Hooks only\n+exist as specially named scripts within `.git/hooks/`.\n+\n+[[stage-1]]\n+==== Stage 1\n+\n+`git hook list --porcelain <hook-event>` is implemented. Users can replace their\n+`.git/hooks/<hook-event>` scripts with a trampoline based on `git hook list`'s\n+output. Modifier commands like `git hook add` and `git hook edit` can be\n+implemented around this time as well.\n+\n+[[stage-2]]\n+==== Stage 2\n+\n+`hook.h:run_hooks()` is taught to include `run-command.h:find_hook()` at the\n+end; calls to `find_hook()` are replaced with calls to `run_hooks()`. Users can\n+opt-in to config-based hooks simply by creating some in their config; otherwise\n+users should remain unaffected by the change.\n+\n+[[stage-3]]\n+==== Stage 3\n+\n+The call to `find_hook()` inside of `run_hooks()` learns to check for a config,\n+`hook.runHookDir`. Users can opt into managing their hooks completely via the\n+config this way.\n+\n+[[stage-4]]\n+==== Stage 4\n+\n+`.git/hooks` is removed from the template and the hook directory is considered\n+deprecated. To avoid breaking older repos, the default of `hook.runHookDir` is\n+not changed, and `find_hook()` is not removed.\n+\n+[[caveats]]\n+== Caveats\n+\n+[[security]]\n+=== Security and repo config\n+\n+Part of the motivation behind this refactor is to mitigate hooks as an attack\n+vector;footnote:[https://lore.kernel.org/git/20171002234517.GV19555@aiede.mtv.corp.google.com/]\n+however, as the design stands, users can still provide hooks in the repo-level\n+config, which is included when a repo is zipped and sent elsewhere.  The\n+security of the repo-level config is still under discussion; this design\n+generally assumes the repo-level config is secure, which is not true yet. The\n+goal is to avoid an overcomplicated design to work around a problem which has\n+ceased to exist.\n+\n+[[ease-of-use]]\n+=== Ease of use\n+\n+The config schema is nontrivial; that's why it's important for the `git hook`\n+modifier commands to be usable. Contributors with UX expertise are encouraged to\n+share their suggestions.\n+\n+[[alternatives]]\n+== Alternative approaches\n+\n+A previous summary of alternatives exists in the\n+archives.footnote:[https://lore.kernel.org/git/20191116011125.GG22855@google.com]\n+\n+[[status-quo]]\n+=== Status quo\n+\n+Today users can implement multihooks themselves by using a \"trampoline script\"\n+as their hook, and pointing that script to a directory or list of other scripts\n+they wish to run.\n+\n+[[hook-directories]]\n+=== Hook directories\n+\n+Other contributors have suggested Git learn about the existence of a directory\n+such as `.git/hooks/<hookname>.d` and execute those hooks in alphabetical order.\n+\n+[[comparison]]\n+=== Comparison table\n+\n+.Comparison of alternatives\n+|===\n+|Feature |Config-based hooks |Hook directories |Status quo\n+\n+|Supports multiple hooks\n+|Natively\n+|Natively\n+|With user effort\n+\n+|Safer for zipped repos\n+|A little\n+|No\n+|No\n+\n+|Previous hooks just work\n+|If configured\n+|Yes\n+|Yes\n+\n+|Can install one hook to many repos\n+|Yes\n+|No\n+|No\n+\n+|Discoverability\n+|Better (in `git help git`)\n+|Same as before\n+|Same as before\n+\n+|Hard to run unexpected hook\n+|If configured\n+|No\n+|No\n+|===\n+\n+[[future-work]]\n+== Future work\n+\n+[[execution-ordering]]\n+=== Execution ordering\n+\n+We may find that config order is insufficient for some users; for example,\n+config order makes it difficult to add a new hook to the system or global config\n+which runs at the end of the hook list. A new ordering schema should be:\n+\n+1) Specified by a `hook.order` config, so that users will not unexpectedly see\n+their order change;\n+\n+2) Either dependency or numerically based.\n+\n+Dependency-based ordering is prone to classic linked-list problems, like a\n+cycles and handling of missing dependencies. But, it paves the way for enabling\n+parallelization if some tasks truly depend on others.\n+\n+Numerical ordering makes it tricky for Git to generate suggested ordering\n+numbers for each command, but is easy to determine a definitive order.\n+\n+[[parallelization]]\n+=== Parallelization\n+\n+Users with many hooks might want to run them simultaneously, if the hooks don't\n+modify state; if one hook depends on another's output, then users will want to\n+specify those dependencies. If we decide to solve this problem, we may want to\n+look to modern build systems for inspiration on how to manage dependencies and\n+parallel tasks.\n+\n+[[securing-hookdir-hooks]]\n+=== Securing hookdir hooks\n+\n+With the design as written in this doc, it's still possible for a malicious user\n+to modify `.git/config` to include `hook.pre-receive.command = rm -rf /`, then\n+zip their repo and send it to another user. It may be necessary to teach Git to\n+only allow inlined hooks like this if they were configured outside of the local\n+scope (in other words, only run hookcmds, and only allow hookcmds to be\n+configured in global or system scope); or another approach, like a list of safe\n+projects, might be useful. It may also be sufficient (or at least useful) to\n+teach a `hook.disableAll` config or similar flag to the Git executable.\n+\n+[[submodule-inheritance]]\n+=== Submodule inheritance\n+\n+It's possible some submodules may want to run the identical set of hooks that\n+their superrepo runs. While a globally-configured hook set is helpful, it's not\n+a great solution for users who have multiple repos-with-submodules under the\n+same user. It would be useful for submodules to learn how to run hooks from\n+their superrepo's config, or inherit that hook setting.\n+\n+[[per-hook-event-settings]]\n+=== Per-hook-event settings\n+\n+It might be desirable to keep settings specifically for some hook events, but\n+not for others - for example, a user may wish to disable hookdir hooks for all\n+events but pre-commit, which they haven't had time to convert yet; or, a user\n+may wish for execution order settings to differ based on hook event. In that\n+case, it would be useful to set something like `hook.pre-commit.executionOrder`\n+which would not apply to the 'prepare-commit-msg' hook, for example.\n+\n+[[glossary]]\n+== Glossary\n+\n+*hook event*\n+\n+A point during Git's execution where user scripts may be run, for example,\n+_prepare-commit-msg_ or _pre-push_.\n+\n+*hook command*\n+\n+A user script or executable which will be run on one or more hook events.\n-- \n2.28.0.rc0.142.g3c755180ce-goog\n\n"},{"id":"407565","messageId":"20201014232447.3050579-6-emilyshaffer@google.com","threadId":"54421","inReplyTo":"20201014232447.3050579-1-emilyshaffer@google.com","subject":"[PATCH v5 5/8] hook: implement hookcmd.<name>.skip","fromName":"Emily Shaffer","fromEmail":"emilyshaffer@google.com","sentAt":"2020-10-14T23:24:44Z","receivedAt":"2020-10-15T01:54:46Z","isPatch":true,"sender":{"key":"nasamuffin@google.com","avatar":"https://avatars.githubusercontent.com/u/1606826?v=4"},"body":"If a user wants a specific repo to skip execution of a hook which is set\nat a global or system level, they can now do so by specifying 'skip' in\ntheir repo config:\n\n~/.gitconfig\n  [hook.pre-commit]\n    command = skippable-oneliner\n    command = skippable-hookcmd\n\n  [hookcmd.skippable-hookcmd]\n    command = foo.sh\n\n$GIT_DIR/.git/config\n  [hookcmd.skippable-oneliner]\n    skip = true\n  [hookcmd.skippable-hookcmd]\n    skip = true\n\nLater it may make sense to add an option like\n\"hookcmd.<name>.<hook-event>-skip\" - but for simplicity, let's start\nwith a universal skip setting like this.\n\nSigned-off-by: Emily Shaffer <emilyshaffer@google.com>\n---\n\nNotes:\n    New since v4.\n    \n    During the Google team's review club I was reminded about this whole\n    'skip' option I never implemented. It's true that it's impossible to\n    exclude a given hook without this; however, I think I have some more\n    work to do on it, so consider it RFC for now and tell me what you think\n    :)\n     - Emily\n\n hook.c                        | 37 +++++++++++++++++++++++++----------\n t/t1360-config-based-hooks.sh | 23 ++++++++++++++++++++++\n 2 files changed, 50 insertions(+), 10 deletions(-)\n\ndiff --git a/hook.c b/hook.c\nindex 340e5a35c8..f4084e33c8 100644\n--- a/hook.c\n+++ b/hook.c\n@@ -12,23 +12,24 @@ void free_hook(struct hook *ptr)\n \t}\n }\n \n-static void append_or_move_hook(struct list_head *head, const char *command)\n+static struct hook* find_hook_by_command(struct list_head *head, const char *command)\n {\n \tstruct list_head *pos = NULL, *tmp = NULL;\n-\tstruct hook *to_add = NULL;\n+\tstruct hook *found = NULL;\n \n-\t/*\n-\t * remove the prior entry with this command; we'll replace it at the\n-\t * end.\n-\t */\n \tlist_for_each_safe(pos, tmp, head) {\n \t\tstruct hook *it = list_entry(pos, struct hook, list);\n \t\tif (!strcmp(it->command.buf, command)) {\n \t\t    list_del(pos);\n-\t\t    /* we'll simply move the hook to the end */\n-\t\t    to_add = it;\n+\t\t    found = it;\n \t\t}\n \t}\n+\treturn found;\n+}\n+\n+static void append_or_move_hook(struct list_head *head, const char *command)\n+{\n+\tstruct hook *to_add = find_hook_by_command(head, command);\n \n \tif (!to_add) {\n \t\t/* adding a new hook, not moving an old one */\n@@ -41,7 +42,7 @@ static void append_or_move_hook(struct list_head *head, const char *command)\n \t/* re-set the scope so we show where an override was specified */\n \tto_add->origin = current_config_scope();\n \n-\tlist_add_tail(&to_add->list, pos);\n+\tlist_add_tail(&to_add->list, head);\n }\n \n static void remove_hook(struct list_head *to_remove)\n@@ -73,8 +74,18 @@ static int hook_config_lookup(const char *key, const char *value, void *cb_data)\n \tif (!strcmp(key, hook_key)) {\n \t\tconst char *command = value;\n \t\tstruct strbuf hookcmd_name = STRBUF_INIT;\n+\t\tint skip = 0;\n+\n+\t\t/*\n+\t\t * Check if we're removing that hook instead. Hookcmds are\n+\t\t * removed by name, and inlined hooks are removed by command\n+\t\t * content.\n+\t\t */\n+\t\tstrbuf_addf(&hookcmd_name, \"hookcmd.%s.skip\", command);\n+\t\tgit_config_get_bool(hookcmd_name.buf, &skip);\n \n \t\t/* Check if a hookcmd with that name exists. */\n+\t\tstrbuf_reset(&hookcmd_name);\n \t\tstrbuf_addf(&hookcmd_name, \"hookcmd.%s.command\", command);\n \t\tgit_config_get_value(hookcmd_name.buf, &command);\n \n@@ -89,7 +100,13 @@ static int hook_config_lookup(const char *key, const char *value, void *cb_data)\n \t\t *   for each key+value, do_callback(key, value, cb_data)\n \t\t */\n \n-\t\tappend_or_move_hook(head, command);\n+\t\tif (skip) {\n+\t\t\tstruct hook *to_remove = find_hook_by_command(head, command);\n+\t\t\tif (to_remove)\n+\t\t\t\tremove_hook(&(to_remove->list));\n+\t\t} else {\n+\t\t\tappend_or_move_hook(head, command);\n+\t\t}\n \n \t\tstrbuf_release(&hookcmd_name);\n \t}\ndiff --git a/t/t1360-config-based-hooks.sh b/t/t1360-config-based-hooks.sh\nindex 91127a50a4..ebd3bc623f 100755\n--- a/t/t1360-config-based-hooks.sh\n+++ b/t/t1360-config-based-hooks.sh\n@@ -132,6 +132,29 @@ test_expect_success 'hook.runHookDir = warn is respected by list' '\n \ttest_i18ncmp expected actual\n '\n \n+test_expect_success 'git hook list removes skipped hookcmd' '\n+\tsetup_hookcmd &&\n+\ttest_config hookcmd.abc.skip \"true\" --add &&\n+\n+\tcat >expected <<-EOF &&\n+\tno commands configured for hook '\\''pre-commit'\\''\n+\tEOF\n+\n+\tgit hook list pre-commit >actual &&\n+\ttest_i18ncmp expected actual\n+'\n+\n+test_expect_success 'git hook list removes skipped inlined hook' '\n+\tsetup_hooks &&\n+\ttest_config hookcmd.\"$ROOT/path/ghi\".skip \"true\" --add &&\n+\n+\tcat >expected <<-EOF &&\n+\tglobal: $ROOT/path/def\n+\tEOF\n+\n+\tgit hook list pre-commit >actual &&\n+\ttest_cmp expected actual\n+'\n \n test_expect_success 'hook.runHookDir = interactive is respected by list' '\n \tsetup_hookdir &&\n-- \n2.28.0.rc0.142.g3c755180ce-goog\n\n"},{"id":"407567","messageId":"20201014232447.3050579-5-emilyshaffer@google.com","threadId":"54421","inReplyTo":"20201014232447.3050579-1-emilyshaffer@google.com","subject":"[PATCH v5 4/8] hook: include hookdir hook in list","fromName":"Emily Shaffer","fromEmail":"emilyshaffer@google.com","sentAt":"2020-10-14T23:24:43Z","receivedAt":"2020-10-15T01:54:56Z","isPatch":true,"sender":{"key":"nasamuffin@google.com","avatar":"https://avatars.githubusercontent.com/u/1606826?v=4"},"body":"Historically, hooks are declared by placing an executable into\n$GIT_DIR/hooks/$HOOKNAME (or $HOOKDIR/$HOOKNAME). Although hooks taken\nfrom the config are more featureful than hooks placed in the $HOOKDIR,\nthose hooks should not stop working for users who already have them.\n\nWhen we add hooks from $HOOKDIR to the list of all hooks to run, to\nsupport paths with spaces in them, quote legacy hook paths.\n\nSigned-off-by: Emily Shaffer <emilyshaffer@google.com>\n---\n\nNotes:\n    Newly split into its own commit since v4, and taking place much sooner.\n    \n    An unfortunate side effect of adding this support *before* the\n    hook.runHookDir support is that the labels on the list are not clear -\n    because we aren't yet flagging which hooks are from the hookdir versus\n    the config. I suppose we could move the addition of that field to the\n    struct hook up to this patch, but it didn't make a lot of sense to me to\n    do it just for cosmetic purposes.\n\n Documentation/config/hook.txt |  5 +++\n builtin/hook.c                | 70 +++++++++++++++++++++++++++++++----\n hook.c                        | 36 ++++++++++++++++++\n hook.h                        | 16 ++++++++\n t/t1360-config-based-hooks.sh | 62 +++++++++++++++++++++++++++++++\n 5 files changed, 182 insertions(+), 7 deletions(-)\n\ndiff --git a/Documentation/config/hook.txt b/Documentation/config/hook.txt\nindex 71449ecbc7..75312754ae 100644\n--- a/Documentation/config/hook.txt\n+++ b/Documentation/config/hook.txt\n@@ -7,3 +7,8 @@ hookcmd.<name>.command::\n \tA command to execute during a hook for which <name> has been specified\n \tas a command. This can be an executable on your device or a oneliner for\n \tyour shell. See linkgit:git-hook[1].\n+\n+hook.runHookDir::\n+\tControls how hooks contained in your hookdir are executed. Can be any of\n+\t\"yes\", \"warn\", \"interactive\", or \"no\". Defaults to \"yes\". See\n+\tlinkgit:git-hook[1] and linkgit:git-config[1] \"core.hooksPath\").\ndiff --git a/builtin/hook.c b/builtin/hook.c\nindex 4d36de52f8..16324d4195 100644\n--- a/builtin/hook.c\n+++ b/builtin/hook.c\n@@ -11,11 +11,14 @@ static const char * const builtin_hook_usage[] = {\n \tNULL\n };\n \n+static enum hookdir_opt should_run_hookdir;\n+\n static int list(int argc, const char **argv, const char *prefix)\n {\n \tstruct list_head *head, *pos;\n \tstruct hook *item;\n \tstruct strbuf hookname = STRBUF_INIT;\n+\tstruct strbuf hookdir_annotation = STRBUF_INIT;\n \n \tstruct option list_options[] = {\n \t\tOPT_END(),\n@@ -40,12 +43,39 @@ static int list(int argc, const char **argv, const char *prefix)\n \t\treturn 0;\n \t}\n \n+\tswitch (should_run_hookdir) {\n+\t\tcase hookdir_no:\n+\t\t\tstrbuf_addstr(&hookdir_annotation, _(\" (will not run)\"));\n+\t\t\tbreak;\n+\t\tcase hookdir_interactive:\n+\t\t\tstrbuf_addstr(&hookdir_annotation, _(\" (will prompt)\"));\n+\t\t\tbreak;\n+\t\tcase hookdir_warn:\n+\t\tcase hookdir_unknown:\n+\t\t\tstrbuf_addstr(&hookdir_annotation, _(\" (will warn)\"));\n+\t\t\tbreak;\n+\t\tcase hookdir_yes:\n+\t\t/*\n+\t\t * The default behavior should agree with\n+\t\t * hook.c:configured_hookdir_opt().\n+\t\t */\n+\t\tdefault:\n+\t\t\tbreak;\n+\t}\n+\n \tlist_for_each(pos, head) {\n \t\titem = list_entry(pos, struct hook, list);\n-\t\tif (item)\n-\t\t\tprintf(\"%s: %s\\n\",\n-\t\t\t       config_scope_name(item->origin),\n-\t\t\t       item->command.buf);\n+\t\tif (item) {\n+\t\t\t/* Don't translate 'hookdir' - it matches the config */\n+\t\t\tprintf(\"%s: %s%s\\n\",\n+\t\t\t       (item->from_hookdir\n+\t\t\t\t? \"hookdir\"\n+\t\t\t\t: config_scope_name(item->origin)),\n+\t\t\t       item->command.buf,\n+\t\t\t       (item->from_hookdir\n+\t\t\t\t? hookdir_annotation.buf\n+\t\t\t\t: \"\"));\n+\t\t}\n \t}\n \n \tclear_hook_list(head);\n@@ -56,14 +86,40 @@ static int list(int argc, const char **argv, const char *prefix)\n \n int cmd_hook(int argc, const char **argv, const char *prefix)\n {\n+\tconst char *run_hookdir = NULL;\n+\n \tstruct option builtin_hook_options[] = {\n+\t\tOPT_STRING(0, \"run-hookdir\", &run_hookdir, N_(\"option\"),\n+\t\t\t   N_(\"what to do with hooks found in the hookdir\")),\n \t\tOPT_END(),\n \t};\n-\tif (argc < 2)\n+\n+\targc = parse_options(argc, argv, prefix, builtin_hook_options,\n+\t\t\t     builtin_hook_usage, 0);\n+\n+\t/* after the parse, we should have \"<command> <hookname> <args...>\" */\n+\tif (argc < 1)\n \t\tusage_with_options(builtin_hook_usage, builtin_hook_options);\n \n-\tif (!strcmp(argv[1], \"list\"))\n-\t\treturn list(argc - 1, argv + 1, prefix);\n+\n+\t/* argument > config */\n+\tif (run_hookdir)\n+\t\tif (!strcmp(run_hookdir, \"no\"))\n+\t\t\tshould_run_hookdir = hookdir_no;\n+\t\telse if (!strcmp(run_hookdir, \"yes\"))\n+\t\t\tshould_run_hookdir = hookdir_yes;\n+\t\telse if (!strcmp(run_hookdir, \"warn\"))\n+\t\t\tshould_run_hookdir = hookdir_warn;\n+\t\telse if (!strcmp(run_hookdir, \"interactive\"))\n+\t\t\tshould_run_hookdir = hookdir_interactive;\n+\t\telse\n+\t\t\tdie(_(\"'%s' is not a valid option for --run-hookdir \"\n+\t\t\t      \"(yes, warn, interactive, no)\"), run_hookdir);\n+\telse\n+\t\tshould_run_hookdir = configured_hookdir_opt();\n+\n+\tif (!strcmp(argv[0], \"list\"))\n+\t\treturn list(argc, argv, prefix);\n \n \tusage_with_options(builtin_hook_usage, builtin_hook_options);\n }\ndiff --git a/hook.c b/hook.c\nindex 937dc768c8..340e5a35c8 100644\n--- a/hook.c\n+++ b/hook.c\n@@ -2,6 +2,7 @@\n \n #include \"hook.h\"\n #include \"config.h\"\n+#include \"run-command.h\"\n \n void free_hook(struct hook *ptr)\n {\n@@ -34,6 +35,7 @@ static void append_or_move_hook(struct list_head *head, const char *command)\n \t\tto_add = xmalloc(sizeof(struct hook));\n \t\tstrbuf_init(&to_add->command, 0);\n \t\tstrbuf_addstr(&to_add->command, command);\n+\t\tto_add->from_hookdir = 0;\n \t}\n \n \t/* re-set the scope so we show where an override was specified */\n@@ -95,11 +97,33 @@ static int hook_config_lookup(const char *key, const char *value, void *cb_data)\n \treturn 0;\n }\n \n+enum hookdir_opt configured_hookdir_opt(void)\n+{\n+\tconst char *key;\n+\tif (git_config_get_value(\"hook.runhookdir\", &key))\n+\t\treturn hookdir_yes; /* by default, just run it. */\n+\n+\tif (!strcmp(key, \"no\"))\n+\t\treturn hookdir_no;\n+\n+\tif (!strcmp(key, \"yes\"))\n+\t\treturn hookdir_yes;\n+\n+\tif (!strcmp(key, \"warn\"))\n+\t\treturn hookdir_warn;\n+\n+\tif (!strcmp(key, \"interactive\"))\n+\t\treturn hookdir_interactive;\n+\n+\treturn hookdir_unknown;\n+}\n+\n struct list_head* hook_list(const struct strbuf* hookname)\n {\n \tstruct strbuf hook_key = STRBUF_INIT;\n \tstruct list_head *hook_head = xmalloc(sizeof(struct list_head));\n \tstruct hook_config_cb cb_data = { &hook_key, hook_head };\n+\tconst char *legacy_hook_path = NULL;\n \n \tINIT_LIST_HEAD(hook_head);\n \n@@ -110,6 +134,18 @@ struct list_head* hook_list(const struct strbuf* hookname)\n \n \tgit_config(hook_config_lookup, (void*)&cb_data);\n \n+\tif (have_git_dir())\n+\t\tlegacy_hook_path = find_hook(hookname->buf);\n+\n+\t/* Unconditionally add legacy hook, but annotate it. */\n+\tif (legacy_hook_path) {\n+\t\tstruct hook *legacy_hook;\n+\n+\t\tappend_or_move_hook(hook_head, absolute_path(legacy_hook_path));\n+\t\tlegacy_hook = list_entry(hook_head->prev, struct hook, list);\n+\t\tlegacy_hook->from_hookdir = 1;\n+\t}\n+\n \tstrbuf_release(&hook_key);\n \treturn hook_head;\n }\ndiff --git a/hook.h b/hook.h\nindex 8ffc4f14b6..ca45d388d3 100644\n--- a/hook.h\n+++ b/hook.h\n@@ -12,6 +12,7 @@ struct hook\n \tenum config_scope origin;\n \t/* The literal command to run. */\n \tstruct strbuf command;\n+\tint from_hookdir;\n };\n \n /*\n@@ -20,6 +21,21 @@ struct hook\n  */\n struct list_head* hook_list(const struct strbuf *hookname);\n \n+enum hookdir_opt\n+{\n+\thookdir_no,\n+\thookdir_warn,\n+\thookdir_interactive,\n+\thookdir_yes,\n+\thookdir_unknown,\n+};\n+\n+/*\n+ * Provides the hookdir_opt specified in the config without consulting any\n+ * command line arguments.\n+ */\n+enum hookdir_opt configured_hookdir_opt(void);\n+\n /* Free memory associated with a 'struct hook' */\n void free_hook(struct hook *ptr);\n /* Empties the list at 'head', calling 'free_hook()' on each entry */\ndiff --git a/t/t1360-config-based-hooks.sh b/t/t1360-config-based-hooks.sh\nindex 6e4a3e763f..91127a50a4 100755\n--- a/t/t1360-config-based-hooks.sh\n+++ b/t/t1360-config-based-hooks.sh\n@@ -23,6 +23,14 @@ setup_hookcmd () {\n \ttest_config_global hookcmd.abc.command \"/path/abc\" --add\n }\n \n+setup_hookdir () {\n+\tmkdir .git/hooks\n+\twrite_script .git/hooks/pre-commit <<-EOF\n+\techo \\\"Legacy Hook\\\"\n+\tEOF\n+\ttest_when_finished rm -rf .git/hooks\n+}\n+\n test_expect_success 'git hook rejects commands without a mode' '\n \ttest_must_fail git hook pre-commit\n '\n@@ -85,4 +93,58 @@ test_expect_success 'git hook list reorders on duplicate commands' '\n \ttest_cmp expected actual\n '\n \n+test_expect_success 'git hook list shows hooks from the hookdir' '\n+\tsetup_hookdir &&\n+\n+\tcat >expected <<-EOF &&\n+\thookdir: $(pwd)/.git/hooks/pre-commit\n+\tEOF\n+\n+\tgit hook list pre-commit >actual &&\n+\ttest_cmp expected actual\n+'\n+\n+test_expect_success 'hook.runHookDir = no is respected by list' '\n+\tsetup_hookdir &&\n+\n+\ttest_config hook.runHookDir \"no\" &&\n+\n+\tcat >expected <<-EOF &&\n+\thookdir: $(pwd)/.git/hooks/pre-commit (will not run)\n+\tEOF\n+\n+\tgit hook list pre-commit >actual &&\n+\t# the hookdir annotation is translated\n+\ttest_i18ncmp expected actual\n+'\n+\n+test_expect_success 'hook.runHookDir = warn is respected by list' '\n+\tsetup_hookdir &&\n+\n+\ttest_config hook.runHookDir \"warn\" &&\n+\n+\tcat >expected <<-EOF &&\n+\thookdir: $(pwd)/.git/hooks/pre-commit (will warn)\n+\tEOF\n+\n+\tgit hook list pre-commit >actual &&\n+\t# the hookdir annotation is translated\n+\ttest_i18ncmp expected actual\n+'\n+\n+\n+test_expect_success 'hook.runHookDir = interactive is respected by list' '\n+\tsetup_hookdir &&\n+\n+\ttest_config hook.runHookDir \"interactive\" &&\n+\n+\tcat >expected <<-EOF &&\n+\thookdir: $(pwd)/.git/hooks/pre-commit (will prompt)\n+\tEOF\n+\n+\tgit hook list pre-commit >actual &&\n+\t# the hookdir annotation is translated\n+\ttest_i18ncmp expected actual\n+'\n+\n test_done\n-- \n2.28.0.rc0.142.g3c755180ce-goog\n\n"},{"id":"407568","messageId":"20201014232447.3050579-3-emilyshaffer@google.com","threadId":"54421","inReplyTo":"20201014232447.3050579-1-emilyshaffer@google.com","subject":"[PATCH v5 2/8] hook: scaffolding for git-hook subcommand","fromName":"Emily Shaffer","fromEmail":"emilyshaffer@google.com","sentAt":"2020-10-14T23:24:41Z","receivedAt":"2020-10-15T01:55:12Z","isPatch":true,"sender":{"key":"nasamuffin@google.com","avatar":"https://avatars.githubusercontent.com/u/1606826?v=4"},"body":"Introduce infrastructure for a new subcommand, git-hook, which will be\nused to ease config-based hook management. This command will handle\nparsing configs to compose a list of hooks to run for a given event, as\nwell as adding or modifying hook configs in an interactive fashion.\n\nSigned-off-by: Emily Shaffer <emilyshaffer@google.com>\n---\n\nNotes:\n    Since v4, mainly changed to RUN_SETUP_GENTLY so that 'git hook list' can\n    be executed outside of a repo.\n\n .gitignore                    |  1 +\n Documentation/git-hook.txt    | 20 ++++++++++++++++++++\n Makefile                      |  1 +\n builtin.h                     |  1 +\n builtin/hook.c                | 21 +++++++++++++++++++++\n git.c                         |  1 +\n t/t1360-config-based-hooks.sh | 11 +++++++++++\n 7 files changed, 56 insertions(+)\n create mode 100644 Documentation/git-hook.txt\n create mode 100644 builtin/hook.c\n create mode 100755 t/t1360-config-based-hooks.sh\n\ndiff --git a/.gitignore b/.gitignore\nindex 6232d33924..432e0b11cb 100644\n--- a/.gitignore\n+++ b/.gitignore\n@@ -75,6 +75,7 @@\n /git-grep\n /git-hash-object\n /git-help\n+/git-hook\n /git-http-backend\n /git-http-fetch\n /git-http-push\ndiff --git a/Documentation/git-hook.txt b/Documentation/git-hook.txt\nnew file mode 100644\nindex 0000000000..9eeab0009d\n--- /dev/null\n+++ b/Documentation/git-hook.txt\n@@ -0,0 +1,20 @@\n+git-hook(1)\n+===========\n+\n+NAME\n+----\n+git-hook - Manage configured hooks\n+\n+SYNOPSIS\n+--------\n+[verse]\n+'git hook'\n+\n+DESCRIPTION\n+-----------\n+A placeholder command. Later, you will be able to list, add, and modify hooks\n+with this command.\n+\n+GIT\n+---\n+Part of the linkgit:git[1] suite\ndiff --git a/Makefile b/Makefile\nindex 5311b1d2c4..9152f6d7c8 100644\n--- a/Makefile\n+++ b/Makefile\n@@ -1095,6 +1095,7 @@ BUILTIN_OBJS += builtin/get-tar-commit-id.o\n BUILTIN_OBJS += builtin/grep.o\n BUILTIN_OBJS += builtin/hash-object.o\n BUILTIN_OBJS += builtin/help.o\n+BUILTIN_OBJS += builtin/hook.o\n BUILTIN_OBJS += builtin/index-pack.o\n BUILTIN_OBJS += builtin/init-db.o\n BUILTIN_OBJS += builtin/interpret-trailers.o\ndiff --git a/builtin.h b/builtin.h\nindex 53fb290963..3b20689d1a 100644\n--- a/builtin.h\n+++ b/builtin.h\n@@ -162,6 +162,7 @@ int cmd_get_tar_commit_id(int argc, const char **argv, const char *prefix);\n int cmd_grep(int argc, const char **argv, const char *prefix);\n int cmd_hash_object(int argc, const char **argv, const char *prefix);\n int cmd_help(int argc, const char **argv, const char *prefix);\n+int cmd_hook(int argc, const char **argv, const char *prefix);\n int cmd_index_pack(int argc, const char **argv, const char *prefix);\n int cmd_init_db(int argc, const char **argv, const char *prefix);\n int cmd_interpret_trailers(int argc, const char **argv, const char *prefix);\ndiff --git a/builtin/hook.c b/builtin/hook.c\nnew file mode 100644\nindex 0000000000..b2bbc84d4d\n--- /dev/null\n+++ b/builtin/hook.c\n@@ -0,0 +1,21 @@\n+#include \"cache.h\"\n+\n+#include \"builtin.h\"\n+#include \"parse-options.h\"\n+\n+static const char * const builtin_hook_usage[] = {\n+\tN_(\"git hook\"),\n+\tNULL\n+};\n+\n+int cmd_hook(int argc, const char **argv, const char *prefix)\n+{\n+\tstruct option builtin_hook_options[] = {\n+\t\tOPT_END(),\n+\t};\n+\n+\targc = parse_options(argc, argv, prefix, builtin_hook_options,\n+\t\t\t     builtin_hook_usage, 0);\n+\n+\treturn 0;\n+}\ndiff --git a/git.c b/git.c\nindex f1e8b56d99..caad1c877f 100644\n--- a/git.c\n+++ b/git.c\n@@ -524,6 +524,7 @@ static struct cmd_struct commands[] = {\n \t{ \"grep\", cmd_grep, RUN_SETUP_GENTLY },\n \t{ \"hash-object\", cmd_hash_object },\n \t{ \"help\", cmd_help },\n+\t{ \"hook\", cmd_hook, RUN_SETUP_GENTLY },\n \t{ \"index-pack\", cmd_index_pack, RUN_SETUP_GENTLY | NO_PARSEOPT },\n \t{ \"init\", cmd_init_db },\n \t{ \"init-db\", cmd_init_db },\ndiff --git a/t/t1360-config-based-hooks.sh b/t/t1360-config-based-hooks.sh\nnew file mode 100755\nindex 0000000000..34b0df5216\n--- /dev/null\n+++ b/t/t1360-config-based-hooks.sh\n@@ -0,0 +1,11 @@\n+#!/bin/bash\n+\n+test_description='config-managed multihooks, including git-hook command'\n+\n+. ./test-lib.sh\n+\n+test_expect_success 'git hook command does not crash' '\n+\tgit hook\n+'\n+\n+test_done\n-- \n2.28.0.rc0.142.g3c755180ce-goog\n\n"},{"id":"407569","messageId":"20201014232447.3050579-8-emilyshaffer@google.com","threadId":"54421","inReplyTo":"20201014232447.3050579-1-emilyshaffer@google.com","subject":"[PATCH v5 7/8] hook: add 'run' subcommand","fromName":"Emily Shaffer","fromEmail":"emilyshaffer@google.com","sentAt":"2020-10-14T23:24:46Z","receivedAt":"2020-10-15T01:55:13Z","isPatch":true,"sender":{"key":"nasamuffin@google.com","avatar":"https://avatars.githubusercontent.com/u/1606826?v=4"},"body":"In order to enable hooks to be run as an external process, by a\nstandalone Git command, or by tools which wrap Git, provide an external\nmeans to run all configured hook commands for a given hook event.\n\nFor now, the hook commands will run in config order, in series. As\nalternate ordering or parallelism is supported in the future, we should\nadd knobs to use those to the command line as well.\n\nAs with the legacy hook implementation, all stdout generated by hook\ncommands is redirected to stderr. Piping from stdin is not yet\nsupported.\n\nLegacy hooks (those present in $GITDIR/hooks) are run at the end of the\nexecution list. For now, there is no way to disable them.\n\nUsers may wish to provide hook commands like 'git config\nhook.pre-commit.command \"~/linter.sh --pre-commit\"'. To enable this, the\ncontents of the 'hook.*.command' and 'hookcmd.*.command' strings are\nfirst split by space or quotes into an argv_array, then expanded with\n'expand_user_path()'.\n\nSigned-off-by: Emily Shaffer <emilyshaffer@google.com>\n---\n\nNotes:\n    Since v4, updated the docs, and did less local application of single\n    quotes. In order for hookdir hooks to run successfully with a space in\n    the path, though, they must not be run with 'sh -c'. So we can treat the\n    hookdir hooks specially, and warn users via doc about special\n    considerations for configured hooks with spaces in their path.\n\n Documentation/git-hook.txt    |  12 +++-\n builtin/hook.c                |  40 +++++++++++++-\n hook.c                        | 100 ++++++++++++++++++++++++++++++++++\n hook.h                        |   7 +++\n t/t1360-config-based-hooks.sh |  65 +++++++++++++++++++++-\n 5 files changed, 218 insertions(+), 6 deletions(-)\n\ndiff --git a/Documentation/git-hook.txt b/Documentation/git-hook.txt\nindex f19875ed68..95d3687905 100644\n--- a/Documentation/git-hook.txt\n+++ b/Documentation/git-hook.txt\n@@ -9,11 +9,12 @@ SYNOPSIS\n --------\n [verse]\n 'git hook' list <hook-name>\n+'git hook' run <hook-name>\n \n DESCRIPTION\n -----------\n-You can list configured hooks with this command. Later, you will be able to run,\n-add, and modify hooks with this command.\n+You can list and run configured hooks with this command. Later, you will be able\n+to add and modify hooks with this command.\n \n This command parses the default configuration files for sections `hook` and\n `hookcmd`. `hook` is used to describe the commands which will be run during a\n@@ -64,6 +65,13 @@ in the order they should be run, and print the config scope where the relevant\n `hook.<hook-name>.command` was specified, not the `hookcmd` (if applicable).\n This output is human-readable and the format is subject to change over time.\n \n+run `<hook-name>`::\n+\n+Runs hooks configured for `<hook-name>`, in the same order displayed by `git\n+hook list`. Hooks configured this way are run prepended with `sh -c`, so paths\n+containing special characters or spaces should be wrapped in single quotes:\n+`command = '/my/path with spaces/script.sh' some args`.\n+\n CONFIGURATION\n -------------\n include::config/hook.txt[]\ndiff --git a/builtin/hook.c b/builtin/hook.c\nindex 16324d4195..64aad28e54 100644\n--- a/builtin/hook.c\n+++ b/builtin/hook.c\n@@ -5,9 +5,11 @@\n #include \"hook.h\"\n #include \"parse-options.h\"\n #include \"strbuf.h\"\n+#include \"strvec.h\"\n \n static const char * const builtin_hook_usage[] = {\n \tN_(\"git hook list <hookname>\"),\n+\tN_(\"git hook run [(-e|--env)=<var>...] [(-a|--arg)=<arg>...] <hookname>\"),\n \tNULL\n };\n \n@@ -84,6 +86,40 @@ static int list(int argc, const char **argv, const char *prefix)\n \treturn 0;\n }\n \n+static int run(int argc, const char **argv, const char *prefix)\n+{\n+\tstruct strbuf hookname = STRBUF_INIT;\n+\tstruct strvec envs = STRVEC_INIT;\n+\tstruct strvec args = STRVEC_INIT;\n+\n+\tstruct option run_options[] = {\n+\t\tOPT_STRVEC('e', \"env\", &envs, N_(\"var\"),\n+\t\t\t   N_(\"environment variables for hook to use\")),\n+\t\tOPT_STRVEC('a', \"arg\", &args, N_(\"args\"),\n+\t\t\t   N_(\"argument to pass to hook\")),\n+\t\tOPT_END(),\n+\t};\n+\n+\t/*\n+\t * While it makes sense to list hooks out-of-repo, it doesn't make sense\n+\t * to execute them. Hooks usually want to look at repository artifacts.\n+\t */\n+\tif (!have_git_dir())\n+\t\tusage_msg_opt(_(\"You must be in a Git repo to execute hooks.\"),\n+\t\t\t      builtin_hook_usage, run_options);\n+\n+\targc = parse_options(argc, argv, prefix, run_options,\n+\t\t\t     builtin_hook_usage, 0);\n+\n+\tif (argc < 1)\n+\t\tusage_msg_opt(_(\"You must specify a hook event to run.\"),\n+\t\t\t      builtin_hook_usage, run_options);\n+\n+\tstrbuf_addstr(&hookname, argv[0]);\n+\n+\treturn run_hooks(envs.v, hookname.buf, &args, should_run_hookdir);\n+}\n+\n int cmd_hook(int argc, const char **argv, const char *prefix)\n {\n \tconst char *run_hookdir = NULL;\n@@ -98,7 +134,7 @@ int cmd_hook(int argc, const char **argv, const char *prefix)\n \t\t\t     builtin_hook_usage, 0);\n \n \t/* after the parse, we should have \"<command> <hookname> <args...>\" */\n-\tif (argc < 1)\n+\tif (argc < 2)\n \t\tusage_with_options(builtin_hook_usage, builtin_hook_options);\n \n \n@@ -120,6 +156,8 @@ int cmd_hook(int argc, const char **argv, const char *prefix)\n \n \tif (!strcmp(argv[0], \"list\"))\n \t\treturn list(argc, argv, prefix);\n+\tif (!strcmp(argv[0], \"run\"))\n+\t\treturn run(argc, argv, prefix);\n \n \tusage_with_options(builtin_hook_usage, builtin_hook_options);\n }\ndiff --git a/hook.c b/hook.c\nindex f4084e33c8..1494a32c1a 100644\n--- a/hook.c\n+++ b/hook.c\n@@ -3,6 +3,7 @@\n #include \"hook.h\"\n #include \"config.h\"\n #include \"run-command.h\"\n+#include \"prompt.h\"\n \n void free_hook(struct hook *ptr)\n {\n@@ -135,6 +136,56 @@ enum hookdir_opt configured_hookdir_opt(void)\n \treturn hookdir_unknown;\n }\n \n+static int should_include_hookdir(const char *path, enum hookdir_opt cfg)\n+{\n+\tstruct strbuf prompt = STRBUF_INIT;\n+\t/*\n+\t * If the path doesn't exist, don't bother adding the empty hook and\n+\t * don't bother checking the config or prompting the user.\n+\t */\n+\tif (!path)\n+\t\treturn 0;\n+\n+\tswitch (cfg)\n+\t{\n+\t\tcase hookdir_no:\n+\t\t\treturn 0;\n+\t\tcase hookdir_unknown:\n+\t\t\tfprintf(stderr,\n+\t\t\t\t_(\"Unrecognized value for 'hook.runHookDir'. \"\n+\t\t\t\t  \"Is there a typo? \"));\n+\t\t\t/* FALLTHROUGH */\n+\t\tcase hookdir_warn:\n+\t\t\tfprintf(stderr, _(\"Running legacy hook at '%s'\\n\"),\n+\t\t\t\tpath);\n+\t\t\treturn 1;\n+\t\tcase hookdir_interactive:\n+\t\t\tdo {\n+\t\t\t\t/*\n+\t\t\t\t * TRANSLATORS: Make sure to include [Y] and [n]\n+\t\t\t\t * in your translation. Only English input is\n+\t\t\t\t * accepted. Default option is \"yes\".\n+\t\t\t\t */\n+\t\t\t\tfprintf(stderr, _(\"Run '%s'? [Yn] \"), path);\n+\t\t\t\tgit_read_line_interactively(&prompt);\n+\t\t\t\tstrbuf_tolower(&prompt);\n+\t\t\t\tif (starts_with(prompt.buf, \"n\")) {\n+\t\t\t\t\tstrbuf_release(&prompt);\n+\t\t\t\t\treturn 0;\n+\t\t\t\t} else if (starts_with(prompt.buf, \"y\")) {\n+\t\t\t\t\tstrbuf_release(&prompt);\n+\t\t\t\t\treturn 1;\n+\t\t\t\t}\n+\t\t\t\t/* otherwise, we didn't understand the input */\n+\t\t\t} while (prompt.len); /* an empty reply means \"Yes\" */\n+\t\t\tstrbuf_release(&prompt);\n+\t\t\treturn 1;\n+\t\tcase hookdir_yes:\n+\t\tdefault:\n+\t\t\treturn 1;\n+\t}\n+}\n+\n struct list_head* hook_list(const struct strbuf* hookname)\n {\n \tstruct strbuf hook_key = STRBUF_INIT;\n@@ -166,3 +217,52 @@ struct list_head* hook_list(const struct strbuf* hookname)\n \tstrbuf_release(&hook_key);\n \treturn hook_head;\n }\n+\n+\n+int run_hooks(const char *const *env, const char *hookname,\n+\t      const struct strvec *args, enum hookdir_opt run_hookdir)\n+{\n+\tstruct strbuf hookname_str = STRBUF_INIT;\n+\tstruct list_head *to_run, *pos = NULL, *tmp = NULL;\n+\tint rc = 0;\n+\n+\tstrbuf_addstr(&hookname_str, hookname);\n+\n+\tto_run = hook_list(&hookname_str);\n+\n+\tlist_for_each_safe(pos, tmp, to_run) {\n+\t\tstruct child_process hook_proc = CHILD_PROCESS_INIT;\n+\t\tstruct hook *hook = list_entry(pos, struct hook, list);\n+\n+\t\thook_proc.env = env;\n+\t\thook_proc.no_stdin = 1;\n+\t\thook_proc.stdout_to_stderr = 1;\n+\t\thook_proc.trace2_hook_name = hook->command.buf;\n+\t\thook_proc.use_shell = 1;\n+\n+\n+\t\tif (hook->from_hookdir) {\n+\t\t    if (!should_include_hookdir(hook->command.buf, run_hookdir))\n+\t\t\tcontinue;\n+\t\t    /*\n+\t\t     * Commands from the config could be oneliners, but we know\n+\t\t     * for certain that hookdir commands are not.\n+\t\t     */\n+\t\t    hook_proc.use_shell = 0;\n+\t\t}\n+\n+\t\t/* add command */\n+\t\tstrvec_push(&hook_proc.args, hook->command.buf);\n+\n+\t\t/*\n+\t\t * add passed-in argv, without expanding - let the user get back\n+\t\t * exactly what they put in\n+\t\t */\n+\t\tif (args)\n+\t\t\tstrvec_pushv(&hook_proc.args, args->v);\n+\n+\t\trc |= run_command(&hook_proc);\n+\t}\n+\n+\treturn rc;\n+}\ndiff --git a/hook.h b/hook.h\nindex ca45d388d3..6eb1dc99c4 100644\n--- a/hook.h\n+++ b/hook.h\n@@ -1,6 +1,7 @@\n #include \"config.h\"\n #include \"list.h\"\n #include \"strbuf.h\"\n+#include \"strvec.h\"\n \n struct hook\n {\n@@ -35,6 +36,12 @@ enum hookdir_opt\n  * command line arguments.\n  */\n enum hookdir_opt configured_hookdir_opt(void);\n+/*\n+ * Runs all hooks associated to the 'hookname' event in order. Each hook will be\n+ * passed 'env' and 'args'.\n+ */\n+int run_hooks(const char *const *env, const char *hookname,\n+\t      const struct strvec *args, enum hookdir_opt run_hookdir);\n \n /* Free memory associated with a 'struct hook' */\n void free_hook(struct hook *ptr);\ndiff --git a/t/t1360-config-based-hooks.sh b/t/t1360-config-based-hooks.sh\nindex ebd3bc623f..5b3003d59b 100755\n--- a/t/t1360-config-based-hooks.sh\n+++ b/t/t1360-config-based-hooks.sh\n@@ -115,7 +115,10 @@ test_expect_success 'hook.runHookDir = no is respected by list' '\n \n \tgit hook list pre-commit >actual &&\n \t# the hookdir annotation is translated\n-\ttest_i18ncmp expected actual\n+\ttest_i18ncmp expected actual &&\n+\n+\tgit hook run pre-commit 2>actual &&\n+\ttest_must_be_empty actual\n '\n \n test_expect_success 'hook.runHookDir = warn is respected by list' '\n@@ -129,6 +132,14 @@ test_expect_success 'hook.runHookDir = warn is respected by list' '\n \n \tgit hook list pre-commit >actual &&\n \t# the hookdir annotation is translated\n+\ttest_i18ncmp expected actual &&\n+\n+\tcat >expected <<-EOF &&\n+\tRunning legacy hook at '\\''$(pwd)/.git/hooks/pre-commit'\\''\n+\t\"Legacy Hook\"\n+\tEOF\n+\n+\tgit hook run pre-commit 2>actual &&\n \ttest_i18ncmp expected actual\n '\n \n@@ -156,7 +167,7 @@ test_expect_success 'git hook list removes skipped inlined hook' '\n \ttest_cmp expected actual\n '\n \n-test_expect_success 'hook.runHookDir = interactive is respected by list' '\n+test_expect_success 'hook.runHookDir = interactive is respected by list and run' '\n \tsetup_hookdir &&\n \n \ttest_config hook.runHookDir \"interactive\" &&\n@@ -167,7 +178,55 @@ test_expect_success 'hook.runHookDir = interactive is respected by list' '\n \n \tgit hook list pre-commit >actual &&\n \t# the hookdir annotation is translated\n-\ttest_i18ncmp expected actual\n+\ttest_i18ncmp expected actual &&\n+\n+\ttest_write_lines n | git hook run pre-commit 2>actual &&\n+\t! grep \"Legacy Hook\" actual &&\n+\n+\ttest_write_lines y | git hook run pre-commit 2>actual &&\n+\tgrep \"Legacy Hook\" actual\n+'\n+\n+test_expect_success 'inline hook definitions execute oneliners' '\n+\ttest_config hook.pre-commit.command \"echo \\\"Hello World\\\"\" &&\n+\n+\techo \"Hello World\" >expected &&\n+\n+\t# hooks are run with stdout_to_stderr = 1\n+\tgit hook run pre-commit 2>actual &&\n+\ttest_cmp expected actual\n+'\n+\n+test_expect_success 'inline hook definitions resolve paths' '\n+\twrite_script sample-hook.sh <<-EOF &&\n+\techo \\\"Sample Hook\\\"\n+\tEOF\n+\n+\ttest_when_finished \"rm sample-hook.sh\" &&\n+\n+\ttest_config hook.pre-commit.command \"\\\"$(pwd)/sample-hook.sh\\\"\" &&\n+\n+\techo \\\"Sample Hook\\\" >expected &&\n+\n+\t# hooks are run with stdout_to_stderr = 1\n+\tgit hook run pre-commit 2>actual &&\n+\ttest_cmp expected actual\n+'\n+\n+test_expect_success 'hookdir hook included in git hook run' '\n+\tsetup_hookdir &&\n+\n+\techo \\\"Legacy Hook\\\" >expected &&\n+\n+\t# hooks are run with stdout_to_stderr = 1\n+\tgit hook run pre-commit 2>actual &&\n+\ttest_cmp expected actual\n+'\n+\n+test_expect_success 'out-of-repo runs excluded' '\n+\tsetup_hooks &&\n+\n+\tnongit test_must_fail git hook run pre-commit\n '\n \n test_done\n-- \n2.28.0.rc0.142.g3c755180ce-goog\n\n"},{"id":"407570","messageId":"20201014232447.3050579-1-emilyshaffer@google.com","threadId":"54421","inReplyTo":"20200909004939.1942347-1-emilyshaffer@google.com","subject":"[PATCH v5 0/8] propose config-based hooks (part I)","fromName":"Emily Shaffer","fromEmail":"emilyshaffer@google.com","sentAt":"2020-10-14T23:24:39Z","receivedAt":"2020-10-15T01:55:15Z","isPatch":true,"sender":{"key":"nasamuffin@google.com","avatar":"https://avatars.githubusercontent.com/u/1606826?v=4"},"body":"Since v4:\n- Reordered the commits. Hookdir support is added sooner and conversion\n  of existing hooks is moved to another branch (part II) for hopefully\n  more granular reviewing. If folks hate this, let me know and I'll\n  reintegrate the two topics.\n- Removed the --porcelain option on 'git hook list'. General consensus\n  is that this should use a format string instead, and I didn't want to\n  write that new feature while I had been promising v5 \"any day now\".\n- Added functionality for 'skip' to remove hooks from the execution\n  list.\n- General nits from folks.\n\nComing soon:\n- 'git hook list --format'\n- More conversions (in the other topic)\n- As required by new conversions, stdin support for hooks\n\nComing much later:\n- 'git hook add'/'git hook edit'. The config isn't too ugly to manually\n  edit, for now, so I'd like to get the hooks themselves all figured out\n  before adding these convenience tools. I do still think they're a good\n  idea, as they'll increase the discoverability of the feature for new\n  users.\n\nMore detailed notes in each commit. Thanks all for your patience and\nreviews.\n\n - Emily\n\nEmily Shaffer (8):\n  doc: propose hooks managed by the config\n  hook: scaffolding for git-hook subcommand\n  hook: add list command\n  hook: include hookdir hook in list\n  hook: implement hookcmd.<name>.skip\n  parse-options: parse into strvec\n  hook: add 'run' subcommand\n  hook: replace find_hook() with hook_exists()\n\n .gitignore                                    |   1 +\n Documentation/Makefile                        |   1 +\n Documentation/config/hook.txt                 |  14 +\n Documentation/git-hook.txt                    |  81 ++++\n Documentation/technical/api-parse-options.txt |   5 +\n .../technical/config-based-hooks.txt          | 367 ++++++++++++++++++\n Makefile                                      |   2 +\n builtin.h                                     |   1 +\n builtin/hook.c                                | 163 ++++++++\n git.c                                         |   1 +\n hook.c                                        | 282 ++++++++++++++\n hook.h                                        |  58 +++\n parse-options-cb.c                            |  16 +\n parse-options.h                               |   4 +\n t/t1360-config-based-hooks.sh                 | 232 +++++++++++\n 15 files changed, 1228 insertions(+)\n create mode 100644 Documentation/config/hook.txt\n create mode 100644 Documentation/git-hook.txt\n create mode 100644 Documentation/technical/config-based-hooks.txt\n create mode 100644 builtin/hook.c\n create mode 100644 hook.c\n create mode 100644 hook.h\n create mode 100755 t/t1360-config-based-hooks.sh\n\n-- \n2.28.0.rc0.142.g3c755180ce-goog\n\n"},{"id":"407625","messageId":"87d01jih7w.fsf@evledraar.gmail.com","threadId":"54421","inReplyTo":"20201014232447.3050579-2-emilyshaffer@google.com","subject":"Re: [PATCH v5 1/8] doc: propose hooks managed by the config","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2020-10-15T16:31:15Z","receivedAt":"2020-10-15T16:31:20Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"\nOn Thu, Oct 15 2020, Emily Shaffer wrote:\n\n> Notes:\n>     Since v4, addressed comments from Jonathan Tan about wording.\n\nI had some extensive comments on the v4 here:\nhttps://lore.kernel.org/git/87mu0ygzk1.fsf@evledraar.gmail.com/\n\nYour CL & this patch don't mention it. I'd be interested in\ncollaborating on this depending on if/how our goals/wants align, but I'd\nlke to get your thoughts on that feedback first.\n"},{"id":"407730","messageId":"xmqqimbacc4q.fsf@gitster.c.googlers.com","threadId":"54421","inReplyTo":"87d01jih7w.fsf@evledraar.gmail.com","subject":"Re: [PATCH v5 1/8] doc: propose hooks managed by the config","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2020-10-16T17:29:57Z","receivedAt":"2020-10-16T17:30:01Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Ævar Arnfjörð Bjarmason <avarab@gmail.com> writes:\n\n> On Thu, Oct 15 2020, Emily Shaffer wrote:\n>\n>> Notes:\n>>     Since v4, addressed comments from Jonathan Tan about wording.\n>\n> I had some extensive comments on the v4 here:\n> https://lore.kernel.org/git/87mu0ygzk1.fsf@evledraar.gmail.com/\n>\n> Your CL & this patch don't mention it. I'd be interested in\n> collaborating on this depending on if/how our goals/wants align, but I'd\n> lke to get your thoughts on that feedback first.\n\nTrue.\n\nIt seems that it wasn't responded (not even a single-liner \"Thanks,\nI'll get to it later\" or \"Thanks, but the goal I am aiming is\ndifferent from yours and your experience does not translate directly\nhere\") and I can only conclude that it somehow was overlooked?\n\nEmily?\n\nSide note: perhaps it is just me, but after making a review and\ngiving extensive comments and suggestions, it is often disorienting\nto read the next round without getting any hint on which parts of\nthe comments were heard and which other parts were dismissed (and\nwhy).  I think your earlier review is a kind that deserves a\nseparate response before the updated patchset.\n\nThanks.\n\n\n"},{"id":"408127","messageId":"20201021233749.GC2774782@google.com","threadId":"54421","inReplyTo":"87d01jih7w.fsf@evledraar.gmail.com","subject":"Re: [PATCH v5 1/8] doc: propose hooks managed by the config","fromName":"Emily Shaffer","fromEmail":"emilyshaffer@google.com","sentAt":"2020-10-21T23:37:49Z","receivedAt":"2020-10-21T23:37:57Z","isPatch":true,"sender":{"key":"nasamuffin@google.com","avatar":"https://avatars.githubusercontent.com/u/1606826?v=4"},"body":"On Thu, Oct 15, 2020 at 06:31:15PM +0200, Ævar Arnfjörð Bjarmason wrote:\n> \n> \n> On Thu, Oct 15 2020, Emily Shaffer wrote:\n> \n> > Notes:\n> >     Since v4, addressed comments from Jonathan Tan about wording.\n> \n> I had some extensive comments on the v4 here:\n> https://lore.kernel.org/git/87mu0ygzk1.fsf@evledraar.gmail.com/\n\nHum, it seems I completely missed it. I'm sorry - that was very rude of\nme! I'll have a look now and reply there.\n\n - Emily\n"}]}