{"thread":{"id":"25671","subject":"[PATCH v2] Run global hooks from the directory at hooks.dir","startedAt":"2010-11-08T12:32:00Z","lastAt":"2010-11-09T03:22:43Z","messageCount":6,"participants":["Brian Collins","Johannes Sixt","Jonathan Nieder","Junio C Hamano"],"isPatch":true,"patchVersion":2,"patchTotal":null},"messages":[{"id":"155376","messageId":"1289219520-37435-1-git-send-email-bricollins@gmail.com","threadId":"25671","inReplyTo":null,"subject":"[PATCH v2] Run global hooks from the directory at hooks.dir","fromName":"Brian Collins","fromEmail":"bricollins@gmail.com","sentAt":"2010-11-08T12:32:00Z","receivedAt":"2010-11-08T12:32:00Z","isPatch":true,"sender":{"key":"bricollins@gmail.com","avatar":"https://avatars.githubusercontent.com/u/22232?v=4"},"body":"Run global hooks in the directory specified by the config variable\nhooks.dir before every attempt at running a local hook. If the\nglobal hook fails, the local hook will not run. If the global hook is\nabsent, the local hook runs normally. This is useful because it means\nyou can have scripts that run as hooks for multiple repositories, for\nexample coding style enforcement for an entire organization, or\nsystem-wide commit analytics.\n\nSigned-off-by: Brian Collins <bricollins@gmail.com>\n---\n\nThe possibility of adding this feature was previously discussed here:\nhttp://marc.info/?l=git&m=127808782807807&w=2\n\nFixed some bad whitespace in contrib/completion/git-completion.bash\n\nCheers,\nBrian\n\n Documentation/config.txt               |    5 ++\n cache.h                                |    2 +\n config.c                               |   12 ++++\n contrib/completion/git-completion.bash |    1 +\n run-command.c                          |   51 +++++++++++++-----\n t/t7510-global-hooks.sh                |   88 ++++++++++++++++++++++++++++++++\n 6 files changed, 144 insertions(+), 15 deletions(-)\n create mode 100755 t/t7510-global-hooks.sh\n\ndiff --git a/Documentation/config.txt b/Documentation/config.txt\nindex 538ebb5..3f18208 100644\n--- a/Documentation/config.txt\n+++ b/Documentation/config.txt\n@@ -1199,6 +1199,11 @@ help.autocorrect::\n \tvalue is 0 - the command will be just shown but not executed.\n \tThis is the default.\n \n+hooks.dir::\n+\tDirectory containing executable hooks. Usage is identical to\n+\t.git/hooks except they are triggered for any repository used\n+\twhile this configuration is set.\n+\n http.proxy::\n \tOverride the HTTP proxy, normally configured using the 'http_proxy'\n \tenvironment variable (see linkgit:curl[1]).  This can be overridden\ndiff --git a/cache.h b/cache.h\nindex 33decd9..c75a444 100644\n--- a/cache.h\n+++ b/cache.h\n@@ -1018,6 +1018,8 @@ extern const char *git_commit_encoding;\n extern const char *git_log_output_encoding;\n extern const char *git_mailmap_file;\n \n+extern const char *git_hooks_dir;\n+\n /* IO helper functions */\n extern void maybe_flush_or_die(FILE *, const char *);\n extern int copy_fd(int ifd, int ofd);\ndiff --git a/config.c b/config.c\nindex 4b0a820..1a9d5ee 100644\n--- a/config.c\n+++ b/config.c\n@@ -745,6 +745,15 @@ static int git_default_mailmap_config(const char *var, const char *value)\n \treturn 0;\n }\n \n+static int git_default_hooks_config(const char *var, const char *value)\n+{\n+\tif (!strcmp(var, \"hooks.dir\"))\n+\t\treturn git_config_pathname(&git_hooks_dir, var, value);\n+\n+\t/* Add other config variables here and to Documentation/config.txt. */\n+\treturn 0;\n+}\n+\n int git_default_config(const char *var, const char *value, void *dummy)\n {\n \tif (!prefixcmp(var, \"core.\"))\n@@ -768,6 +777,9 @@ int git_default_config(const char *var, const char *value, void *dummy)\n \tif (!prefixcmp(var, \"advice.\"))\n \t\treturn git_default_advice_config(var, value);\n \n+\tif (!prefixcmp(var, \"hooks.\"))\n+\t\treturn git_default_hooks_config(var, value);\n+\n \tif (!strcmp(var, \"pager.color\") || !strcmp(var, \"color.pager\")) {\n \t\tpager_use_color = git_config_bool(var,value);\n \t\treturn 0;\ndiff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash\nindex 168669b..b266f83 100755\n--- a/contrib/completion/git-completion.bash\n+++ b/contrib/completion/git-completion.bash\n@@ -1913,6 +1913,7 @@ _git_config ()\n \t\thelp.autocorrect\n \t\thelp.browser\n \t\thelp.format\n+\t\thooks.dir\n \t\thttp.lowSpeedLimit\n \t\thttp.lowSpeedTime\n \t\thttp.maxRequests\ndiff --git a/run-command.c b/run-command.c\nindex 2a1041e..8e33b80 100644\n--- a/run-command.c\n+++ b/run-command.c\n@@ -2,6 +2,8 @@\n #include \"run-command.h\"\n #include \"exec_cmd.h\"\n \n+const char *git_hooks_dir;\n+\n static inline void close_pair(int fd[2])\n {\n \tclose(fd[0]);\n@@ -603,26 +605,16 @@ int finish_async(struct async *async)\n #endif\n }\n \n-int run_hook(const char *index_file, const char *name, ...)\n+int run_hook_file(const char *path, const char *index_file, const char **argv)\n {\n \tstruct child_process hook;\n-\tconst char **argv = NULL, *env[2];\n \tchar index[PATH_MAX];\n-\tva_list args;\n-\tint ret;\n-\tsize_t i = 0, alloc = 0;\n+\tconst char *env[2];\n \n-\tif (access(git_path(\"hooks/%s\", name), X_OK) < 0)\n+\tif (access(path, X_OK) < 0)\n \t\treturn 0;\n \n-\tva_start(args, name);\n-\tALLOC_GROW(argv, i + 1, alloc);\n-\targv[i++] = git_path(\"hooks/%s\", name);\n-\twhile (argv[i-1]) {\n-\t\tALLOC_GROW(argv, i + 1, alloc);\n-\t\targv[i++] = va_arg(args, const char *);\n-\t}\n-\tva_end(args);\n+\targv[0] = path;\n \n \tmemset(&hook, 0, sizeof(hook));\n \thook.argv = argv;\n@@ -635,7 +627,36 @@ int run_hook(const char *index_file, const char *name, ...)\n \t\thook.env = env;\n \t}\n \n-\tret = run_command(&hook);\n+\treturn run_command(&hook);\n+}\n+\n+int run_hook(const char *index_file, const char *name, ...)\n+{\n+\tconst char **argv = NULL;\n+\tva_list args;\n+\tint ret;\n+\tsize_t i = 1, alloc = 0;\n+\tconst char *local_path = git_path(\"hooks/%s\", name);\n+\tconst char *global_path = NULL;\n+\tif (git_hooks_dir != NULL)\n+\t\tglobal_path = mkpath(\"%s/%s\", git_hooks_dir, name);\n+\n+\tif ((global_path == NULL || access(global_path, X_OK) > 0) &&\n+\t\t\taccess(local_path, X_OK) > 0)\n+\t\treturn 0;\n+\n+\tva_start(args, name);\n+\tdo {\n+\t\tALLOC_GROW(argv, i + 1, alloc);\n+\t\targv[i++] = va_arg(args, const char *);\n+\t} while (argv[i-1]);\n+\tva_end(args);\n+\n+\tif (global_path != NULL)\n+\t\tret = run_hook_file(global_path, index_file, argv) ||\n+\t\t\trun_hook_file(local_path, index_file, argv);\n+\telse\n+\t\tret = run_hook_file(local_path, index_file, argv);\n \tfree(argv);\n \treturn ret;\n }\ndiff --git a/t/t7510-global-hooks.sh b/t/t7510-global-hooks.sh\nnew file mode 100755\nindex 0000000..d2481ea\n--- /dev/null\n+++ b/t/t7510-global-hooks.sh\n@@ -0,0 +1,88 @@\n+#!/bin/sh\n+\n+test_description='global hooks'\n+\n+. ./test-lib.sh\n+\n+test_expect_success 'with no hooks' '\n+\techo \"foo\" > file &&\n+\tgit add file &&\n+\tgit commit -m \"first\"\n+'\n+\n+GIT_DIR=\"$(git rev-parse --git-dir)\"\n+\n+# now install global hook that always succeeds\n+GLOBALHOOKDIR=\"global-hooks\"\n+GLOBALHOOK=\"$GLOBALHOOKDIR/pre-commit\"\n+mkdir -p \"$GLOBALHOOKDIR\"\n+cat > $GLOBALHOOK <<EOF\n+#!/bin/sh\n+touch $GIT_DIR/global-hook-fired\n+exit 0\n+EOF\n+\n+chmod +x \"$GLOBALHOOK\"\n+\n+# and a local hook that always succeeds\n+HOOKDIR=\"$GIT_DIR/hooks\"\n+HOOK=\"$HOOKDIR/pre-commit\"\n+mkdir -p \"$HOOKDIR\"\n+cat > $HOOK <<EOF\n+#!/bin/sh\n+touch $GIT_DIR/local-hook-fired\n+exit 0\n+EOF\n+\n+chmod +x \"$HOOK\"\n+\n+\n+test_expect_success 'configure global hooks directory' '\n+\tgit config hooks.dir ${GLOBALHOOKDIR}\n+'\n+\n+test_expect_success 'with succeeding hook' '\n+\techo \"more\" >> file &&\n+\tgit add file &&\n+\tgit commit -m \"more\" &&\n+\ttest -e ${GIT_DIR}/global-hook-fired &&\n+\ttest -e ${GIT_DIR}/local-hook-fired\n+'\n+\n+rm $GIT_DIR/global-hook-fired\n+rm $GIT_DIR/local-hook-fired\n+\n+# now a local hook that fails, both hooks should fire\n+cat > $HOOK <<EOF\n+#!/bin/sh\n+touch $GIT_DIR/local-hook-fired\n+exit 1\n+EOF\n+\n+test_expect_success 'with failing global hook' '\n+\techo \"another\" >> file &&\n+\tgit add file &&\n+\ttest_must_fail git commit -m \"another\" &&\n+\ttest -e ${GIT_DIR}/global-hook-fired &&\n+\ttest -e ${GIT_DIR}/local-hook-fired\n+'\n+\n+rm $GIT_DIR/global-hook-fired\n+rm $GIT_DIR/local-hook-fired\n+\n+# now a global hook that fails, the local hook shouldn't fire\n+cat > \"$GLOBALHOOK\" <<EOF\n+#!/bin/sh\n+touch $GIT_DIR/global-hook-fired\n+exit 1\n+EOF\n+\n+test_expect_success 'with failing global hook' '\n+\techo \"another\" >> file &&\n+\tgit add file &&\n+\ttest_must_fail git commit -m \"another\" &&\n+\ttest -e ${GIT_DIR}/global-hook-fired &&\n+\ttest_must_fail test -e ${GIT_DIR}/local-hook-fired\n+'\n+\n+test_done\n-- \n1.7.3.2.162.g4671\n"},{"id":"155380","messageId":"4CD7FB37.4050107@viscovery.net","threadId":"25671","inReplyTo":"1289219520-37435-1-git-send-email-bricollins@gmail.com","subject":"Re: [PATCH v2] Run global hooks from the directory at hooks.dir","fromName":"Johannes Sixt","fromEmail":"j.sixt@viscovery.net","sentAt":"2010-11-08T13:29:27Z","receivedAt":"2010-11-08T13:29:27Z","isPatch":true,"sender":{"key":"j6t@kdbg.org","avatar":"https://avatars.githubusercontent.com/u/14810926?v=4"},"body":"Am 11/8/2010 13:32, schrieb Brian Collins:\n> Run global hooks in the directory specified by the config variable\n> hooks.dir before every attempt at running a local hook. If the\n> global hook fails, the local hook will not run. If the global hook is\n> absent, the local hook runs normally. This is useful because it means\n> you can have scripts that run as hooks for multiple repositories, for\n> example coding style enforcement for an entire organization, or\n> system-wide commit analytics.\n> \n> Signed-off-by: Brian Collins <bricollins@gmail.com>\n> ---\n> \n> The possibility of adding this feature was previously discussed here:\n> http://marc.info/?l=git&m=127808782807807&w=2\n\nI'm not in favor of this change, as a number of alternatives were\nsuggested in the thread you cite (Gmane:\nhttp://thread.gmane.org/gmane.comp.version-control.git/150141).\n\nA few hints, just in case you deploy this code:\n\n> +\tif ((global_path == NULL || access(global_path, X_OK) > 0) &&\n> +\t\t\taccess(local_path, X_OK) > 0)\n> +\t\treturn 0;\n\naccess(2) returns zero on success, negative on failure.\n\n> +\tif (global_path != NULL)\n> +\t\tret = run_hook_file(global_path, index_file, argv) ||\n> +\t\t\trun_hook_file(local_path, index_file, argv);\n\nWhat is the value of ret if the hook in global_path fails? Is it the exit\ncode, or is it 1? What should it be?\n\n-- Hannes\n"},{"id":"155385","messageId":"AANLkTikJACZxf6socuvXFMu7WUZ0y+s0ZRA2kYEKb19q@mail.gmail.com","threadId":"25671","inReplyTo":"AANLkTi=kFuoaV5Ur_a7FJPg_oUs3svOpq=wEVhdpuoai@mail.gmail.com","subject":"Re: [PATCH v2] Run global hooks from the directory at hooks.dir","fromName":"Brian Collins","fromEmail":"bricollins@gmail.com","sentAt":"2010-11-08T13:59:35Z","receivedAt":"2010-11-08T13:59:35Z","isPatch":true,"sender":{"key":"bricollins@gmail.com","avatar":"https://avatars.githubusercontent.com/u/22232?v=4"},"body":"> I'm not in favor of this change, as a number of alternatives were\n> suggested in the thread you cite (Gmane:\n> http://thread.gmane.org/gmane.comp.version-control.git/150141).\n\nThe alternatives suggested need to be applied individually to each\nrepository. This can be problematic because you may want repository-specific\nscripts as well. Then when you update your global hooks you have to deal\nwith merging with your repository-specific scripts. Using templates is not\nideal because it is system-wide and they still need to be renamed for every\nclone or init.\n\n> A few hints, just in case you deploy this code:\n\nThanks, I have fixed those issues.\n"},{"id":"155391","messageId":"20101108171311.GA3619@burratino","threadId":"25671","inReplyTo":"AANLkTikJACZxf6socuvXFMu7WUZ0y+s0ZRA2kYEKb19q@mail.gmail.com","subject":"Re: [PATCH v2] Run global hooks from the directory at hooks.dir","fromName":"Jonathan Nieder","fromEmail":"jrnieder@gmail.com","sentAt":"2010-11-08T17:13:11Z","receivedAt":"2010-11-08T17:13:11Z","isPatch":true,"sender":{"key":"jrnieder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/281595?v=4"},"body":"Brian Collins wrote:\n\n>                                                     Using templates is not\n> ideal because it is system-wide and they still need to be renamed for every\n> clone or init.\n\nWhy do they have to be renamed?  And can we do better?\n"},{"id":"155429","messageId":"7v39rbpn1c.fsf@alter.siamese.dyndns.org","threadId":"25671","inReplyTo":"1289219520-37435-1-git-send-email-bricollins@gmail.com","subject":"Re: [PATCH v2] Run global hooks from the directory at hooks.dir","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2010-11-08T20:39:59Z","receivedAt":"2010-11-08T20:39:59Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Brian Collins <bricollins@gmail.com> writes:\n\n> Run global hooks in the directory specified by the config variable\n> hooks.dir before every attempt at running a local hook. If the\n> global hook fails, the local hook will not run. If the global hook is\n> absent, the local hook runs normally.\n\nIt is left unspecified what happens when the global hook exists and it\nsucceeds.  Watch out for hooks that read from their standard input.\n\nIn any case, the above is totally backwards from the usual practice and\nexpectation of local things overriding the global default.  If you want a\nsite-wide policy suggestion, default templates would be a more acceptable\nway (and the implementation of hooks you install to developer repositories\ncan choose to look at $GIT_DIR/hooks/local-foo-hook).\n\nBy the way, with a distributed scm, anything-wide policy enforcement at\nthe level of developer's individual working repositories is a lost cause.\nYou are giving freedom to do anything on their own copy of the history to\nthe developers; the project-wide policy is to be enforced at the perimeter\nof your authoritative repository of the project.\n"},{"id":"155457","messageId":"AANLkTimx5V7P4yFY0HYP6TNOKa+9fZ1vQUkhoMVL7OeR@mail.gmail.com","threadId":"25671","inReplyTo":"7v39rbpn1c.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH v2] Run global hooks from the directory at hooks.dir","fromName":"Brian Collins","fromEmail":"bricollins@gmail.com","sentAt":"2010-11-09T03:22:43Z","receivedAt":"2010-11-09T03:22:43Z","isPatch":true,"sender":{"key":"bricollins@gmail.com","avatar":"https://avatars.githubusercontent.com/u/22232?v=4"},"body":"> It is left unspecified what happens when the global hook exists and it\n> succeeds.  Watch out for hooks that read from their standard input.\n\nThe local hook will execute. They are chained.\n\n> In any case, the above is totally backwards from the usual practice and\n> expectation of local things overriding the global default.  If you want a\n> site-wide policy suggestion, default templates would be a more acceptable\n> way (and the implementation of hooks you install to developer repositories\n> can choose to look at $GIT_DIR/hooks/local-foo-hook).\n\nThe global hook doesn't override the local hook, it supplements it. In any\ncase, a developer can set hooks.dir to be something else in a specific\nrepository, and it will override the global default.\n\n> By the way, with a distributed scm, anything-wide policy enforcement at\n> the level of developer's individual working repositories is a lost cause.\n> You are giving freedom to do anything on their own copy of the history to\n> the developers; the project-wide policy is to be enforced at the perimeter\n> of your authoritative repository of the project.\n\nThe developer is in total control, it is a user configuration option.\nIt is the same\nas setting colours for diffs. Just a preference that makes the developer's life\neasier by adding some way of automating certain tasks. Say you want to\npost to IRC every time you commit, this could do that. Or if you want to set up\na shell script to jump to your most recently used repository. There are lots of\npossible applications. I admit \"site-wide coding style enforcement\" was a bad\nexample, and I certainly don't plan to use it for that.\n"}]}