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

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

[Ævar Arnfjörð Bjarmason]

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,
+	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

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

[SZEDER Gábor]

> 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/

> +	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.

> +
> +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.

> +	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.

> +'
> +
> +test_done
> -- 
> 2.1.3

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

[Ævar Arnfjörð Bjarmason]

On Sat, Apr 23, 2016 at 2:13 AM, SZEDER Gábor <szeder@ira.uka.de> wrote:
>
>> 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.

> 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.

>> +     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.

>> +
>> +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.

>> +     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.

>> +'
>> +
>> +test_done
>> --
>> 2.1.3

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

[Ævar Arnfjörð Bjarmason]

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

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

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

[Ævar Arnfjörð Bjarmason]

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

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

[Johannes Sixt]

Am 24.04.2016 um 23:18 schrieb Ævar Arnfjörð Bjarmason:
> +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...

> +	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.

> +	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

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

[Junio C Hamano]

Æ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?

>  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.

> 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

> +	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

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

[Ævar Arnfjörð Bjarmason]

On Mon, Apr 25, 2016 at 10:33 PM, Junio C Hamano <gitster@pobox.com> wrote:
> Æ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...

>>  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..

>> 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.

>         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

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

[Junio C Hamano]

Æ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.

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

[Ævar Arnfjörð Bjarmason]

On Tue, Apr 26, 2016 at 9:16 PM, Junio C Hamano <gitster@pobox.com> wrote:
> Æ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.