threads / patch / 42118

patchhooks: Add ability to specify where the hook directory is

Subject: [PATCH] hooks: Add ability to specify where the hook directory is

## tl;dr

10 messages between Apr 22, 2016 and Apr 26, 2016. Diffs are folded; open one to read it.

replies: 9people: 4as markdown or json

Ævar Arnfjörð Bjarmason· Apr 22, 2016, 23:33 UTC · lore

Change the hardcoded lookup for .git/hooks/* to optionally lookup in $(git config core.hooksDirectory)/* instead if that config key is set.

This is essentially a more intrusive version of the git-init ability to specify hooks on init time via init templates.

The difference between that facility and this feature is that this can be set up after the fact via e.g. ~/.gitconfig or /etc/gitconfig to apply for all your personal repositories, or all repositories on the system.

I plan on using this on a centralized Git server where users can create arbitrary repositories under /gitroot, but I'd like to manage all the hooks that should be run centrally via a unified dispatch mechanism.

Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
---
 Documentation/config.txt          | 10 ++++++++++
 Documentation/githooks.txt        |  5 ++++-
 cache.h                           |  1 +
 config.c                          |  3 +++
 environment.c                     |  1 +
 run-command.c                     |  5 ++++-
 t/t1350-config-hooks-directory.sh | 35 +++++++++++++++++++++++++++++++++++
 7 files changed, 58 insertions(+), 2 deletions(-)
 create mode 100755 t/t1350-config-hooks-directory.sh
Show changes to 7 files +58 −2

Documentation/config.txt, Documentation/githooks.txt, cache.h, config.c, environment.c, run-command.c, t/t1350-config-hooks-directory.sh

diff --git a/Documentation/config.txt b/Documentation/config.txt
index 42d2b50..2faf3c0 100644
--- a/Documentation/config.txt
+++ b/Documentation/config.txt
@@ -618,6 +618,16 @@ core.attributesFile::
 	$XDG_CONFIG_HOME/git/attributes. If $XDG_CONFIG_HOME is either not
 	set or empty, $HOME/.config/git/attributes is used instead.
 
+core.hooksDirectory::
+	By default Git will look for your hooks in the '$GIT_DIR/hooks'
+	directory. Set this to different absolute directory name,
+	e.g. '/etc/git/hooks', and Git will try to find your hooks that
+	directory, e.g. '/etc/git/hooks/pre-receive' instead of in
+	'$GIT_DIR/hooks'.
++
+This is useful in cases where you'd like to centrally configure your
+Git hooks instead of configuring them on a per-repository basis.
+
 core.editor::
 	Commands such as `commit` and `tag` that lets you edit
 	messages by launching an editor uses the value of this
diff --git a/Documentation/githooks.txt b/Documentation/githooks.txt
index a2f59b1..e1fe66d 100644
--- a/Documentation/githooks.txt
+++ b/Documentation/githooks.txt
@@ -13,13 +13,16 @@ $GIT_DIR/hooks/*
 DESCRIPTION
 -----------
 
-Hooks are little scripts you can place in `$GIT_DIR/hooks`
+Hooks are little scripts you can place in the `hooks`
 directory to trigger action at certain points.  When
 'git init' is run, a handful of example hooks are copied into the
 `hooks` directory of the new repository, but by default they are
 all disabled.  To enable a hook, rename it by removing its `.sample`
 suffix.
 
+By default the hooks directory is `$GIT_DIR/hooks`, but that can be
+changed via the `core.hooksDirectory` (see linkgit:git-config[1]).
+
 NOTE: It is also a requirement for a given hook to be executable.
 However - in a freshly initialized repository - the `.sample` files are
 executable by default.
diff --git a/cache.h b/cache.h
index 2711048..85ad594 100644
--- a/cache.h
+++ b/cache.h
@@ -654,6 +654,7 @@ extern int warn_on_object_refname_ambiguity;
 extern const char *apply_default_whitespace;
 extern const char *apply_default_ignorewhitespace;
 extern const char *git_attributes_file;
+extern const char *git_hooks_directory;
 extern int zlib_compression_level;
 extern int core_compression_level;
 extern int core_compression_seen;
diff --git a/config.c b/config.c
index 10b5c95..543de4e 100644
--- a/config.c
+++ b/config.c
@@ -717,6 +717,9 @@ static int git_default_core_config(const char *var, const char *value)
 	if (!strcmp(var, "core.attributesfile"))
 		return git_config_pathname(&git_attributes_file, var, value);
 
+	if (!strcmp(var, "core.hooksdirectory"))
+		return git_config_pathname(&git_hooks_directory, var, value);
+
 	if (!strcmp(var, "core.bare")) {
 		is_bare_repository_cfg = git_config_bool(var, value);
 		return 0;
diff --git a/environment.c b/environment.c
index 57acb2f..ffb5dec 100644
--- a/environment.c
+++ b/environment.c
@@ -31,6 +31,7 @@ const char *git_log_output_encoding;
 const char *apply_default_whitespace;
 const char *apply_default_ignorewhitespace;
 const char *git_attributes_file;
+const char *git_hooks_directory;
 int zlib_compression_level = Z_BEST_SPEED;
 int core_compression_level;
 int core_compression_seen;
diff --git a/run-command.c b/run-command.c
index 8c7115a..ae8e470 100644
--- a/run-command.c
+++ b/run-command.c
@@ -815,7 +815,10 @@ const char *find_hook(const char *name)
 	static struct strbuf path = STRBUF_INIT;
 
 	strbuf_reset(&path);
-	strbuf_git_path(&path, "hooks/%s", name);
+	if (git_hooks_directory)
+		strbuf_addf(&path, "%s/%s", git_hooks_directory, name);
+	else
+		strbuf_git_path(&path, "hooks/%s", name);
 	if (access(path.buf, X_OK) < 0)
 		return NULL;
 	return path.buf;
diff --git a/t/t1350-config-hooks-directory.sh b/t/t1350-config-hooks-directory.sh
new file mode 100755
index 0000000..556c1d3
--- /dev/null
+++ b/t/t1350-config-hooks-directory.sh
@@ -0,0 +1,35 @@
+#!/bin/sh
+
+test_description='Test the core.hooksDirectory configuration variable'
+
+. ./test-lib.sh
+
+test_expect_success 'set up a pre-commit hook in core.hooksDirectory' '
+	mkdir -p .git/custom-hooks .git/hooks &&
+	cat >.git/custom-hooks/pre-commit <<EOF &&
+#!$SHELL_PATH
+printf "%s" "." >>.git/PRE-COMMIT-HOOK-WAS-CALLED
+EOF
+	cat >.git/hooks/pre-commit <<EOF &&
+	chmod +x .git/hooks/pre-commit
+#!$SHELL_PATH
+printf "%s" "SHOULD NOT BE CALLED" >>.git/PRE-COMMIT-HOOK-WAS-CALLED
+EOF
+	chmod +x .git/custom-hooks/pre-commit
+'
+
+test_expect_success 'Check that various forms of specifying core.hooksDirectory work' '
+	test_commit no_custom_hook &&
+	git config core.hooksDirectory .git/custom-hooks &&
+	test_commit have_custom_hook &&
+	git config core.hooksDirectory .git/custom-hooks/ &&
+	test_commit have_custom_hook_trailing_slash &&
+	git config core.hooksDirectory "$PWD/.git/custom-hooks" &&
+	test_commit have_custom_hook_abs_path &&
+	git config core.hooksDirectory "$PWD/.git/custom-hooks/" &&
+	test_commit have_custom_hook_abs_path_trailing_slash &&
+    printf "%s" "...." >.git/PRE-COMMIT-HOOK-WAS-CALLED.expect &&
+    test_cmp .git/PRE-COMMIT-HOOK-WAS-CALLED.expect .git/PRE-COMMIT-HOOK-WAS-CALLED
+'
+
+test_done
-- 
2.1.3
SZEDER Gábor· Apr 23, 2016, 00:13 UTC · re: Ævar Arnfjörð Bjarmason · lore

Re: [PATCH] hooks: Add ability to specify where the hook directory is

Show 38 quoted lines
> Change the hardcoded lookup for .git/hooks/* to optionally lookup in
> $(git config core.hooksDirectory)/* instead if that config key is set.
> 
> This is essentially a more intrusive version of the git-init ability to
> specify hooks on init time via init templates.
> 
> The difference between that facility and this feature is that this can
> be set up after the fact via e.g. ~/.gitconfig or /etc/gitconfig to
> apply for all your personal repositories, or all repositories on the
> system.
> 
> I plan on using this on a centralized Git server where users can create
> arbitrary repositories under /gitroot, but I'd like to manage all the
> hooks that should be run centrally via a unified dispatch mechanism.
> 
> Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
> ---
>  Documentation/config.txt          | 10 ++++++++++
>  Documentation/githooks.txt        |  5 ++++-
>  cache.h                           |  1 +
>  config.c                          |  3 +++
>  environment.c                     |  1 +
>  run-command.c                     |  5 ++++-
>  t/t1350-config-hooks-directory.sh | 35 +++++++++++++++++++++++++++++++++++
>  7 files changed, 58 insertions(+), 2 deletions(-)
>  create mode 100755 t/t1350-config-hooks-directory.sh
> 
> diff --git a/Documentation/config.txt b/Documentation/config.txt
> index 42d2b50..2faf3c0 100644
> --- a/Documentation/config.txt
> +++ b/Documentation/config.txt
> @@ -618,6 +618,16 @@ core.attributesFile::
>  	$XDG_CONFIG_HOME/git/attributes. If $XDG_CONFIG_HOME is either not
>  	set or empty, $HOME/.config/git/attributes is used instead.
>  
> +core.hooksDirectory::
> +	By default Git will look for your hooks in the '$GIT_DIR/hooks'
> +	directory. Set this to different absolute directory name,
Mental note: here you say that it should be an absolute directory.
> +	e.g. '/etc/git/hooks', and Git will try to find your hooks that
s/hooks that/hooks in that/
Show 9 quoted lines
> +	directory, e.g. '/etc/git/hooks/pre-receive' instead of in
> +	'$GIT_DIR/hooks'.
> ++
> +This is useful in cases where you'd like to centrally configure your
> +Git hooks instead of configuring them on a per-repository basis.
> +
>  core.editor::
>  	Commands such as `commit` and `tag` that lets you edit
>  	messages by launching an editor uses the value of this
Show 25 quoted lines
> diff --git a/t/t1350-config-hooks-directory.sh b/t/t1350-config-hooks-directory.sh
> new file mode 100755
> index 0000000..556c1d3
> --- /dev/null
> +++ b/t/t1350-config-hooks-directory.sh
> @@ -0,0 +1,35 @@
> +#!/bin/sh
> +
> +test_description='Test the core.hooksDirectory configuration variable'
> +
> +. ./test-lib.sh
> +
> +test_expect_success 'set up a pre-commit hook in core.hooksDirectory' '
> +	mkdir -p .git/custom-hooks .git/hooks &&
> +	cat >.git/custom-hooks/pre-commit <<EOF &&
> +#!$SHELL_PATH
> +printf "%s" "." >>.git/PRE-COMMIT-HOOK-WAS-CALLED
> +EOF
> +	cat >.git/hooks/pre-commit <<EOF &&
> +	chmod +x .git/hooks/pre-commit
> +#!$SHELL_PATH
> +printf "%s" "SHOULD NOT BE CALLED" >>.git/PRE-COMMIT-HOOK-WAS-CALLED
> +EOF
> +	chmod +x .git/custom-hooks/pre-commit
> +'
Please use the 'write_script' helper for, well, writing scripts.
Show 7 quoted lines
> +
> +test_expect_success 'Check that various forms of specifying core.hooksDirectory work' '
> +	test_commit no_custom_hook &&
> +	git config core.hooksDirectory .git/custom-hooks &&
> +	test_commit have_custom_hook &&
> +	git config core.hooksDirectory .git/custom-hooks/ &&
> +	test_commit have_custom_hook_trailing_slash &&

These two cases ensure that it should work even when the configured hook directory is given as a relative path, though the docs say it should be an absolute path.

Show 6 quoted lines
> +	git config core.hooksDirectory "$PWD/.git/custom-hooks" &&
> +	test_commit have_custom_hook_abs_path &&
> +	git config core.hooksDirectory "$PWD/.git/custom-hooks/" &&
> +	test_commit have_custom_hook_abs_path_trailing_slash &&
> +    printf "%s" "...." >.git/PRE-COMMIT-HOOK-WAS-CALLED.expect &&
> +    test_cmp .git/PRE-COMMIT-HOOK-WAS-CALLED.expect .git/PRE-COMMIT-HOOK-WAS-CALLED
Indentation with spaces.
Show 5 quoted lines
> +'
> +
> +test_done
> -- 
> 2.1.3
Ævar Arnfjörð Bjarmason· Apr 23, 2016, 00:44 UTC · re: SZEDER Gábor · lore

Re: [PATCH] hooks: Add ability to specify where the hook directory is

On Sat, Apr 23, 2016 at 2:13 AM, SZEDER Gábor <szeder@ira.uka.de> wrote:
Show 39 quoted lines
>
>> Change the hardcoded lookup for .git/hooks/* to optionally lookup in
>> $(git config core.hooksDirectory)/* instead if that config key is set.
>>
>> This is essentially a more intrusive version of the git-init ability to
>> specify hooks on init time via init templates.
>>
>> The difference between that facility and this feature is that this can
>> be set up after the fact via e.g. ~/.gitconfig or /etc/gitconfig to
>> apply for all your personal repositories, or all repositories on the
>> system.
>>
>> I plan on using this on a centralized Git server where users can create
>> arbitrary repositories under /gitroot, but I'd like to manage all the
>> hooks that should be run centrally via a unified dispatch mechanism.
>>
>> Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
>> ---
>>  Documentation/config.txt          | 10 ++++++++++
>>  Documentation/githooks.txt        |  5 ++++-
>>  cache.h                           |  1 +
>>  config.c                          |  3 +++
>>  environment.c                     |  1 +
>>  run-command.c                     |  5 ++++-
>>  t/t1350-config-hooks-directory.sh | 35 +++++++++++++++++++++++++++++++++++
>>  7 files changed, 58 insertions(+), 2 deletions(-)
>>  create mode 100755 t/t1350-config-hooks-directory.sh
>>
>> diff --git a/Documentation/config.txt b/Documentation/config.txt
>> index 42d2b50..2faf3c0 100644
>> --- a/Documentation/config.txt
>> +++ b/Documentation/config.txt
>> @@ -618,6 +618,16 @@ core.attributesFile::
>>       $XDG_CONFIG_HOME/git/attributes. If $XDG_CONFIG_HOME is either not
>>       set or empty, $HOME/.config/git/attributes is used instead.
>>
>> +core.hooksDirectory::
>> +     By default Git will look for your hooks in the '$GIT_DIR/hooks'
>> +     directory. Set this to different absolute directory name,

Thanks for the review. I'll submit a new patch with fixes pending any more comments from others to reduce list churn.

Show 5 quoted lines
> Mental note: here you say that it should be an absolute directory.
>
>> +     e.g. '/etc/git/hooks', and Git will try to find your hooks that
>
> s/hooks that/hooks in that/
Will fix.
Show 38 quoted lines
>> +     directory, e.g. '/etc/git/hooks/pre-receive' instead of in
>> +     '$GIT_DIR/hooks'.
>> ++
>> +This is useful in cases where you'd like to centrally configure your
>> +Git hooks instead of configuring them on a per-repository basis.
>> +
>>  core.editor::
>>       Commands such as `commit` and `tag` that lets you edit
>>       messages by launching an editor uses the value of this
>
>
>> diff --git a/t/t1350-config-hooks-directory.sh b/t/t1350-config-hooks-directory.sh
>> new file mode 100755
>> index 0000000..556c1d3
>> --- /dev/null
>> +++ b/t/t1350-config-hooks-directory.sh
>> @@ -0,0 +1,35 @@
>> +#!/bin/sh
>> +
>> +test_description='Test the core.hooksDirectory configuration variable'
>> +
>> +. ./test-lib.sh
>> +
>> +test_expect_success 'set up a pre-commit hook in core.hooksDirectory' '
>> +     mkdir -p .git/custom-hooks .git/hooks &&
>> +     cat >.git/custom-hooks/pre-commit <<EOF &&
>> +#!$SHELL_PATH
>> +printf "%s" "." >>.git/PRE-COMMIT-HOOK-WAS-CALLED
>> +EOF
>> +     cat >.git/hooks/pre-commit <<EOF &&
>> +     chmod +x .git/hooks/pre-commit
>> +#!$SHELL_PATH
>> +printf "%s" "SHOULD NOT BE CALLED" >>.git/PRE-COMMIT-HOOK-WAS-CALLED
>> +EOF
>> +     chmod +x .git/custom-hooks/pre-commit
>> +'
>
> Please use the 'write_script' helper for, well, writing scripts.
Will fix.
Show 11 quoted lines
>> +
>> +test_expect_success 'Check that various forms of specifying core.hooksDirectory work' '
>> +     test_commit no_custom_hook &&
>> +     git config core.hooksDirectory .git/custom-hooks &&
>> +     test_commit have_custom_hook &&
>> +     git config core.hooksDirectory .git/custom-hooks/ &&
>> +     test_commit have_custom_hook_trailing_slash &&
>
> These two cases ensure that it should work even when the configured
> hook directory is given as a relative path, though the docs say it
> should be an absolute path.

Yeah I didn't mention this in the docs or commit message, but I figured not promising anything else in the docs made sense.

I.e. a relative path of e.g. .git/custom-hook/* won't work if you're cd'd into a subdirectory of your repository, so telling users that it has to be absolute is probably an OK white lie.

Show 8 quoted lines
>> +     git config core.hooksDirectory "$PWD/.git/custom-hooks" &&
>> +     test_commit have_custom_hook_abs_path &&
>> +     git config core.hooksDirectory "$PWD/.git/custom-hooks/" &&
>> +     test_commit have_custom_hook_abs_path_trailing_slash &&
>> +    printf "%s" "...." >.git/PRE-COMMIT-HOOK-WAS-CALLED.expect &&
>> +    test_cmp .git/PRE-COMMIT-HOOK-WAS-CALLED.expect .git/PRE-COMMIT-HOOK-WAS-CALLED
>
> Indentation with spaces.
Will fix.
Show 5 quoted lines
>> +'
>> +
>> +test_done
>> --
>> 2.1.3
Ævar Arnfjörð Bjarmason· Apr 24, 2016, 21:18 UTC · re: Ævar Arnfjörð Bjarmason · lore

[PATCH v2] hooks: Add ability to specify where the hook directory is

Change the hardcoded lookup for .git/hooks/* to optionally lookup in $(git config core.hooksPath)/* instead.

This is essentially a more intrusive version of the git-init ability to specify hooks on init time via init templates.

The difference between that facility and this feature is that this can be set up after the fact via e.g. ~/.gitconfig or /etc/gitconfig to apply for all your personal repositories, or all repositories on the system.

I plan on using this on a centralized Git server where users can create arbitrary repositories under /gitroot, but I'd like to manage all the hooks that should be run centrally via a unified dispatch mechanism.

Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
---

This is based off the githooks.txt series I just sent, I thought it made the most sense to address the questions the previous version raised about how we treat the working directory with unrelated githooks.txt doc improvements.

This version also address all the other issues SZEDER Gábor noted.
 Documentation/config.txt     | 17 +++++++++++++++++
 Documentation/githooks.txt   | 12 ++++++++----
 cache.h                      |  1 +
 config.c                     |  3 +++
 environment.c                |  1 +
 run-command.c                |  5 ++++-
 t/t1350-config-hooks-path.sh | 33 +++++++++++++++++++++++++++++++++
 7 files changed, 67 insertions(+), 5 deletions(-)
 create mode 100755 t/t1350-config-hooks-path.sh
Show changes to 7 files +67 −5

Documentation/config.txt, Documentation/githooks.txt, cache.h, config.c, environment.c, run-command.c, t/t1350-config-hooks-path.sh

diff --git a/Documentation/config.txt b/Documentation/config.txt
index 42d2b50..6a079c1 100644
--- a/Documentation/config.txt
+++ b/Documentation/config.txt
@@ -618,6 +618,23 @@ core.attributesFile::
 	$XDG_CONFIG_HOME/git/attributes. If $XDG_CONFIG_HOME is either not
 	set or empty, $HOME/.config/git/attributes is used instead.
 
+core.hooksPath::
+	By default Git will look for your hooks in the
+	'$GIT_DIR/hooks' directory. Set this to different path,
+	e.g. '/etc/git/hooks', and Git will try to find your hooks in
+	that directory, e.g. '/etc/git/hooks/pre-receive' instead of
+	in '$GIT_DIR/hooks/pre-receive'.
++
+The path can either be absolute or relative. In the latter case see
+the discussion in the "DESCRIPTION" section of linkgit:githooks[5]
+about what the relative path will be relative to.
++
+This configuration is useful in cases where you'd like to
+e.g. centrally configure your Git hooks instead of configuring them on
+a per-repository basis, or a more flexible and centralized alterantive
+having an `init.templateDir` where you've changed the 'hooks'
+directory.
+
 core.editor::
 	Commands such as `commit` and `tag` that lets you edit
 	messages by launching an editor uses the value of this
diff --git a/Documentation/githooks.txt b/Documentation/githooks.txt
index d30492c..3b1591f 100644
--- a/Documentation/githooks.txt
+++ b/Documentation/githooks.txt
@@ -7,15 +7,19 @@ githooks - Hooks used by Git
 
 SYNOPSIS
 --------
-$GIT_DIR/hooks/*
+$GIT_DIR/hooks/* (or `git config core.hooksPath`/*)
 
 
 DESCRIPTION
 -----------
 
-Hooks are programs you can place in the `$GIT_DIR/hooks` directory to
-trigger action at certain points. Hooks that don't have the executable
-bit set are ignored.
+Hooks are programs you can place in a hooks directory to trigger action
+at certain points. Hooks that don't have the executable bit set are
+ignored.
+
+By default the hooks directory is `$GIT_DIR/hooks`, but that can be
+changed via the `core.hooksPath` configuration variable (see
+linkgit:git-config[1]).
 
 When a hook is called in a non-bare repository the working directory
 is guaranteed to be the root of the working tree, in a bare repository
diff --git a/cache.h b/cache.h
index 2711048..4f7d222 100644
--- a/cache.h
+++ b/cache.h
@@ -654,6 +654,7 @@ extern int warn_on_object_refname_ambiguity;
 extern const char *apply_default_whitespace;
 extern const char *apply_default_ignorewhitespace;
 extern const char *git_attributes_file;
+extern const char *git_hooks_path;
 extern int zlib_compression_level;
 extern int core_compression_level;
 extern int core_compression_seen;
diff --git a/config.c b/config.c
index 10b5c95..51f80e4 100644
--- a/config.c
+++ b/config.c
@@ -717,6 +717,9 @@ static int git_default_core_config(const char *var, const char *value)
 	if (!strcmp(var, "core.attributesfile"))
 		return git_config_pathname(&git_attributes_file, var, value);
 
+	if (!strcmp(var, "core.hookspath"))
+		return git_config_pathname(&git_hooks_path, var, value);
+
 	if (!strcmp(var, "core.bare")) {
 		is_bare_repository_cfg = git_config_bool(var, value);
 		return 0;
diff --git a/environment.c b/environment.c
index 57acb2f..2857e3f 100644
--- a/environment.c
+++ b/environment.c
@@ -31,6 +31,7 @@ const char *git_log_output_encoding;
 const char *apply_default_whitespace;
 const char *apply_default_ignorewhitespace;
 const char *git_attributes_file;
+const char *git_hooks_path;
 int zlib_compression_level = Z_BEST_SPEED;
 int core_compression_level;
 int core_compression_seen;
diff --git a/run-command.c b/run-command.c
index 8c7115a..39d7237 100644
--- a/run-command.c
+++ b/run-command.c
@@ -815,7 +815,10 @@ const char *find_hook(const char *name)
 	static struct strbuf path = STRBUF_INIT;
 
 	strbuf_reset(&path);
-	strbuf_git_path(&path, "hooks/%s", name);
+	if (git_hooks_path)
+		strbuf_addf(&path, "%s/%s", git_hooks_path, name);
+	else
+		strbuf_git_path(&path, "hooks/%s", name);
 	if (access(path.buf, X_OK) < 0)
 		return NULL;
 	return path.buf;
diff --git a/t/t1350-config-hooks-path.sh b/t/t1350-config-hooks-path.sh
new file mode 100755
index 0000000..31461aa
--- /dev/null
+++ b/t/t1350-config-hooks-path.sh
@@ -0,0 +1,33 @@
+#!/bin/sh
+
+test_description='Test the core.hooksPath configuration variable'
+
+. ./test-lib.sh
+
+test_expect_success 'set up a pre-commit hook in core.hooksPath' '
+	mkdir -p .git/custom-hooks .git/hooks &&
+	write_script .git/custom-hooks/pre-commit <<EOF &&
+printf "%s" "." >>.git/PRE-COMMIT-HOOK-WAS-CALLED
+EOF
+	cat >.git/hooks/pre-commit <<EOF &&
+	write_script .git/hooks/pre-commit &&
+printf "%s" "SHOULD NOT BE CALLED" >>.git/PRE-COMMIT-HOOK-WAS-CALLED
+EOF
+	chmod +x .git/custom-hooks/pre-commit
+'
+
+test_expect_success 'Check that various forms of specifying core.hooksPath work' '
+	test_commit no_custom_hook &&
+	git config core.hooksPath .git/custom-hooks &&
+	test_commit have_custom_hook &&
+	git config core.hooksPath .git/custom-hooks/ &&
+	test_commit have_custom_hook_trailing_slash &&
+	git config core.hooksPath "$PWD/.git/custom-hooks" &&
+	test_commit have_custom_hook_abs_path &&
+	git config core.hooksPath "$PWD/.git/custom-hooks/" &&
+	test_commit have_custom_hook_abs_path_trailing_slash &&
+	printf "%s" "...." >.git/PRE-COMMIT-HOOK-WAS-CALLED.expect &&
+	test_cmp .git/PRE-COMMIT-HOOK-WAS-CALLED.expect .git/PRE-COMMIT-HOOK-WAS-CALLED
+'
+
+test_done
-- 
2.1.3
Ævar Arnfjörð Bjarmason· Apr 25, 2016, 14:19 UTC · re: Ævar Arnfjörð Bjarmason · lore

Re: [PATCH v2] hooks: Add ability to specify where the hook directory is

On Sun, Apr 24, 2016 at 11:18 PM, Ævar Arnfjörð Bjarmason <avarab@gmail.com> wrote:

Show 24 quoted lines
> Change the hardcoded lookup for .git/hooks/* to optionally lookup in
> $(git config core.hooksPath)/* instead.
>
> This is essentially a more intrusive version of the git-init ability to
> specify hooks on init time via init templates.
>
> The difference between that facility and this feature is that this can
> be set up after the fact via e.g. ~/.gitconfig or /etc/gitconfig to
> apply for all your personal repositories, or all repositories on the
> system.
>
> I plan on using this on a centralized Git server where users can create
> arbitrary repositories under /gitroot, but I'd like to manage all the
> hooks that should be run centrally via a unified dispatch mechanism.
>
> Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
> ---
>
> This is based off the githooks.txt series I just sent, I thought it
> made the most sense to address the questions the previous version
> raised about how we treat the working directory with unrelated
> githooks.txt doc improvements.
>
> This version also address all the other issues SZEDER Gábor noted.
This still applies cleanly on top of the v3 githooks.txt series I just sent.

I forgot to note one thing, when I re-rolled this I changed the configuration variable from core.hooksDirectory to core.hooksPath, everything else in config.txt that dealt with paths was *Path something, so I thought that made more sense for consistency.

Johannes Sixt· Apr 25, 2016, 19:11 UTC · re: Ævar Arnfjörð Bjarmason · lore

Re: [PATCH v2] hooks: Add ability to specify where the hook directory is

Am 24.04.2016 um 23:18 schrieb Ævar Arnfjörð Bjarmason:
Show 10 quoted lines
> +test_expect_success 'set up a pre-commit hook in core.hooksPath' '
> +	mkdir -p .git/custom-hooks .git/hooks &&
> +	write_script .git/custom-hooks/pre-commit <<EOF &&
> +printf "%s" "." >>.git/PRE-COMMIT-HOOK-WAS-CALLED
> +EOF
> +	cat >.git/hooks/pre-commit <<EOF &&
> +	write_script .git/hooks/pre-commit &&
> +printf "%s" "SHOULD NOT BE CALLED" >>.git/PRE-COMMIT-HOOK-WAS-CALLED
> +EOF
> +	chmod +x .git/custom-hooks/pre-commit

Here I see a half-baked attempt to use write_script. Once you've fixed that, we have a pre-commit hook in the regular hook directory. Obviously, the hook is expected not to be called...

> +'
> +
> +test_expect_success 'Check that various forms of specifying core.hooksPath work' '
> +	test_commit no_custom_hook &&
... but at this point, it *will* be called...
Show 9 quoted lines
> +	git config core.hooksPath .git/custom-hooks &&
> +	test_commit have_custom_hook &&
> +	git config core.hooksPath .git/custom-hooks/ &&
> +	test_commit have_custom_hook_trailing_slash &&
> +	git config core.hooksPath "$PWD/.git/custom-hooks" &&
> +	test_commit have_custom_hook_abs_path &&
> +	git config core.hooksPath "$PWD/.git/custom-hooks/" &&
> +	test_commit have_custom_hook_abs_path_trailing_slash &&
> +	printf "%s" "...." >.git/PRE-COMMIT-HOOK-WAS-CALLED.expect &&
... and this expectation is wrong.
Show 5 quoted lines
> +	test_cmp .git/PRE-COMMIT-HOOK-WAS-CALLED.expect .git/PRE-COMMIT-HOOK-WAS-CALLED
> +'
> +
> +test_done
>

I feel a bit uneasy that expected and actual files are not POSIXly correct text files, i.e., the LF at the end is missing...

-- Hannes
Junio C Hamano· Apr 25, 2016, 20:33 UTC · re: Ævar Arnfjörð Bjarmason · lore

Re: [PATCH v2] hooks: Add ability to specify where the hook directory is

Ævar Arnfjörð Bjarmason  <avarab@gmail.com> writes:
Show 10 quoted lines
> +core.hooksPath::
> +	By default Git will look for your hooks in the
> +	'$GIT_DIR/hooks' directory. Set this to different path,
> +	e.g. '/etc/git/hooks', and Git will try to find your hooks in
> +	that directory, e.g. '/etc/git/hooks/pre-receive' instead of
> +	in '$GIT_DIR/hooks/pre-receive'.
> ++
> +The path can either be absolute or relative. In the latter case see
> +the discussion in the "DESCRIPTION" section of linkgit:githooks[5]
> +about what the relative path will be relative to.
... which does not seem to appear there, it seems?
Show 13 quoted lines
>  DESCRIPTION
>  -----------
>  
> -Hooks are programs you can place in the `$GIT_DIR/hooks` directory to
> -trigger action at certain points. Hooks that don't have the executable
> -bit set are ignored.
> +Hooks are programs you can place in a hooks directory to trigger action
> +at certain points. Hooks that don't have the executable bit set are
> +ignored.
> +
> +By default the hooks directory is `$GIT_DIR/hooks`, but that can be
> +changed via the `core.hooksPath` configuration variable (see
> +linkgit:git-config[1]).

The section talks about what the cwd of the process that runs the hook is, but it is not clear at all from these three lines in core.hooksPath description above how the cwd of the process is related with the directory the relative path will be relative to.

Unless the readers know that the implementation makes sure that the process chdir'ed to that final directory before calling find_hook(), that is. And I do not think you want to assume that knowledge on the side of the readers.

Show 17 quoted lines
> diff --git a/t/t1350-config-hooks-path.sh b/t/t1350-config-hooks-path.sh
> new file mode 100755
> index 0000000..31461aa
> --- /dev/null
> +++ b/t/t1350-config-hooks-path.sh
> @@ -0,0 +1,33 @@
> +#!/bin/sh
> +
> +test_description='Test the core.hooksPath configuration variable'
> +
> +. ./test-lib.sh
> +
> +test_expect_success 'set up a pre-commit hook in core.hooksPath' '
> +	mkdir -p .git/custom-hooks .git/hooks &&
> +	write_script .git/custom-hooks/pre-commit <<EOF &&
> +printf "%s" "." >>.git/PRE-COMMIT-HOOK-WAS-CALLED
> +EOF

Because there is no funny interpolation going on, it would help readers to signal that fact by quoting the end-of-here-text marker. And it makes the entire test block reads nicer if you indent the body of hte here-text by prefixing the end-of-here-text marker with a dash, i.e.

	write_script .git/custom-hooks/pre-commit <<-\EOF &&
	printf "%s" "." >>.git/PRE-COMMIT-HOOK-WAS-CALLED
	EOF
Show 5 quoted lines
> +	cat >.git/hooks/pre-commit <<EOF &&
> +	write_script .git/hooks/pre-commit &&
> +printf "%s" "SHOULD NOT BE CALLED" >>.git/PRE-COMMIT-HOOK-WAS-CALLED
> +EOF
> +	chmod +x .git/custom-hooks/pre-commit
You didn't want cat and chmod here?
Show 17 quoted lines
> +'
> +
> +test_expect_success 'Check that various forms of specifying core.hooksPath work' '
> +	test_commit no_custom_hook &&
> +	git config core.hooksPath .git/custom-hooks &&
> +	test_commit have_custom_hook &&
> +	git config core.hooksPath .git/custom-hooks/ &&
> +	test_commit have_custom_hook_trailing_slash &&
> +	git config core.hooksPath "$PWD/.git/custom-hooks" &&
> +	test_commit have_custom_hook_abs_path &&
> +	git config core.hooksPath "$PWD/.git/custom-hooks/" &&
> +	test_commit have_custom_hook_abs_path_trailing_slash &&
> +	printf "%s" "...." >.git/PRE-COMMIT-HOOK-WAS-CALLED.expect &&
> +	test_cmp .git/PRE-COMMIT-HOOK-WAS-CALLED.expect .git/PRE-COMMIT-HOOK-WAS-CALLED
> +'
> +
> +test_done
Ævar Arnfjörð Bjarmason· Apr 26, 2016, 16:31 UTC · re: Junio C Hamano · lore

Re: [PATCH v2] hooks: Add ability to specify where the hook directory is

On Mon, Apr 25, 2016 at 10:33 PM, Junio C Hamano <gitster@pobox.com> wrote:
Show 14 quoted lines
> Ævar Arnfjörð Bjarmason  <avarab@gmail.com> writes:
>
>> +core.hooksPath::
>> +     By default Git will look for your hooks in the
>> +     '$GIT_DIR/hooks' directory. Set this to different path,
>> +     e.g. '/etc/git/hooks', and Git will try to find your hooks in
>> +     that directory, e.g. '/etc/git/hooks/pre-receive' instead of
>> +     in '$GIT_DIR/hooks/pre-receive'.
>> ++
>> +The path can either be absolute or relative. In the latter case see
>> +the discussion in the "DESCRIPTION" section of linkgit:githooks[5]
>> +about what the relative path will be relative to.
>
> ... which does not seem to appear there, it seems?
I think it does. Read on...
Show 18 quoted lines
>>  DESCRIPTION
>>  -----------
>>
>> -Hooks are programs you can place in the `$GIT_DIR/hooks` directory to
>> -trigger action at certain points. Hooks that don't have the executable
>> -bit set are ignored.
>> +Hooks are programs you can place in a hooks directory to trigger action
>> +at certain points. Hooks that don't have the executable bit set are
>> +ignored.
>> +
>> +By default the hooks directory is `$GIT_DIR/hooks`, but that can be
>> +changed via the `core.hooksPath` configuration variable (see
>> +linkgit:git-config[1]).
>
> The section talks about what the cwd of the process that runs the
> hook is, but it is not clear at all from these three lines in
> core.hooksPath description above how the cwd of the process is
> related with the directory the relative path will be relative to.

I think the documentation mostly makes sense, but that the context of this patch is confusing.

I.e. when I say:
> The path can either be absolute or relative. In the latter case see
> the discussion in the "DESCRIPTION" section of linkgit:githooks[5]
> about what the relative path will be relative to.

In config.txt, I'm not talking about the patch to githooks.txt I'm adding in this commit, but the first patch in the githooks.txt series, i.e. this section:

> When a hook is called in a non-bare repository the working directory
> is guaranteed to be the root of the working tree, in a bare repository
> the working directory will be the path to the repository. I.e. hooks
> don't need to worry about the user's current working directory.

I.e. I'm not talking about the "by default the hooks directory [blah blah]" part I'm adding here.

> Unless the readers know that the implementation makes sure that the
> process chdir'ed to that final directory before calling find_hook(),
> that is.  And I do not think you want to assume that knowledge on
> the side of the readers.

This should not be explicitly covered in githooks.txt in my previous series per the above..

Show 23 quoted lines
>> diff --git a/t/t1350-config-hooks-path.sh b/t/t1350-config-hooks-path.sh
>> new file mode 100755
>> index 0000000..31461aa
>> --- /dev/null
>> +++ b/t/t1350-config-hooks-path.sh
>> @@ -0,0 +1,33 @@
>> +#!/bin/sh
>> +
>> +test_description='Test the core.hooksPath configuration variable'
>> +
>> +. ./test-lib.sh
>> +
>> +test_expect_success 'set up a pre-commit hook in core.hooksPath' '
>> +     mkdir -p .git/custom-hooks .git/hooks &&
>> +     write_script .git/custom-hooks/pre-commit <<EOF &&
>> +printf "%s" "." >>.git/PRE-COMMIT-HOOK-WAS-CALLED
>> +EOF
>
> Because there is no funny interpolation going on, it would help
> readers to signal that fact by quoting the end-of-here-text marker.
> And it makes the entire test block reads nicer if you indent the
> body of hte here-text by prefixing the end-of-here-text marker with
> a dash, i.e.

Yes this and all the other stuff you and Johannes mentioned in this thread just made no sense and was just some brainfart of mine. I'm fixing all these issues up for the next re-send.

Show 29 quoted lines
>         write_script .git/custom-hooks/pre-commit <<-\EOF &&
>         printf "%s" "." >>.git/PRE-COMMIT-HOOK-WAS-CALLED
>         EOF
>
>> +     cat >.git/hooks/pre-commit <<EOF &&
>> +     write_script .git/hooks/pre-commit &&
>> +printf "%s" "SHOULD NOT BE CALLED" >>.git/PRE-COMMIT-HOOK-WAS-CALLED
>> +EOF
>> +     chmod +x .git/custom-hooks/pre-commit
>
> You didn't want cat and chmod here?
>
>> +'
>> +
>> +test_expect_success 'Check that various forms of specifying core.hooksPath work' '
>> +     test_commit no_custom_hook &&
>> +     git config core.hooksPath .git/custom-hooks &&
>> +     test_commit have_custom_hook &&
>> +     git config core.hooksPath .git/custom-hooks/ &&
>> +     test_commit have_custom_hook_trailing_slash &&
>> +     git config core.hooksPath "$PWD/.git/custom-hooks" &&
>> +     test_commit have_custom_hook_abs_path &&
>> +     git config core.hooksPath "$PWD/.git/custom-hooks/" &&
>> +     test_commit have_custom_hook_abs_path_trailing_slash &&
>> +     printf "%s" "...." >.git/PRE-COMMIT-HOOK-WAS-CALLED.expect &&
>> +     test_cmp .git/PRE-COMMIT-HOOK-WAS-CALLED.expect .git/PRE-COMMIT-HOOK-WAS-CALLED
>> +'
>> +
>> +test_done
Junio C Hamano· Apr 26, 2016, 19:16 UTC · re: Ævar Arnfjörð Bjarmason · lore

Re: [PATCH v2] hooks: Add ability to specify where the hook directory is

Ævar Arnfjörð Bjarmason <avarab@gmail.com> writes:
Show 7 quoted lines
>>> +The path can either be absolute or relative. In the latter case see
>>> +the discussion in the "DESCRIPTION" section of linkgit:githooks[5]
>>> +about what the relative path will be relative to.
>>
>> ... which does not seem to appear there, it seems?
>
> I think it does. Read on...

I actually read the result of applying the patch before sending the review above.

Show 39 quoted lines
>>>  DESCRIPTION
>>>  -----------
>>>
>>> -Hooks are programs you can place in the `$GIT_DIR/hooks` directory to
>>> -trigger action at certain points. Hooks that don't have the executable
>>> -bit set are ignored.
>>> +Hooks are programs you can place in a hooks directory to trigger action
>>> +at certain points. Hooks that don't have the executable bit set are
>>> +ignored.
>>> +
>>> +By default the hooks directory is `$GIT_DIR/hooks`, but that can be
>>> +changed via the `core.hooksPath` configuration variable (see
>>> +linkgit:git-config[1]).
>>
>> The section talks about what the cwd of the process that runs the
>> hook is, but it is not clear at all from these three lines in
>> core.hooksPath description above how the cwd of the process is
>> related with the directory the relative path will be relative to.
>
> I think the documentation mostly makes sense, but that the context of
> this patch is confusing.
>
> I.e. when I say:
>
>> The path can either be absolute or relative. In the latter case see
>> the discussion in the "DESCRIPTION" section of linkgit:githooks[5]
>> about what the relative path will be relative to.
>
> In config.txt, I'm not talking about the patch to githooks.txt I'm
> adding in this commit, but the first patch in the githooks.txt series,
> i.e. this section:
>
>> When a hook is called in a non-bare repository the working directory
>> is guaranteed to be the root of the working tree, in a bare repository
>> the working directory will be the path to the repository. I.e. hooks
>> don't need to worry about the user's current working directory.
>
> I.e. I'm not talking about the "by default the hooks directory [blah
> blah]" part I'm adding here.
I know.  What it boils down to I think is this.
If somebody said:
    The path to the hooks directory can be specified relative, and
    it is relative to something described elsewhere.
    Hooks are run either at the root of the working tree or in
    GIT_DIR, and they are not affected where the user's current
    directory is (they cannot even know where it is).

you interpret, with the knowledge that "we first determine in which directory to run a hook with a given name, go there, and then look for the named hook", the directory hooks are run in is NATURALLY the directory relative paths the hooks are found are relative to.

My problem was that it is only natural if you have that knowledge.

A reader who starts with a mindset "Git first finds the hook to run, and then goes to the directory to run it in", it is not naturally clear. The latter is specified by two rules, one for a bare and the other for a non-bare repository, and it is very clear. The former is specified nowhere, unless you give a hint to fix the mindset of such a reader.

Ævar Arnfjörð Bjarmason· Apr 26, 2016, 19:19 UTC · re: Junio C Hamano · lore

Re: [PATCH v2] hooks: Add ability to specify where the hook directory is

On Tue, Apr 26, 2016 at 9:16 PM, Junio C Hamano <gitster@pobox.com> wrote:
Show 77 quoted lines
> Ævar Arnfjörð Bjarmason <avarab@gmail.com> writes:
>
>>>> +The path can either be absolute or relative. In the latter case see
>>>> +the discussion in the "DESCRIPTION" section of linkgit:githooks[5]
>>>> +about what the relative path will be relative to.
>>>
>>> ... which does not seem to appear there, it seems?
>>
>> I think it does. Read on...
>
> I actually read the result of applying the patch before sending the
> review above.
>
>>>>  DESCRIPTION
>>>>  -----------
>>>>
>>>> -Hooks are programs you can place in the `$GIT_DIR/hooks` directory to
>>>> -trigger action at certain points. Hooks that don't have the executable
>>>> -bit set are ignored.
>>>> +Hooks are programs you can place in a hooks directory to trigger action
>>>> +at certain points. Hooks that don't have the executable bit set are
>>>> +ignored.
>>>> +
>>>> +By default the hooks directory is `$GIT_DIR/hooks`, but that can be
>>>> +changed via the `core.hooksPath` configuration variable (see
>>>> +linkgit:git-config[1]).
>>>
>>> The section talks about what the cwd of the process that runs the
>>> hook is, but it is not clear at all from these three lines in
>>> core.hooksPath description above how the cwd of the process is
>>> related with the directory the relative path will be relative to.
>>
>> I think the documentation mostly makes sense, but that the context of
>> this patch is confusing.
>>
>> I.e. when I say:
>>
>>> The path can either be absolute or relative. In the latter case see
>>> the discussion in the "DESCRIPTION" section of linkgit:githooks[5]
>>> about what the relative path will be relative to.
>>
>> In config.txt, I'm not talking about the patch to githooks.txt I'm
>> adding in this commit, but the first patch in the githooks.txt series,
>> i.e. this section:
>>
>>> When a hook is called in a non-bare repository the working directory
>>> is guaranteed to be the root of the working tree, in a bare repository
>>> the working directory will be the path to the repository. I.e. hooks
>>> don't need to worry about the user's current working directory.
>>
>> I.e. I'm not talking about the "by default the hooks directory [blah
>> blah]" part I'm adding here.
>
> I know.  What it boils down to I think is this.
>
> If somebody said:
>
>     The path to the hooks directory can be specified relative, and
>     it is relative to something described elsewhere.
>
>     Hooks are run either at the root of the working tree or in
>     GIT_DIR, and they are not affected where the user's current
>     directory is (they cannot even know where it is).
>
> you interpret, with the knowledge that "we first determine in which
> directory to run a hook with a given name, go there, and then look
> for the named hook", the directory hooks are run in is NATURALLY the
> directory relative paths the hooks are found are relative to.
>
> My problem was that it is only natural if you have that knowledge.
>
> A reader who starts with a mindset "Git first finds the hook to run,
> and then goes to the directory to run it in", it is not naturally
> clear.  The latter is specified by two rules, one for a bare and the
> other for a non-bare repository, and it is very clear.  The former
> is specified nowhere, unless you give a hint to fix the mindset of
> such a reader.

Right. I changed the wording for all of this to hopefully be more clear in my v4.

← back to recent threads