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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Mon, Apr 25, 2016 at 10:33 PM, Junio C Hamano <gitster@xxxxxxxxx> wrote:
> Ævar Arnfjörð Bjarmason  <avarab@xxxxxxxxx> 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
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html



[Index of Archives]     [Linux Kernel Development]     [Gcc Help]     [IETF Annouce]     [DCCP]     [Netdev]     [Networking]     [Security]     [V4L]     [Bugtraq]     [Yosemite]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux SCSI]     [Fedora Users]