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

From: Junio C Hamano <gitster@pobox.com>
Date: Apr 26, 2016, 19:16 UTC
Message-ID: <xmqqvb34faed.fsf@gitster.mtv.corp.google.com>
In-Reply-To: <CACBZZX5X_7guR2b+uQFcxzzC6xCv55z=KiMUO6kEmJdQ-U1Gcw@mail.gmail.com>

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