All of lore.kernel.org
 help / color / mirror / Atom feed
From: Junio C Hamano <gitster@pobox.com>
To: "Ævar Arnfjörð Bjarmason" <avarab@gmail.com>
Cc: Git <git@vger.kernel.org>, "SZEDER Gábor" <szeder@ira.uka.de>,
	"Johannes Sixt" <j6t@kdbg.org>
Subject: Re: [PATCH v2] hooks: Add ability to specify where the hook directory is
Date: Tue, 26 Apr 2016 12:16:42 -0700	[thread overview]
Message-ID: <xmqqvb34faed.fsf@gitster.mtv.corp.google.com> (raw)
In-Reply-To: <CACBZZX5X_7guR2b+uQFcxzzC6xCv55z=KiMUO6kEmJdQ-U1Gcw@mail.gmail.com> ("Ævar Arnfjörð Bjarmason"'s message of "Tue, 26 Apr 2016 18:31:55 +0200")

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

  reply	other threads:[~2016-04-26 19:16 UTC|newest]

Thread overview: 10+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2016-04-22 23:33 [PATCH] hooks: Add ability to specify where the hook directory is Ævar Arnfjörð Bjarmason
2016-04-23  0:13 ` SZEDER Gábor
2016-04-23  0:44   ` Ævar Arnfjörð Bjarmason
2016-04-24 21:18     ` [PATCH v2] " Ævar Arnfjörð Bjarmason
2016-04-25 14:19       ` Ævar Arnfjörð Bjarmason
2016-04-25 19:11       ` Johannes Sixt
2016-04-25 20:33       ` Junio C Hamano
2016-04-26 16:31         ` Ævar Arnfjörð Bjarmason
2016-04-26 19:16           ` Junio C Hamano [this message]
2016-04-26 19:19             ` Ævar Arnfjörð Bjarmason

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=xmqqvb34faed.fsf@gitster.mtv.corp.google.com \
    --to=gitster@pobox.com \
    --cc=avarab@gmail.com \
    --cc=git@vger.kernel.org \
    --cc=j6t@kdbg.org \
    --cc=szeder@ira.uka.de \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.