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

From: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
Date: Apr 23, 2016, 00:44 UTC
Message-ID: <CACBZZX7jTHqSq0ADJxO0cP+1MZMWAowLMMj4J0NHkFTYOYsJiw@mail.gmail.com>
In-Reply-To: <1461370382-16524-1-git-send-email-szeder@ira.uka.de>

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