threads / patch / 25671

v2Run global hooks from the directory at hooks.dir

Subject: [PATCH v2] Run global hooks from the directory at hooks.dir

## tl;dr

6 messages between Nov 8, 2010 and Nov 9, 2010. Diffs are folded; open one to read it.

replies: 5people: 4as markdown or json

Brian Collins· Nov 8, 2010, 12:32 UTC · lore

Run global hooks in the directory specified by the config variable hooks.dir before every attempt at running a local hook. If the global hook fails, the local hook will not run. If the global hook is absent, the local hook runs normally. This is useful because it means you can have scripts that run as hooks for multiple repositories, for example coding style enforcement for an entire organization, or system-wide commit analytics.

Signed-off-by: Brian Collins <bricollins@gmail.com>
---

The possibility of adding this feature was previously discussed here: http://marc.info/?l=git&m=127808782807807&w=2

Fixed some bad whitespace in contrib/completion/git-completion.bash

Cheers, Brian

 Documentation/config.txt               |    5 ++
 cache.h                                |    2 +
 config.c                               |   12 ++++
 contrib/completion/git-completion.bash |    1 +
 run-command.c                          |   51 +++++++++++++-----
 t/t7510-global-hooks.sh                |   88 ++++++++++++++++++++++++++++++++
 6 files changed, 144 insertions(+), 15 deletions(-)
 create mode 100755 t/t7510-global-hooks.sh
Show changes to 6 files +144 −15

Documentation/config.txt, cache.h, config.c, contrib/completion/git-completion.bash, run-command.c, t/t7510-global-hooks.sh

diff --git a/Documentation/config.txt b/Documentation/config.txt
index 538ebb5..3f18208 100644
--- a/Documentation/config.txt
+++ b/Documentation/config.txt
@@ -1199,6 +1199,11 @@ help.autocorrect::
 	value is 0 - the command will be just shown but not executed.
 	This is the default.
 
+hooks.dir::
+	Directory containing executable hooks. Usage is identical to
+	.git/hooks except they are triggered for any repository used
+	while this configuration is set.
+
 http.proxy::
 	Override the HTTP proxy, normally configured using the 'http_proxy'
 	environment variable (see linkgit:curl[1]).  This can be overridden
diff --git a/cache.h b/cache.h
index 33decd9..c75a444 100644
--- a/cache.h
+++ b/cache.h
@@ -1018,6 +1018,8 @@ extern const char *git_commit_encoding;
 extern const char *git_log_output_encoding;
 extern const char *git_mailmap_file;
 
+extern const char *git_hooks_dir;
+
 /* IO helper functions */
 extern void maybe_flush_or_die(FILE *, const char *);
 extern int copy_fd(int ifd, int ofd);
diff --git a/config.c b/config.c
index 4b0a820..1a9d5ee 100644
--- a/config.c
+++ b/config.c
@@ -745,6 +745,15 @@ static int git_default_mailmap_config(const char *var, const char *value)
 	return 0;
 }
 
+static int git_default_hooks_config(const char *var, const char *value)
+{
+	if (!strcmp(var, "hooks.dir"))
+		return git_config_pathname(&git_hooks_dir, var, value);
+
+	/* Add other config variables here and to Documentation/config.txt. */
+	return 0;
+}
+
 int git_default_config(const char *var, const char *value, void *dummy)
 {
 	if (!prefixcmp(var, "core."))
@@ -768,6 +777,9 @@ int git_default_config(const char *var, const char *value, void *dummy)
 	if (!prefixcmp(var, "advice."))
 		return git_default_advice_config(var, value);
 
+	if (!prefixcmp(var, "hooks."))
+		return git_default_hooks_config(var, value);
+
 	if (!strcmp(var, "pager.color") || !strcmp(var, "color.pager")) {
 		pager_use_color = git_config_bool(var,value);
 		return 0;
diff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash
index 168669b..b266f83 100755
--- a/contrib/completion/git-completion.bash
+++ b/contrib/completion/git-completion.bash
@@ -1913,6 +1913,7 @@ _git_config ()
 		help.autocorrect
 		help.browser
 		help.format
+		hooks.dir
 		http.lowSpeedLimit
 		http.lowSpeedTime
 		http.maxRequests
diff --git a/run-command.c b/run-command.c
index 2a1041e..8e33b80 100644
--- a/run-command.c
+++ b/run-command.c
@@ -2,6 +2,8 @@
 #include "run-command.h"
 #include "exec_cmd.h"
 
+const char *git_hooks_dir;
+
 static inline void close_pair(int fd[2])
 {
 	close(fd[0]);
@@ -603,26 +605,16 @@ int finish_async(struct async *async)
 #endif
 }
 
-int run_hook(const char *index_file, const char *name, ...)
+int run_hook_file(const char *path, const char *index_file, const char **argv)
 {
 	struct child_process hook;
-	const char **argv = NULL, *env[2];
 	char index[PATH_MAX];
-	va_list args;
-	int ret;
-	size_t i = 0, alloc = 0;
+	const char *env[2];
 
-	if (access(git_path("hooks/%s", name), X_OK) < 0)
+	if (access(path, X_OK) < 0)
 		return 0;
 
-	va_start(args, name);
-	ALLOC_GROW(argv, i + 1, alloc);
-	argv[i++] = git_path("hooks/%s", name);
-	while (argv[i-1]) {
-		ALLOC_GROW(argv, i + 1, alloc);
-		argv[i++] = va_arg(args, const char *);
-	}
-	va_end(args);
+	argv[0] = path;
 
 	memset(&hook, 0, sizeof(hook));
 	hook.argv = argv;
@@ -635,7 +627,36 @@ int run_hook(const char *index_file, const char *name, ...)
 		hook.env = env;
 	}
 
-	ret = run_command(&hook);
+	return run_command(&hook);
+}
+
+int run_hook(const char *index_file, const char *name, ...)
+{
+	const char **argv = NULL;
+	va_list args;
+	int ret;
+	size_t i = 1, alloc = 0;
+	const char *local_path = git_path("hooks/%s", name);
+	const char *global_path = NULL;
+	if (git_hooks_dir != NULL)
+		global_path = mkpath("%s/%s", git_hooks_dir, name);
+
+	if ((global_path == NULL || access(global_path, X_OK) > 0) &&
+			access(local_path, X_OK) > 0)
+		return 0;
+
+	va_start(args, name);
+	do {
+		ALLOC_GROW(argv, i + 1, alloc);
+		argv[i++] = va_arg(args, const char *);
+	} while (argv[i-1]);
+	va_end(args);
+
+	if (global_path != NULL)
+		ret = run_hook_file(global_path, index_file, argv) ||
+			run_hook_file(local_path, index_file, argv);
+	else
+		ret = run_hook_file(local_path, index_file, argv);
 	free(argv);
 	return ret;
 }
diff --git a/t/t7510-global-hooks.sh b/t/t7510-global-hooks.sh
new file mode 100755
index 0000000..d2481ea
--- /dev/null
+++ b/t/t7510-global-hooks.sh
@@ -0,0 +1,88 @@
+#!/bin/sh
+
+test_description='global hooks'
+
+. ./test-lib.sh
+
+test_expect_success 'with no hooks' '
+	echo "foo" > file &&
+	git add file &&
+	git commit -m "first"
+'
+
+GIT_DIR="$(git rev-parse --git-dir)"
+
+# now install global hook that always succeeds
+GLOBALHOOKDIR="global-hooks"
+GLOBALHOOK="$GLOBALHOOKDIR/pre-commit"
+mkdir -p "$GLOBALHOOKDIR"
+cat > $GLOBALHOOK <<EOF
+#!/bin/sh
+touch $GIT_DIR/global-hook-fired
+exit 0
+EOF
+
+chmod +x "$GLOBALHOOK"
+
+# and a local hook that always succeeds
+HOOKDIR="$GIT_DIR/hooks"
+HOOK="$HOOKDIR/pre-commit"
+mkdir -p "$HOOKDIR"
+cat > $HOOK <<EOF
+#!/bin/sh
+touch $GIT_DIR/local-hook-fired
+exit 0
+EOF
+
+chmod +x "$HOOK"
+
+
+test_expect_success 'configure global hooks directory' '
+	git config hooks.dir ${GLOBALHOOKDIR}
+'
+
+test_expect_success 'with succeeding hook' '
+	echo "more" >> file &&
+	git add file &&
+	git commit -m "more" &&
+	test -e ${GIT_DIR}/global-hook-fired &&
+	test -e ${GIT_DIR}/local-hook-fired
+'
+
+rm $GIT_DIR/global-hook-fired
+rm $GIT_DIR/local-hook-fired
+
+# now a local hook that fails, both hooks should fire
+cat > $HOOK <<EOF
+#!/bin/sh
+touch $GIT_DIR/local-hook-fired
+exit 1
+EOF
+
+test_expect_success 'with failing global hook' '
+	echo "another" >> file &&
+	git add file &&
+	test_must_fail git commit -m "another" &&
+	test -e ${GIT_DIR}/global-hook-fired &&
+	test -e ${GIT_DIR}/local-hook-fired
+'
+
+rm $GIT_DIR/global-hook-fired
+rm $GIT_DIR/local-hook-fired
+
+# now a global hook that fails, the local hook shouldn't fire
+cat > "$GLOBALHOOK" <<EOF
+#!/bin/sh
+touch $GIT_DIR/global-hook-fired
+exit 1
+EOF
+
+test_expect_success 'with failing global hook' '
+	echo "another" >> file &&
+	git add file &&
+	test_must_fail git commit -m "another" &&
+	test -e ${GIT_DIR}/global-hook-fired &&
+	test_must_fail test -e ${GIT_DIR}/local-hook-fired
+'
+
+test_done
-- 
1.7.3.2.162.g4671
Johannes Sixt· Nov 8, 2010, 13:29 UTC · re: Brian Collins · lore

Re: [PATCH v2] Run global hooks from the directory at hooks.dir

Am 11/8/2010 13:32, schrieb Brian Collins:
Show 13 quoted lines
> Run global hooks in the directory specified by the config variable
> hooks.dir before every attempt at running a local hook. If the
> global hook fails, the local hook will not run. If the global hook is
> absent, the local hook runs normally. This is useful because it means
> you can have scripts that run as hooks for multiple repositories, for
> example coding style enforcement for an entire organization, or
> system-wide commit analytics.
> 
> Signed-off-by: Brian Collins <bricollins@gmail.com>
> ---
> 
> The possibility of adding this feature was previously discussed here:
> http://marc.info/?l=git&m=127808782807807&w=2

I'm not in favor of this change, as a number of alternatives were suggested in the thread you cite (Gmane: http://thread.gmane.org/gmane.comp.version-control.git/150141).

A few hints, just in case you deploy this code:
> +	if ((global_path == NULL || access(global_path, X_OK) > 0) &&
> +			access(local_path, X_OK) > 0)
> +		return 0;
access(2) returns zero on success, negative on failure.
> +	if (global_path != NULL)
> +		ret = run_hook_file(global_path, index_file, argv) ||
> +			run_hook_file(local_path, index_file, argv);

What is the value of ret if the hook in global_path fails? Is it the exit code, or is it 1? What should it be?

-- Hannes
Junio C Hamano· Nov 8, 2010, 20:39 UTC · re: Brian Collins · lore

Re: [PATCH v2] Run global hooks from the directory at hooks.dir

Brian Collins <bricollins@gmail.com> writes:
> Run global hooks in the directory specified by the config variable
> hooks.dir before every attempt at running a local hook. If the
> global hook fails, the local hook will not run. If the global hook is
> absent, the local hook runs normally.

It is left unspecified what happens when the global hook exists and it succeeds. Watch out for hooks that read from their standard input.

In any case, the above is totally backwards from the usual practice and expectation of local things overriding the global default. If you want a site-wide policy suggestion, default templates would be a more acceptable way (and the implementation of hooks you install to developer repositories can choose to look at $GIT_DIR/hooks/local-foo-hook).

By the way, with a distributed scm, anything-wide policy enforcement at the level of developer's individual working repositories is a lost cause. You are giving freedom to do anything on their own copy of the history to the developers; the project-wide policy is to be enforced at the perimeter of your authoritative repository of the project.

Brian Collins· Nov 9, 2010, 03:22 UTC · re: Junio C Hamano · lore

Re: [PATCH v2] Run global hooks from the directory at hooks.dir

> It is left unspecified what happens when the global hook exists and it
> succeeds.  Watch out for hooks that read from their standard input.
The local hook will execute. They are chained.
Show 5 quoted lines
> In any case, the above is totally backwards from the usual practice and
> expectation of local things overriding the global default.  If you want a
> site-wide policy suggestion, default templates would be a more acceptable
> way (and the implementation of hooks you install to developer repositories
> can choose to look at $GIT_DIR/hooks/local-foo-hook).

The global hook doesn't override the local hook, it supplements it. In any case, a developer can set hooks.dir to be something else in a specific repository, and it will override the global default.

Show 5 quoted lines
> By the way, with a distributed scm, anything-wide policy enforcement at
> the level of developer's individual working repositories is a lost cause.
> You are giving freedom to do anything on their own copy of the history to
> the developers; the project-wide policy is to be enforced at the perimeter
> of your authoritative repository of the project.

The developer is in total control, it is a user configuration option. It is the same as setting colours for diffs. Just a preference that makes the developer's life easier by adding some way of automating certain tasks. Say you want to post to IRC every time you commit, this could do that. Or if you want to set up a shell script to jump to your most recently used repository. There are lots of possible applications. I admit "site-wide coding style enforcement" was a bad example, and I certainly don't plan to use it for that.

Brian Collins· Nov 8, 2010, 13:59 UTC · lore

Re: [PATCH v2] Run global hooks from the directory at hooks.dir

> I'm not in favor of this change, as a number of alternatives were
> suggested in the thread you cite (Gmane:
> http://thread.gmane.org/gmane.comp.version-control.git/150141).

The alternatives suggested need to be applied individually to each repository. This can be problematic because you may want repository-specific scripts as well. Then when you update your global hooks you have to deal with merging with your repository-specific scripts. Using templates is not ideal because it is system-wide and they still need to be renamed for every clone or init.

> A few hints, just in case you deploy this code:
Thanks, I have fixed those issues.
Jonathan Nieder· Nov 8, 2010, 17:13 UTC · re: Brian Collins · lore

Re: [PATCH v2] Run global hooks from the directory at hooks.dir

Brian Collins wrote:
>                                                     Using templates is not
> ideal because it is system-wide and they still need to be renamed for every
> clone or init.
Why do they have to be renamed?  And can we do better?

← back to recent threads