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

["Gabriel Nützi" <gnuetzi@gmail.com>]

[-- Attachment #1.1: Type: text/plain, Size: 4966 bytes --]

Hi Patrick,

Thanks for clarifications!
Could it work if for (2) -> call the reference-transaction hook after the HEAD has been initialized, meaning that Git would internally cache the
different reference transactions and then call the hooks in one go at once after the creation of the repo, such that it is initialized properly?
This might be probably a more elaborate change which introduces too many technicalities?

Gabriel

-----Original Message-----  
**From**: Patrick Steinhardt <[ps@pks.im](mailto:Patrick%20Steinhardt%20%3cps@pks.im%3e)>  
**To**: Gabriel Nützi <[gnuetzi@gmail.com](mailto:Gabriel%20%3d%3fISO-8859-1%3fQ%3fN%3dFCtzi%3f%3d%20%3cgnuetzi@gmail.com%3e)>  
**Cc**: [git@vger.kernel.org](mailto:git@vger.kernel.org), Karthik Nayak <[karthik.188@gmail.com](mailto:Karthik%20Nayak%20%3ckarthik.188@gmail.com%3e)>  
**Subject**: Re: Bug: `git init` with hook `reference-transaction` running `git rev-parse --git-dir` fails  
**Date**: 10/07/2024 10:03:30 AM

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

> 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](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

[-- Attachment #1.2: Type: text/html, Size: 5706 bytes --]

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 228 bytes --]