{"thread":{"id":"62151","subject":"Bug: `git init` with hook `reference-transaction` running `git rev-parse --git-dir` fails","startedAt":"2024-09-20T10:07:57Z","lastAt":"2024-10-14T12:25:32Z","messageCount":14,"participants":["Gabriel Nützi","Patrick Steinhardt","Junio C Hamano","Karthik Nayak"],"isPatch":false,"patchVersion":null,"patchTotal":null},"messages":[{"id":"503119","messageId":"0084cc18b6d90ba14849a7f788939f4a1d0d61c9.camel@gmail.com","threadId":"62151","inReplyTo":null,"subject":"Bug: `git init` with hook `reference-transaction` running `git rev-parse --git-dir` fails","fromName":"Gabriel Nützi","fromEmail":"gnuetzi@gmail.com","sentAt":"2024-09-20T10:07:53Z","receivedAt":"2024-09-20T10:07:57Z","isPatch":false,"sender":{"key":"gnuetzi@gmail.com","avatar":"https://gravatar.com/avatar/eda99c30173930819ae1c26c3dff249edd4b3c735c3886d268c0d6a71c6307c9?d=mp&s=160"},"body":"Thank you for filling out a Git bug report!\nPlease answer the following questions to help us understand your issue.\n\nWhat did you do before the bug happened? (Steps to reproduce your\nissue)\n\nI set `git config --global core.hooksPath ~/myhooks` and placed a\n`reference-transaction` hook in `~/myhooks/reference-transaction`\nwith the content:\n\n```shell\n#!/usr/bin/env bash\n\nset -e\necho \"$GIT_DIR\"\ngit rev-parse --absolute-git-dir\n```\n\nthen I ran\n\n```shell\nmkdir ~/test && cd test\ngit init\n```\n\nWhat did you expect to happen? (Expected behavior)\n\nThe Git repo `~/test` should have been initialized (and the hook\n`reference-transaction` would have passed successfully.)\n\n\nWhat happened instead? (Actual behavior)\n\nThe hook `reference-transaction` crashes since `git rev-parse --\nabsolute-git-dir` with\n```\nfailed: not a git repository: ...\n```\n\nWhat's different between what you expected and what actually happened?\n\nThe documentation says that `git rev-parse --absolute-git-dir` inside\nthe `reference-transaction` hooks read \"$GIT_DIR\" if defined (which is\ndefined!) so the `reference-transaction` should have passed. I assume\nthat hooks should be executed on properly initialized repositories,\nright? Therefore I do not understand why `git rev-parse --absolute-git-\ndir` fails -> Bug?\n\nAnything else you want to add:\n\nThis came up with `Githooks` hooks manager\nhttps://github.com/gabyx/Githooks where we use this command\nto locate the current Git dir...\n\nPlease review the rest of the bug report below.\nYou can delete any lines you don't wish to share.\n\n\n[System Info]\ngit version:\ngit version 2.46.0\ncpu: x86_64\nno commit associated with this build\nsizeof-long: 8\nsizeof-size_t: 8\nshell-path: /nix/store/izpf49b74i15pcr9708s3xdwyqs4jxwl-bash-\n5.2p32/bin/bash\nlibcurl: 8.9.1\nOpenSSL: OpenSSL 3.0.14 4 Jun 2024\nzlib: 1.3.1\nuname: Linux 6.6.45 #1-NixOS SMP PREEMPT_DYNAMIC Sun Aug 11 10:47:28\nUTC 2024 x86_64\ncompiler info: gnuc: 13.3\nlibc info: glibc: 2.39\n$SHELL (typically, interactive shell): /run/current-system/sw/bin/zsh\n\n"},{"id":"503121","messageId":"6102c928cb2f2e9afbc93e89737556c9d66b68eb.camel@gmail.com","threadId":"62151","inReplyTo":"0084cc18b6d90ba14849a7f788939f4a1d0d61c9.camel@gmail.com","subject":"Re: Bug: `git init` with hook `reference-transaction` running `git rev-parse --git-dir` fails","fromName":"Gabriel Nützi","fromEmail":"gnuetzi@gmail.com","sentAt":"2024-09-20T10:42:13Z","receivedAt":"2024-09-20T10:42:16Z","isPatch":false,"sender":{"key":"gnuetzi@gmail.com","avatar":"https://gravatar.com/avatar/eda99c30173930819ae1c26c3dff249edd4b3c735c3886d268c0d6a71c6307c9?d=mp&s=160"},"body":"The same happens with `git rev-parse --git-comon-dir` and \n`git config a.a \"hello\"` (which might be ok, since the config file is not yet setup at this point...)\n\nOn Fri, 2024-09-20 at 12:07 +0200, Gabriel Nützi wrote:\n> Thank you for filling out a Git bug report!  \n> Please answer the following questions to help us understand your issue.\n> \n> What did you do before the bug happened? (Steps to reproduce your  \n> issue)\n> \n> I set `git config --global core.hooksPath ~/myhooks` and placed a  \n> `reference-transaction` hook in `~/myhooks/reference-transaction`  \n> with the content:\n> \n> ```shell  \n> #!/usr/bin/env bash\n> \n> set -e  \n> echo \"$GIT_DIR\"  \n> git rev-parse --absolute-git-dir  \n> ```\n> \n> then I ran\n> \n> ```shell  \n> mkdir ~/test && cd test  \n> git init  \n> ```\n> \n> What did you expect to happen? (Expected behavior)\n> \n> The Git repo `~/test` should have been initialized (and the hook  \n> `reference-transaction` would have passed successfully.)\n> \n> \n> What happened instead? (Actual behavior)\n> \n> The hook `reference-transaction` crashes since `git rev-parse --  \n> absolute-git-dir` with  \n> ```  \n> failed: not a git repository: ...  \n> ```\n> \n> What's different between what you expected and what actually happened?\n> \n> The documentation says that `git rev-parse --absolute-git-dir` inside  \n> the `reference-transaction` hooks read \"$GIT_DIR\" if defined (which is  \n> defined!) so the `reference-transaction` should have passed. I assume  \n> that hooks should be executed on properly initialized repositories,  \n> right? Therefore I do not understand why `git rev-parse --absolute-git-  \n> dir` fails -> Bug?\n> \n> Anything else you want to add:\n> \n> This came up with `Githooks` hooks manager  \n> [https://github.com/gabyx/Githooks](https://github.com/gabyx/Githooks) where we use this command  \n> to locate the current Git dir...\n> \n> Please review the rest of the bug report below.  \n> You can delete any lines you don't wish to share.\n> \n> \n> [System Info]  \n> git version:  \n> git version 2.46.0  \n> cpu: x86_64  \n> no commit associated with this build  \n> sizeof-long: 8  \n> sizeof-size_t: 8  \n> shell-path: /nix/store/izpf49b74i15pcr9708s3xdwyqs4jxwl-bash-  \n> 5.2p32/bin/bash  \n> libcurl: 8.9.1  \n> OpenSSL: OpenSSL 3.0.14 4 Jun 2024  \n> zlib: 1.3.1  \n> uname: Linux 6.6.45 #1-NixOS SMP PREEMPT_DYNAMIC Sun Aug 11 10:47:28  \n> UTC 2024 x86_64  \n> compiler info: gnuc: 13.3  \n> libc info: glibc: 2.39  \n> $SHELL (typically, interactive shell): /run/current-system/sw/bin/zsh\n> \n"},{"id":"504263","messageId":"ZwOVy4FltrEjxHn_@pks.im","threadId":"62151","inReplyTo":"0084cc18b6d90ba14849a7f788939f4a1d0d61c9.camel@gmail.com","subject":"Re: Bug: `git init` with hook `reference-transaction` running `git rev-parse --git-dir` fails","fromName":"Patrick Steinhardt","fromEmail":"ps@pks.im","sentAt":"2024-10-07T08:03:30Z","receivedAt":"2024-10-07T08:03:38Z","isPatch":false,"sender":{"key":"ps@pks.im","avatar":"https://avatars.githubusercontent.com/u/4056630?v=4"},"body":"On Fri, Sep 20, 2024 at 12:07:53PM +0200, Gabriel Nützi wrote:\n> Thank you for filling out a Git bug report!\n> Please answer the following questions to help us understand your issue.\n> \n> What did you do before the bug happened? (Steps to reproduce your\n> issue)\n> \n> I set `git config --global core.hooksPath ~/myhooks` and placed a\n> `reference-transaction` hook in `~/myhooks/reference-transaction`\n> with the content:\n> \n> ```shell\n> #!/usr/bin/env bash\n> \n> set -e\n> echo \"$GIT_DIR\"\n> git rev-parse --absolute-git-dir\n> ```\n> \n> then I ran\n> \n> ```shell\n> mkdir ~/test && cd test\n> git init\n> ```\n> \n> What did you expect to happen? (Expected behavior)\n> \n> The Git repo `~/test` should have been initialized (and the hook\n> `reference-transaction` would have passed successfully.)\n> \n> \n> What happened instead? (Actual behavior)\n> \n> The hook `reference-transaction` crashes since `git rev-parse --\n> absolute-git-dir` with\n> ```\n> failed: not a git repository: ...\n> ```\n> \n> What's different between what you expected and what actually happened?\n> \n> The documentation says that `git rev-parse --absolute-git-dir` inside\n> the `reference-transaction` hooks read \"$GIT_DIR\" if defined (which is\n> defined!) so the `reference-transaction` should have passed. I assume\n> that hooks should be executed on properly initialized repositories,\n> right? Therefore I do not understand why `git rev-parse --absolute-git-\n> dir` fails -> Bug?\n> \n> Anything else you want to add:\n> \n> This came up with `Githooks` hooks manager\n> https://github.com/gabyx/Githooks where we use this command\n> to locate the current Git dir...\n> \n> Please review the rest of the bug report below.\n> You can delete any lines you don't wish to share.\n\nThanks for your bug report, and sorry for taking so long to respond.\nReproducing the observed behaviour is quite simple:\n\n    test_expect_success 'git-init with global hook' '\n        test_when_finished \"rm -rf hooks repo\" &&\n        mkdir hooks &&\n        write_script hooks/reference-transaction <<-EOF &&\n        git rev-parse --absolute-git-dir >>\"$(pwd)/reftx-logs\"\n        EOF\n        test_config --global core.hooksPath \"$(pwd)/hooks\" &&\n        git init repo\n    '\n\nThis breakage is new in Git v2.46 and comes from the patch series that\nintroduces support for symbolic refs in the reftx hook via a8ae923f85\n(refs: support symrefs in 'reference-transaction' hook, 2024-05-07) .\nBefore that change we didn't execute the hook for \"HEAD\" in the first\nplace, now we do.\n\nNow the question is whether this is a bug or not. When the reftx hook\nexecutes the first time it is when we are creating the \"HEAD\" ref in\nthe repo. Consequently, that file did not yet exist beforehand. And as\nGit only considers something a repository when the \"HEAD\" file exists it\nrightfully complains that this is not a valid Git repository when you\nask it to resolve the repo paths. So conceptually, the behaviour here is\ncorrect.\n\nThere are two ways we could fix this that I can think of:\n\n  - We can create a dummy \"HEAD\" file with invalid contents such that we\n    do have a proper Git repository when creating \"HEAD\". It feels like\n    a bit of a hack though, but we play similar games in git-clone(1).\n\n  - We can skip execution of the \"reference-transaction\" hook during\n    initialization of the repository. But this would make us miss some\n    ref updates, which feels conceptually wrong.\n\nI'd rule out (2), but (1) could be feasible if we label this a bug. I'm\nnot a 100% sure whether we should, as you could also argue that this is\nreflecting the actual state of the repo. I'd be happy to hear arguments\nin either direction.\n\nAlso Cc'd Karthik, the author of the menitoned change.\n\nPatrick\n"},{"id":"504304","messageId":"867f42bea10de470532d2f30eacceafa2b03d8de.camel@gmail.com","threadId":"62151","inReplyTo":"ZwOVy4FltrEjxHn_@pks.im","subject":"Re: Bug: `git init` with hook `reference-transaction` running `git rev-parse --git-dir` fails","fromName":"Gabriel Nützi","fromEmail":"gnuetzi@gmail.com","sentAt":"2024-10-07T10:54:36Z","receivedAt":"2024-10-07T10:54:39Z","isPatch":false,"sender":{"key":"gnuetzi@gmail.com","avatar":"https://gravatar.com/avatar/eda99c30173930819ae1c26c3dff249edd4b3c735c3886d268c0d6a71c6307c9?d=mp&s=160"},"body":"Hi Patrick,\n\nThanks for clarifications!\nCould it work if for (2) -> call the reference-transaction hook after the HEAD has been initialized, meaning that Git would internally cache the\ndifferent reference transactions and then call the hooks in one go at once after the creation of the repo, such that it is initialized properly?\nThis might be probably a more elaborate change which introduces too many technicalities?\n\nGabriel\n\n-----Original Message-----  \n**From**: Patrick Steinhardt <[ps@pks.im](mailto:Patrick%20Steinhardt%20%3cps@pks.im%3e)>  \n**To**: Gabriel Nützi <[gnuetzi@gmail.com](mailto:Gabriel%20%3d%3fISO-8859-1%3fQ%3fN%3dFCtzi%3f%3d%20%3cgnuetzi@gmail.com%3e)>  \n**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)>  \n**Subject**: Re: Bug: `git init` with hook `reference-transaction` running `git rev-parse --git-dir` fails  \n**Date**: 10/07/2024 10:03:30 AM\n\nOn Fri, Sep 20, 2024 at 12:07:53PM +0200, Gabriel Nützi wrote:\n\n> Thank you for filling out a Git bug report!\n> Please answer the following questions to help us understand your issue.\n> \n> What did you do before the bug happened? (Steps to reproduce your\n> issue)\n> \n> I set `git config --global core.hooksPath ~/myhooks` and placed a\n> `reference-transaction` hook in `~/myhooks/reference-transaction`\n> with the content:\n> \n> ```shell\n> #!/usr/bin/env bash\n> \n> set -e\n> echo \"$GIT_DIR\"\n> git rev-parse --absolute-git-dir\n> ```\n> \n> then I ran\n> \n> ```shell\n> mkdir ~/test && cd test\n> git init\n> ```\n> \n> What did you expect to happen? (Expected behavior)\n> \n> The Git repo `~/test` should have been initialized (and the hook\n> `reference-transaction` would have passed successfully.)\n> \n> \n> What happened instead? (Actual behavior)\n> \n> The hook `reference-transaction` crashes since `git rev-parse --\n> absolute-git-dir` with\n> ```\n> failed: not a git repository: ...\n> ```\n> \n> What's different between what you expected and what actually happened?\n> \n> The documentation says that `git rev-parse --absolute-git-dir` inside\n> the `reference-transaction` hooks read \"$GIT_DIR\" if defined (which is\n> defined!) so the `reference-transaction` should have passed. I assume\n> that hooks should be executed on properly initialized repositories,\n> right? Therefore I do not understand why `git rev-parse --absolute-git-\n> dir` fails -> Bug?\n> \n> Anything else you want to add:\n> \n> This came up with `Githooks` hooks manager\n> [https://github.com/gabyx/Githooks](https://github.com/gabyx/Githooks) where we use this command\n> to locate the current Git dir...\n> \n> Please review the rest of the bug report below.\n> You can delete any lines you don't wish to share.\n\n\nThanks for your bug report, and sorry for taking so long to respond.  \nReproducing the observed behaviour is quite simple:\n\n    test_expect_success 'git-init with global hook' '  \n        test_when_finished \"rm -rf hooks repo\" &&  \n        mkdir hooks &&  \n        write_script hooks/reference-transaction <<-EOF &&  \n        git rev-parse --absolute-git-dir >>\"$(pwd)/reftx-logs\"  \n        EOF  \n        test_config --global core.hooksPath \"$(pwd)/hooks\" &&  \n        git init repo  \n    '\n\nThis breakage is new in Git v2.46 and comes from the patch series that  \nintroduces support for symbolic refs in the reftx hook via a8ae923f85  \n(refs: support symrefs in 'reference-transaction' hook, 2024-05-07) .  \nBefore that change we didn't execute the hook for \"HEAD\" in the first  \nplace, now we do.\n\nNow the question is whether this is a bug or not. When the reftx hook  \nexecutes the first time it is when we are creating the \"HEAD\" ref in  \nthe repo. Consequently, that file did not yet exist beforehand. And as  \nGit only considers something a repository when the \"HEAD\" file exists it  \nrightfully complains that this is not a valid Git repository when you  \nask it to resolve the repo paths. So conceptually, the behaviour here is  \ncorrect.\n\nThere are two ways we could fix this that I can think of:\n\n  - We can create a dummy \"HEAD\" file with invalid contents such that we  \n    do have a proper Git repository when creating \"HEAD\". It feels like  \n    a bit of a hack though, but we play similar games in git-clone(1).\n\n  - We can skip execution of the \"reference-transaction\" hook during  \n    initialization of the repository. But this would make us miss some  \n    ref updates, which feels conceptually wrong.\n\nI'd rule out (2), but (1) could be feasible if we label this a bug. I'm  \nnot a 100% sure whether we should, as you could also argue that this is  \nreflecting the actual state of the repo. I'd be happy to hear arguments  \nin either direction.\n\nAlso Cc'd Karthik, the author of the menitoned change.\n\nPatrick\n"},{"id":"504305","messageId":"ZwO-j0C59vuYsEnt@pks.im","threadId":"62151","inReplyTo":"867f42bea10de470532d2f30eacceafa2b03d8de.camel@gmail.com","subject":"Re: Bug: `git init` with hook `reference-transaction` running `git rev-parse --git-dir` fails","fromName":"Patrick Steinhardt","fromEmail":"ps@pks.im","sentAt":"2024-10-07T10:57:19Z","receivedAt":"2024-10-07T10:57:24Z","isPatch":false,"sender":{"key":"ps@pks.im","avatar":"https://avatars.githubusercontent.com/u/4056630?v=4"},"body":"On Mon, Oct 07, 2024 at 12:54:36PM +0200, Gabriel Nützi wrote:\n> Hi Patrick,\n> \n> Thanks for clarifications!\n> Could it work if for (2) -> call the reference-transaction hook after the HEAD has been initialized, meaning that Git would internally cache the\n> 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?\n> This might be probably a more elaborate change which introduces too many technicalities?\n\nThat would break basic assumptions of how the hook operates. It is\nexpected that we abort the transaction when the hook returns an error,\nso if we were to run the hook _after_ `HEAD` has been created that\nexpectation would be broken.\n\nSo the only viable solution is to create a stub `HEAD`, but as said I'm\nnot a 100% sure whether we want to go there or not.\n\nPatrick\n"},{"id":"504306","messageId":"a8807b44026458097267932f429ef61d5b77aa4c.camel@gmail.com","threadId":"62151","inReplyTo":"ZwO-j0C59vuYsEnt@pks.im","subject":"Re: Bug: `git init` with hook `reference-transaction` running `git rev-parse --git-dir` fails","fromName":"Gabriel Nützi","fromEmail":"gnuetzi@gmail.com","sentAt":"2024-10-07T11:02:56Z","receivedAt":"2024-10-07T11:02:59Z","isPatch":false,"sender":{"key":"gnuetzi@gmail.com","avatar":"https://gravatar.com/avatar/eda99c30173930819ae1c26c3dff249edd4b3c735c3886d268c0d6a71c6307c9?d=mp&s=160"},"body":"Ah, jeah thats true:\n\nThe command in question which is `git rev-parse --git-dir` to find the actual GIT_DIR is actually the issue here:\nIt relies on having a proper HEAD despite the docs saying it only reports GIT_DIR if set etc...\n\nCould we maybe make these commands more agnostic? \nMaking an empty HEAD probably directly also resolves these commands to work correctly.\n\nGabriel\n\n\n-----Original Message-----  \n**From**: Patrick Steinhardt <[ps@pks.im](mailto:Patrick%20Steinhardt%20%3cps@pks.im%3e)>  \n**To**: Gabriel Nützi <[gnuetzi@gmail.com](mailto:Gabriel%20%3d%3fISO-8859-1%3fQ%3fN%3dFCtzi%3f%3d%20%3cgnuetzi@gmail.com%3e)>  \n**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)>  \n**Subject**: Re: Bug: `git init` with hook `reference-transaction` running `git rev-parse --git-dir` fails  \n**Date**: 10/07/2024 12:57:19 PM\n\ngit rev-parse --git-dir\n"},{"id":"504318","messageId":"ZwPEwL-fKHBkUOcm@pks.im","threadId":"62151","inReplyTo":"a8807b44026458097267932f429ef61d5b77aa4c.camel@gmail.com","subject":"Re: Bug: `git init` with hook `reference-transaction` running `git rev-parse --git-dir` fails","fromName":"Patrick Steinhardt","fromEmail":"ps@pks.im","sentAt":"2024-10-07T11:24:00Z","receivedAt":"2024-10-07T11:24:07Z","isPatch":false,"sender":{"key":"ps@pks.im","avatar":"https://avatars.githubusercontent.com/u/4056630?v=4"},"body":"On Mon, Oct 07, 2024 at 01:02:56PM +0200, Gabriel Nützi wrote:\n> Ah, jeah thats true:\n> \n> The command in question which is `git rev-parse --git-dir` to find the actual GIT_DIR is actually the issue here:\n> It relies on having a proper HEAD despite the docs saying it only reports GIT_DIR if set etc...\n\nThat's not quite true. The second paragraph for the `--git-dir` option\nin git-rev-parse(1) has this to say:\n\n    If $GIT_DIR is not defined and the current directory is not detected to lie in a Git\n    repository or work tree print a message to stderr and exit with nonzero status.\n\nBut in our case you're correct: `$GIT_DIR` _is_ defined during hook\nexecution. So in theory, if git-rev-parse(1) behaved exactly as\ndocumented, it shouldn't even care whether or not it is executing in a\nrepository.\n\n> Could we maybe make these commands more agnostic? \n\nSo this could indeed be a viable fix, but I guess we'd rather play\nwhack-a-mole because other Git commands that one may want to execute in\nthis repository would also be broken.\n\n> Making an empty HEAD probably directly also resolves these commands to work correctly.\n\nSo agreed, creating a stub HEAD is the most sensible option we have. I'd\nlike to wait a bit for more feedback from other contributors before we\ndo so though.\n\nPatrick\n"},{"id":"504361","messageId":"xmqqiku38w51.fsf@gitster.g","threadId":"62151","inReplyTo":"ZwPEwL-fKHBkUOcm@pks.im","subject":"Re: Bug: `git init` with hook `reference-transaction` running `git rev-parse --git-dir` fails","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2024-10-07T21:02:02Z","receivedAt":"2024-10-07T21:02:04Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Patrick Steinhardt <ps@pks.im> writes:\n\n> `$GIT_DIR` _is_ defined during hook\n> execution. So in theory, if git-rev-parse(1) behaved exactly as\n> documented, it shouldn't even care whether or not it is executing in a\n> repository.\n\nI've always considered \"git rev-parse --git-dir\" and friends were to\nverify the validity of the repository before returning \"Your GIT_DIR\nis here\".  Otherwise there is no easy way to ask \"I have this directory.\nIs it a valid repository you can work with?\".\n\nSo, I am not sure I agree with the above.\n\nFor what is worth, I am skeptical to the \"solution\" that tentively\ncreates a bogus HEAD file while the repository is being initialized.\nThe code today may ignore certain bogosity in such a HEAD (like the\n\".invalid\" magic used during \"git clone\"), but there is no guarantee\nthat a random third-party add-on a hook script may invoke do the\nsame as we do, and more importantly, what a repository with its\ninitialization complete look like may change over time and it may\nnot be enough to have HEAD pointing at \"refs/heads/.invalid\" to fool\nour bootstrap process.  If we were to go that route, I would rather\nsee us pick a pointee that is *not* bogus at the mechanical level\n(i.e., \"git symbolic-ref HEAD refs/heads/.invalid\" would fail) but\nis clearly a placeholder value to humans, soon to be updated.\n\nLet's say if we were to create a repository with the name of initial\nbranch as 'main', we could create HEAD that points at refs/heads/main\nbypassing any hook intervention, then call the hook to see if it\napproves the name.  We'd need to make sure that we fail the\nrepository creation when the hook declines, as it is refusing to set\na HEAD, one critical element in the repository that has to exist,\nand probably remove the directory if we are not reinitializing.\n\nOr we could use a name that is clearly bogus to humans but is still\nstructurally OK, say \"refs/heads/hook-failed-during-initialization\",\nask the hook if it is OK to repoint HEAD to \"refs/heads/main\" from\nthat state, and (1) if it approves, HEAD will point at \"refs/heads/main\"\nand \"hook-failed-during-initialization\" will be seen nowhere but the\nreflog of HEAD, or (2) if it refuses, we stop, and the user will be\nleft on an unborn branch with a long descriptive name that explains\nthe situation.\n\nA much simpler alternative would be to simply ignore any hooks,\ntraces, or anything that want to look into the directory we are\nworking to turn into a repository but haven't completed doing so,\nduring repository initialization, I would think, though.\n\n"},{"id":"504532","messageId":"ZwZPSA-J1RFQKYrw@pks.im","threadId":"62151","inReplyTo":"xmqqiku38w51.fsf@gitster.g","subject":"Re: Bug: `git init` with hook `reference-transaction` running `git rev-parse --git-dir` fails","fromName":"Patrick Steinhardt","fromEmail":"ps@pks.im","sentAt":"2024-10-09T09:39:26Z","receivedAt":"2024-10-09T09:39:31Z","isPatch":false,"sender":{"key":"ps@pks.im","avatar":"https://avatars.githubusercontent.com/u/4056630?v=4"},"body":"On Mon, Oct 07, 2024 at 02:02:02PM -0700, Junio C Hamano wrote:\n> Patrick Steinhardt <ps@pks.im> writes:\n> \n> > `$GIT_DIR` _is_ defined during hook\n> > execution. So in theory, if git-rev-parse(1) behaved exactly as\n> > documented, it shouldn't even care whether or not it is executing in a\n> > repository.\n> \n> I've always considered \"git rev-parse --git-dir\" and friends were to\n> verify the validity of the repository before returning \"Your GIT_DIR\n> is here\".  Otherwise there is no easy way to ask \"I have this directory.\n> Is it a valid repository you can work with?\".\n> \n> So, I am not sure I agree with the above.\n\nWell, I'm not sure either. I was merely pointing out that the documented\nbehaviour is different than the actual behaviour. Which of both is the\nmore sensible one is a different question.\n\n> For what is worth, I am skeptical to the \"solution\" that tentively\n> creates a bogus HEAD file while the repository is being initialized.\n> The code today may ignore certain bogosity in such a HEAD (like the\n> \".invalid\" magic used during \"git clone\"), but there is no guarantee\n> that a random third-party add-on a hook script may invoke do the\n> same as we do, and more importantly, what a repository with its\n> initialization complete look like may change over time and it may\n> not be enough to have HEAD pointing at \"refs/heads/.invalid\" to fool\n> our bootstrap process.  If we were to go that route, I would rather\n> see us pick a pointee that is *not* bogus at the mechanical level\n> (i.e., \"git symbolic-ref HEAD refs/heads/.invalid\" would fail) but\n> is clearly a placeholder value to humans, soon to be updated.\n> \n> Let's say if we were to create a repository with the name of initial\n> branch as 'main', we could create HEAD that points at refs/heads/main\n> bypassing any hook intervention, then call the hook to see if it\n> approves the name.  We'd need to make sure that we fail the\n> repository creation when the hook declines, as it is refusing to set\n> a HEAD, one critical element in the repository that has to exist,\n> and probably remove the directory if we are not reinitializing.\n> \n> Or we could use a name that is clearly bogus to humans but is still\n> structurally OK, say \"refs/heads/hook-failed-during-initialization\",\n> ask the hook if it is OK to repoint HEAD to \"refs/heads/main\" from\n> that state, and (1) if it approves, HEAD will point at \"refs/heads/main\"\n> and \"hook-failed-during-initialization\" will be seen nowhere but the\n> reflog of HEAD, or (2) if it refuses, we stop, and the user will be\n> left on an unborn branch with a long descriptive name that explains\n> the situation.\n\nI dunno. It all feels rather complex.\n\n> A much simpler alternative would be to simply ignore any hooks,\n> traces, or anything that want to look into the directory we are\n> working to turn into a repository but haven't completed doing so,\n> during repository initialization, I would think, though.\n\nThat could work, yes, but it would limit the usefulness of the hook. In\ntheory, you can create a full log of all changes in the repository from\nits inception. If we didn't log the first item, that log would be\nincomplete.\n\nWe have another solution that is even simpler: just do nothing. I do not\nthink that the behaviour we exhibit is wrong. Unwieldy? Maybe. But it is\nmerely stating facts: we are executing a transaction in a repository\nthat is not yet fully set up. If you don't want that, either don't set\nup a global reference-transaction hook, or alternatively handle that\nedge case in your script.\n\nPatrick\n"},{"id":"504534","messageId":"CAOLa=ZRMqRnUn0daZ2O-TCbyJzmc6LVGx9Yc+V7VJapB+FyPCQ@mail.gmail.com","threadId":"62151","inReplyTo":"ZwZPSA-J1RFQKYrw@pks.im","subject":"Re: Bug: `git init` with hook `reference-transaction` running `git rev-parse --git-dir` fails","fromName":"Karthik Nayak","fromEmail":"karthik.188@gmail.com","sentAt":"2024-10-09T10:09:36Z","receivedAt":"2024-10-09T10:09:39Z","isPatch":false,"sender":{"key":"karthik.188@gmail.com","avatar":"https://avatars.githubusercontent.com/u/1786334?v=4"},"body":"Patrick Steinhardt <ps@pks.im> writes:\n\n> On Mon, Oct 07, 2024 at 02:02:02PM -0700, Junio C Hamano wrote:\n>> Patrick Steinhardt <ps@pks.im> writes:\n>>\n>> > `$GIT_DIR` _is_ defined during hook\n>> > execution. So in theory, if git-rev-parse(1) behaved exactly as\n>> > documented, it shouldn't even care whether or not it is executing in a\n>> > repository.\n>>\n>> I've always considered \"git rev-parse --git-dir\" and friends were to\n>> verify the validity of the repository before returning \"Your GIT_DIR\n>> is here\".  Otherwise there is no easy way to ask \"I have this directory.\n>> Is it a valid repository you can work with?\".\n>>\n>> So, I am not sure I agree with the above.\n>\n> Well, I'm not sure either. I was merely pointing out that the documented\n> behaviour is different than the actual behaviour. Which of both is the\n> more sensible one is a different question.\n>\n>> For what is worth, I am skeptical to the \"solution\" that tentively\n>> creates a bogus HEAD file while the repository is being initialized.\n>> The code today may ignore certain bogosity in such a HEAD (like the\n>> \".invalid\" magic used during \"git clone\"), but there is no guarantee\n>> that a random third-party add-on a hook script may invoke do the\n>> same as we do, and more importantly, what a repository with its\n>> initialization complete look like may change over time and it may\n>> not be enough to have HEAD pointing at \"refs/heads/.invalid\" to fool\n>> our bootstrap process.  If we were to go that route, I would rather\n>> see us pick a pointee that is *not* bogus at the mechanical level\n>> (i.e., \"git symbolic-ref HEAD refs/heads/.invalid\" would fail) but\n>> is clearly a placeholder value to humans, soon to be updated.\n>>\n>> Let's say if we were to create a repository with the name of initial\n>> branch as 'main', we could create HEAD that points at refs/heads/main\n>> bypassing any hook intervention, then call the hook to see if it\n>> approves the name.  We'd need to make sure that we fail the\n>> repository creation when the hook declines, as it is refusing to set\n>> a HEAD, one critical element in the repository that has to exist,\n>> and probably remove the directory if we are not reinitializing.\n>>\n>> Or we could use a name that is clearly bogus to humans but is still\n>> structurally OK, say \"refs/heads/hook-failed-during-initialization\",\n>> ask the hook if it is OK to repoint HEAD to \"refs/heads/main\" from\n>> that state, and (1) if it approves, HEAD will point at \"refs/heads/main\"\n>> and \"hook-failed-during-initialization\" will be seen nowhere but the\n>> reflog of HEAD, or (2) if it refuses, we stop, and the user will be\n>> left on an unborn branch with a long descriptive name that explains\n>> the situation.\n>\n> I dunno. It all feels rather complex.\n>\n>> A much simpler alternative would be to simply ignore any hooks,\n>> traces, or anything that want to look into the directory we are\n>> working to turn into a repository but haven't completed doing so,\n>> during repository initialization, I would think, though.\n>\n> That could work, yes, but it would limit the usefulness of the hook. In\n> theory, you can create a full log of all changes in the repository from\n> its inception. If we didn't log the first item, that log would be\n> incomplete.\n>\n> We have another solution that is even simpler: just do nothing. I do not\n> think that the behaviour we exhibit is wrong. Unwieldy? Maybe. But it is\n> merely stating facts: we are executing a transaction in a repository\n> that is not yet fully set up. If you don't want that, either don't set\n> up a global reference-transaction hook, or alternatively handle that\n> edge case in your script.\n\nThis does seem like we're putting the onus on the users, but ultimately\nI agree with this. What the hook is doing is as expected.\n\nThanks Patrick for responding while I was away.\n\nKarthik\n"},{"id":"504545","messageId":"5cd07a8d3c554815d4fce20996c018c6294c63e6.camel@gmail.com","threadId":"62151","inReplyTo":"ZwZPSA-J1RFQKYrw@pks.im","subject":"Re: Bug: `git init` with hook `reference-transaction` running `git rev-parse --git-dir` fails","fromName":"Gabriel Nützi","fromEmail":"gnuetzi@gmail.com","sentAt":"2024-10-09T11:53:17Z","receivedAt":"2024-10-09T11:53:22Z","isPatch":false,"sender":{"key":"gnuetzi@gmail.com","avatar":"https://gravatar.com/avatar/eda99c30173930819ae1c26c3dff249edd4b3c735c3886d268c0d6a71c6307c9?d=mp&s=160"},"body":"\n> We have another solution that is even simpler: just do nothing. I do not  \n> think that the behaviour we exhibit is wrong. Unwieldy? Maybe. But it is  \n> merely stating facts: we are executing a transaction in a repository  \n> that is not yet fully set up. If you don't want that, either don't set  \n> up a global reference-transaction hook, or alternatively handle that  \n> edge case in your script.\n\nBut then Git should at least give the hooks maintainers a chance to know what it is doing.\nCould we have a new env. variable or mechanism stating that the repository is not yet setup or so?\nThe issue here comes from the fact that Githooks is a hooks manager which basically calls `reference-transaction` for you, but before\nit 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.\nI 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...\n\n\nGabriel\n\n-----Original Message-----  \n**From**: Patrick Steinhardt <[ps@pks.im](mailto:Patrick%20Steinhardt%20%3cps@pks.im%3e)>  \n**To**: Junio C Hamano <[gitster@pobox.com](mailto:Junio%20C%20Hamano%20%3cgitster@pobox.com%3e)>  \n**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)>  \n**Subject**: Re: Bug: `git init` with hook `reference-transaction` running `git rev-parse --git-dir` fails  \n**Date**: 10/09/2024 11:39:26 AM\n\nOn Mon, Oct 07, 2024 at 02:02:02PM -0700, Junio C Hamano wrote:\n\n> Patrick Steinhardt <[ps@pks.im](mailto:ps@pks.im)> writes:\n> \n> \n> > `$GIT_DIR` _is_ defined during hook\n> > execution. So in theory, if git-rev-parse(1) behaved exactly as\n> > documented, it shouldn't even care whether or not it is executing in a\n> > repository.\n> \n> \n> I've always considered \"git rev-parse --git-dir\" and friends were to\n> verify the validity of the repository before returning \"Your GIT_DIR\n> is here\".  Otherwise there is no easy way to ask \"I have this directory.\n> Is it a valid repository you can work with?\".\n> \n> So, I am not sure I agree with the above.\n\n\nWell, I'm not sure either. I was merely pointing out that the documented  \nbehaviour is different than the actual behaviour. Which of both is the  \nmore sensible one is a different question.\n\n\n> For what is worth, I am skeptical to the \"solution\" that tentively\n> creates a bogus HEAD file while the repository is being initialized.\n> The code today may ignore certain bogosity in such a HEAD (like the\n> \".invalid\" magic used during \"git clone\"), but there is no guarantee\n> that a random third-party add-on a hook script may invoke do the\n> same as we do, and more importantly, what a repository with its\n> initialization complete look like may change over time and it may\n> not be enough to have HEAD pointing at \"refs/heads/.invalid\" to fool\n> our bootstrap process.  If we were to go that route, I would rather\n> see us pick a pointee that is *not* bogus at the mechanical level\n> (i.e., \"git symbolic-ref HEAD refs/heads/.invalid\" would fail) but\n> is clearly a placeholder value to humans, soon to be updated.\n> \n> Let's say if we were to create a repository with the name of initial\n> branch as 'main', we could create HEAD that points at refs/heads/main\n> bypassing any hook intervention, then call the hook to see if it\n> approves the name.  We'd need to make sure that we fail the\n> repository creation when the hook declines, as it is refusing to set\n> a HEAD, one critical element in the repository that has to exist,\n> and probably remove the directory if we are not reinitializing.\n> \n> Or we could use a name that is clearly bogus to humans but is still\n> structurally OK, say \"refs/heads/hook-failed-during-initialization\",\n> ask the hook if it is OK to repoint HEAD to \"refs/heads/main\" from\n> that state, and (1) if it approves, HEAD will point at \"refs/heads/main\"\n> and \"hook-failed-during-initialization\" will be seen nowhere but the\n> reflog of HEAD, or (2) if it refuses, we stop, and the user will be\n> left on an unborn branch with a long descriptive name that explains\n> the situation.\n\n\nI dunno. It all feels rather complex.\n\n\n> A much simpler alternative would be to simply ignore any hooks,\n> traces, or anything that want to look into the directory we are\n> working to turn into a repository but haven't completed doing so,\n> during repository initialization, I would think, though.\n\n\nThat could work, yes, but it would limit the usefulness of the hook. In  \ntheory, you can create a full log of all changes in the repository from  \nits inception. If we didn't log the first item, that log would be  \nincomplete.\n\nWe have another solution that is even simpler: just do nothing. I do not  \nthink that the behaviour we exhibit is wrong. Unwieldy? Maybe. But it is  \nmerely stating facts: we are executing a transaction in a repository  \nthat is not yet fully set up. If you don't want that, either don't set  \nup a global reference-transaction hook, or alternatively handle that  \nedge case in your script.\n\nPatrick\n"},{"id":"504549","messageId":"ZwZ01-mtwm9Gcjub@pks.im","threadId":"62151","inReplyTo":"5cd07a8d3c554815d4fce20996c018c6294c63e6.camel@gmail.com","subject":"Re: Bug: `git init` with hook `reference-transaction` running `git rev-parse --git-dir` fails","fromName":"Patrick Steinhardt","fromEmail":"ps@pks.im","sentAt":"2024-10-09T12:19:45Z","receivedAt":"2024-10-09T12:19:51Z","isPatch":false,"sender":{"key":"ps@pks.im","avatar":"https://avatars.githubusercontent.com/u/4056630?v=4"},"body":"On Wed, Oct 09, 2024 at 01:53:17PM +0200, Gabriel Nützi wrote:\n> \n> > We have another solution that is even simpler: just do nothing. I do not  \n> > think that the behaviour we exhibit is wrong. Unwieldy? Maybe. But it is  \n> > merely stating facts: we are executing a transaction in a repository  \n> > that is not yet fully set up. If you don't want that, either don't set  \n> > up a global reference-transaction hook, or alternatively handle that  \n> > edge case in your script.\n> \n> But then Git should at least give the hooks maintainers a chance to know what it is doing.\n> Could we have a new env. variable or mechanism stating that the repository is not yet setup or so?\n> The issue here comes from the fact that Githooks is a hooks manager which basically calls `reference-transaction` for you, but before\n> 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.\n> 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...\n\nYeah, that's fair indeed, thanks for the background.\n\nIn practice you already have a unique way to identify whether the repo\nis about to be created or whether you're using \"normal\" operations,\nnamely by checking whether the HEAD file exists in the Git directory. Is\nthat an acceptable workaround for you?\n\nAs said, we're currently just exploring options. I'm not yet saying that\nthis is the way we'll go, even though it seems like the most sensible\nsolution to me right now. That picture of course changes when there are\nvalid usecases like what you present here.\n\nPatrick\n"},{"id":"504612","messageId":"xmqqfrp5uqra.fsf@gitster.g","threadId":"62151","inReplyTo":"ZwZPSA-J1RFQKYrw@pks.im","subject":"Re: Bug: `git init` with hook `reference-transaction` running `git rev-parse --git-dir` fails","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2024-10-09T17:31:53Z","receivedAt":"2024-10-09T17:31:55Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Patrick Steinhardt <ps@pks.im> writes:\n\n> We have another solution that is even simpler: just do nothing. I do not\n> think that the behaviour we exhibit is wrong. Unwieldy? Maybe. But it is\n> merely stating facts: we are executing a transaction in a repository\n> that is not yet fully set up. If you don't want that, either don't set\n> up a global reference-transaction hook, or alternatively handle that\n> edge case in your script.\n\nI like that over all the other choices.\n\nMy art in leaving out what I really want to see happen and let\nothers say it is improving, it seems ;-).\n"},{"id":"504998","messageId":"Zw0NskRJUSmpNxLe@pks.im","threadId":"62151","inReplyTo":"B0631C6D-0914-4C25-AAF7-E742129836FC@gmail.com","subject":"Re: Bug: `git init` with hook `reference-transaction` running `git rev-parse --git-dir` fails","fromName":"Patrick Steinhardt","fromEmail":"ps@pks.im","sentAt":"2024-10-14T12:25:27Z","receivedAt":"2024-10-14T12:25:32Z","isPatch":false,"sender":{"key":"ps@pks.im","avatar":"https://avatars.githubusercontent.com/u/4056630?v=4"},"body":"On Fri, Oct 11, 2024 at 12:09:18PM +0200, Gabriel Nützi wrote:\n> > namely by checking whether the HEAD file exists in the Git directory. \n> \n> The problem here is the chicken-egg problem: How do I know inside a\n> hook where to look for HEAD, so the non. env. way of doing this would\n> be 'git rev-parse —git-dir' which crahshes… So there should be cleary\n> some better way of knowing if a Git repo is on the way of beeing\n> created, and when it is fully initialized. What about\n> GIT_DIR_INITIALIZING and this variable does not exist once it is done\n> or whats about making ‚git rev-parse —git-dir‘ not panic if the env.\n> variables are defined. I guess for a good solution we need both?\n\nOur documentation in githooks(5) says that the GIT_DIR environment\nvariable is always set so that the hook can identify the location of the\nGit repository. So I'd argue that using git-rev-parse(1) is unnecessary\nin the first place. Or am I missing something?\n\nPatrick\n"}]}