{"thread":{"id":"49262","subject":"Feature request: hooks directory","startedAt":"2018-08-30T13:20:05Z","lastAt":"2018-09-03T08:50:30Z","messageCount":8,"participants":["Wesley Schwengle","Ævar Arnfjörð Bjarmason","Jonathan Nieder","Christian Couder"],"isPatch":false,"patchVersion":null,"patchTotal":null},"messages":[{"id":"356971","messageId":"CAEpdsiYHri8FJ8VohnwxmPwDM4-0J4J9Zb9wTZjYYRnYqsb=nA@mail.gmail.com","threadId":"49262","inReplyTo":null,"subject":"Feature request: hooks directory","fromName":"Wesley Schwengle","fromEmail":"wesley@mintlab.nl","sentAt":"2018-08-30T13:20:03Z","receivedAt":"2018-08-30T13:20:05Z","isPatch":false,"sender":{"key":"wesley@mintlab.nl","avatar":null},"body":"Hello all,\n\nI would like to ask if it is worth my time looking into the following\nsolution to a problem we have at work.\n\nProblem:\nWe want to have some git-hooks and we want to provide them to the\nuser. In a most recent example we have a post-checkout hook that deals\nwith some Docker things. However, if we update that post-checkout hook\nmy local overrides in that post-checkout hook are going to be\noverwritten.\n\nSolution:\nWe discussed this at work and we thought about making a .d directory\nfor the hooks, eg. $GIT_DIR/hooks/post-commit.d, where a user can put\nthe post-commit hooks in. This allows us to provide post commit hooks\nand allows the user to add additional hooks him/herself. We could\nimplement this in our own code base. But we were wondering if this\napproach could be shared with the git community and if this behavior\nis wanted in git itself.\n\n\nCheers,\nWesley\n\n-- \nWesley Schwengle, Developer\nMintlab B.V., https://www.zaaksysteem.nl\nE: wesley@mintlab.nl\nT: +31 20 737 00 05\n"},{"id":"356977","messageId":"87pnxzdib7.fsf@evledraar.gmail.com","threadId":"49262","inReplyTo":"CAEpdsiYHri8FJ8VohnwxmPwDM4-0J4J9Zb9wTZjYYRnYqsb=nA@mail.gmail.com","subject":"Re: Feature request: hooks directory","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2018-08-30T14:45:32Z","receivedAt":"2018-08-30T14:45:37Z","isPatch":false,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"\nOn Thu, Aug 30 2018, Wesley Schwengle wrote:\n\n> Hello all,\n>\n> I would like to ask if it is worth my time looking into the following\n> solution to a problem we have at work.\n>\n> Problem:\n> We want to have some git-hooks and we want to provide them to the\n> user. In a most recent example we have a post-checkout hook that deals\n> with some Docker things. However, if we update that post-checkout hook\n> my local overrides in that post-checkout hook are going to be\n> overwritten.\n>\n> Solution:\n> We discussed this at work and we thought about making a .d directory\n> for the hooks, eg. $GIT_DIR/hooks/post-commit.d, where a user can put\n> the post-commit hooks in. This allows us to provide post commit hooks\n> and allows the user to add additional hooks him/herself. We could\n> implement this in our own code base. But we were wondering if this\n> approach could be shared with the git community and if this behavior\n> is wanted in git itself.\n\nThere is interest in this. This E-Mail of mine gives a good summary of\nprior discussions about this:\nhttps://public-inbox.org/git/877eqqnq22.fsf@evledraar.gmail.com/\n\nI.e. it's something I've personally been interested in doing in the\npast, there's various bolt-on solutions to do it (basically local hook\nrunners) used by various projects.\n"},{"id":"357060","messageId":"20180831031654.GB98946@aiede.svl.corp.google.com","threadId":"49262","inReplyTo":"87pnxzdib7.fsf@evledraar.gmail.com","subject":"Re: Feature request: hooks directory","fromName":"Jonathan Nieder","fromEmail":"jrnieder@gmail.com","sentAt":"2018-08-31T03:16:54Z","receivedAt":"2018-08-31T03:17:00Z","isPatch":false,"sender":{"key":"jrnieder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/281595?v=4"},"body":"Ævar Arnfjörð Bjarmason wrote:\n\n> There is interest in this. This E-Mail of mine gives a good summary of\n> prior discussions about this:\n> https://public-inbox.org/git/877eqqnq22.fsf@evledraar.gmail.com/\n>\n> I.e. it's something I've personally been interested in doing in the\n> past, there's various bolt-on solutions to do it (basically local hook\n> runners) used by various projects.\n\nA few unrelated thoughts, to expand on this.\n\nReports of experience from using local hook runners would be very\nwelcome so we can benefit from their good ideas and avoid their bad\nones. That was part of the motivation for not building this in for so\nlong: we want people to experiment so that the result can be something\nthat works well for a lot of people.\n\nSeparately from that, in [1] I mentioned that I want to revamp how\nhooks work somewhat, to avoid the attack described there (or the more\ncommon attack also described there that involves a zip file). Such a\nrevamp would be likely to also handle this multiple-hook use case.\n\nAs a word of caution, today we support having multiple credential\nhelpers in use and it's a nightmare to support. The layering model is\ncomplicated and users don't understand it. So we might want to try to\navoid whatever went wrong there. ;-)\n\nThanks,\nJonathan\n\n[1] https://public-inbox.org/git/20171002234517.GV19555@aiede.mtv.corp.google.com/\n"},{"id":"357084","messageId":"CAEpdsiZFMphQTdJnqFYH03M80W8CcrCbr2Uewktm0jy4D+Fz1A@mail.gmail.com","threadId":"49262","inReplyTo":"87pnxzdib7.fsf@evledraar.gmail.com","subject":"Re: Feature request: hooks directory","fromName":"Wesley Schwengle","fromEmail":"wesley@mintlab.nl","sentAt":"2018-08-31T12:38:48Z","receivedAt":"2018-08-31T12:38:52Z","isPatch":false,"sender":{"key":"wesley@mintlab.nl","avatar":null},"body":"Hop,\n\n2018-08-30 16:45 GMT+02:00 Ævar Arnfjörð Bjarmason :\n\n>> Solution:\n>> We discussed this at work and we thought about making a .d directory\n>> for the hooks, eg. $GIT_DIR/hooks/post-commit.d, where a user can put\n>> the post-commit hooks in. This allows us to provide post commit hooks\n>> and allows the user to add additional hooks him/herself. We could\n>> implement this in our own code base. But we were wondering if this\n>> approach could be shared with the git community and if this behavior\n>> is wanted in git itself.\n>\n> There is interest in this. This E-Mail of mine gives a good summary of\n> prior discussions about this:\n> https://public-inbox.org/git/877eqqnq22.fsf@evledraar.gmail.com/\n>\n> I.e. it's something I've personally been interested in doing in the\n> past, there's various bolt-on solutions to do it (basically local hook\n> runners) used by various projects.\n\nThank you for the input. Do you by any chance still have that branch?\nOr would you advise me to start fresh, if so, do you have any pointers\non where to look as I'm brand new to the git source code?\n\nFrom the thread I've extracted three stories:\n\n1) As a developer I want to have 'hooks.multiHooks' to enable\nmulti-hook support in git\nInput is welcome for another name.\n\n2) As a developer I want natural sort order executing for my githooks\nso I can predict executions\nSee https://public-inbox.org/git/CACBZZX6AYBYeb5S4nEBhYbx1r=icJ81JGYBx5=H4wacPhHjFbQ@mail.gmail.com/\nfor more information\n\n3) As a developer I want to run $GIT_DIR/hooks/ before\n$GIT_DIR/hooks/.d/*\nReference: https://public-inbox.org/git/CACBZZX6j6q2DUN_Z-Pnent1u714dVNPFBrL_PiEQyLmCzLUVxg@mail.gmail.com/\n\nThe following story would be.. nice to have I think. I'm not sure I\nwould want to implement this from the get go as I don't have a use\ncase for it.\n4) As a developer I want a way to have a hook report an error and let\nanother hook decide if we want to pass or not.\nReference: https://public-inbox.org/git/xmqq60v4don1.fsf@gitster.mtv.corp.google.com/\n\n\n2018-08-31 5:16 GMT+02:00 Jonathan Nieder :\n> A few unrelated thoughts, to expand on this.\n>\n> Separately from that, in [1] I mentioned that I want to revamp how\n> hooks work somewhat, to avoid the attack described there (or the more\n> common attack also described there that involves a zip file). Such a\n> revamp would be likely to also handle this multiple-hook use case.\n>\n> [1] https://public-inbox.org/git/20171002234517.GV19555@aiede.mtv.corp.google.com/\n\nThe zip file attack vector doesn't change with adding a hook.d\ndirectory structure? If I have one file or multiple files, the attack\nstays the same?\nI think I'm asking if this would be a show stopper for the feature.\n\n\nCheers,\nWesley\n\n-- \nWesley Schwengle, Developer\nMintlab B.V., https://www.zaaksysteem.nl\nE: wesley@mintlab.nl\nT: +31 20 737 00 05\n"},{"id":"357086","messageId":"874lfad537.fsf@evledraar.gmail.com","threadId":"49262","inReplyTo":"CAEpdsiZFMphQTdJnqFYH03M80W8CcrCbr2Uewktm0jy4D+Fz1A@mail.gmail.com","subject":"Re: Feature request: hooks directory","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2018-08-31T13:43:24Z","receivedAt":"2018-08-31T13:43:31Z","isPatch":false,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"\nOn Fri, Aug 31 2018, Wesley Schwengle wrote:\n\n> Hop,\n>\n> 2018-08-30 16:45 GMT+02:00 Ævar Arnfjörð Bjarmason :\n>\n>>> Solution:\n>>> We discussed this at work and we thought about making a .d directory\n>>> for the hooks, eg. $GIT_DIR/hooks/post-commit.d, where a user can put\n>>> the post-commit hooks in. This allows us to provide post commit hooks\n>>> and allows the user to add additional hooks him/herself. We could\n>>> implement this in our own code base. But we were wondering if this\n>>> approach could be shared with the git community and if this behavior\n>>> is wanted in git itself.\n>>\n>> There is interest in this. This E-Mail of mine gives a good summary of\n>> prior discussions about this:\n>> https://public-inbox.org/git/877eqqnq22.fsf@evledraar.gmail.com/\n>>\n>> I.e. it's something I've personally been interested in doing in the\n>> past, there's various bolt-on solutions to do it (basically local hook\n>> runners) used by various projects.\n>\n> Thank you for the input. Do you by any chance still have that branch?\n> Or would you advise me to start fresh, if so, do you have any pointers\n> on where to look as I'm brand new to the git source code?\n\nNo, sorry. Start by grepping the hook names found in the githooks\nmanpage in the C code.\n\nOne of the things that's hard, well not hard, just tedious about this,\nis that most of them are implementing their own ad-hoc way of doing\nstuff. E.g. the *-receive hooks are in receive-pack.c in\nrun_receive_hook().\n\nThere is run_hook_le() and friends, but it's not used by everything.\n\nSo e.g. for the pre-receive hook in order to run 2 of them instead of 1\nyou need to untangle the state where we're feeding the hook with the\ninput (and potentially buffer it, not stream it), instead of doing it as\na one-off as we're doing now.\n\nThen some hooks get nothing on stdin, some get stuff on stdin, some\nproduce output on stdout/stderr etc.\n\nAs a first approximation, just add a e.g. support for a pre-receive.2\nhook, that gets run after pre-receive, to see what needs to be done to\nrun it twice.\n\n> From the thread I've extracted three stories:\n>\n> 1) As a developer I want to have 'hooks.multiHooks' to enable\n> multi-hook support in git\n> Input is welcome for another name.\n\nYeah maybe we should have a setting, but FWIW I think we should just\nstat() whether the hooks/$hook_name.d directory exist, and then use it,\nalthough maybe we'll need stuff like hooks.multiHooks to give the likes\nof GitLab (which already do that themselves) an upgrade path...\n\nYou can see their implementation here:\nhttps://gitlab.com/gitlab-org/gitlab-shell/blob/master/lib/gitlab_custom_hook.rb\n\n> 2) As a developer I want natural sort order executing for my githooks\n> so I can predict executions\n> See https://public-inbox.org/git/CACBZZX6AYBYeb5S4nEBhYbx1r=icJ81JGYBx5=H4wacPhHjFbQ@mail.gmail.com/\n> for more information\n\nYeah, any sane implementation of this will execute the hooks in\nhooks/$hook_name.d in glob() order.\n\n> 3) As a developer I want to run $GIT_DIR/hooks/ before\n> $GIT_DIR/hooks/.d/*\n> Reference: https://public-inbox.org/git/CACBZZX6j6q2DUN_Z-Pnent1u714dVNPFBrL_PiEQyLmCzLUVxg@mail.gmail.com/\n\nFor e.g. GitLab the hook/pre-receive is a runner that'll run all\nhook/pre-receive.d/*, so this probably makes sense to hide behind a\nconfig setting. I think it's sensible as a default to move to to just\ntry to move away from hooks/ and use hook/.d/* instead.\n\n> The following story would be.. nice to have I think. I'm not sure I\n> would want to implement this from the get go as I don't have a use\n> case for it.\n> 4) As a developer I want a way to have a hook report an error and let\n> another hook decide if we want to pass or not.\n> Reference: https://public-inbox.org/git/xmqq60v4don1.fsf@gitster.mtv.corp.google.com/\n\nI think a default that makes more sense is a while ! ret = glob()\nloop, i.e. a failure will do early exit. But yeah. Junio seemed to want\nthis to be configurable.\n\n> 2018-08-31 5:16 GMT+02:00 Jonathan Nieder :\n>> A few unrelated thoughts, to expand on this.\n>>\n>> Separately from that, in [1] I mentioned that I want to revamp how\n>> hooks work somewhat, to avoid the attack described there (or the more\n>> common attack also described there that involves a zip file). Such a\n>> revamp would be likely to also handle this multiple-hook use case.\n>>\n>> [1] https://public-inbox.org/git/20171002234517.GV19555@aiede.mtv.corp.google.com/\n>\n> The zip file attack vector doesn't change with adding a hook.d\n> directory structure? If I have one file or multiple files, the attack\n> stays the same?\n> I think I'm asking if this would be a show stopper for the feature.\n\nYeah I don't see how what Jonathan's talking about there has any\nrelevance to whether we run 1 or 100 hooks.\n"},{"id":"357192","messageId":"CAEpdsiZSNe6e5JvGF4UHaf5+3zBq61uJAgQ5YDSr1v4er7inhQ@mail.gmail.com","threadId":"49262","inReplyTo":"874lfad537.fsf@evledraar.gmail.com","subject":"Re: Feature request: hooks directory","fromName":"Wesley Schwengle","fromEmail":"wesley@mintlab.nl","sentAt":"2018-09-02T21:38:08Z","receivedAt":"2018-09-02T21:38:13Z","isPatch":false,"sender":{"key":"wesley@mintlab.nl","avatar":null},"body":"Hi all,\n\nI've made some progress with the hook.d implementation. It isn't\nfinished, as it is my first C project I'm still somewhat rocky with\nhow pointers and such work, but I'm getting somewhere. I haven't\nbroken any tests \\o/.\n You have a nice testsuite btw. Feel free to comment on the code.\n\nI've moved some of the hooks-code found in run-command.c to hooks.c\n\nYou can see the progress on gitlab: https://gitlab.com/waterkip/git\nor on github: https://github.com/waterkip/git\nThe output of format-patch is down below.\n\nI have some questions regarding the following two functions in run-command.c:\n* run_hook_le\n* run_hook_ve\n\nWhat do the postfixes le and ve mean?\n\nCheers,\nWesley\n\nformat-patch:\n\nFrom 129d8aff8257b22210beadc155cdbcae99b0fc4b Mon Sep 17 00:00:00 2001\nFrom: Wesley Schwengle \nDate: Sun, 2 Sep 2018 02:40:04 +0200\nSubject: [PATCH] WIP: Add hook.d support in git\n\nAdd a generic mechanism to find and execute one or multiple hooks found\nin $GIT_DIR/hooks/ and/or $GIT_DIR/hooks/.d/*\n\nThe API is as follows:\n\n#include \"hooks.h\"\n\narray hooks = find_hooks('pre-receive');\nint hooks_ran = run_hooks(hooks);\n\nThe implemented behaviour is:\n\n* If we find a hooks/.d directory and the hooks.multiHook flag isn't\n set we make use of that directory.\n\n* If we find a hooks/.d and we also have hooks/ and the\n hooks.multiHook isn't set or set to false we don't use the hook.d\n directory. If the hook isn't set we issue a warning to the user\n telling him/her that we support multiple hooks via the .d directory\n structure\n\n* If the hooks.multiHook is set to true we use the hooks/ and all\n the entries found in hooks/.d\n\n* All the scripts are executed and fail on the first error\n---\n Makefile | 1 +\n TODO-hooks.md | 38 ++++++++++++\n builtin/am.c | 4 +-\n builtin/commit.c | 4 +-\n builtin/receive-pack.c | 10 +--\n builtin/worktree.c | 3 +-\n cache.h | 1 +\n config.c | 5 ++\n environment.c | 1 +\n hooks.c | 134 +++++++++++++++++++++++++++++++++++++++++\n hooks.h | 35 +++++++++++\n run-command.c | 36 +----------\n run-command.h | 6 --\n sequencer.c | 7 ++-\n transport.c | 3 +-\n 15 files changed, 237 insertions(+), 51 deletions(-)\n create mode 100644 TODO-hooks.md\n create mode 100644 hooks.c\n create mode 100644 hooks.h\n\ndiff --git a/Makefile b/Makefile\nindex 5a969f583..f5eddf1d7 100644\n--- a/Makefile\n+++ b/Makefile\n@@ -810,6 +810,7 @@ LIB_H = $(shell $(FIND) . \\\n -name Documentation -prune -o \\\n -name '*.h' -print)\n\n+LIB_OBJS += hooks.o\n LIB_OBJS += abspath.o\n LIB_OBJS += advice.o\n LIB_OBJS += alias.o\ndiff --git a/TODO-hooks.md b/TODO-hooks.md\nnew file mode 100644\nindex 000000000..13b15bffb\n--- /dev/null\n+++ b/TODO-hooks.md\n@@ -0,0 +1,38 @@\n+# All hooks\n+# See Documentation/githooks.txt for more information about each and every hook\n+# that git knows about\n+commit-msg\n+fsmoninor-watchman\n+p4-pre-submit\n+post-applypatch\n+post-checkout\n+post-commit\n+post-merge\n+post-receive\n+post-rewrite\n+post-update\n+pre-applypatch\n+pre-auto-gc\n+pre-commit\n+pre-push\n+pre-rebase\n+pre-receive\n+prepare-commit-msg\n+push-to-checkout\n+sendemail-validate\n+update\n+\n+# builtin/receive-pack.c\n+feed_recieve_hook\n+find_hook\n+find_receive_hook\n+push_to_checkout_hook\n+receive_hook_feed_state\n+run_and_feed_hook\n+run_hook_le\n+run_receive_hook\n+run_update_hook\n+\n+\n+# run-command.c\n+find_hook\ndiff --git a/builtin/am.c b/builtin/am.c\nindex 9f7ecf6ec..d1276dd47 100644\n--- a/builtin/am.c\n+++ b/builtin/am.c\n@@ -34,6 +34,8 @@\n #include \"packfile.h\"\n #include \"repository.h\"\n\n+#include \"hooks.h\"\n+\n /**\n * Returns 1 if the file is empty or does not exist, 0 otherwise.\n */\n@@ -509,7 +511,7 @@ static int run_applypatch_msg_hook(struct am_state *state)\n static int run_post_rewrite_hook(const struct am_state *state)\n {\n struct child_process cp = CHILD_PROCESS_INIT;\n- const char *hook = find_hook(\"post-rewrite\");\n+ const char *hook = find_hooks(\"post-rewrite\");\n int ret;\n\n if (!hook)\ndiff --git a/builtin/commit.c b/builtin/commit.c\nindex 0d9828e29..389224d66 100644\n--- a/builtin/commit.c\n+++ b/builtin/commit.c\n@@ -34,6 +34,8 @@\n #include \"mailmap.h\"\n #include \"help.h\"\n\n+#include \"hooks.h\"\n+\n static const char * const builtin_commit_usage[] = {\n N_(\"git commit [] [--] ...\"),\n NULL\n@@ -932,7 +934,7 @@ static int prepare_to_commit(const char\n*index_file, const char *prefix,\n return 0;\n }\n\n- if (!no_verify && find_hook(\"pre-commit\")) {\n+ if (!no_verify && find_hooks(\"pre-commit\")) {\n /*\n * Re-read the index as pre-commit hook could have updated it,\n * and write it out as a tree. We must do this before we invoke\ndiff --git a/builtin/receive-pack.c b/builtin/receive-pack.c\nindex c17ce94e1..dd10ef4af 100644\n--- a/builtin/receive-pack.c\n+++ b/builtin/receive-pack.c\n@@ -28,6 +28,8 @@\n #include \"object-store.h\"\n #include \"protocol.h\"\n\n+#include \"hooks.h\"\n+\n static const char * const receive_pack_usage[] = {\n N_(\"git receive-pack \"),\n NULL\n@@ -685,7 +687,7 @@ static int run_and_feed_hook(const char\n*hook_name, feed_fn feed,\n const char *argv[2];\n int code;\n\n- argv[0] = find_hook(hook_name);\n+ argv[0] = find_hooks(hook_name);\n if (!argv[0])\n return 0;\n\n@@ -794,7 +796,7 @@ static int run_update_hook(struct command *cmd)\n struct child_process proc = CHILD_PROCESS_INIT;\n int code;\n\n- argv[0] = find_hook(\"update\");\n+ argv[0] = find_hooks(\"update\");\n if (!argv[0])\n return 0;\n\n@@ -1008,7 +1010,7 @@ static const char *update_worktree(unsigned char *sha1)\n\n argv_array_pushf(&env, \"GIT_DIR=%s\", absolute_path(get_git_dir()));\n\n- if (!find_hook(push_to_checkout_hook))\n+ if (!find_hooks(push_to_checkout_hook))\n retval = push_to_deploy(sha1, &env, work_tree);\n else\n retval = push_to_checkout(sha1, &env, work_tree);\n@@ -1167,7 +1169,7 @@ static void run_update_post_hook(struct command *commands)\n struct child_process proc = CHILD_PROCESS_INIT;\n const char *hook;\n\n- hook = find_hook(\"post-update\");\n+ hook = find_hooks(\"post-update\");\n if (!hook)\n return;\n\ndiff --git a/builtin/worktree.c b/builtin/worktree.c\nindex 41e771439..93fd4c175 100644\n--- a/builtin/worktree.c\n+++ b/builtin/worktree.c\n@@ -12,6 +12,7 @@\n #include \"refs.h\"\n #include \"utf8.h\"\n #include \"worktree.h\"\n+#include \"hooks.h\"\n\n static const char * const worktree_usage[] = {\n N_(\"git worktree add [] []\"),\n@@ -344,7 +345,7 @@ static int add_worktree(const char *path, const\nchar *refname,\n * is_junk is cleared, but do return appropriate code when hook fails.\n */\n if (!ret && opts->checkout) {\n- const char *hook = find_hook(\"post-checkout\");\n+ const char *hook = find_hooks(\"post-checkout\");\n if (hook) {\n const char *env[] = { \"GIT_DIR\", \"GIT_WORK_TREE\", NULL };\n cp.git_cmd = 0;\ndiff --git a/cache.h b/cache.h\nindex 4d014541a..04cb06bba 100644\n--- a/cache.h\n+++ b/cache.h\n@@ -837,6 +837,7 @@ extern int quote_path_fully;\n extern int has_symlinks;\n extern int minimum_abbrev, default_abbrev;\n extern int ignore_case;\n+extern int hookd_enabled;\n extern int assume_unchanged;\n extern int prefer_symlink_refs;\n extern int warn_ambiguous_refs;\ndiff --git a/config.c b/config.c\nindex 3461993f0..07a073ab9 100644\n--- a/config.c\n+++ b/config.c\n@@ -1491,6 +1491,11 @@ int git_default_config(const char *var, const\nchar *value, void *dummy)\n pack_compression_seen = 1;\n return 0;\n }\n+ if (!strcmp(var, \"hooks.multihook\")) {\n+ hookd_enabled = git_config_bool(var, value);\n+ return 0;\n+ }\n+\n\n /* Add other config variables here and to Documentation/config.txt. */\n return 0;\ndiff --git a/environment.c b/environment.c\nindex 3f3c8746c..27569e671 100644\n--- a/environment.c\n+++ b/environment.c\n@@ -73,6 +73,7 @@ int precomposed_unicode = -1; /* see\nprobe_utf8_pathname_composition() */\n unsigned long pack_size_limit_cfg;\n enum hide_dotfiles_type hide_dotfiles = HIDE_DOTFILES_DOTGITONLY;\n enum log_refs_config log_all_ref_updates = LOG_REFS_UNSET;\n+int hookd_enabled = -1;\n\n #ifndef PROTECT_HFS_DEFAULT\n #define PROTECT_HFS_DEFAULT 0\ndiff --git a/hooks.c b/hooks.c\nnew file mode 100644\nindex 000000000..d2950a2da\n--- /dev/null\n+++ b/hooks.c\n@@ -0,0 +1,134 @@\n+#include \"hooks.h\"\n+#include \"cache.h\"\n+#include \"run-command.h\"\n+#include \"string-list.h\"\n+#include \"config.h\"\n+\n+//#include \n+//#include \n+//#include \n+\n+static const char *_get_hook(struct strbuf *path)\n+{\n+ char *name = path->buf;\n+ int err;\n+ if (access(name, X_OK) >= 0) {\n+ return name;\n+ }\n+ err = errno;\n+\n+// Check for .exe\n+#ifdef STRIP_EXTENSION\n+ strbuf_addstr(path, STRIP_EXTENSION);\n+ name = path->buf;\n+ if (access(name, X_OK) >= 0)\n+ return name;\n+ if (errno == EACCES)\n+ err = errno;\n+#endif\n+\n+ if (err == EACCES && advice_ignored_hook) {\n+ static struct string_list advise_given = STRING_LIST_INIT_DUP;\n+\n+ if (!string_list_lookup(&advise_given, name)) {\n+ string_list_insert(&advise_given, name);\n+ advise(_(\"The '%s' hook was ignored because \"\n+ \"it's not set as executable.\\n\"\n+ \"You can disable this warning with \"\n+ \"`git config advice.ignoredHook false`.\"),\n+ name);\n+ }\n+ }\n+ return NULL;\n+}\n+\n+static void get_hooks_from_directory(const char *name, struct\nstring_list *list)\n+{\n+\n+ struct strbuf path = STRBUF_INIT;\n+ strbuf_git_path(&path, \"hooks/%s.d\", name);\n+\n+ struct dirent *de;\n+ DIR *hooksd_dir;\n+ static struct strbuf hooksd_script = STRBUF_INIT;\n+\n+ if ((hooksd_dir = opendir(path.buf)) == NULL) {\n+ return;\n+ }\n+\n+ while ((de = readdir(hooksd_dir)) != NULL) {\n+ struct stat stbuf;\n+\n+ strbuf_reset(&hooksd_script);\n+ strbuf_addf(&hooksd_script, \"%s/%s\", path.buf, de->d_name);\n+\n+ if (stat(hooksd_script.buf, &stbuf) == -1) {\n+ continue;\n+ }\n+ else if ((stbuf.st_mode & S_IFMT) == S_IFDIR) {\n+ continue;\n+ }\n+\n+ const char *hook = _get_hook(&hooksd_script);\n+ if (hook) {\n+ //printf(\"Found hook script %s\\n\", hook);\n+ string_list_append(list, hook);\n+ }\n+\n+ strbuf_release(&hooksd_script);\n+ }\n+\n+ strbuf_release(&hooksd_script);\n+ strbuf_release(&path);\n+ return;\n+}\n+\n+//const struct string_list *find_hooks(const char *name)\n+const char *find_hooks(const char *name)\n+{\n+\n+ struct string_list list = STRING_LIST_INIT_NODUP;\n+ const char *hook_path;\n+\n+ struct strbuf path = STRBUF_INIT;\n+ strbuf_git_path(&path, \"hooks/%s\", name);\n+\n+ hook_path = _get_hook(&path);\n+\n+ if (hook_path) {\n+ string_list_append(&list, hook_path);\n+ }\n+\n+ if (hookd_enabled == 1) {\n+ get_hooks_from_directory(name, &list);\n+\n+ if (hook_path)\n+ return hook_path;\n+ return NULL;\n+ }\n+ else if (hookd_enabled == 0) {\n+ if (hook_path)\n+ return hook_path;\n+ return NULL;\n+ }\n+ else {\n+ static struct string_list advise_given = STRING_LIST_INIT_DUP;\n+\n+ get_hooks_from_directory(name, &list);\n+ if ( hook_path && list.nr > 1\n+ && !string_list_lookup(&advise_given, name)) {\n+ string_list_insert(&advise_given, name);\n+ advise(_(\"You have a hook plus hook.d dir for %s. This\"\n+ \" behaviour is now supported by git.\\nYou can\"\n+ \" disable this warning with `git config\"\n+ \" hooks.multiHook false` to disable reading\\n\"\n+ \"the hook.d directory or run `git config\"\n+ \" hooks.multiHook true` to enable the\\nhook.d\"\n+ \" directory. Ignoring the hook.d directory\"\n+ \" for now\"), name);\n+ }\n+ if (hook_path)\n+ return hook_path;\n+ return NULL;\n+ }\n+}\ndiff --git a/hooks.h b/hooks.h\nnew file mode 100644\nindex 000000000..278d63344\n--- /dev/null\n+++ b/hooks.h\n@@ -0,0 +1,35 @@\n+#ifndef HOOKS_H\n+#define HOOKS_H\n+\n+#ifndef NO_PTHREADS\n+#include \n+#endif\n+/*\n+ * Returns all the hooks found in either\n+ * $GIT_DIR/hooks/$hook and/or $GIT_DIR/hooks/$hook.d/\n+ *\n+ * Note that this points to static storage that will be\n+ * overwritten by further calls to find_hooks and run_hook_*.\n+ */\n+//extern const struct string_list *find_hooks(const char *name);\n+extern const char *find_hooks(const char *name);\n+\n+/* Unsure what this does/is/etc */\n+//LAST_ARG_MUST_BE_NULL\n+\n+/*\n+ * Run all the runnable hooks found in\n+ * $GIT_DIR/hooks/$hook and/or $GIT_DIR/hooks/$hook.d/\n+ *\n+ */\n+//extern int run_hooks_le(const char *const *env, const char *name, ...);\n+//extern int run_hooks_ve(const char *const *env, const char *name,\nva_list args);\n+\n+#endif\n+\n+/* Workings of hooks\n+ *\n+ * array_of_hooks = find_hooks(name);\n+ * number_of_hooks_ran = run_hooks(array_of_hooks);\n+ *\n+ */\ndiff --git a/run-command.c b/run-command.c\nindex 84b883c21..6b57147fa 100644\n--- a/run-command.c\n+++ b/run-command.c\n@@ -7,6 +7,7 @@\n #include \"strbuf.h\"\n #include \"string-list.h\"\n #include \"quote.h\"\n+#include \"hooks.h\"\n\n void child_process_init(struct child_process *child)\n {\n@@ -1246,46 +1247,13 @@ int finish_async(struct async *async)\n #endif\n }\n\n-const char *find_hook(const char *name)\n-{\n- static struct strbuf path = STRBUF_INIT;\n-\n- strbuf_reset(&path);\n- strbuf_git_path(&path, \"hooks/%s\", name);\n- if (access(path.buf, X_OK) < 0) {\n- int err = errno;\n-\n-#ifdef STRIP_EXTENSION\n- strbuf_addstr(&path, STRIP_EXTENSION);\n- if (access(path.buf, X_OK) >= 0)\n- return path.buf;\n- if (errno == EACCES)\n- err = errno;\n-#endif\n-\n- if (err == EACCES && advice_ignored_hook) {\n- static struct string_list advise_given = STRING_LIST_INIT_DUP;\n-\n- if (!string_list_lookup(&advise_given, name)) {\n- string_list_insert(&advise_given, name);\n- advise(_(\"The '%s' hook was ignored because \"\n- \"it's not set as executable.\\n\"\n- \"You can disable this warning with \"\n- \"`git config advice.ignoredHook false`.\"),\n- path.buf);\n- }\n- }\n- return NULL;\n- }\n- return path.buf;\n-}\n\n int run_hook_ve(const char *const *env, const char *name, va_list args)\n {\n struct child_process hook = CHILD_PROCESS_INIT;\n const char *p;\n\n- p = find_hook(name);\n+ p = find_hooks(name);\n if (!p)\n return 0;\n\ndiff --git a/run-command.h b/run-command.h\nindex 3932420ec..3009e49b1 100644\n--- a/run-command.h\n+++ b/run-command.h\n@@ -58,12 +58,6 @@ int finish_command(struct child_process *);\n int finish_command_in_signal(struct child_process *);\n int run_command(struct child_process *);\n\n-/*\n- * Returns the path to the hook file, or NULL if the hook is missing\n- * or disabled. Note that this points to static storage that will be\n- * overwritten by further calls to find_hook and run_hook_*.\n- */\n-extern const char *find_hook(const char *name);\n LAST_ARG_MUST_BE_NULL\n extern int run_hook_le(const char *const *env, const char *name, ...);\n extern int run_hook_ve(const char *const *env, const char *name, va_list args);\ndiff --git a/sequencer.c b/sequencer.c\nindex 84bf598c3..dfa58fae1 100644\n--- a/sequencer.c\n+++ b/sequencer.c\n@@ -30,6 +30,7 @@\n #include \"oidset.h\"\n #include \"commit-slab.h\"\n #include \"alias.h\"\n+#include \"hooks.h\"\n\n #define GIT_REFLOG_ACTION \"GIT_REFLOG_ACTION\"\n\n@@ -996,7 +997,7 @@ static int run_rewrite_hook(const struct object_id *oldoid,\n int code;\n struct strbuf sb = STRBUF_INIT;\n\n- argv[0] = find_hook(\"post-rewrite\");\n+ argv[0] = find_hooks(\"post-rewrite\");\n if (!argv[0])\n return 0;\n\n@@ -1265,7 +1266,7 @@ static int try_to_commit(struct strbuf *msg,\nconst char *author,\n goto out;\n }\n\n- if (find_hook(\"prepare-commit-msg\")) {\n+ if (find_hooks(\"prepare-commit-msg\")) {\n res = run_prepare_commit_msg_hook(msg, hook_commit);\n if (res)\n goto out;\n@@ -3496,7 +3497,7 @@ static int pick_commits(struct todo_list\n*todo_list, struct replay_opts *opts)\n st.st_size > 0) {\n struct child_process child = CHILD_PROCESS_INIT;\n const char *post_rewrite_hook =\n- find_hook(\"post-rewrite\");\n+ find_hooks(\"post-rewrite\");\n\n child.in = open(rebase_path_rewritten_list(), O_RDONLY);\n child.git_cmd = 1;\ndiff --git a/transport.c b/transport.c\nindex 06ffea277..a2930f4fd 100644\n--- a/transport.c\n+++ b/transport.c\n@@ -22,6 +22,7 @@\n #include \"protocol.h\"\n #include \"object-store.h\"\n #include \"color.h\"\n+#include \"hooks.h\"\n\n static int transport_use_color = -1;\n static char transport_colors[][COLOR_MAXLEN] = {\n@@ -1019,7 +1020,7 @@ static int run_pre_push_hook(struct transport *transport,\n struct strbuf buf;\n const char *argv[4];\n\n- if (!(argv[0] = find_hook(\"pre-push\")))\n+ if (!(argv[0] = find_hooks(\"pre-push\")))\n return 0;\n\n argv[1] = transport->remote->name;\n\n-- \nWesley Schwengle, Developer\nMintlab B.V., https://www.zaaksysteem.nl\nE: wesley@mintlab.nl\nT: +31 20 737 00 05\n"},{"id":"357193","messageId":"CAP8UFD0u2ZULWMDtWMwJCH27o=JLS3xiSmzvH+M4To0EWuX9Vg@mail.gmail.com","threadId":"49262","inReplyTo":"CAEpdsiZSNe6e5JvGF4UHaf5+3zBq61uJAgQ5YDSr1v4er7inhQ@mail.gmail.com","subject":"Re: Feature request: hooks directory","fromName":"Christian Couder","fromEmail":"christian.couder@gmail.com","sentAt":"2018-09-03T04:00:08Z","receivedAt":"2018-09-03T04:00:12Z","isPatch":false,"sender":{"key":"christian.couder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/208954?v=4"},"body":"Hi Wesley,\n\nOn Sun, Sep 2, 2018 at 11:38 PM, Wesley Schwengle wrote:\n> Hi all,\n>\n> I've made some progress with the hook.d implementation. It isn't\n> finished, as it is my first C project I'm still somewhat rocky with\n> how pointers and such work, but I'm getting somewhere. I haven't\n> broken any tests \\o/.\n\nGreat! Welcome to the Git community!\n\n> You have a nice testsuite btw. Feel free to comment on the code.\n>\n> I've moved some of the hooks-code found in run-command.c to hooks.c\n>\n> You can see the progress on gitlab: https://gitlab.com/waterkip/git\n> or on github: https://github.com/waterkip/git\n> The output of format-patch is down below.\n\nIt would be nicer if you could give links that let us see more\ndirectly the commits you made, for example:\nhttps://gitlab.com/waterkip/git/commits/multi-hooks\n\n> I have some questions regarding the following two functions in run-command.c:\n> * run_hook_le\n> * run_hook_ve\n>\n> What do the postfixes le and ve mean?\n\nIt's about the arguments the function accepts, in a similar way to\nexec*() functions, see `man execve` and `man execle`.\nIn short 'l' means list, 'v' means array of pointer to strings and 'e'\nmeans environment.\n\n> format-patch:\n>\n> From 129d8aff8257b22210beadc155cdbcae99b0fc4b Mon Sep 17 00:00:00 2001\n> From: Wesley Schwengle \n> Date: Sun, 2 Sep 2018 02:40:04 +0200\n> Subject: [PATCH] WIP: Add hook.d support in git\n\nThis is not the best way to embed a patch in an email. There is\nDocumentation/SubmittingPatches in the code base, that should explain\nbetter ways to send patches to the mailing list.\n\n> Add a generic mechanism to find and execute one or multiple hooks found\n> in $GIT_DIR/hooks/ and/or $GIT_DIR/hooks/.d/*\n>\n> The API is as follows:\n>\n> #include \"hooks.h\"\n>\n> array hooks = find_hooks('pre-receive');\n> int hooks_ran = run_hooks(hooks);\n>\n> The implemented behaviour is:\n>\n> * If we find a hooks/.d directory and the hooks.multiHook flag isn't\n> set we make use of that directory.\n>\n> * If we find a hooks/.d and we also have hooks/ and the\n> hooks.multiHook isn't set or set to false we don't use the hook.d\n> directory. If the hook isn't set we issue a warning to the user\n> telling him/her that we support multiple hooks via the .d directory\n> structure\n>\n> * If the hooks.multiHook is set to true we use the hooks/ and all\n> the entries found in hooks/.d\n>\n> * All the scripts are executed and fail on the first error\n\nMaybe the above documentation should be fully embedded as comments in\n\"hooks.h\" (or perhaps added to a new file in\n\"Documentation/technical/\", though it looks like we prefer to embed\ndoc in header files these days).\n\n> diff --git a/hooks.h b/hooks.h\n> new file mode 100644\n> index 000000000..278d63344\n> --- /dev/null\n> +++ b/hooks.h\n> @@ -0,0 +1,35 @@\n> +#ifndef HOOKS_H\n> +#define HOOKS_H\n> +\n> +#ifndef NO_PTHREADS\n> +#include \n> +#endif\n> +/*\n> + * Returns all the hooks found in either\n> + * $GIT_DIR/hooks/$hook and/or $GIT_DIR/hooks/$hook.d/\n> + *\n> + * Note that this points to static storage that will be\n> + * overwritten by further calls to find_hooks and run_hook_*.\n> + */\n> +//extern const struct string_list *find_hooks(const char *name);\n\nThe above comment is using \"//\" which we forbid and should probably be\nremoved anyway.\n\n> +extern const char *find_hooks(const char *name);\n> +\n> +/* Unsure what this does/is/etc */\n> +//LAST_ARG_MUST_BE_NULL\n\nThis is to make it easier for tools like compilers to check that a\nfunction is used correctly. You should not remove such macros.\n\n> +/*\n> + * Run all the runnable hooks found in\n> + * $GIT_DIR/hooks/$hook and/or $GIT_DIR/hooks/$hook.d/\n> + *\n> + */\n> +//extern int run_hooks_le(const char *const *env, const char *name, ...);\n> +//extern int run_hooks_ve(const char *const *env, const char *name,\n> va_list args);\n\nStrange that these functions are commented out.\n\n> +#endif\n> +\n> +/* Workings of hooks\n> + *\n> + * array_of_hooks = find_hooks(name);\n> + * number_of_hooks_ran = run_hooks(array_of_hooks);\n> + *\n> + */\n\nThis kind of documentation should probably be at the beginning of the\nfile, see strbuf.h for example.\n\nThanks,\nChristian.\n"},{"id":"357196","messageId":"CAEpdsiaCMy1vd=x4W7rsFRN+Rj1vuMNLLA64OaF4GLqfjteuPg@mail.gmail.com","threadId":"49262","inReplyTo":"CAP8UFD0u2ZULWMDtWMwJCH27o=JLS3xiSmzvH+M4To0EWuX9Vg@mail.gmail.com","subject":"Re: Feature request: hooks directory","fromName":"Wesley Schwengle","fromEmail":"wesley@mintlab.nl","sentAt":"2018-09-03T08:50:27Z","receivedAt":"2018-09-03T08:50:30Z","isPatch":false,"sender":{"key":"wesley@mintlab.nl","avatar":null},"body":"Hello Christian,\n\n2018-09-03 6:00 GMT+02:00 Christian Couder :\n> Hi Wesley,\n>\n> On Sun, Sep 2, 2018 at 11:38 PM, Wesley Schwengle wrote:\n>> Hi all,\n>>\n>> I've made some progress with the hook.d implementation. It isn't\n>> finished, as it is my first C project I'm still somewhat rocky with\n>> how pointers and such work, but I'm getting somewhere. I haven't\n>> broken any tests \\o/.\n>\n> Great! Welcome to the Git community!\n\nThank you!\n\n>> You have a nice testsuite btw. Feel free to comment on the code.\n>>\n>> I've moved some of the hooks-code found in run-command.c to hooks.c\n>>\n>> You can see the progress on gitlab: https://gitlab.com/waterkip/git\n>> or on github: https://github.com/waterkip/git\n>> The output of format-patch is down below.\n>\n> It would be nicer if you could give links that let us see more\n> directly the commits you made, for example:\n> https://gitlab.com/waterkip/git/commits/multi-hooks\n\nYeah.. sorry about that. Let's just say I was excited to send my\nprogress to the list.\n\n>> I have some questions regarding the following two functions in run-command.c:\n>> * run_hook_le\n>> * run_hook_ve\n>>\n>> What do the postfixes le and ve mean?\n>\n> It's about the arguments the function accepts, in a similar way to\n> exec*() functions, see `man execve` and `man execle`.\n> In short 'l' means list, 'v' means array of pointer to strings and 'e'\n> means environment.\n\nThanks, I'll have a look at these functions later today.\n\n>> format-patch:\n>>\n>> From 129d8aff8257b22210beadc155cdbcae99b0fc4b Mon Sep 17 00:00:00 2001\n>> From: Wesley Schwengle \n>> Date: Sun, 2 Sep 2018 02:40:04 +0200\n>> Subject: [PATCH] WIP: Add hook.d support in git\n>\n> This is not the best way to embed a patch in an email. There is\n> Documentation/SubmittingPatches in the code base, that should explain\n> better ways to send patches to the mailing list.\n\nI saw that as well, after I've submitted the e-mail and was looking at\nthe travis documentation. I'll promise I'll do better for my next\npatch submission. Sorry about this..\n\n>> Add a generic mechanism to find and execute one or multiple hooks found\n>> in $GIT_DIR/hooks/ and/or $GIT_DIR/hooks/.d/*\n>>\n>> [snip]\n>>\n>> * All the scripts are executed and fail on the first error\n>\n> Maybe the above documentation should be fully embedded as comments in\n> \"hooks.h\" (or perhaps added to a new file in\n> \"Documentation/technical/\", though it looks like we prefer to embed\n> doc in header files these days).\n\nI've added this to \"hooks.h\". If you guys want some documentation in\n\"Documentation/technical\", I'm ok with adding a new file there as\nwell.\n\n>> diff --git a/hooks.h b/hooks.h\n>> new file mode 100644\n>> index 000000000..278d63344\n>> --- /dev/null\n>> +++ b/hooks.h\n>> @@ -0,0 +1,35 @@\n>> +#ifndef HOOKS_H\n>> +#define HOOKS_H\n>> +\n>> +#ifndef NO_PTHREADS\n>> +#include \n>> +#endif\n>> +/*\n>> + * Returns all the hooks found in either\n>> + * $GIT_DIR/hooks/$hook and/or $GIT_DIR/hooks/$hook.d/\n>> + *\n>> + * Note that this points to static storage that will be\n>> + * overwritten by further calls to find_hooks and run_hook_*.\n>> + */\n>> +//extern const struct string_list *find_hooks(const char *name);\n>\n> The above comment is using \"//\" which we forbid and should probably be\n> removed anyway.\n\nThanks, I have a \"//\" comment elsewhere, I'll change/remove it.\n\n>> +extern const char *find_hooks(const char *name);\n>> +\n>> +/* Unsure what this does/is/etc */\n>> +//LAST_ARG_MUST_BE_NULL\n>\n> This is to make it easier for tools like compilers to check that a\n> function is used correctly. You should not remove such macros.\n\nCheck.\n\n>> +/*\n>> + * Run all the runnable hooks found in\n>> + * $GIT_DIR/hooks/$hook and/or $GIT_DIR/hooks/$hook.d/\n>> + *\n>> + */\n>> +//extern int run_hooks_le(const char *const *env, const char *name, ...);\n>> +//extern int run_hooks_ve(const char *const *env, const char *name,\n>> va_list args);\n>\n> Strange that these functions are commented out.\n\nThese two functions are still in \"run-command.h\" and I want to move\nthem to \"hooks.h\" and friends. But I first wanted to make sure\n\"find_hooks\" worked as intended. This is still on my TODO for this\nweek.\n\n>> +#endif\n>> +\n>> +/* Workings of hooks\n>> + *\n>> + * array_of_hooks = find_hooks(name);\n>> + * number_of_hooks_ran = run_hooks(array_of_hooks);\n>> + *\n>> + */\n>\n> This kind of documentation should probably be at the beginning of the\n> file, see strbuf.h for example.\n\nSince I added the better part of the commit message in \"hooks.h\" I\nremoved this bit.\n\nAn additional question:\n\nIn my patch I've added \"hooks.multiHook\", which I think I should\nrename to \"hooks.hooksd\". It is wanted to change \"core.hooksPath\" to\nthe \"hooks\" namespace? Or should I rename my new config item to\n\"core.hooksd\"?\n\nCheers,\nWesley\n\n-- \nWesley Schwengle, Developer\nMintlab B.V., https://www.zaaksysteem.nl\nE: wesley@mintlab.nl\nT: +31 20 737 00 05\n"}]}