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

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

[Gabriel Nützi]

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

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.


[System Info]
git version:
git version 2.46.0
cpu: x86_64
no commit associated with this build
sizeof-long: 8
sizeof-size_t: 8
shell-path: /nix/store/izpf49b74i15pcr9708s3xdwyqs4jxwl-bash-
5.2p32/bin/bash
libcurl: 8.9.1
OpenSSL: OpenSSL 3.0.14 4 Jun 2024
zlib: 1.3.1
uname: Linux 6.6.45 #1-NixOS SMP PREEMPT_DYNAMIC Sun Aug 11 10:47:28
UTC 2024 x86_64
compiler info: gnuc: 13.3
libc info: glibc: 2.39
$SHELL (typically, interactive shell): /run/current-system/sw/bin/zsh


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

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

[Gabriel Nützi]

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

The same happens with `git rev-parse --git-comon-dir` and 
`git config a.a "hello"` (which might be ok, since the config file is not yet setup at this point...)

On Fri, 2024-09-20 at 12:07 +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.
> 
> 
> [System Info]  
> git version:  
> git version 2.46.0  
> cpu: x86_64  
> no commit associated with this build  
> sizeof-long: 8  
> sizeof-size_t: 8  
> shell-path: /nix/store/izpf49b74i15pcr9708s3xdwyqs4jxwl-bash-  
> 5.2p32/bin/bash  
> libcurl: 8.9.1  
> OpenSSL: OpenSSL 3.0.14 4 Jun 2024  
> zlib: 1.3.1  
> uname: Linux 6.6.45 #1-NixOS SMP PREEMPT_DYNAMIC Sun Aug 11 10:47:28  
> UTC 2024 x86_64  
> compiler info: gnuc: 13.3  
> libc info: glibc: 2.39  
> $SHELL (typically, interactive shell): /run/current-system/sw/bin/zsh
> 

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

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

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

[Patrick Steinhardt]

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

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

[Gabriel Nützi]

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

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

[Patrick Steinhardt]

On Mon, Oct 07, 2024 at 12:54:36PM +0200, Gabriel Nützi wrote:
> 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?

That would break basic assumptions of how the hook operates. It is
expected that we abort the transaction when the hook returns an error,
so if we were to run the hook _after_ `HEAD` has been created that
expectation would be broken.

So the only viable solution is to create a stub `HEAD`, but as said I'm
not a 100% sure whether we want to go there or not.

Patrick

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

[Gabriel Nützi]

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

Ah, jeah thats true:

The command in question which is `git rev-parse --git-dir` to find the actual GIT_DIR is actually the issue here:
It relies on having a proper HEAD despite the docs saying it only reports GIT_DIR if set etc...

Could we maybe make these commands more agnostic? 
Making an empty HEAD probably directly also resolves these commands to work correctly.

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 12:57:19 PM

git rev-parse --git-dir

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

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

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

[Patrick Steinhardt]

On Mon, Oct 07, 2024 at 01:02:56PM +0200, Gabriel Nützi wrote:
> Ah, jeah thats true:
> 
> The command in question which is `git rev-parse --git-dir` to find the actual GIT_DIR is actually the issue here:
> It relies on having a proper HEAD despite the docs saying it only reports GIT_DIR if set etc...

That's not quite true. The second paragraph for the `--git-dir` option
in git-rev-parse(1) has this to say:

    If $GIT_DIR is not defined and the current directory is not detected to lie in a Git
    repository or work tree print a message to stderr and exit with nonzero status.

But in our case you're correct: `$GIT_DIR` _is_ defined during hook
execution. So in theory, if git-rev-parse(1) behaved exactly as
documented, it shouldn't even care whether or not it is executing in a
repository.

> Could we maybe make these commands more agnostic? 

So this could indeed be a viable fix, but I guess we'd rather play
whack-a-mole because other Git commands that one may want to execute in
this repository would also be broken.

> Making an empty HEAD probably directly also resolves these commands to work correctly.

So agreed, creating a stub HEAD is the most sensible option we have. I'd
like to wait a bit for more feedback from other contributors before we
do so though.

Patrick

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

[Junio C Hamano]

Patrick Steinhardt <ps@pks.im> writes:

> `$GIT_DIR` _is_ defined during hook
> execution. So in theory, if git-rev-parse(1) behaved exactly as
> documented, it shouldn't even care whether or not it is executing in a
> repository.

I've always considered "git rev-parse --git-dir" and friends were to
verify the validity of the repository before returning "Your GIT_DIR
is here".  Otherwise there is no easy way to ask "I have this directory.
Is it a valid repository you can work with?".

So, I am not sure I agree with the above.

For what is worth, I am skeptical to the "solution" that tentively
creates a bogus HEAD file while the repository is being initialized.
The code today may ignore certain bogosity in such a HEAD (like the
".invalid" magic used during "git clone"), but there is no guarantee
that a random third-party add-on a hook script may invoke do the
same as we do, and more importantly, what a repository with its
initialization complete look like may change over time and it may
not be enough to have HEAD pointing at "refs/heads/.invalid" to fool
our bootstrap process.  If we were to go that route, I would rather
see us pick a pointee that is *not* bogus at the mechanical level
(i.e., "git symbolic-ref HEAD refs/heads/.invalid" would fail) but
is clearly a placeholder value to humans, soon to be updated.

Let's say if we were to create a repository with the name of initial
branch as 'main', we could create HEAD that points at refs/heads/main
bypassing any hook intervention, then call the hook to see if it
approves the name.  We'd need to make sure that we fail the
repository creation when the hook declines, as it is refusing to set
a HEAD, one critical element in the repository that has to exist,
and probably remove the directory if we are not reinitializing.

Or we could use a name that is clearly bogus to humans but is still
structurally OK, say "refs/heads/hook-failed-during-initialization",
ask the hook if it is OK to repoint HEAD to "refs/heads/main" from
that state, and (1) if it approves, HEAD will point at "refs/heads/main"
and "hook-failed-during-initialization" will be seen nowhere but the
reflog of HEAD, or (2) if it refuses, we stop, and the user will be
left on an unborn branch with a long descriptive name that explains
the situation.

A much simpler alternative would be to simply ignore any hooks,
traces, or anything that want to look into the directory we are
working to turn into a repository but haven't completed doing so,
during repository initialization, I would think, though.

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

[Patrick Steinhardt]

On Mon, Oct 07, 2024 at 02:02:02PM -0700, Junio C Hamano wrote:
> Patrick Steinhardt <ps@pks.im> writes:
> 
> > `$GIT_DIR` _is_ defined during hook
> > execution. So in theory, if git-rev-parse(1) behaved exactly as
> > documented, it shouldn't even care whether or not it is executing in a
> > repository.
> 
> I've always considered "git rev-parse --git-dir" and friends were to
> verify the validity of the repository before returning "Your GIT_DIR
> is here".  Otherwise there is no easy way to ask "I have this directory.
> Is it a valid repository you can work with?".
> 
> So, I am not sure I agree with the above.

Well, I'm not sure either. I was merely pointing out that the documented
behaviour is different than the actual behaviour. Which of both is the
more sensible one is a different question.

> For what is worth, I am skeptical to the "solution" that tentively
> creates a bogus HEAD file while the repository is being initialized.
> The code today may ignore certain bogosity in such a HEAD (like the
> ".invalid" magic used during "git clone"), but there is no guarantee
> that a random third-party add-on a hook script may invoke do the
> same as we do, and more importantly, what a repository with its
> initialization complete look like may change over time and it may
> not be enough to have HEAD pointing at "refs/heads/.invalid" to fool
> our bootstrap process.  If we were to go that route, I would rather
> see us pick a pointee that is *not* bogus at the mechanical level
> (i.e., "git symbolic-ref HEAD refs/heads/.invalid" would fail) but
> is clearly a placeholder value to humans, soon to be updated.
> 
> Let's say if we were to create a repository with the name of initial
> branch as 'main', we could create HEAD that points at refs/heads/main
> bypassing any hook intervention, then call the hook to see if it
> approves the name.  We'd need to make sure that we fail the
> repository creation when the hook declines, as it is refusing to set
> a HEAD, one critical element in the repository that has to exist,
> and probably remove the directory if we are not reinitializing.
> 
> Or we could use a name that is clearly bogus to humans but is still
> structurally OK, say "refs/heads/hook-failed-during-initialization",
> ask the hook if it is OK to repoint HEAD to "refs/heads/main" from
> that state, and (1) if it approves, HEAD will point at "refs/heads/main"
> and "hook-failed-during-initialization" will be seen nowhere but the
> reflog of HEAD, or (2) if it refuses, we stop, and the user will be
> left on an unborn branch with a long descriptive name that explains
> the situation.

I dunno. It all feels rather complex.

> A much simpler alternative would be to simply ignore any hooks,
> traces, or anything that want to look into the directory we are
> working to turn into a repository but haven't completed doing so,
> during repository initialization, I would think, though.

That could work, yes, but it would limit the usefulness of the hook. In
theory, you can create a full log of all changes in the repository from
its inception. If we didn't log the first item, that log would be
incomplete.

We have another solution that is even simpler: just do nothing. I do not
think that the behaviour we exhibit is wrong. Unwieldy? Maybe. But it is
merely stating facts: we are executing a transaction in a repository
that is not yet fully set up. If you don't want that, either don't set
up a global reference-transaction hook, or alternatively handle that
edge case in your script.

Patrick

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

[Karthik Nayak]

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

Patrick Steinhardt <ps@pks.im> writes:

> On Mon, Oct 07, 2024 at 02:02:02PM -0700, Junio C Hamano wrote:
>> Patrick Steinhardt <ps@pks.im> writes:
>>
>> > `$GIT_DIR` _is_ defined during hook
>> > execution. So in theory, if git-rev-parse(1) behaved exactly as
>> > documented, it shouldn't even care whether or not it is executing in a
>> > repository.
>>
>> I've always considered "git rev-parse --git-dir" and friends were to
>> verify the validity of the repository before returning "Your GIT_DIR
>> is here".  Otherwise there is no easy way to ask "I have this directory.
>> Is it a valid repository you can work with?".
>>
>> So, I am not sure I agree with the above.
>
> Well, I'm not sure either. I was merely pointing out that the documented
> behaviour is different than the actual behaviour. Which of both is the
> more sensible one is a different question.
>
>> For what is worth, I am skeptical to the "solution" that tentively
>> creates a bogus HEAD file while the repository is being initialized.
>> The code today may ignore certain bogosity in such a HEAD (like the
>> ".invalid" magic used during "git clone"), but there is no guarantee
>> that a random third-party add-on a hook script may invoke do the
>> same as we do, and more importantly, what a repository with its
>> initialization complete look like may change over time and it may
>> not be enough to have HEAD pointing at "refs/heads/.invalid" to fool
>> our bootstrap process.  If we were to go that route, I would rather
>> see us pick a pointee that is *not* bogus at the mechanical level
>> (i.e., "git symbolic-ref HEAD refs/heads/.invalid" would fail) but
>> is clearly a placeholder value to humans, soon to be updated.
>>
>> Let's say if we were to create a repository with the name of initial
>> branch as 'main', we could create HEAD that points at refs/heads/main
>> bypassing any hook intervention, then call the hook to see if it
>> approves the name.  We'd need to make sure that we fail the
>> repository creation when the hook declines, as it is refusing to set
>> a HEAD, one critical element in the repository that has to exist,
>> and probably remove the directory if we are not reinitializing.
>>
>> Or we could use a name that is clearly bogus to humans but is still
>> structurally OK, say "refs/heads/hook-failed-during-initialization",
>> ask the hook if it is OK to repoint HEAD to "refs/heads/main" from
>> that state, and (1) if it approves, HEAD will point at "refs/heads/main"
>> and "hook-failed-during-initialization" will be seen nowhere but the
>> reflog of HEAD, or (2) if it refuses, we stop, and the user will be
>> left on an unborn branch with a long descriptive name that explains
>> the situation.
>
> I dunno. It all feels rather complex.
>
>> A much simpler alternative would be to simply ignore any hooks,
>> traces, or anything that want to look into the directory we are
>> working to turn into a repository but haven't completed doing so,
>> during repository initialization, I would think, though.
>
> That could work, yes, but it would limit the usefulness of the hook. In
> theory, you can create a full log of all changes in the repository from
> its inception. If we didn't log the first item, that log would be
> incomplete.
>
> We have another solution that is even simpler: just do nothing. I do not
> think that the behaviour we exhibit is wrong. Unwieldy? Maybe. But it is
> merely stating facts: we are executing a transaction in a repository
> that is not yet fully set up. If you don't want that, either don't set
> up a global reference-transaction hook, or alternatively handle that
> edge case in your script.

This does seem like we're putting the onus on the users, but ultimately
I agree with this. What the hook is doing is as expected.

Thanks Patrick for responding while I was away.

Karthik

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

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

[Gabriel Nützi]

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


> We have another solution that is even simpler: just do nothing. I do not  
> think that the behaviour we exhibit is wrong. Unwieldy? Maybe. But it is  
> merely stating facts: we are executing a transaction in a repository  
> that is not yet fully set up. If you don't want that, either don't set  
> up a global reference-transaction hook, or alternatively handle that  
> edge case in your script.

But then Git should at least give the hooks maintainers a chance to know what it is doing.
Could we have a new env. variable or mechanism stating that the repository is not yet setup or so?
The issue here comes from the fact that Githooks is a hooks manager which basically calls `reference-transaction` for you, but before
it does it calls `git rev-parse --git-dir` to get the correct Git directory, basically not doing env. lookup on GIT_DIR which this command actually does, but that apparently only works on a "initialized" repo, so currently I do a workaround and looking directly at GIT_DIR.
I would be really happy also if I could somehow know when Git is creating a new repository (clone, or init), that would also improve other scenarios I came across. Basically you cannot react on hook `post-checkout` only when a new repo is created...


Gabriel

-----Original Message-----  
**From**: Patrick Steinhardt <[ps@pks.im](mailto:Patrick%20Steinhardt%20%3cps@pks.im%3e)>  
**To**: Junio C Hamano <[gitster@pobox.com](mailto:Junio%20C%20Hamano%20%3cgitster@pobox.com%3e)>  
**Cc**: Gabriel Nützi <[gnuetzi@gmail.com](mailto:Gabriel%20%3d%3fISO-8859-1%3fQ%3fN%3dFCtzi%3f%3d%20%3cgnuetzi@gmail.com%3e)>, [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/09/2024 11:39:26 AM

On Mon, Oct 07, 2024 at 02:02:02PM -0700, Junio C Hamano wrote:

> Patrick Steinhardt <[ps@pks.im](mailto:ps@pks.im)> writes:
> 
> 
> > `$GIT_DIR` _is_ defined during hook
> > execution. So in theory, if git-rev-parse(1) behaved exactly as
> > documented, it shouldn't even care whether or not it is executing in a
> > repository.
> 
> 
> I've always considered "git rev-parse --git-dir" and friends were to
> verify the validity of the repository before returning "Your GIT_DIR
> is here".  Otherwise there is no easy way to ask "I have this directory.
> Is it a valid repository you can work with?".
> 
> So, I am not sure I agree with the above.


Well, I'm not sure either. I was merely pointing out that the documented  
behaviour is different than the actual behaviour. Which of both is the  
more sensible one is a different question.


> For what is worth, I am skeptical to the "solution" that tentively
> creates a bogus HEAD file while the repository is being initialized.
> The code today may ignore certain bogosity in such a HEAD (like the
> ".invalid" magic used during "git clone"), but there is no guarantee
> that a random third-party add-on a hook script may invoke do the
> same as we do, and more importantly, what a repository with its
> initialization complete look like may change over time and it may
> not be enough to have HEAD pointing at "refs/heads/.invalid" to fool
> our bootstrap process.  If we were to go that route, I would rather
> see us pick a pointee that is *not* bogus at the mechanical level
> (i.e., "git symbolic-ref HEAD refs/heads/.invalid" would fail) but
> is clearly a placeholder value to humans, soon to be updated.
> 
> Let's say if we were to create a repository with the name of initial
> branch as 'main', we could create HEAD that points at refs/heads/main
> bypassing any hook intervention, then call the hook to see if it
> approves the name.  We'd need to make sure that we fail the
> repository creation when the hook declines, as it is refusing to set
> a HEAD, one critical element in the repository that has to exist,
> and probably remove the directory if we are not reinitializing.
> 
> Or we could use a name that is clearly bogus to humans but is still
> structurally OK, say "refs/heads/hook-failed-during-initialization",
> ask the hook if it is OK to repoint HEAD to "refs/heads/main" from
> that state, and (1) if it approves, HEAD will point at "refs/heads/main"
> and "hook-failed-during-initialization" will be seen nowhere but the
> reflog of HEAD, or (2) if it refuses, we stop, and the user will be
> left on an unborn branch with a long descriptive name that explains
> the situation.


I dunno. It all feels rather complex.


> A much simpler alternative would be to simply ignore any hooks,
> traces, or anything that want to look into the directory we are
> working to turn into a repository but haven't completed doing so,
> during repository initialization, I would think, though.


That could work, yes, but it would limit the usefulness of the hook. In  
theory, you can create a full log of all changes in the repository from  
its inception. If we didn't log the first item, that log would be  
incomplete.

We have another solution that is even simpler: just do nothing. I do not  
think that the behaviour we exhibit is wrong. Unwieldy? Maybe. But it is  
merely stating facts: we are executing a transaction in a repository  
that is not yet fully set up. If you don't want that, either don't set  
up a global reference-transaction hook, or alternatively handle that  
edge case in your script.

Patrick

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

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

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

[Patrick Steinhardt]

On Wed, Oct 09, 2024 at 01:53:17PM +0200, Gabriel Nützi wrote:
> 
> > We have another solution that is even simpler: just do nothing. I do not  
> > think that the behaviour we exhibit is wrong. Unwieldy? Maybe. But it is  
> > merely stating facts: we are executing a transaction in a repository  
> > that is not yet fully set up. If you don't want that, either don't set  
> > up a global reference-transaction hook, or alternatively handle that  
> > edge case in your script.
> 
> But then Git should at least give the hooks maintainers a chance to know what it is doing.
> Could we have a new env. variable or mechanism stating that the repository is not yet setup or so?
> The issue here comes from the fact that Githooks is a hooks manager which basically calls `reference-transaction` for you, but before
> it does it calls `git rev-parse --git-dir` to get the correct Git directory, basically not doing env. lookup on GIT_DIR which this command actually does, but that apparently only works on a "initialized" repo, so currently I do a workaround and looking directly at GIT_DIR.
> I would be really happy also if I could somehow know when Git is creating a new repository (clone, or init), that would also improve other scenarios I came across. Basically you cannot react on hook `post-checkout` only when a new repo is created...

Yeah, that's fair indeed, thanks for the background.

In practice you already have a unique way to identify whether the repo
is about to be created or whether you're using "normal" operations,
namely by checking whether the HEAD file exists in the Git directory. Is
that an acceptable workaround for you?

As said, we're currently just exploring options. I'm not yet saying that
this is the way we'll go, even though it seems like the most sensible
solution to me right now. That picture of course changes when there are
valid usecases like what you present here.

Patrick

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

[Junio C Hamano]

Patrick Steinhardt <ps@pks.im> writes:

> We have another solution that is even simpler: just do nothing. I do not
> think that the behaviour we exhibit is wrong. Unwieldy? Maybe. But it is
> merely stating facts: we are executing a transaction in a repository
> that is not yet fully set up. If you don't want that, either don't set
> up a global reference-transaction hook, or alternatively handle that
> edge case in your script.

I like that over all the other choices.

My art in leaving out what I really want to see happen and let
others say it is improving, it seems ;-).

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

[Patrick Steinhardt]

On Fri, Oct 11, 2024 at 12:09:18PM +0200, Gabriel Nützi wrote:
> > namely by checking whether the HEAD file exists in the Git directory. 
> 
> The problem here is the chicken-egg problem: How do I know inside a
> hook where to look for HEAD, so the non. env. way of doing this would
> be 'git rev-parse —git-dir' which crahshes… So there should be cleary
> some better way of knowing if a Git repo is on the way of beeing
> created, and when it is fully initialized. What about
> GIT_DIR_INITIALIZING and this variable does not exist once it is done
> or whats about making ‚git rev-parse —git-dir‘ not panic if the env.
> variables are defined. I guess for a good solution we need both?

Our documentation in githooks(5) says that the GIT_DIR environment
variable is always set so that the hook can identify the location of the
Git repository. So I'd argue that using git-rev-parse(1) is unnecessary
in the first place. Or am I missing something?

Patrick