{"thread":{"id":"52420","subject":"[PATCH 0/6] configuration-based hook management","startedAt":"2019-12-10T02:33:53Z","lastAt":"2019-12-12T10:59:01Z","messageCount":19,"participants":["Emily Shaffer","Philip Oakley","Junio C Hamano","Bert Wesarg","SZEDER Gábor"],"isPatch":true,"patchVersion":1,"patchTotal":6},"messages":[{"id":"387863","messageId":"20191210023335.49987-1-emilyshaffer@google.com","threadId":"52420","inReplyTo":null,"subject":"[PATCH 0/6] configuration-based hook management","fromName":"Emily Shaffer","fromEmail":"emilyshaffer@google.com","sentAt":"2019-12-10T02:33:29Z","receivedAt":"2019-12-10T02:33:53Z","isPatch":true,"sender":{"key":"nasamuffin@google.com","avatar":"https://avatars.githubusercontent.com/u/1606826?v=4"},"body":"An implementation of the first piece of the proposal given in\nlore.kernel.org/git/20191116011125.GG22855@google.com.\n\nTeaches a new command, 'git hook', which will someday include 'git hook\n--add ...', 'git hook --edit ...', and maybe more. For now, just teach\nit how to check the config files with 'git hook --list ...'.\n\nThe hooks-to-run list is collected in a new library, hook.o, which can\nsomeday reimplement find_hook() or otherwise be invoked to run all hooks\nfor a given hookname (e.g. \"pre-commit\").\n\nThe change to config.[ch] allows us to display a similar scope name to\nthe one a user may use to 'git config --add' or later 'git hook --add' a\nhook at a certain scope, e.g.:\n\n  $ git hook --list pre-commit\n  001\tglobal\t~/foo.sh\n  $ git hook --add --global pre-commit 005 ~/bar.sh\n  Added.\n  001\tglobal\t~/foo.sh\n  005\tglobal\t~/bar.sh\n\nThere are config examples in many of the commit messages in this chain.\n\nBefore I consider \"--list\" to be done, I also want to add support to\ncheck \"hook.runHookDir\" and take .git/hooks/* into account. But I wanted\nus to spend time chewing on the config format for a while before I got\ntoo far.\n\nIt's also very possible (likely, even!) to put this feature behind an\nexperimental flag, which gives us more room to change the config format\nif we want before the feature is \"done\".\n\nIn the discussion thread with brian, I also mentioned a self-paced\ndeprecation of hooks which live in .git/hooks/, which I'm aware some\nusers may not want to follow. However, it occurred to me that we may be\nable to hide a Git-paced deprecation behind a config macro (since those\nare new and shiny) which is opt-in, and handles something like:\n\n  hook.runHookDir = true\n  hook.warnHookDir = false\n\n  {some months pass, we are sure config-based hooks are working nicely}\n\n  hook.runHookDir = true\n  hook.warnHookDir = true\n\n  {so start yelling at users to move away, and wait some more\n  months/years}\n\n  hook.runHookDir = false\n\n  {users who have opted into the hookdir phaseout macro are no longer\n  using the hookdir}\n\nAs it's opt-in (and easily reversible by changing configs) this might be\na good middle ground for the \"deprecate or not\" discussion brian and I\nhad.\n\nThanks.\n - Emily\n\nEmily Shaffer (6):\n  hook: scaffolding for git-hook subcommand\n  config: add string mapping for enum config_scope\n  hook: add --list mode\n  hook: support reordering of hook list\n  hook: remove prior hook with '---'\n  hook: teach --porcelain mode\n\n .gitignore                    |  1 +\n Documentation/git-hook.txt    | 53 ++++++++++++++++++++\n Makefile                      |  2 +\n builtin.h                     |  1 +\n builtin/hook.c                | 80 ++++++++++++++++++++++++++++++\n config.c                      | 17 +++++++\n config.h                      |  1 +\n git.c                         |  1 +\n hook.c                        | 93 +++++++++++++++++++++++++++++++++++\n hook.h                        | 14 ++++++\n t/t1360-config-based-hooks.sh | 89 +++++++++++++++++++++++++++++++++\n 11 files changed, 352 insertions(+)\n create mode 100644 Documentation/git-hook.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.24.0.393.g34dc348eaf-goog\n\n"},{"id":"387864","messageId":"20191210023335.49987-2-emilyshaffer@google.com","threadId":"52420","inReplyTo":"20191210023335.49987-1-emilyshaffer@google.com","subject":"[PATCH 1/6] hook: scaffolding for git-hook subcommand","fromName":"Emily Shaffer","fromEmail":"emilyshaffer@google.com","sentAt":"2019-12-10T02:33:30Z","receivedAt":"2019-12-10T02:33:55Z","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 .gitignore                    |  1 +\n Documentation/git-hook.txt    | 19 +++++++++++++++++++\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, 55 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 89b3b79c1a..9ef59b9baa 100644\n--- a/.gitignore\n+++ b/.gitignore\n@@ -74,6 +74,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..2d50c414cc\n--- /dev/null\n+++ b/Documentation/git-hook.txt\n@@ -0,0 +1,19 @@\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+You can list, add, and modify hooks with this command.\n+\n+GIT\n+---\n+Part of the linkgit:git[1] suite\ndiff --git a/Makefile b/Makefile\nindex 58b92af54b..83263505c0 100644\n--- a/Makefile\n+++ b/Makefile\n@@ -1074,6 +1074,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 5cf5df69f7..d4ca2ac9a5 100644\n--- a/builtin.h\n+++ b/builtin.h\n@@ -173,6 +173,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 ce6ab0ece2..c8344b9ab7 100644\n--- a/git.c\n+++ b/git.c\n@@ -513,6 +513,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 },\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.24.0.393.g34dc348eaf-goog\n\n"},{"id":"387865","messageId":"20191210023335.49987-3-emilyshaffer@google.com","threadId":"52420","inReplyTo":"20191210023335.49987-1-emilyshaffer@google.com","subject":"[PATCH 2/6] config: add string mapping for enum config_scope","fromName":"Emily Shaffer","fromEmail":"emilyshaffer@google.com","sentAt":"2019-12-10T02:33:31Z","receivedAt":"2019-12-10T02:33:57Z","isPatch":true,"sender":{"key":"nasamuffin@google.com","avatar":"https://avatars.githubusercontent.com/u/1606826?v=4"},"body":"If a user is interacting with their config files primarily by the 'git\nconfig' command, using the location flags (--global, --system, etc) then\nthey may be more interested to see the scope of the config file they are\nediting, rather than the filepath.\n\nSigned-off-by: Emily Shaffer <emilyshaffer@google.com>\n---\n config.c | 17 +++++++++++++++++\n config.h |  1 +\n 2 files changed, 18 insertions(+)\n\ndiff --git a/config.c b/config.c\nindex e7052b3977..a20110e016 100644\n--- a/config.c\n+++ b/config.c\n@@ -3312,6 +3312,23 @@ enum config_scope current_config_scope(void)\n \t\treturn current_parsing_scope;\n }\n \n+const char *config_scope_to_string(enum config_scope scope)\n+{\n+\tswitch (scope) {\n+\tcase CONFIG_SCOPE_SYSTEM:\n+\t\treturn _(\"system\");\n+\tcase CONFIG_SCOPE_GLOBAL:\n+\t\treturn _(\"global\");\n+\tcase CONFIG_SCOPE_REPO:\n+\t\treturn _(\"repo\");\n+\tcase CONFIG_SCOPE_CMDLINE:\n+\t\treturn _(\"cmdline\");\n+\tcase CONFIG_SCOPE_UNKNOWN:\n+\tdefault:\n+\t\treturn _(\"unknown\");\n+\t}\n+}\n+\n int lookup_config(const char **mapping, int nr_mapping, const char *var)\n {\n \tint i;\ndiff --git a/config.h b/config.h\nindex f0ed464004..612f43acd0 100644\n--- a/config.h\n+++ b/config.h\n@@ -139,6 +139,7 @@ enum config_scope {\n };\n \n enum config_scope current_config_scope(void);\n+const char *config_scope_to_string(enum config_scope);\n const char *current_config_origin_type(void);\n const char *current_config_name(void);\n \n-- \n2.24.0.393.g34dc348eaf-goog\n\n"},{"id":"387866","messageId":"20191210023335.49987-4-emilyshaffer@google.com","threadId":"52420","inReplyTo":"20191210023335.49987-1-emilyshaffer@google.com","subject":"[PATCH 3/6] hook: add --list mode","fromName":"Emily Shaffer","fromEmail":"emilyshaffer@google.com","sentAt":"2019-12-10T02:33:32Z","receivedAt":"2019-12-10T02:34:00Z","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\nThe hook config format is \"hook.<hookname> = <order>:<path-to-hook>\".\nThis paves the way for multiple hook support; hooks should be run in the\norder specified by the user in the config, and in the case of an order\nnumber collision, configuration order should be used (e.g. global hook\n004 will run before repo hook 004).\n\nFor example:\n\n  $ grep -A2 \"\\[hook\\]\" ~/.gitconfig\n  [hook]\n          pre-commit = 001:~/test.sh\n          pre-commit = 999:~/baz.sh\n\n  $ grep -A1 \"\\[hook\\]\" ~/git/.git/config\n  [hook]\n          pre-commit = 900:~/bar.sh\n\n  $ ./bin-wrappers/git hook --list pre-commit\n  001     global  ~/test.sh\n  900     repo    ~/bar.sh\n  999     global  ~/baz.sh\n\nSigned-off-by: Emily Shaffer <emilyshaffer@google.com>\n---\n Documentation/git-hook.txt    | 17 +++++++-\n Makefile                      |  1 +\n builtin/hook.c                | 54 ++++++++++++++++++++++-\n hook.c                        | 81 +++++++++++++++++++++++++++++++++++\n hook.h                        | 14 ++++++\n t/t1360-config-based-hooks.sh | 43 ++++++++++++++++++-\n 6 files changed, 206 insertions(+), 4 deletions(-)\n create mode 100644 hook.c\n create mode 100644 hook.h\n\ndiff --git a/Documentation/git-hook.txt b/Documentation/git-hook.txt\nindex 2d50c414cc..a141884239 100644\n--- a/Documentation/git-hook.txt\n+++ b/Documentation/git-hook.txt\n@@ -8,12 +8,27 @@ git-hook - Manage configured hooks\n SYNOPSIS\n --------\n [verse]\n-'git hook'\n+'git hook' -l | --list <hook-name>\n \n DESCRIPTION\n -----------\n You can list, add, and modify hooks with this command.\n \n+This command parses the default configuration files for lines which look like\n+\"hook.<hook-name> = <order number>:<hook command>\", e.g. \"hook.pre-commit =\n+010:/path/to/script.sh\". In this way, multiple scripts can be run during a\n+single hook. Hooks are sorted in ascending order by order number; in the event\n+of an order number conflict, they are sorted in configuration order.\n+\n+OPTIONS\n+-------\n+\n+-l::\n+--list::\n+\tList the hooks which have been configured for <hook-name>. Hooks appear\n+\tin the order they should be run. Output of this command follows the\n+\tformat '<order number> <origin config> <hook command>'.\n+\n GIT\n ---\n Part of the linkgit:git[1] suite\ndiff --git a/Makefile b/Makefile\nindex 83263505c0..21b3a82208 100644\n--- a/Makefile\n+++ b/Makefile\n@@ -892,6 +892,7 @@ LIB_OBJS += hashmap.o\n LIB_OBJS += linear-assignment.o\n LIB_OBJS += help.o\n LIB_OBJS += hex.o\n+LIB_OBJS += hook.o\n LIB_OBJS += ident.o\n LIB_OBJS += interdiff.o\n LIB_OBJS += json-writer.o\ndiff --git a/builtin/hook.c b/builtin/hook.c\nindex b2bbc84d4d..8261302b27 100644\n--- a/builtin/hook.c\n+++ b/builtin/hook.c\n@@ -1,21 +1,73 @@\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+enum hook_command {\n+\tHOOK_NO_COMMAND = 0,\n+\tHOOK_LIST,\n+};\n+\n+static int print_hook_list(const struct strbuf *hookname)\n+{\n+\tstruct list_head *head, *pos;\n+\tstruct hook *item;\n+\n+\thead = hook_list(hookname);\n+\n+\tlist_for_each(pos, head) {\n+\t\titem = list_entry(pos, struct hook, list);\n+\t\tif (item)\n+\t\t\tprintf(\"%.3d\\t%s\\t%s\\n\", item->order,\n+\t\t\t       config_scope_to_string(item->origin),\n+\t\t\t       item->command.buf);\n+\t}\n+\n+\treturn 0;\n+}\n+\n int cmd_hook(int argc, const char **argv, const char *prefix)\n {\n+\tenum hook_command command = 0;\n+\tstruct strbuf hookname = STRBUF_INIT;\n+\n \tstruct option builtin_hook_options[] = {\n+\t\tOPT_CMDMODE('l', \"list\", &command,\n+\t\t\t    N_(\"list scripts which will be run for <hookname>\"),\n+\t\t\t    HOOK_LIST),\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+\tif (argc < 1) {\n+\t\tusage_msg_opt(\"a hookname must be provided to operate on.\",\n+\t\t\t      builtin_hook_usage, builtin_hook_options);\n+\t}\n+\n+\tstrbuf_addstr(&hookname, \"hook.\");\n+\tstrbuf_addstr(&hookname, argv[0]);\n+\n+\tswitch(command) {\n+\t\tcase HOOK_LIST:\n+\t\t\treturn print_hook_list(&hookname);\n+\t\t\tbreak;\n+\t\tdefault:\n+\t\t\tusage_msg_opt(\"no command given.\", builtin_hook_usage,\n+\t\t\t\t      builtin_hook_options);\n+\t}\n+\n+\tclear_hook_list();\n+\tstrbuf_release(&hookname);\n+\n \treturn 0;\n }\ndiff --git a/hook.c b/hook.c\nnew file mode 100644\nindex 0000000000..f8d1109084\n--- /dev/null\n+++ b/hook.c\n@@ -0,0 +1,81 @@\n+#include \"cache.h\"\n+\n+#include \"hook.h\"\n+#include \"config.h\"\n+\n+static LIST_HEAD(hook_head);\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 emplace_hook(struct list_head *pos, int order, const char *command)\n+{\n+\tstruct hook *to_add = malloc(sizeof(struct hook));\n+\tto_add->order = order;\n+\tto_add->origin = current_config_scope();\n+\tstrbuf_init(&to_add->command, 0);\n+\tstrbuf_addstr(&to_add->command, command);\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()\n+{\n+\tstruct list_head *pos, *tmp;\n+\tlist_for_each_safe(pos, tmp, &hook_head)\n+\t\tremove_hook(pos);\n+}\n+\n+static int check_config_for_hooks(const char *var, const char *value, void *hookname)\n+{\n+\tstruct list_head *pos, *p;\n+\tstruct hook *item;\n+\tconst struct strbuf *hookname_strbuf = hookname;\n+\n+\tif (!strcmp(var, hookname_strbuf->buf)) {\n+\t\tint order = 0;\n+\t\t// TODO this is bad - open to overflows\n+\t\tchar command[256];\n+\t\tint added = 0;\n+\t\tif (!sscanf(value, \"%d:%s\", &order, command))\n+\t\t\tdie(_(\"hook config '%s' doesn't match expected format\"),\n+\t\t\t    value);\n+\n+\t\tlist_for_each_safe(pos, p, &hook_head) {\n+\t\t\titem = list_entry(pos, struct hook, list);\n+\n+\t\t\t/*\n+\t\t\t * the new entry should go just before the first entry\n+\t\t\t * which has a higher order number than it.\n+\t\t\t */\n+\t\t\tif (item->order > order && !added) {\n+\t\t\t\templace_hook(pos, order, command);\n+\t\t\t\tadded = 1;\n+\t\t\t}\n+\t\t}\n+\n+\t\tif (!added)\n+\t\t\templace_hook(pos, order, command);\n+\t}\n+\n+\treturn 0;\n+}\n+\n+struct list_head* hook_list(const struct strbuf* hookname)\n+{\n+\tgit_config(check_config_for_hooks, (void*)hookname);\n+\n+\treturn &hook_head;\n+}\ndiff --git a/hook.h b/hook.h\nnew file mode 100644\nindex 0000000000..104df4c088\n--- /dev/null\n+++ b/hook.h\n@@ -0,0 +1,14 @@\n+#include \"config.h\"\n+\n+struct hook\n+{\n+\tstruct list_head list;\n+\tint order;\n+\tenum config_scope origin;\n+\tstruct strbuf command;\n+};\n+\n+struct list_head* hook_list(const struct strbuf *hookname);\n+\n+void free_hook(struct hook *ptr);\n+void clear_hook_list();\ndiff --git a/t/t1360-config-based-hooks.sh b/t/t1360-config-based-hooks.sh\nindex 34b0df5216..1434051db3 100755\n--- a/t/t1360-config-based-hooks.sh\n+++ b/t/t1360-config-based-hooks.sh\n@@ -4,8 +4,47 @@ 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+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 'setup hooks in system, global, and local' '\n+\tgit config --add --global hook.pre-commit \"010:/path/def\" &&\n+\tgit config --add --global hook.pre-commit \"999:/path/uvw\" &&\n+\n+\tgit config --add --local hook.pre-commit \"100:/path/ghi\" &&\n+\tgit config --add --local hook.pre-commit \"990:/path/rst\"\n+'\n+\n+test_expect_success 'git hook --list orders by order number' '\n+\tcat >expected <<-\\EOF &&\n+\t010\tglobal\t/path/def\n+\t100\trepo\t/path/ghi\n+\t990\trepo\t/path/rst\n+\t999\tglobal\t/path/uvw\n+\tEOF\n+\n+\tgit hook --list pre-commit >actual &&\n+\ttest_cmp expected actual\n+'\n+\n+test_expect_success 'order number collisions resolved in config order' '\n+\tcat >expected <<-\\EOF &&\n+\t010\tglobal\t/path/def\n+\t010\trepo\t/path/abc\n+\t100\trepo\t/path/ghi\n+\t990\trepo\t/path/rst\n+\t999\tglobal\t/path/uvw\n+\tEOF\n+\n+\tgit config --add --local hook.pre-commit \"010:/path/abc\" &&\n+\tgit hook --list pre-commit >actual &&\n+\ttest_cmp expected actual\n '\n \n test_done\n-- \n2.24.0.393.g34dc348eaf-goog\n\n"},{"id":"387867","messageId":"20191210023335.49987-5-emilyshaffer@google.com","threadId":"52420","inReplyTo":"20191210023335.49987-1-emilyshaffer@google.com","subject":"[PATCH 4/6] hook: support reordering of hook list","fromName":"Emily Shaffer","fromEmail":"emilyshaffer@google.com","sentAt":"2019-12-10T02:33:33Z","receivedAt":"2019-12-10T02:34:02Z","isPatch":true,"sender":{"key":"nasamuffin@google.com","avatar":"https://avatars.githubusercontent.com/u/1606826?v=4"},"body":"It's possible that in most cases a user wants to run pre-commit hook\n'A', but in exactly one repo that user wants to run pre-commit hook 'A'\nfirst instead. Teach 'git hook' to support this by allowing a user to\nspecify a new order number for a hook after the initial hook has been\nspecified.\n\nFor example:\n\n  $ grep -A2 \"\\[hook\\]\" ~/.gitconfig\n  [hook]\n          pre-commit = 001:~/test.sh\n          pre-commit = 999:~/baz.sh\n  $ grep -A2 \"\\[hook\\]\" ~/git/.git/config\n  [hook]\n          pre-commit = 900:~/bar.sh\n          pre-commit = 050:~/baz.sh\n  $ ./bin-wrappers/git hook --list pre-commit\n  001     global  ~/test.sh\n  050     repo    ~/baz.sh\n  900     repo    ~/bar.sh\n\nIn the above example, '~/baz.sh' is provided in the global config with\norder position 999. Then, in the local config, that order is overridden\nto 050. Instead of running ~/baz.sh twice (at order 050 and at order\n999), only run it once, in the position specified last in config order.\n\nSigned-off-by: Emily Shaffer <emilyshaffer@google.com>\n---\n Documentation/git-hook.txt    |  8 ++++++++\n hook.c                        |  7 +++++++\n t/t1360-config-based-hooks.sh | 14 ++++++++++++++\n 3 files changed, 29 insertions(+)\n\ndiff --git a/Documentation/git-hook.txt b/Documentation/git-hook.txt\nindex a141884239..0f7115f826 100644\n--- a/Documentation/git-hook.txt\n+++ b/Documentation/git-hook.txt\n@@ -20,6 +20,14 @@ This command parses the default configuration files for lines which look like\n single hook. Hooks are sorted in ascending order by order number; in the event\n of an order number conflict, they are sorted in configuration order.\n \n+The order number of a hook can be changed at a more local scope, e.g.:\n+\n+  git config --add --global hook.pre-commit \"001:/foo.sh\"\n+  git config --add --local hook.pre-commit \"005:/foo.sh\"\n+\n+When the order number is respecified this way, the previously specified hook\n+configuration is overridden.\n+\n OPTIONS\n -------\n \ndiff --git a/hook.c b/hook.c\nindex f8d1109084..a7dcd18a2e 100644\n--- a/hook.c\n+++ b/hook.c\n@@ -64,6 +64,13 @@ static int check_config_for_hooks(const char *var, const char *value, void *hook\n \t\t\t\templace_hook(pos, order, command);\n \t\t\t\tadded = 1;\n \t\t\t}\n+\n+\t\t\t/*\n+\t\t\t * if the command already exists, this entry should be\n+\t\t\t * replacing it.\n+\t\t\t */\n+\t\t\tif (!strcmp(item->command.buf, command))\n+\t\t\t\tremove_hook(pos);\n \t\t}\n \n \t\tif (!added)\ndiff --git a/t/t1360-config-based-hooks.sh b/t/t1360-config-based-hooks.sh\nindex 1434051db3..1af43ef18d 100755\n--- a/t/t1360-config-based-hooks.sh\n+++ b/t/t1360-config-based-hooks.sh\n@@ -47,4 +47,18 @@ test_expect_success 'order number collisions resolved in config order' '\n \ttest_cmp expected actual\n '\n \n+test_expect_success 'adding a command with a different number reorders list' '\n+\tcat >expected <<-\\EOF &&\n+\t010\trepo\t/path/abc\n+\t050\trepo\t/path/def\n+\t100\trepo\t/path/ghi\n+\t990\trepo\t/path/rst\n+\t999\tglobal\t/path/uvw\n+\tEOF\n+\n+\tgit config --add --local hook.pre-commit \"050:/path/def\" &&\n+\tgit hook --list pre-commit >actual &&\n+\ttest_cmp expected actual\n+'\n+\n test_done\n-- \n2.24.0.393.g34dc348eaf-goog\n\n"},{"id":"387868","messageId":"20191210023335.49987-6-emilyshaffer@google.com","threadId":"52420","inReplyTo":"20191210023335.49987-1-emilyshaffer@google.com","subject":"[PATCH 5/6] hook: remove prior hook with '---'","fromName":"Emily Shaffer","fromEmail":"emilyshaffer@google.com","sentAt":"2019-12-10T02:33:34Z","receivedAt":"2019-12-10T02:34:04Z","isPatch":true,"sender":{"key":"nasamuffin@google.com","avatar":"https://avatars.githubusercontent.com/u/1606826?v=4"},"body":"It's possible a user may want to run a hook for nearly every repo,\nexcept for one. Rather than requiring the user to specify the hook\nlocally in all but one repo, teach 'git hook' how to interpret config\nlines intended to remove hooks specified earlier during the config\nparse. This means a user can specify such a hook at the system or global\nlevel and override it at the local level.\n\nFor example:\n\n$ grep -A2 \"\\[hook\\]\" ~/.gitconfig\n[hook]\n        pre-commit = 001:~/test.sh\n        pre-commit = 999:~/baz.sh\n$ grep -A2 \"\\[hook\\]\" ~/git/.git/config\n[hook]\n        pre-commit = 900:~/bar.sh\n        pre-commit = ---:~/baz.sh\n$ ./bin-wrappers/git hook --list pre-commit\n001     global  ~/test.sh\n900     repo    ~/bar.sh\n\nSigned-off-by: Emily Shaffer <emilyshaffer@google.com>\n---\n Documentation/git-hook.txt    |  8 ++++++++\n hook.c                        | 15 ++++++++++-----\n t/t1360-config-based-hooks.sh | 13 +++++++++++++\n 3 files changed, 31 insertions(+), 5 deletions(-)\n\ndiff --git a/Documentation/git-hook.txt b/Documentation/git-hook.txt\nindex 0f7115f826..b4a992d43f 100644\n--- a/Documentation/git-hook.txt\n+++ b/Documentation/git-hook.txt\n@@ -28,6 +28,14 @@ The order number of a hook can be changed at a more local scope, e.g.:\n When the order number is respecified this way, the previously specified hook\n configuration is overridden.\n \n+A hook specified at a more global scope can be removed by specifying \"---\"\n+instead of an order number, e.g.:\n+\n+  git config --add --global hook.pre-commit \"001:/foo.sh\"\n+  git config --add --local hook.pre-commit \"---:/foo.sh\"\n+\n+When the hook is removed in this way, `/foo.sh` will not be run at all.\n+\n OPTIONS\n -------\n \ndiff --git a/hook.c b/hook.c\nindex a7dcd18a2e..e7afa140c8 100644\n--- a/hook.c\n+++ b/hook.c\n@@ -49,9 +49,14 @@ static int check_config_for_hooks(const char *var, const char *value, void *hook\n \t\t// TODO this is bad - open to overflows\n \t\tchar command[256];\n \t\tint added = 0;\n-\t\tif (!sscanf(value, \"%d:%s\", &order, command))\n-\t\t\tdie(_(\"hook config '%s' doesn't match expected format\"),\n-\t\t\t    value);\n+\t\tint remove = 0;\n+\t\tif (!sscanf(value, \"%d:%s\", &order, command)) {\n+\t\t\tif (sscanf(value, \"---:%s\", command))\n+\t\t\t\tremove = 1;\n+\t\t\telse\n+\t\t\t\tdie(_(\"hook config '%s' doesn't match expected format\"),\n+\t\t\t\t    value);\n+\t\t}\n \n \t\tlist_for_each_safe(pos, p, &hook_head) {\n \t\t\titem = list_entry(pos, struct hook, list);\n@@ -60,7 +65,7 @@ static int check_config_for_hooks(const char *var, const char *value, void *hook\n \t\t\t * the new entry should go just before the first entry\n \t\t\t * which has a higher order number than it.\n \t\t\t */\n-\t\t\tif (item->order > order && !added) {\n+\t\t\tif (item->order > order && !added && !remove) {\n \t\t\t\templace_hook(pos, order, command);\n \t\t\t\tadded = 1;\n \t\t\t}\n@@ -73,7 +78,7 @@ static int check_config_for_hooks(const char *var, const char *value, void *hook\n \t\t\t\tremove_hook(pos);\n \t\t}\n \n-\t\tif (!added)\n+\t\tif (!added && !remove)\n \t\t\templace_hook(pos, order, command);\n \t}\n \ndiff --git a/t/t1360-config-based-hooks.sh b/t/t1360-config-based-hooks.sh\nindex 1af43ef18d..66e70ae222 100755\n--- a/t/t1360-config-based-hooks.sh\n+++ b/t/t1360-config-based-hooks.sh\n@@ -61,4 +61,17 @@ test_expect_success 'adding a command with a different number reorders list' '\n \ttest_cmp expected actual\n '\n \n+test_expect_success 'remove a command with \"---:/path/to/cmd\"' '\n+\tcat >expected <<-\\EOF &&\n+\t010\trepo\t/path/abc\n+\t050\trepo\t/path/def\n+\t100\trepo\t/path/ghi\n+\t990\trepo\t/path/rst\n+\tEOF\n+\n+\tgit config --add --local hook.pre-commit \"---:/path/uvw\" &&\n+\tgit hook --list pre-commit >actual &&\n+\ttest_cmp expected actual\n+'\n+\n test_done\n-- \n2.24.0.393.g34dc348eaf-goog\n\n"},{"id":"387869","messageId":"20191210023335.49987-7-emilyshaffer@google.com","threadId":"52420","inReplyTo":"20191210023335.49987-1-emilyshaffer@google.com","subject":"[PATCH 6/6] hook: teach --porcelain mode","fromName":"Emily Shaffer","fromEmail":"emilyshaffer@google.com","sentAt":"2019-12-10T02:33:35Z","receivedAt":"2019-12-10T02:34:07Z","isPatch":true,"sender":{"key":"nasamuffin@google.com","avatar":"https://avatars.githubusercontent.com/u/1606826?v=4"},"body":"It might be desirable - for a user script, or a scripted Git command -\nto run the appropriate set of hooks from outside of the compiled Git\nbinary. So, teach --porcelain in a way that enables the following:\n\n  git hook --list --porcelain pre-commit | xargs -I% sh \"%\"\n\nSigned-off-by: Emily Shaffer <emilyshaffer@google.com>\n---\n Documentation/git-hook.txt    |  5 ++++-\n builtin/hook.c                | 19 +++++++++++++------\n t/t1360-config-based-hooks.sh | 12 ++++++++++++\n 3 files changed, 29 insertions(+), 7 deletions(-)\n\ndiff --git a/Documentation/git-hook.txt b/Documentation/git-hook.txt\nindex b4a992d43f..34276f5bce 100644\n--- a/Documentation/git-hook.txt\n+++ b/Documentation/git-hook.txt\n@@ -8,7 +8,7 @@ git-hook - Manage configured hooks\n SYNOPSIS\n --------\n [verse]\n-'git hook' -l | --list <hook-name>\n+'git hook' -l | --list [--porcelain] <hook-name>\n \n DESCRIPTION\n -----------\n@@ -45,6 +45,9 @@ OPTIONS\n \tin the order they should be run. Output of this command follows the\n \tformat '<order number> <origin config> <hook command>'.\n \n+--porcelain::\n+\tPrint in a machine-readable format suitable for scripting.\n+\n GIT\n ---\n Part of the linkgit:git[1] suite\ndiff --git a/builtin/hook.c b/builtin/hook.c\nindex 8261302b27..b76dd3ad8f 100644\n--- a/builtin/hook.c\n+++ b/builtin/hook.c\n@@ -16,7 +16,7 @@ enum hook_command {\n \tHOOK_LIST,\n };\n \n-static int print_hook_list(const struct strbuf *hookname)\n+static int print_hook_list(const struct strbuf *hookname, int porcelain)\n {\n \tstruct list_head *head, *pos;\n \tstruct hook *item;\n@@ -25,10 +25,14 @@ static int print_hook_list(const struct strbuf *hookname)\n \n \tlist_for_each(pos, head) {\n \t\titem = list_entry(pos, struct hook, list);\n-\t\tif (item)\n-\t\t\tprintf(\"%.3d\\t%s\\t%s\\n\", item->order,\n-\t\t\t       config_scope_to_string(item->origin),\n-\t\t\t       item->command.buf);\n+\t\tif (item) {\n+\t\t\tif (porcelain)\n+\t\t\t\tprintf(\"%s\\n\", item->command.buf);\n+\t\t\telse\n+\t\t\t\tprintf(\"%.3d\\t%s\\t%s\\n\", item->order,\n+\t\t\t\t       config_scope_to_string(item->origin),\n+\t\t\t\t       item->command.buf);\n+\t\t}\n \t}\n \n \treturn 0;\n@@ -38,11 +42,14 @@ int cmd_hook(int argc, const char **argv, const char *prefix)\n {\n \tenum hook_command command = 0;\n \tstruct strbuf hookname = STRBUF_INIT;\n+\tint porcelain = 0;\n \n \tstruct option builtin_hook_options[] = {\n \t\tOPT_CMDMODE('l', \"list\", &command,\n \t\t\t    N_(\"list scripts which will be run for <hookname>\"),\n \t\t\t    HOOK_LIST),\n+\t\tOPT_BOOL(0, \"porcelain\", &porcelain,\n+\t\t\t N_(\"display in machine parseable format\")),\n \t\tOPT_END(),\n \t};\n \n@@ -59,7 +66,7 @@ int cmd_hook(int argc, const char **argv, const char *prefix)\n \n \tswitch(command) {\n \t\tcase HOOK_LIST:\n-\t\t\treturn print_hook_list(&hookname);\n+\t\t\treturn print_hook_list(&hookname, porcelain);\n \t\t\tbreak;\n \t\tdefault:\n \t\t\tusage_msg_opt(\"no command given.\", builtin_hook_usage,\ndiff --git a/t/t1360-config-based-hooks.sh b/t/t1360-config-based-hooks.sh\nindex 66e70ae222..6f16ea1dd8 100755\n--- a/t/t1360-config-based-hooks.sh\n+++ b/t/t1360-config-based-hooks.sh\n@@ -33,6 +33,18 @@ test_expect_success 'git hook --list orders by order number' '\n \ttest_cmp expected actual\n '\n \n+test_expect_success 'git hook --list --porcelain' '\n+\tcat >expected <<-\\EOF &&\n+\t/path/def\n+\t/path/ghi\n+\t/path/rst\n+\t/path/uvw\n+\tEOF\n+\n+\tgit hook --list --porcelain pre-commit >actual &&\n+\ttest_cmp expected actual\n+'\n+\n test_expect_success 'order number collisions resolved in config order' '\n \tcat >expected <<-\\EOF &&\n \t010\tglobal\t/path/def\n-- \n2.24.0.393.g34dc348eaf-goog\n\n"},{"id":"387880","messageId":"e0bcedec-dd1c-9cb7-1b0f-a8fda0bbf610@iee.email","threadId":"52420","inReplyTo":"20191210023335.49987-3-emilyshaffer@google.com","subject":"Re: [PATCH 2/6] config: add string mapping for enum config_scope","fromName":"Philip Oakley","fromEmail":"philipoakley@iee.email","sentAt":"2019-12-10T11:16:44Z","receivedAt":"2019-12-10T11:16:49Z","isPatch":true,"sender":{"key":"philipoakley@iee.email","avatar":"https://avatars.githubusercontent.com/u/914343?v=4"},"body":"Hi Emily,\n\nOn 10/12/2019 02:33, Emily Shaffer wrote:\n> If a user is interacting with their config files primarily by the 'git\n> config' command, using the location flags (--global, --system, etc) then\n> they may be more interested to see the scope of the config file they are\n> editing, rather than the filepath.\nThere's asimilar issue being worked on under Git-for-Windows with some \nproposed code for this very 'problem'\nhttps://github.com/git-for-windows/git/pull/2399 and a GitGitGadget PR \nhttps://github.com/gitgitgadget/git/pull/478\n\ncc'ing Matthew to help coordination.\n\nPhilip\n>\n> Signed-off-by: Emily Shaffer <emilyshaffer@google.com>\n> ---\n>   config.c | 17 +++++++++++++++++\n>   config.h |  1 +\n>   2 files changed, 18 insertions(+)\n>\n> diff --git a/config.c b/config.c\n> index e7052b3977..a20110e016 100644\n> --- a/config.c\n> +++ b/config.c\n> @@ -3312,6 +3312,23 @@ enum config_scope current_config_scope(void)\n>   \t\treturn current_parsing_scope;\n>   }\n>   \n> +const char *config_scope_to_string(enum config_scope scope)\n> +{\n> +\tswitch (scope) {\n> +\tcase CONFIG_SCOPE_SYSTEM:\n> +\t\treturn _(\"system\");\n> +\tcase CONFIG_SCOPE_GLOBAL:\n> +\t\treturn _(\"global\");\n> +\tcase CONFIG_SCOPE_REPO:\n> +\t\treturn _(\"repo\");\n> +\tcase CONFIG_SCOPE_CMDLINE:\n> +\t\treturn _(\"cmdline\");\n> +\tcase CONFIG_SCOPE_UNKNOWN:\n> +\tdefault:\n> +\t\treturn _(\"unknown\");\n> +\t}\n> +}\n> +\n>   int lookup_config(const char **mapping, int nr_mapping, const char *var)\n>   {\n>   \tint i;\n> diff --git a/config.h b/config.h\n> index f0ed464004..612f43acd0 100644\n> --- a/config.h\n> +++ b/config.h\n> @@ -139,6 +139,7 @@ enum config_scope {\n>   };\n>   \n>   enum config_scope current_config_scope(void);\n> +const char *config_scope_to_string(enum config_scope);\n>   const char *current_config_origin_type(void);\n>   const char *current_config_name(void);\n>   \n\n"},{"id":"387900","messageId":"f5d109b0-edc3-914a-1b73-5bae8ddd9eee@iee.email","threadId":"52420","inReplyTo":"e0bcedec-dd1c-9cb7-1b0f-a8fda0bbf610@iee.email","subject":"Re: [PATCH 2/6] config: add string mapping for enum config_scope","fromName":"Philip Oakley","fromEmail":"philipoakley@iee.email","sentAt":"2019-12-10T17:21:36Z","receivedAt":"2019-12-10T17:21:39Z","isPatch":true,"sender":{"key":"philipoakley@iee.email","avatar":"https://avatars.githubusercontent.com/u/914343?v=4"},"body":"correcting Matt's email address.\noriginal thread \nhttps://lore.kernel.org/git/20191210023335.49987-3-emilyshaffer@google.com/\n\nOn 10/12/2019 11:16, Philip Oakley wrote:\n> On 10/12/2019 02:33, Emily Shaffer wrote:\n>> If a user is interacting with their config files primarily by the 'git\n>> config' command, using the location flags (--global, --system, etc) then\n>> they may be more interested to see the scope of the config file they are\n>> editing, rather than the filepath.\n> There's asimilar issue being worked on under Git-for-Windows with some \n> proposed code for this very 'problem'\n> https://github.com/git-for-windows/git/pull/2399 and a GitGitGadget PR \n> https://github.com/gitgitgadget/git/pull/478\n>\n> cc'ing Matthew to help coordination.\nPhilip\n"},{"id":"387969","messageId":"xmqqlfrioegv.fsf@gitster-ct.c.googlers.com","threadId":"52420","inReplyTo":"20191210023335.49987-5-emilyshaffer@google.com","subject":"Re: [PATCH 4/6] hook: support reordering of hook list","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2019-12-11T19:21:36Z","receivedAt":"2019-12-11T19:21:44Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Emily Shaffer <emilyshaffer@google.com> writes:\n\n>   $ grep -A2 \"\\[hook\\]\" ~/.gitconfig\n>   [hook]\n>           pre-commit = 001:~/test.sh\n>           pre-commit = 999:~/baz.sh\n>   $ grep -A2 \"\\[hook\\]\" ~/git/.git/config\n>   [hook]\n>           pre-commit = 900:~/bar.sh\n>           pre-commit = 050:~/baz.sh\n>   $ ./bin-wrappers/git hook --list pre-commit\n>   001     global  ~/test.sh\n>   050     repo    ~/baz.sh\n>   900     repo    ~/bar.sh\n>\n> In the above example, '~/baz.sh' is provided in the global config with\n> order position 999. Then, in the local config, that order is overridden\n> to 050. Instead of running ~/baz.sh twice (at order 050 and at order\n> 999), only run it once, in the position specified last in config order.\n\nDoesn't that depend on the nature of the hook?  A hook that is\ngeneral enough to be used to inspect if another hook's effect is\nsane and reject the result may want to be run after invocation of\neach hook that is not itself, so I would prefer to avoid a design\nthat forbids the same command to be specified twice.\n\nI would love it if it were possible without the precedence order and\ninstead the order of appearance in git_config() stream were usable\nto decide the order these hooks are executed.  Unfortunately, there\nis a fixed order that the configuration files are read, and I do not\nsee a way short of adding <number>: prefix like this design does to\nensure that a hook defined in the local config can run before or\nafter a hook defined in the global config, so <number>: in the above\ndesign is probably a necessary evil X-<.\n\nHaving said that, I have a suspicion that the config file itself\nshould be kept simple---if a hook appears twice with different\nnumbers, they would be run twice, for example---and the tooling\naround it (e.g. \"git hook add/edit/replace/reorder\") should\nimplement such a policy (e.g. \"the same hook can run only once, so\nremove the other entries when adding the same\") if desired.\n\nWhich would mean that overriding/disabling an entry in the same\nconfiguration file should be done by replacing or removing the\nentry.  Adding another entry for the same command with different\nprecedence should mean the command would run twice.\n\nAnd you would need a notation to override or disable an entry in a\ndifferent configuration file (e.g. global tells us to run foo.sh at\nlevel 50 with \"hook.pre-commit=50:foo.sh\"; repository wants to say\nnot to run it at all, or run it at 80 instead).  I would think you'd\njust need a notation to kill an existing entry (e.g. the local one\nadds \"hook.pre-commit=-50:foo.sh\" to countermand the entry in the\nearlier example, and then can add another one at level 80 if it\ndesires).\n\nI am also tempted to say that the precedence level may not stay to\nbe the only attribute for a <hookname, executable> pair wants to\nkeep.  Instead of\n\n\t[hook]\n                pre-commit = 900:bar.sh\n\nit may have to become more like\n\n\t[hook \"pre-commit\"]\n\t\tlevel = 900\n\t\tpath = bar.sh\n\nif we do not want to paint us into a corner from which we cannot get\nout of.  I dunno.\n\nDoesn't this require coordination between the three configuration\nsources how numbers are assigned and used, by the way?  Between the\nper-user and the per-repository config, they are set by the same\nperson anyway, so there is not much to coordinate, but I am not sure\nwhat the expectations are to allow reading from the system-wide\nconfiguration (or, should we just disable reading from the\nsystem-wide configuration for, say, security reasons?)\n"},{"id":"387971","messageId":"xmqqimmmodwt.fsf@gitster-ct.c.googlers.com","threadId":"52420","inReplyTo":"20191210023335.49987-7-emilyshaffer@google.com","subject":"Re: [PATCH 6/6] hook: teach --porcelain mode","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2019-12-11T19:33:38Z","receivedAt":"2019-12-11T19:33:46Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Emily Shaffer <emilyshaffer@google.com> writes:\n\n> It might be desirable - for a user script, or a scripted Git command -\n> to run the appropriate set of hooks from outside of the compiled Git\n> binary. So, teach --porcelain in a way that enables the following:\n>\n>   git hook --list --porcelain pre-commit | xargs -I% sh \"%\"\n>\n> Signed-off-by: Emily Shaffer <emilyshaffer@google.com>\n> ---\n\n> +--porcelain::\n> +\tPrint in a machine-readable format suitable for scripting.\n> +\n> ...\n> +static int print_hook_list(const struct strbuf *hookname, int porcelain)\n>  {\n>  \tstruct list_head *head, *pos;\n>  \tstruct hook *item;\n> @@ -25,10 +25,14 @@ static int print_hook_list(const struct strbuf *hookname)\n>  \n>  \tlist_for_each(pos, head) {\n>  \t\titem = list_entry(pos, struct hook, list);\n> +\t\tif (item) {\n> +\t\t\tif (porcelain)\n> +\t\t\t\tprintf(\"%s\\n\", item->command.buf);\n> +\t\t\telse\n> +\t\t\t\tprintf(\"%.3d\\t%s\\t%s\\n\", item->order,\n> +\t\t\t\t       config_scope_to_string(item->origin),\n> +\t\t\t\t       item->command.buf);\n> +\t\t}\n\nSo, a Porcelain script cannot learn where the hook command comes\nfrom, or what the precedence order of each line of the output is?\n\n"},{"id":"387982","messageId":"20191211220039.GB8464@google.com","threadId":"52420","inReplyTo":"xmqqimmmodwt.fsf@gitster-ct.c.googlers.com","subject":"Re: [PATCH 6/6] hook: teach --porcelain mode","fromName":"Emily Shaffer","fromEmail":"emilyshaffer@google.com","sentAt":"2019-12-11T22:00:39Z","receivedAt":"2019-12-11T22:00:46Z","isPatch":true,"sender":{"key":"nasamuffin@google.com","avatar":"https://avatars.githubusercontent.com/u/1606826?v=4"},"body":"On Wed, Dec 11, 2019 at 11:33:38AM -0800, Junio C Hamano wrote:\n> Emily Shaffer <emilyshaffer@google.com> writes:\n> \n> > It might be desirable - for a user script, or a scripted Git command -\n> > to run the appropriate set of hooks from outside of the compiled Git\n> > binary. So, teach --porcelain in a way that enables the following:\n> >\n> >   git hook --list --porcelain pre-commit | xargs -I% sh \"%\"\n> >\n> > Signed-off-by: Emily Shaffer <emilyshaffer@google.com>\n> > ---\n> \n> > +--porcelain::\n> > +\tPrint in a machine-readable format suitable for scripting.\n> > +\n> > ...\n> > +static int print_hook_list(const struct strbuf *hookname, int porcelain)\n> >  {\n> >  \tstruct list_head *head, *pos;\n> >  \tstruct hook *item;\n> > @@ -25,10 +25,14 @@ static int print_hook_list(const struct strbuf *hookname)\n> >  \n> >  \tlist_for_each(pos, head) {\n> >  \t\titem = list_entry(pos, struct hook, list);\n> > +\t\tif (item) {\n> > +\t\t\tif (porcelain)\n> > +\t\t\t\tprintf(\"%s\\n\", item->command.buf);\n> > +\t\t\telse\n> > +\t\t\t\tprintf(\"%.3d\\t%s\\t%s\\n\", item->order,\n> > +\t\t\t\t       config_scope_to_string(item->origin),\n> > +\t\t\t\t       item->command.buf);\n> > +\t\t}\n> \n> So, a Porcelain script cannot learn where the hook command comes\n> from,\n\nNot as I had envisioned.\n\n> or what the precedence order of each line of the output is?\n> \n\nThey're printed in the order they should be executed; the explicit order\nisn't provided.\n\n\nI suppose I had considered really just the one use case listed in the\ncommit message, especially since other inquiry into the hooks to be run\ncan be done against the config files themselves. But - I'm of course\nopen to use cases. What did you have in mind?\n\nMaybe this can be solved better with a --pretty=format type of argument.\n"},{"id":"387984","messageId":"xmqqwob2ms7i.fsf@gitster-ct.c.googlers.com","threadId":"52420","inReplyTo":"20191211220039.GB8464@google.com","subject":"Re: [PATCH 6/6] hook: teach --porcelain mode","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2019-12-11T22:07:45Z","receivedAt":"2019-12-11T22:07:56Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Emily Shaffer <emilyshaffer@google.com> writes:\n\n>> So, a Porcelain script cannot learn where the hook command comes\n>> from,\n>\n> Not as I had envisioned.\n>\n>> or what the precedence order of each line of the output is?\n>> \n>\n> They're printed in the order they should be executed; the explicit order\n> isn't provided.\n>\n>\n> I suppose I had considered really just the one use case listed in the\n> commit message, especially since other inquiry into the hooks to be run\n> can be done against the config files themselves. But - I'm of course\n> open to use cases. What did you have in mind?\n\nA tool to diagnose why the hooks are not firing in the order the\nuser intended them to, for example?\n\nOr a tool to help editing the list of hooks.\n\n"},{"id":"387988","messageId":"xmqqo8wemqlr.fsf@gitster-ct.c.googlers.com","threadId":"52420","inReplyTo":"20191210023335.49987-1-emilyshaffer@google.com","subject":"Re: [PATCH 0/6] configuration-based hook management","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2019-12-11T22:42:24Z","receivedAt":"2019-12-11T22:42:33Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Emily Shaffer <emilyshaffer@google.com> writes:\n\n> An implementation of the first piece of the proposal given in\n> lore.kernel.org/git/20191116011125.GG22855@google.com.\n>\n> Teaches a new command, 'git hook', which will someday include 'git hook\n> --add ...', 'git hook --edit ...', and maybe more. For now, just teach\n> it how to check the config files with 'git hook --list ...'.\n>\n> The hooks-to-run list is collected in a new library, hook.o, which can\n> someday reimplement find_hook() or otherwise be invoked to run all hooks\n> for a given hookname (e.g. \"pre-commit\").\n\nNice to see the endgame vision upfront.\n\nA few things that I'd like to see in the endgame that you did not\nmention here are:\n\n - We may probably not want to have an authoritative \"these are the\n   hooks Git runs\" catalog, so it would be great if the resulting\n   system can operate without one.\n\n - There are at least two kinds of hooks wrt the style of input they\n   take.  Some take their input on their command line, which makes\n   it quite easy to run multiple of them in a row.  Others take\n   their input from their standard input stream, which probably\n   means that there needs a cache of the input stream to feed to\n   each such hook script (unless Git process itself is generating\n   the stream to drive the hook, in which case we could run the\n   generation of the stream multiple times) if we want to run\n   multiple of them.  \n\n   . With the design goal of *not* having an authoritiative catalog,\n     we'd probably need some way to annotate each entry in the [hook]\n     configuration which kind of invication the hook program wants.\n\n   . There may be more than the above two styles.  The system should\n     be designed to be extensible to accomodate yet more.\n\n\nThanks.\n"},{"id":"387991","messageId":"20191211231535.GD8464@google.com","threadId":"52420","inReplyTo":"xmqqwob2ms7i.fsf@gitster-ct.c.googlers.com","subject":"Re: [PATCH 6/6] hook: teach --porcelain mode","fromName":"Emily Shaffer","fromEmail":"emilyshaffer@google.com","sentAt":"2019-12-11T23:15:35Z","receivedAt":"2019-12-11T23:15:44Z","isPatch":true,"sender":{"key":"nasamuffin@google.com","avatar":"https://avatars.githubusercontent.com/u/1606826?v=4"},"body":"On Wed, Dec 11, 2019 at 02:07:45PM -0800, Junio C Hamano wrote:\n> Emily Shaffer <emilyshaffer@google.com> writes:\n> \n> >> So, a Porcelain script cannot learn where the hook command comes\n> >> from,\n> >\n> > Not as I had envisioned.\n> >\n> >> or what the precedence order of each line of the output is?\n> >> \n> >\n> > They're printed in the order they should be executed; the explicit order\n> > isn't provided.\n> >\n> >\n> > I suppose I had considered really just the one use case listed in the\n> > commit message, especially since other inquiry into the hooks to be run\n> > can be done against the config files themselves. But - I'm of course\n> > open to use cases. What did you have in mind?\n> \n> A tool to diagnose why the hooks are not firing in the order the\n> user intended them to, for example?\n> \n> Or a tool to help editing the list of hooks.\nFWIW, the next step for this 'git hook' tool is just such a mode,\nalthough I certainly won't argue with anybody who wants to interact with\nthem somewhat differently.\n\nDoes allowing a format string solve this, then? Maybe it's less\nGit-idiomatic, but it seems to me to be a very explicit format contract\nthat the scripter can write, and probably more useful than guessing what\ninfo one might want when scripting. It also doesn't paint us into a\ncorner if we add other interesting info later.\n\nUnless you have a complaint about it, I'll try to add that kind of\nargument instead of --porcelain for this command.\n\n - Emily\n"},{"id":"388007","messageId":"CAKPyHN04hEK7PKZQvF7LkGPkbBnV2jccBAxJSiukrXb=4g4g_Q@mail.gmail.com","threadId":"52420","inReplyTo":"20191210023335.49987-4-emilyshaffer@google.com","subject":"Re: [PATCH 3/6] hook: add --list mode","fromName":"Bert Wesarg","fromEmail":"bert.wesarg@googlemail.com","sentAt":"2019-12-12T09:38:55Z","receivedAt":"2019-12-12T09:39:10Z","isPatch":true,"sender":{"key":"bert.wesarg@googlemail.com","avatar":"https://avatars.githubusercontent.com/u/111934?v=4"},"body":"On Tue, Dec 10, 2019 at 3:34 AM Emily Shaffer <emilyshaffer@google.com> wrote:\n>\n> Teach 'git hook --list <hookname>', which checks the known configs in\n> order to create an ordered list of hooks to run on a given hook event.\n>\n> The hook config format is \"hook.<hookname> = <order>:<path-to-hook>\".\n> This paves the way for multiple hook support; hooks should be run in the\n> order specified by the user in the config, and in the case of an order\n> number collision, configuration order should be used (e.g. global hook\n> 004 will run before repo hook 004).\n>\n> For example:\n>\n>   $ grep -A2 \"\\[hook\\]\" ~/.gitconfig\n>   [hook]\n>           pre-commit = 001:~/test.sh\n>           pre-commit = 999:~/baz.sh\n>\n>   $ grep -A1 \"\\[hook\\]\" ~/git/.git/config\n>   [hook]\n>           pre-commit = 900:~/bar.sh\n>\n>   $ ./bin-wrappers/git hook --list pre-commit\n>   001     global  ~/test.sh\n>   900     repo    ~/bar.sh\n>   999     global  ~/baz.sh\n>\n> Signed-off-by: Emily Shaffer <emilyshaffer@google.com>\n> ---\n>  Documentation/git-hook.txt    | 17 +++++++-\n>  Makefile                      |  1 +\n>  builtin/hook.c                | 54 ++++++++++++++++++++++-\n>  hook.c                        | 81 +++++++++++++++++++++++++++++++++++\n>  hook.h                        | 14 ++++++\n>  t/t1360-config-based-hooks.sh | 43 ++++++++++++++++++-\n>  6 files changed, 206 insertions(+), 4 deletions(-)\n>  create mode 100644 hook.c\n>  create mode 100644 hook.h\n>\n> diff --git a/Documentation/git-hook.txt b/Documentation/git-hook.txt\n> index 2d50c414cc..a141884239 100644\n> --- a/Documentation/git-hook.txt\n> +++ b/Documentation/git-hook.txt\n> @@ -8,12 +8,27 @@ git-hook - Manage configured hooks\n>  SYNOPSIS\n>  --------\n>  [verse]\n> -'git hook'\n> +'git hook' -l | --list <hook-name>\n>\n>  DESCRIPTION\n>  -----------\n>  You can list, add, and modify hooks with this command.\n>\n> +This command parses the default configuration files for lines which look like\n> +\"hook.<hook-name> = <order number>:<hook command>\", e.g. \"hook.pre-commit =\n> +010:/path/to/script.sh\". In this way, multiple scripts can be run during a\n> +single hook. Hooks are sorted in ascending order by order number; in the event\n> +of an order number conflict, they are sorted in configuration order.\n> +\n> +OPTIONS\n> +-------\n> +\n> +-l::\n> +--list::\n> +       List the hooks which have been configured for <hook-name>. Hooks appear\n> +       in the order they should be run. Output of this command follows the\n> +       format '<order number> <origin config> <hook command>'.\n> +\n>  GIT\n>  ---\n>  Part of the linkgit:git[1] suite\n> diff --git a/Makefile b/Makefile\n> index 83263505c0..21b3a82208 100644\n> --- a/Makefile\n> +++ b/Makefile\n> @@ -892,6 +892,7 @@ LIB_OBJS += hashmap.o\n>  LIB_OBJS += linear-assignment.o\n>  LIB_OBJS += help.o\n>  LIB_OBJS += hex.o\n> +LIB_OBJS += hook.o\n>  LIB_OBJS += ident.o\n>  LIB_OBJS += interdiff.o\n>  LIB_OBJS += json-writer.o\n> diff --git a/builtin/hook.c b/builtin/hook.c\n> index b2bbc84d4d..8261302b27 100644\n> --- a/builtin/hook.c\n> +++ b/builtin/hook.c\n> @@ -1,21 +1,73 @@\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> -       N_(\"git hook\"),\n> +       N_(\"git hook --list <hookname>\"),\n\nIts \"<hook-name>\" in Documentation/git-hook.txt\n\n>         NULL\n>  };\n>\n> +enum hook_command {\n> +       HOOK_NO_COMMAND = 0,\n> +       HOOK_LIST,\n> +};\n> +\n> +static int print_hook_list(const struct strbuf *hookname)\n> +{\n> +       struct list_head *head, *pos;\n> +       struct hook *item;\n> +\n> +       head = hook_list(hookname);\n> +\n> +       list_for_each(pos, head) {\n> +               item = list_entry(pos, struct hook, list);\n> +               if (item)\n> +                       printf(\"%.3d\\t%s\\t%s\\n\", item->order,\n> +                              config_scope_to_string(item->origin),\n> +                              item->command.buf);\n> +       }\n> +\n> +       return 0;\n> +}\n> +\n>  int cmd_hook(int argc, const char **argv, const char *prefix)\n>  {\n> +       enum hook_command command = 0;\n> +       struct strbuf hookname = STRBUF_INIT;\n> +\n>         struct option builtin_hook_options[] = {\n> +               OPT_CMDMODE('l', \"list\", &command,\n> +                           N_(\"list scripts which will be run for <hookname>\"),\n\n\nIts \"<hook-name>\" in Documentation/git-hook.txt\n> +                           HOOK_LIST),\n>                 OPT_END(),\n>         };\n>\n>         argc = parse_options(argc, argv, prefix, builtin_hook_options,\n>                              builtin_hook_usage, 0);\n>\n> +       if (argc < 1) {\n> +               usage_msg_opt(\"a hookname must be provided to operate on.\",\n> +                             builtin_hook_usage, builtin_hook_options);\n> +       }\n> +\n> +       strbuf_addstr(&hookname, \"hook.\");\n> +       strbuf_addstr(&hookname, argv[0]);\n\nThe arg is never checked, if this is a valid/known hook.\n\nBert\n\n> +\n> +       switch(command) {\n> +               case HOOK_LIST:\n> +                       return print_hook_list(&hookname);\n> +                       break;\n> +               default:\n> +                       usage_msg_opt(\"no command given.\", builtin_hook_usage,\n> +                                     builtin_hook_options);\n> +       }\n> +\n> +       clear_hook_list();\n> +       strbuf_release(&hookname);\n> +\n>         return 0;\n>  }\n> diff --git a/hook.c b/hook.c\n> new file mode 100644\n> index 0000000000..f8d1109084\n> --- /dev/null\n> +++ b/hook.c\n> @@ -0,0 +1,81 @@\n> +#include \"cache.h\"\n> +\n> +#include \"hook.h\"\n> +#include \"config.h\"\n> +\n> +static LIST_HEAD(hook_head);\n> +\n> +void free_hook(struct hook *ptr)\n> +{\n> +       if (ptr) {\n> +               strbuf_release(&ptr->command);\n> +               free(ptr);\n> +       }\n> +}\n> +\n> +static void emplace_hook(struct list_head *pos, int order, const char *command)\n> +{\n> +       struct hook *to_add = malloc(sizeof(struct hook));\n> +       to_add->order = order;\n> +       to_add->origin = current_config_scope();\n> +       strbuf_init(&to_add->command, 0);\n> +       strbuf_addstr(&to_add->command, command);\n> +\n> +       list_add_tail(&to_add->list, pos);\n> +}\n> +\n> +static void remove_hook(struct list_head *to_remove)\n> +{\n> +       struct hook *hook_to_remove = list_entry(to_remove, struct hook, list);\n> +       list_del(to_remove);\n> +       free_hook(hook_to_remove);\n> +}\n> +\n> +void clear_hook_list()\n> +{\n> +       struct list_head *pos, *tmp;\n> +       list_for_each_safe(pos, tmp, &hook_head)\n> +               remove_hook(pos);\n> +}\n> +\n> +static int check_config_for_hooks(const char *var, const char *value, void *hookname)\n> +{\n> +       struct list_head *pos, *p;\n> +       struct hook *item;\n> +       const struct strbuf *hookname_strbuf = hookname;\n> +\n> +       if (!strcmp(var, hookname_strbuf->buf)) {\n> +               int order = 0;\n> +               // TODO this is bad - open to overflows\n> +               char command[256];\n> +               int added = 0;\n> +               if (!sscanf(value, \"%d:%s\", &order, command))\n> +                       die(_(\"hook config '%s' doesn't match expected format\"),\n> +                           value);\n> +\n> +               list_for_each_safe(pos, p, &hook_head) {\n> +                       item = list_entry(pos, struct hook, list);\n> +\n> +                       /*\n> +                        * the new entry should go just before the first entry\n> +                        * which has a higher order number than it.\n> +                        */\n> +                       if (item->order > order && !added) {\n> +                               emplace_hook(pos, order, command);\n> +                               added = 1;\n> +                       }\n> +               }\n> +\n> +               if (!added)\n> +                       emplace_hook(pos, order, command);\n> +       }\n> +\n> +       return 0;\n> +}\n> +\n> +struct list_head* hook_list(const struct strbuf* hookname)\n> +{\n> +       git_config(check_config_for_hooks, (void*)hookname);\n> +\n> +       return &hook_head;\n> +}\n> diff --git a/hook.h b/hook.h\n> new file mode 100644\n> index 0000000000..104df4c088\n> --- /dev/null\n> +++ b/hook.h\n> @@ -0,0 +1,14 @@\n> +#include \"config.h\"\n> +\n> +struct hook\n> +{\n> +       struct list_head list;\n> +       int order;\n> +       enum config_scope origin;\n> +       struct strbuf command;\n> +};\n> +\n> +struct list_head* hook_list(const struct strbuf *hookname);\n> +\n> +void free_hook(struct hook *ptr);\n> +void clear_hook_list();\n> diff --git a/t/t1360-config-based-hooks.sh b/t/t1360-config-based-hooks.sh\n> index 34b0df5216..1434051db3 100755\n> --- a/t/t1360-config-based-hooks.sh\n> +++ b/t/t1360-config-based-hooks.sh\n> @@ -4,8 +4,47 @@ 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> -       git hook\n> +test_expect_success 'git hook rejects commands without a mode' '\n> +       test_must_fail git hook pre-commit\n> +'\n> +\n> +\n> +test_expect_success 'git hook rejects commands without a hookname' '\n> +       test_must_fail git hook --list\n> +'\n> +\n> +test_expect_success 'setup hooks in system, global, and local' '\n> +       git config --add --global hook.pre-commit \"010:/path/def\" &&\n> +       git config --add --global hook.pre-commit \"999:/path/uvw\" &&\n> +\n> +       git config --add --local hook.pre-commit \"100:/path/ghi\" &&\n> +       git config --add --local hook.pre-commit \"990:/path/rst\"\n> +'\n> +\n> +test_expect_success 'git hook --list orders by order number' '\n> +       cat >expected <<-\\EOF &&\n> +       010     global  /path/def\n> +       100     repo    /path/ghi\n> +       990     repo    /path/rst\n> +       999     global  /path/uvw\n> +       EOF\n> +\n> +       git hook --list pre-commit >actual &&\n> +       test_cmp expected actual\n> +'\n> +\n> +test_expect_success 'order number collisions resolved in config order' '\n> +       cat >expected <<-\\EOF &&\n> +       010     global  /path/def\n> +       010     repo    /path/abc\n> +       100     repo    /path/ghi\n> +       990     repo    /path/rst\n> +       999     global  /path/uvw\n> +       EOF\n> +\n> +       git config --add --local hook.pre-commit \"010:/path/abc\" &&\n> +       git hook --list pre-commit >actual &&\n> +       test_cmp expected actual\n>  '\n>\n>  test_done\n> --\n> 2.24.0.393.g34dc348eaf-goog\n>\n"},{"id":"388008","messageId":"CAKPyHN19uDdredXyGv63sHJKKeUue0t9nM6e31RVPYNYfd=XmQ@mail.gmail.com","threadId":"52420","inReplyTo":"20191210023335.49987-2-emilyshaffer@google.com","subject":"Re: [PATCH 1/6] hook: scaffolding for git-hook subcommand","fromName":"Bert Wesarg","fromEmail":"bert.wesarg@googlemail.com","sentAt":"2019-12-12T09:41:28Z","receivedAt":"2019-12-12T09:41:43Z","isPatch":true,"sender":{"key":"bert.wesarg@googlemail.com","avatar":"https://avatars.githubusercontent.com/u/111934?v=4"},"body":"On Tue, Dec 10, 2019 at 3:34 AM Emily Shaffer <emilyshaffer@google.com> wrote:\n>\n> Introduce infrastructure for a new subcommand, git-hook, which will be\n> used to ease config-based hook management. This command will handle\n> parsing configs to compose a list of hooks to run for a given event, as\n> well as adding or modifying hook configs in an interactive fashion.\n>\n> Signed-off-by: Emily Shaffer <emilyshaffer@google.com>\n> ---\n>  .gitignore                    |  1 +\n>  Documentation/git-hook.txt    | 19 +++++++++++++++++++\n>  Makefile                      |  1 +\n>  builtin.h                     |  1 +\n>  builtin/hook.c                | 21 +++++++++++++++++++++\n>  git.c                         |  1 +\n\nhow about adding also completion support here?\n\nBert\n\n>  t/t1360-config-based-hooks.sh | 11 +++++++++++\n>  7 files changed, 55 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>\n> diff --git a/.gitignore b/.gitignore\n> index 89b3b79c1a..9ef59b9baa 100644\n> --- a/.gitignore\n> +++ b/.gitignore\n> @@ -74,6 +74,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\n> diff --git a/Documentation/git-hook.txt b/Documentation/git-hook.txt\n> new file mode 100644\n> index 0000000000..2d50c414cc\n> --- /dev/null\n> +++ b/Documentation/git-hook.txt\n> @@ -0,0 +1,19 @@\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> +You can list, add, and modify hooks with this command.\n> +\n> +GIT\n> +---\n> +Part of the linkgit:git[1] suite\n> diff --git a/Makefile b/Makefile\n> index 58b92af54b..83263505c0 100644\n> --- a/Makefile\n> +++ b/Makefile\n> @@ -1074,6 +1074,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\n> diff --git a/builtin.h b/builtin.h\n> index 5cf5df69f7..d4ca2ac9a5 100644\n> --- a/builtin.h\n> +++ b/builtin.h\n> @@ -173,6 +173,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);\n> diff --git a/builtin/hook.c b/builtin/hook.c\n> new file mode 100644\n> index 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> +       N_(\"git hook\"),\n> +       NULL\n> +};\n> +\n> +int cmd_hook(int argc, const char **argv, const char *prefix)\n> +{\n> +       struct option builtin_hook_options[] = {\n> +               OPT_END(),\n> +       };\n> +\n> +       argc = parse_options(argc, argv, prefix, builtin_hook_options,\n> +                            builtin_hook_usage, 0);\n> +\n> +       return 0;\n> +}\n> diff --git a/git.c b/git.c\n> index ce6ab0ece2..c8344b9ab7 100644\n> --- a/git.c\n> +++ b/git.c\n> @@ -513,6 +513,7 @@ static struct cmd_struct commands[] = {\n>         { \"grep\", cmd_grep, RUN_SETUP_GENTLY },\n>         { \"hash-object\", cmd_hash_object },\n>         { \"help\", cmd_help },\n> +       { \"hook\", cmd_hook, RUN_SETUP },\n>         { \"index-pack\", cmd_index_pack, RUN_SETUP_GENTLY | NO_PARSEOPT },\n>         { \"init\", cmd_init_db },\n>         { \"init-db\", cmd_init_db },\n> diff --git a/t/t1360-config-based-hooks.sh b/t/t1360-config-based-hooks.sh\n> new file mode 100755\n> index 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> +       git hook\n> +'\n> +\n> +test_done\n> --\n> 2.24.0.393.g34dc348eaf-goog\n>\n"},{"id":"388009","messageId":"20191212104701.GI6527@szeder.dev","threadId":"52420","inReplyTo":"20191210023335.49987-2-emilyshaffer@google.com","subject":"Re: [PATCH 1/6] hook: scaffolding for git-hook subcommand","fromName":"SZEDER Gábor","fromEmail":"szeder.dev@gmail.com","sentAt":"2019-12-12T10:47:01Z","receivedAt":"2019-12-12T10:47:08Z","isPatch":true,"sender":{"key":"szeder.dev@gmail.com","avatar":"https://avatars.githubusercontent.com/u/116324?v=4"},"body":"On Mon, Dec 09, 2019 at 06:33:30PM -0800, Emily Shaffer wrote:\n> Introduce infrastructure for a new subcommand, git-hook, which will be\n> used to ease config-based hook management. This command will handle\n> parsing configs to compose a list of hooks to run for a given event, as\n> well as adding or modifying hook configs in an interactive fashion.\n> \n> Signed-off-by: Emily Shaffer <emilyshaffer@google.com>\n> ---\n>  .gitignore                    |  1 +\n>  Documentation/git-hook.txt    | 19 +++++++++++++++++++\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, 55 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\nWhen adding a new command please don't forget the steps noted in\n4ed5562925 (myfirstcontrib: add 'psuh' to command-list.txt,\n2019-10-31) ;)\n\n\n> diff --git a/t/t1360-config-based-hooks.sh b/t/t1360-config-based-hooks.sh\n> new file mode 100755\n> index 0000000000..34b0df5216\n> --- /dev/null\n> +++ b/t/t1360-config-based-hooks.sh\n> @@ -0,0 +1,11 @@\n> +#!/bin/bash\n\ns/ba//\n\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> -- \n> 2.24.0.393.g34dc348eaf-goog\n> \n"},{"id":"388010","messageId":"20191212105855.GJ6527@szeder.dev","threadId":"52420","inReplyTo":"20191210023335.49987-4-emilyshaffer@google.com","subject":"Re: [PATCH 3/6] hook: add --list mode","fromName":"SZEDER Gábor","fromEmail":"szeder.dev@gmail.com","sentAt":"2019-12-12T10:58:55Z","receivedAt":"2019-12-12T10:59:01Z","isPatch":true,"sender":{"key":"szeder.dev@gmail.com","avatar":"https://avatars.githubusercontent.com/u/116324?v=4"},"body":"On Mon, Dec 09, 2019 at 06:33:32PM -0800, Emily Shaffer wrote:\n> Teach 'git hook --list <hookname>', which checks the known configs in\n> order to create an ordered list of hooks to run on a given hook event.\n> \n> The hook config format is \"hook.<hookname> = <order>:<path-to-hook>\".\n> This paves the way for multiple hook support; hooks should be run in the\n> order specified by the user in the config, and in the case of an order\n> number collision, configuration order should be used (e.g. global hook\n> 004 will run before repo hook 004).\n> \n> For example:\n> \n>   $ grep -A2 \"\\[hook\\]\" ~/.gitconfig\n>   [hook]\n>           pre-commit = 001:~/test.sh\n>           pre-commit = 999:~/baz.sh\n> \n>   $ grep -A1 \"\\[hook\\]\" ~/git/.git/config\n>   [hook]\n>           pre-commit = 900:~/bar.sh\n> \n>   $ ./bin-wrappers/git hook --list pre-commit\n>   001     global  ~/test.sh\n>   900     repo    ~/bar.sh\n>   999     global  ~/baz.sh\n> \n> Signed-off-by: Emily Shaffer <emilyshaffer@google.com>\n> ---\n>  Documentation/git-hook.txt    | 17 +++++++-\n>  Makefile                      |  1 +\n>  builtin/hook.c                | 54 ++++++++++++++++++++++-\n>  hook.c                        | 81 +++++++++++++++++++++++++++++++++++\n>  hook.h                        | 14 ++++++\n>  t/t1360-config-based-hooks.sh | 43 ++++++++++++++++++-\n>  6 files changed, 206 insertions(+), 4 deletions(-)\n>  create mode 100644 hook.c\n>  create mode 100644 hook.h\n> \n> diff --git a/Documentation/git-hook.txt b/Documentation/git-hook.txt\n> index 2d50c414cc..a141884239 100644\n> --- a/Documentation/git-hook.txt\n> +++ b/Documentation/git-hook.txt\n> @@ -8,12 +8,27 @@ git-hook - Manage configured hooks\n>  SYNOPSIS\n>  --------\n>  [verse]\n> -'git hook'\n> +'git hook' -l | --list <hook-name>\n>  \n>  DESCRIPTION\n>  -----------\n>  You can list, add, and modify hooks with this command.\n>  \n> +This command parses the default configuration files for lines which look like\n> +\"hook.<hook-name> = <order number>:<hook command>\", e.g. \"hook.pre-commit =\n> +010:/path/to/script.sh\". In this way, multiple scripts can be run during a\n> +single hook. Hooks are sorted in ascending order by order number; in the event\n> +of an order number conflict, they are sorted in configuration order.\n> +\n> +OPTIONS\n> +-------\n> +\n> +-l::\n> +--list::\n> +\tList the hooks which have been configured for <hook-name>. Hooks appear\n> +\tin the order they should be run. Output of this command follows the\n> +\tformat '<order number> <origin config> <hook command>'.\n> +\n>  GIT\n>  ---\n>  Part of the linkgit:git[1] suite\n\n> diff --git a/builtin/hook.c b/builtin/hook.c\n> index b2bbc84d4d..8261302b27 100644\n> --- a/builtin/hook.c\n> +++ b/builtin/hook.c\n> @@ -1,21 +1,73 @@\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> +enum hook_command {\n> +\tHOOK_NO_COMMAND = 0,\n> +\tHOOK_LIST,\n> +};\n> +\n> +static int print_hook_list(const struct strbuf *hookname)\n> +{\n> +\tstruct list_head *head, *pos;\n> +\tstruct hook *item;\n> +\n> +\thead = hook_list(hookname);\n> +\n> +\tlist_for_each(pos, head) {\n> +\t\titem = list_entry(pos, struct hook, list);\n> +\t\tif (item)\n> +\t\t\tprintf(\"%.3d\\t%s\\t%s\\n\", item->order,\n> +\t\t\t       config_scope_to_string(item->origin),\n> +\t\t\t       item->command.buf);\n> +\t}\n> +\n> +\treturn 0;\n> +}\n> +\n>  int cmd_hook(int argc, const char **argv, const char *prefix)\n>  {\n> +\tenum hook_command command = 0;\n> +\tstruct strbuf hookname = STRBUF_INIT;\n> +\n>  \tstruct option builtin_hook_options[] = {\n> +\t\tOPT_CMDMODE('l', \"list\", &command,\n> +\t\t\t    N_(\"list scripts which will be run for <hookname>\"),\n> +\t\t\t    HOOK_LIST),\n\nI'm not sure about '--list' being an option.  I don't know what other\noperations you have in mind for this 'git hook' command, but I suppose\nthat besides listing configured hooks it will be able to at least add,\nremove, and reorder them as well.  These seem to be better implemented\nas subcommands, along the lines of e.g. how notes and remotes can be\nadded, removed, etc.\n\n>  \t\tOPT_END(),\n>  \t};\n"}]}