Unexpected working directory in post-receive hook in non-bare repository

Unexpected working directory in post-receive hook in non-bare repository

[Simon Ruderich]

[-- Attachment #1: Type: text/plain, Size: 1308 bytes --]

Hello,

The following snippet reproduces the issue for me (note the
remote: line in its output):

    git --version

    rm -rf a b

    git init a
    cd a
    echo first >data
    git add data
    git commit -m initial
    cat >>.git/hooks/post-receive <<EOF
    #!/bin/sh
    pwd
    EOF
    chmod +x .git/hooks/post-receive
    cd ..

    git clone a b
    cd b
    echo second >>data
    git add data
    git commit -m test
    git push origin master:not-master

According to man githooks "Before Git invokes a hook, it changes
its working directory to either the root of the working tree in a
non-bare repository, [...]". In this case "a" is non-bare and I
expected the command to be run in the working tree; but instead
it's run inside .git. (This caused some confusion in my case
because I ran "git merge" in the hook which put files in the .git
directory and I didn't notice it at first. I know running merge
in receive-hooks is "bad practice" but it works fine in my
setup.)

The same happens for all hooks executed by git-receive-pack:
pre-receive, update, post-receive, post-update.

Is this a documentation issue or unexpected behavior?

Regards
Simon
-- 
+ privacy is necessary
+ using gnupg http://gnupg.org
+ public key id: 0x92FEFDB7E44C32F9

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Re: Unexpected working directory in post-receive hook in non-bare repository

[Ævar Arnfjörð Bjarmason]

On Sun, Apr 9, 2017 at 3:01 PM, Simon Ruderich <simon@ruderich.org> wrote:
> The following snippet reproduces the issue for me (note the
> remote: line in its output):
>
>     git --version
>
>     rm -rf a b
>
>     git init a
>     cd a
>     echo first >data
>     git add data
>     git commit -m initial
>     cat >>.git/hooks/post-receive <<EOF
>     #!/bin/sh
>     pwd
>     EOF
>     chmod +x .git/hooks/post-receive
>     cd ..
>
>     git clone a b
>     cd b
>     echo second >>data
>     git add data
>     git commit -m test
>     git push origin master:not-master
>
> According to man githooks "Before Git invokes a hook, it changes
> its working directory to either the root of the working tree in a
> non-bare repository, [...]". In this case "a" is non-bare and I
> expected the command to be run in the working tree; but instead
> it's run inside .git. (This caused some confusion in my case
> because I ran "git merge" in the hook which put files in the .git
> directory and I didn't notice it at first. I know running merge
> in receive-hooks is "bad practice" but it works fine in my
> setup.)
>
> The same happens for all hooks executed by git-receive-pack:
> pre-receive, update, post-receive, post-update.
>
> Is this a documentation issue or unexpected behavior?

It's a documentation issue I think. I added this change to the
githooks manpage last year in 49fa52fd00, but didn't think about the
case of pushing into non-bare repositories. The behavior itself hasn't
changed in a long time.

I wonder how to phrase this so that it's unambiguous & simply states a
general rule. I.e. instead of:

""""
Before Git invokes a hook, it changes its working directory to either
the root of the working tree in a non-bare repository, or to the
$GIT_DIR in a bare repository.
"""

Can we say as we do now that:

* All hooks regardless of type in bare repos execute in the bare repo
* If you have a working tree hooks use that

But add:

* Working trees are ignored by any hooks invoked on your behalf during a push.

Some ad-hoc testing reveals that this rule also goes for the
push-to-checkout hook. Should it? Wouldn't it be more useful if it
broke the pattern, since it's dealing with the working tree on the
other side? Junio?

[PATCH] githooks.txt: clarify push hooks are always executed in $GIT_DIR

[Simon Ruderich]

Listing the specific hooks might feel verbose but without it the
reader is left to wonder which hooks are triggered during the
push. Something which is not immediately obvious when only trying
to find out where the hook is executed.

Signed-off-by: Simon Ruderich <simon@ruderich.org>
---
 Documentation/githooks.txt | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

On Mon, Apr 10, 2017 at 01:13:15PM +0200, Ævar Arnfjörð Bjarmason wrote:
> [snip]
>
> Can we say as we do now that:
>
> * All hooks regardless of type in bare repos execute in the bare repo
> * If you have a working tree hooks use that
>
> But add:
>
> * Working trees are ignored by any hooks invoked on your behalf during a push.

Hello,

Maybe like this? I reordered the cases as it felt more natural
that the general case is first and followed by the one with the
exception.

> Some ad-hoc testing reveals that this rule also goes for the
> push-to-checkout hook. Should it? Wouldn't it be more useful if it
> broke the pattern, since it's dealing with the working tree on the
> other side? Junio?

I added push-to-checkout to the patch. Changing the behavior will
break backwards compatibility so I think that's a no-go.

Regards
Simon

diff --git a/Documentation/githooks.txt b/Documentation/githooks.txt
index 32343ae29..706091a56 100644
--- a/Documentation/githooks.txt
+++ b/Documentation/githooks.txt
@@ -22,8 +22,10 @@ changed via the `core.hooksPath` configuration variable (see
 linkgit:git-config[1]).
 
 Before Git invokes a hook, it changes its working directory to either
-the root of the working tree in a non-bare repository, or to the
-$GIT_DIR in a bare repository.
+$GIT_DIR in a bare repository or the root of the working tree in a non-bare
+repository. An exception are hooks triggered during a push ('pre-receive',
+'update', 'post-receive', 'post-update', 'push-to-checkout') which are always
+executed in $GIT_DIR.
 
 Hooks can get their arguments via the environment, command-line
 arguments, and stdin. See the documentation for each hook below for
-- 
2.11.0

-- 
+ privacy is necessary
+ using gnupg http://gnupg.org
+ public key id: 0x92FEFDB7E44C32F9

Re: [PATCH] githooks.txt: clarify push hooks are always executed in $GIT_DIR

[Ævar Arnfjörð Bjarmason]

On Sat, Apr 29, 2017 at 2:28 PM, Simon Ruderich <simon@ruderich.org> wrote:
> Listing the specific hooks might feel verbose but without it the
> reader is left to wonder which hooks are triggered during the
> push. Something which is not immediately obvious when only trying
> to find out where the hook is executed.
>
> Signed-off-by: Simon Ruderich <simon@ruderich.org>
> ---
>  Documentation/githooks.txt | 6 ++++--
>  1 file changed, 4 insertions(+), 2 deletions(-)
>
> On Mon, Apr 10, 2017 at 01:13:15PM +0200, Ęvar Arnfjörš Bjarmason wrote:
>> [snip]
>>
>> Can we say as we do now that:
>>
>> * All hooks regardless of type in bare repos execute in the bare repo
>> * If you have a working tree hooks use that
>>
>> But add:
>>
>> * Working trees are ignored by any hooks invoked on your behalf during a push.
>
> Hello,
>
> Maybe like this? I reordered the cases as it felt more natural
> that the general case is first and followed by the one with the
> exception.
>
>> Some ad-hoc testing reveals that this rule also goes for the
>> push-to-checkout hook. Should it? Wouldn't it be more useful if it
>> broke the pattern, since it's dealing with the working tree on the
>> other side? Junio?
>
> I added push-to-checkout to the patch. Changing the behavior will
> break backwards compatibility so I think that's a no-go.
>
> Regards
> Simon
>
> diff --git a/Documentation/githooks.txt b/Documentation/githooks.txt
> index 32343ae29..706091a56 100644
> --- a/Documentation/githooks.txt
> +++ b/Documentation/githooks.txt
> @@ -22,8 +22,10 @@ changed via the `core.hooksPath` configuration variable (see
>  linkgit:git-config[1]).
>
>  Before Git invokes a hook, it changes its working directory to either
> -the root of the working tree in a non-bare repository, or to the
> -$GIT_DIR in a bare repository.
> +$GIT_DIR in a bare repository or the root of the working tree in a non-bare
> +repository. An exception are hooks triggered during a push ('pre-receive',
> +'update', 'post-receive', 'post-update', 'push-to-checkout') which are always
> +executed in $GIT_DIR.
>
>  Hooks can get their arguments via the environment, command-line
>  arguments, and stdin. See the documentation for each hook below for

This looks good to me. Thanks for working on this.