Re: Bug: `git init` with hook `reference-transaction` running `git rev-parse --git-dir` fails

From: Patrick Steinhardt <ps@pks.im>
Date: Oct 7, 2024, 08:03 UTC
Message-ID: <ZwOVy4FltrEjxHn_@pks.im>
In-Reply-To: <0084cc18b6d90ba14849a7f788939f4a1d0d61c9.camel@gmail.com>

On Fri, Sep 20, 2024 at 12:07:53PM +0200, Gabriel Nützi wrote:

Show 56 quoted lines

> Thank you for filling out a Git bug report!
> Please answer the following questions to help us understand your issue.
> 
> What did you do before the bug happened? (Steps to reproduce your
> issue)
> 
> I set `git config --global core.hooksPath ~/myhooks` and placed a
> `reference-transaction` hook in `~/myhooks/reference-transaction`
> with the content:
> 
> ```shell
> #!/usr/bin/env bash
> 
> set -e
> echo "$GIT_DIR"
> git rev-parse --absolute-git-dir
> ```
> 
> then I ran
> 
> ```shell
> mkdir ~/test && cd test
> git init
> ```
> 
> What did you expect to happen? (Expected behavior)
> 
> The Git repo `~/test` should have been initialized (and the hook
> `reference-transaction` would have passed successfully.)
> 
> 
> What happened instead? (Actual behavior)
> 
> The hook `reference-transaction` crashes since `git rev-parse --
> absolute-git-dir` with
> ```
> failed: not a git repository: ...
> ```
> 
> What's different between what you expected and what actually happened?
> 
> The documentation says that `git rev-parse --absolute-git-dir` inside
> the `reference-transaction` hooks read "$GIT_DIR" if defined (which is
> defined!) so the `reference-transaction` should have passed. I assume
> that hooks should be executed on properly initialized repositories,
> right? Therefore I do not understand why `git rev-parse --absolute-git-
> dir` fails -> Bug?
> 
> Anything else you want to add:
> 
> This came up with `Githooks` hooks manager
> https://github.com/gabyx/Githooks where we use this command
> to locate the current Git dir...
> 
> Please review the rest of the bug report below.
> You can delete any lines you don't wish to share.

Thanks for your bug report, and sorry for taking so long to respond. Reproducing the observed behaviour is quite simple:

    test_expect_success 'git-init with global hook' '
        test_when_finished "rm -rf hooks repo" &&
        mkdir hooks &&
        write_script hooks/reference-transaction <<-EOF &&
        git rev-parse --absolute-git-dir >>"$(pwd)/reftx-logs"
        EOF
        test_config --global core.hooksPath "$(pwd)/hooks" &&
        git init repo
    '

This breakage is new in Git v2.46 and comes from the patch series that introduces support for symbolic refs in the reftx hook via a8ae923f85 (refs: support symrefs in 'reference-transaction' hook, 2024-05-07) . Before that change we didn't execute the hook for "HEAD" in the first place, now we do.

Now the question is whether this is a bug or not. When the reftx hook executes the first time it is when we are creating the "HEAD" ref in the repo. Consequently, that file did not yet exist beforehand. And as Git only considers something a repository when the "HEAD" file exists it rightfully complains that this is not a valid Git repository when you ask it to resolve the repo paths. So conceptually, the behaviour here is correct.

There are two ways we could fix this that I can think of:

  - We can create a dummy "HEAD" file with invalid contents such that we
    do have a proper Git repository when creating "HEAD". It feels like
    a bit of a hack though, but we play similar games in git-clone(1).

  - We can skip execution of the "reference-transaction" hook during
    initialization of the repository. But this would make us miss some
    ref updates, which feels conceptually wrong.

I'd rule out (2), but (1) could be feasible if we label this a bug. I'm not a 100% sure whether we should, as you could also argue that this is reflecting the actual state of the repo. I'd be happy to hear arguments in either direction.

Also Cc'd Karthik, the author of the menitoned change.

Patrick