{"thread":{"id":"59051","subject":"[PATCH] githooks: discuss Git operations in foreign repositories","startedAt":"2023-01-08T09:59:03Z","lastAt":"2023-01-11T19:01:35Z","messageCount":11,"participants":["Eric Sunshine via GitGitGadget","Preston Tunnell Wilson","Eric Sunshine","Junio C Hamano","Jeff King"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"469910","messageId":"pull.1457.git.1673171924727.gitgitgadget@gmail.com","threadId":"59051","inReplyTo":null,"subject":"[PATCH] githooks: discuss Git operations in foreign repositories","fromName":"Eric Sunshine via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2023-01-08T09:58:44Z","receivedAt":"2023-01-08T09:59:03Z","isPatch":true,"sender":{"key":"sunshine@sunshineco.com","avatar":"https://avatars.githubusercontent.com/u/163641?v=4"},"body":"From: Eric Sunshine \n\nHook authors are periodically caught off-guard by difficult-to-diagnose\nerrors when their hook invokes Git commands in a repository other than\nthe local one. In particular, Git environment variables, such as GIT_DIR\nand GIT_WORK_TREE, which reference the local repository cause the Git\ncommands to operate on the local repository rather than on the\nrepository which the author intended. This is true whether the\nenvironment variables have been set manually by the user or\nautomatically by Git itself. The same problem crops up when a hook\ninvokes Git commands in a different worktree of the same repository, as\nwell.\n\nRecommended best-practice[1,2,3,4,5,6] for avoiding this problem is for\nthe hook to ensure that Git variables are unset before invoking Git\ncommands in foreign repositories or other worktrees:\n\n unset $(git rev-parse --local-env-vars)\n\nHowever, this advice is not documented anywhere. Rectify this\nshortcoming by mentioning it in githooks.txt documentation.\n\n[1]: https://lore.kernel.org/git/YFuHd1MMlJAvtdzb@coredump.intra.peff.net/\n[2]: https://lore.kernel.org/git/20200228190218.GC1408759@coredump.intra.peff.net/\n[3]: https://lore.kernel.org/git/20190516221702.GA11784@sigill.intra.peff.net/\n[4]: https://lore.kernel.org/git/20190422162127.GC9680@sigill.intra.peff.net/\n[5]: https://lore.kernel.org/git/20180716183942.GB22298@sigill.intra.peff.net/\n[6]: https://lore.kernel.org/git/20150203163235.GA9325@peff.net/\n\nSigned-off-by: Eric Sunshine \n---\n githooks: discuss Git operations in foreign repositories\n\nPublished-As: https://github.com/gitgitgadget/git/releases/tag/pr-1457%2Fsunshineco%2Fhookenv-v1\nFetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-1457/sunshineco/hookenv-v1\nPull-Request: https://github.com/gitgitgadget/git/pull/1457\n\n Documentation/githooks.txt | 11 +++++++++++\n 1 file changed, 11 insertions(+)\n\ndiff --git a/Documentation/githooks.txt b/Documentation/githooks.txt\nindex a16e62bc8c8..6e9a5420b7c 100644\n--- a/Documentation/githooks.txt\n+++ b/Documentation/githooks.txt\n@@ -31,6 +31,17 @@ Hooks can get their arguments via the environment, command-line\n arguments, and stdin. See the documentation for each hook below for\n details.\n \n+If your hook needs to invoke Git commands in a foreign repository or in a\n+different working tree of the same repository, then it should clear local Git\n+environment variables, such as `GIT_DIR`, `GIT_WORK_TREE`, etc., which could\n+interfere with Git operations in the foreign repository since those variables\n+will be referencing the local repository and working tree. For example:\n+\n+------------\n+local_desc=$(git describe)\n+foreign_desc=$(unset $(git rev-parse --local-env-vars); git -C ../foreign-repo describe)\n+------------\n+\n `git init` may copy hooks to the new repository, depending on its\n configuration. See the \"TEMPLATE DIRECTORY\" section in\n linkgit:git-init[1] for details. When the rest of this document refers\n\nbase-commit: a38d39a4c50d1275833aba54c4dbdfce9e2e9ca1\n-- \ngitgitgadget\n"},{"id":"469926","messageId":"CAC-j02O6z4sG85LpRNzEZ52Y-McurYDa_VnVXtqFVPBFu9kbug@mail.gmail.com","threadId":"59051","inReplyTo":"pull.1457.git.1673171924727.gitgitgadget@gmail.com","subject":"Re: [PATCH] githooks: discuss Git operations in foreign repositories","fromName":"Preston Tunnell Wilson","fromEmail":"prestontunnellwilson@gmail.com","sentAt":"2023-01-08T19:45:32Z","receivedAt":"2023-01-08T19:45:48Z","isPatch":true,"sender":{"key":"prestontunnellwilson@gmail.com","avatar":null},"body":"Thank you for this wonderful remedy, Eric! I really appreciate the\nbackground context and how you framed the problem that I ran into.\n\nI have two questions:\n1. Documentation is a great first step in addressing this, but I'm\nwondering if this should be automatic? If this is a best practice for\nhook authors, could `git` do this for them automatically when running\nhooks?\n2. Should we add something in the `git-worktree` documentation? In\n`Documentation/git-worktree.txt`, it mentions:\n\n> BUGS\n> ----\n> Multiple checkout in general is still experimental, and the support\n> for submodules is incomplete. ...\n\nWould it be helpful to plant a flag in the above documentation to\npoint to this potential issue?\n\nThank you,\nPreston\n\n\nOn Sun, Jan 8, 2023 at 3:58 AM Eric Sunshine via GitGitGadget\n wrote:\n>\n> From: Eric Sunshine \n>\n> Hook authors are periodically caught off-guard by difficult-to-diagnose\n> errors when their hook invokes Git commands in a repository other than\n> the local one. In particular, Git environment variables, such as GIT_DIR\n> and GIT_WORK_TREE, which reference the local repository cause the Git\n> commands to operate on the local repository rather than on the\n> repository which the author intended. This is true whether the\n> environment variables have been set manually by the user or\n> automatically by Git itself. The same problem crops up when a hook\n> invokes Git commands in a different worktree of the same repository, as\n> well.\n>\n> Recommended best-practice[1,2,3,4,5,6] for avoiding this problem is for\n> the hook to ensure that Git variables are unset before invoking Git\n> commands in foreign repositories or other worktrees:\n>\n> unset $(git rev-parse --local-env-vars)\n>\n> However, this advice is not documented anywhere. Rectify this\n> shortcoming by mentioning it in githooks.txt documentation.\n>\n> [1]: https://lore.kernel.org/git/YFuHd1MMlJAvtdzb@coredump.intra.peff.net/\n> [2]: https://lore.kernel.org/git/20200228190218.GC1408759@coredump.intra.peff.net/\n> [3]: https://lore.kernel.org/git/20190516221702.GA11784@sigill.intra.peff.net/\n> [4]: https://lore.kernel.org/git/20190422162127.GC9680@sigill.intra.peff.net/\n> [5]: https://lore.kernel.org/git/20180716183942.GB22298@sigill.intra.peff.net/\n> [6]: https://lore.kernel.org/git/20150203163235.GA9325@peff.net/\n>\n> Signed-off-by: Eric Sunshine \n> ---\n> githooks: discuss Git operations in foreign repositories\n>\n> Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-1457%2Fsunshineco%2Fhookenv-v1\n> Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-1457/sunshineco/hookenv-v1\n> Pull-Request: https://github.com/gitgitgadget/git/pull/1457\n>\n> Documentation/githooks.txt | 11 +++++++++++\n> 1 file changed, 11 insertions(+)\n>\n> diff --git a/Documentation/githooks.txt b/Documentation/githooks.txt\n> index a16e62bc8c8..6e9a5420b7c 100644\n> --- a/Documentation/githooks.txt\n> +++ b/Documentation/githooks.txt\n> @@ -31,6 +31,17 @@ Hooks can get their arguments via the environment, command-line\n> arguments, and stdin. See the documentation for each hook below for\n> details.\n>\n> +If your hook needs to invoke Git commands in a foreign repository or in a\n> +different working tree of the same repository, then it should clear local Git\n> +environment variables, such as `GIT_DIR`, `GIT_WORK_TREE`, etc., which could\n> +interfere with Git operations in the foreign repository since those variables\n> +will be referencing the local repository and working tree. For example:\n> +\n> +------------\n> +local_desc=$(git describe)\n> +foreign_desc=$(unset $(git rev-parse --local-env-vars); git -C ../foreign-repo describe)\n> +------------\n> +\n> `git init` may copy hooks to the new repository, depending on its\n> configuration. See the \"TEMPLATE DIRECTORY\" section in\n> linkgit:git-init[1] for details. When the rest of this document refers\n>\n> base-commit: a38d39a4c50d1275833aba54c4dbdfce9e2e9ca1\n> --\n> gitgitgadget\n"},{"id":"469927","messageId":"CAPig+cS_dXL-Q6NZtUJxDOL4-Q=MJv8fPEPAnEPuONaNF8-sCA@mail.gmail.com","threadId":"59051","inReplyTo":"CAC-j02O6z4sG85LpRNzEZ52Y-McurYDa_VnVXtqFVPBFu9kbug@mail.gmail.com","subject":"Re: [PATCH] githooks: discuss Git operations in foreign repositories","fromName":"Eric Sunshine","fromEmail":"sunshine@sunshineco.com","sentAt":"2023-01-08T23:25:47Z","receivedAt":"2023-01-08T23:26:12Z","isPatch":true,"sender":{"key":"sunshine@sunshineco.com","avatar":"https://avatars.githubusercontent.com/u/163641?v=4"},"body":"[administrivia: please reply inline rather than top-posting]\n\nOn Sun, Jan 8, 2023 at 2:45 PM Preston Tunnell Wilson\n wrote:\n> Thank you for this wonderful remedy, Eric! I really appreciate the\n> background context and how you framed the problem that I ran into.\n>\n> I have two questions:\n> 1. Documentation is a great first step in addressing this, but I'm\n> wondering if this should be automatic? If this is a best practice for\n> hook authors, could `git` do this for them automatically when running\n> hooks?\n\nFor the general case, probably not. \"Best-practice\" is context\nsensitive. It may be best-practice when a hook needs to invoke Git\ncommands in some other repository (or worktree), but clearing those\nvariables automatically would, in some situations, break the much more\ncommon case of the hook invoking Git commands in the local repository\n(or worktree). The fact that those environment variables may have been\nset manually by the user or automatically by Git further complicates\nthe situation.\n\n> 2. Should we add something in the `git-worktree` documentation? In\n> `Documentation/git-worktree.txt`, it mentions:\n>\n> > BUGS\n> > ----\n> > Multiple checkout in general is still experimental, and the support\n> > for submodules is incomplete. ...\n>\n> Would it be helpful to plant a flag in the above documentation to\n> point to this potential issue?\n\nAs noted above, we can't really call this a bug. Git is behaving as\nintended. Whether the user set the variables manually or whether some\nparent Git process set them automatically, the child Git respects the\nvariables as it should rather than second-guessing about the user's\nintentions, and possibly guessing incorrectly.\n\nSo, no, I don't think this qualifies for the BUGS section of\ngit-wortkree, and mentioning this potential gotcha only in\ngit-worktree but not in any other hook-running command doesn't seem\nideal either. At present, the best place to discuss it seems to be\nDocumentation/githooks.txt, as this patch does. It may be possible to\nargue that gitfaq.txt could talk about it, but considering that this\nissue can manifest in many different ways (various error messages or\nmisbehaviors), it's difficult to come up with any text for the \"Q\"\nwhich people would be likely to find when Googling. That's not to say\nit shouldn't be mentioned elsewhere in the documentation, but rather\nthat I haven't come up with any better places than githooks.txt\nitself.\n"},{"id":"469931","messageId":"CAC-j02MV+Gv0D8-fpCOu7JGUimxPLF+OP1dy7bxfs7ArX05BYg@mail.gmail.com","threadId":"59051","inReplyTo":"CAPig+cS_dXL-Q6NZtUJxDOL4-Q=MJv8fPEPAnEPuONaNF8-sCA@mail.gmail.com","subject":"Re: [PATCH] githooks: discuss Git operations in foreign repositories","fromName":"Preston Tunnell Wilson","fromEmail":"prestontunnellwilson@gmail.com","sentAt":"2023-01-09T02:54:09Z","receivedAt":"2023-01-09T02:54:24Z","isPatch":true,"sender":{"key":"prestontunnellwilson@gmail.com","avatar":null},"body":"> \"Best-practice\" is context\n> sensitive. It may be best-practice when a hook needs to invoke Git\n> commands in some other repository (or worktree), but clearing those\n> variables automatically would, in some situations, break the much more\n> common case of the hook invoking Git commands in the local repository\n> (or worktree). The fact that those environment variables may have been\n> set manually by the user or automatically by Git further complicates\n> the situation.\n\nThat makes sense, thank you for your answer!\n\n> So, no, I don't think this qualifies for the BUGS section of\n> git-wortkree, and mentioning this potential gotcha only in\n> git-worktree but not in any other hook-running command doesn't seem\n> ideal either. At present, the best place to discuss it seems to be\n> Documentation/githooks.txt, as this patch does.\n\nI agree the best place to put it is in Documentation/githooks.txt. I\nalso agree the BUGS section doesn't make sense, but I'm still\nwondering if we should call it out in git-worktree.txt in addition to\ngithooks.txt. When I ran into this issue, I tried to compare my setup\nto that of my coworkers. The difference was that I was using\ngit-worktree, they were not. git-worktree's documentation lists:\n\nWithin a linked worktree, $GIT_DIR is set to point to this private\ndirectory (e.g. /path/main/.git/worktrees/test-next in the example)\nand $GIT_COMMON_DIR is set to point back to the main worktree’s\n$GIT_DIR (e.g. /path/main/.git). These settings are made in a .git\nfile located at the top directory of the linked worktree.\n\nTo me, this is the \"other side of the coin\" of your patch. (Or maybe\none of the many other sides of the coin for commands that can run\ngit-hooks.) Mentioning a potential collision between git-hooks and\nthese variables being set could maybe go in the above snippet, maybe\nin parentheses. It took a lot of working backwards to narrow the issue\nto the interaction between git-worktree and git-hooks rather than the\npackage manager I was using or the tool the hook was calling. Putting\na note in the git-worktree documentation (in addition to the note in\ngit-hooks) might help out someone in the future, but I defer to your\njudgement. If it doesn't make sense, doesn't fit, or adding it here\nwould detract and make the documentation more confusing, I am happy to\nleave it out.\n\nAnd thank you for the administrivia!\n\nOn Sun, Jan 8, 2023 at 5:25 PM Eric Sunshine wrote:\n>\n> [administrivia: please reply inline rather than top-posting]\n>\n> On Sun, Jan 8, 2023 at 2:45 PM Preston Tunnell Wilson\n> wrote:\n> > Thank you for this wonderful remedy, Eric! I really appreciate the\n> > background context and how you framed the problem that I ran into.\n> >\n> > I have two questions:\n> > 1. Documentation is a great first step in addressing this, but I'm\n> > wondering if this should be automatic? If this is a best practice for\n> > hook authors, could `git` do this for them automatically when running\n> > hooks?\n>\n> For the general case, probably not. \"Best-practice\" is context\n> sensitive. It may be best-practice when a hook needs to invoke Git\n> commands in some other repository (or worktree), but clearing those\n> variables automatically would, in some situations, break the much more\n> common case of the hook invoking Git commands in the local repository\n> (or worktree). The fact that those environment variables may have been\n> set manually by the user or automatically by Git further complicates\n> the situation.\n>\n> > 2. Should we add something in the `git-worktree` documentation? In\n> > `Documentation/git-worktree.txt`, it mentions:\n> >\n> > > BUGS\n> > > ----\n> > > Multiple checkout in general is still experimental, and the support\n> > > for submodules is incomplete. ...\n> >\n> > Would it be helpful to plant a flag in the above documentation to\n> > point to this potential issue?\n>\n> As noted above, we can't really call this a bug. Git is behaving as\n> intended. Whether the user set the variables manually or whether some\n> parent Git process set them automatically, the child Git respects the\n> variables as it should rather than second-guessing about the user's\n> intentions, and possibly guessing incorrectly.\n>\n> So, no, I don't think this qualifies for the BUGS section of\n> git-wortkree, and mentioning this potential gotcha only in\n> git-worktree but not in any other hook-running command doesn't seem\n> ideal either. At present, the best place to discuss it seems to be\n> Documentation/githooks.txt, as this patch does. It may be possible to\n> argue that gitfaq.txt could talk about it, but considering that this\n> issue can manifest in many different ways (various error messages or\n> misbehaviors), it's difficult to come up with any text for the \"Q\"\n> which people would be likely to find when Googling. That's not to say\n> it shouldn't be mentioned elsewhere in the documentation, but rather\n> that I haven't come up with any better places than githooks.txt\n> itself.\n"},{"id":"469942","messageId":"xmqqwn5wuwvs.fsf@gitster.g","threadId":"59051","inReplyTo":"pull.1457.git.1673171924727.gitgitgadget@gmail.com","subject":"Re: [PATCH] githooks: discuss Git operations in foreign repositories","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2023-01-09T04:58:47Z","receivedAt":"2023-01-09T04:58:55Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"\"Eric Sunshine via GitGitGadget\" writes:\n\n> diff --git a/Documentation/githooks.txt b/Documentation/githooks.txt\n> index a16e62bc8c8..6e9a5420b7c 100644\n> --- a/Documentation/githooks.txt\n> +++ b/Documentation/githooks.txt\n> @@ -31,6 +31,17 @@ Hooks can get their arguments via the environment, command-line\n> arguments, and stdin. See the documentation for each hook below for\n> details.\n> \n> +If your hook needs to invoke Git commands in a foreign repository or in a\n> +different working tree of the same repository, then it should clear local Git\n> +environment variables, such as `GIT_DIR`, `GIT_WORK_TREE`, etc., which could\n> +interfere with Git operations in the foreign repository since those variables\n> +will be referencing the local repository and working tree. For example:\n> +\n> +------------\n> +local_desc=$(git describe)\n> +foreign_desc=$(unset $(git rev-parse --local-env-vars); git -C ../foreign-repo describe)\n> +------------\n> +\n\nIt is an excellent idea to add the above, but\n\n * I think adding it one paragraph earlier may make it fit better.\n\n * The paragraph, after which the above gets inserted, can use a bit\n of enhancement.\n\nThat is, something like this?\n\n\n\n Documentation/githooks.txt | 15 ++++++++++++++-\n 1 file changed, 14 insertions(+), 1 deletion(-)\n\ndiff --git c/Documentation/githooks.txt w/Documentation/githooks.txt\nindex a16e62bc8c..f3d0404164 100644\n--- c/Documentation/githooks.txt\n+++ w/Documentation/githooks.txt\n@@ -25,7 +25,20 @@ Before Git invokes a hook, it changes its working directory to either\n $GIT_DIR in a bare repository or the root of the working tree in a non-bare\n repository. An exception are hooks triggered during a push ('pre-receive',\n 'update', 'post-receive', 'post-update', 'push-to-checkout') which are always\n-executed in $GIT_DIR.\n+executed in $GIT_DIR. Environment variables like GIT_DIR and GIT_WORK_TREE\n+are exported so that the hook can easily learn which repository it is\n+working with.\n+\n+If your hook needs to invoke Git commands in a foreign repository or in a\n+different working tree of the same repository, then it should clear local Git\n+environment variables, such as `GIT_DIR`, `GIT_WORK_TREE`, etc., which could\n+interfere with Git operations in the foreign repository since those variables\n+will be referencing the local repository and working tree. For example:\n+\n+------------\n+local_desc=$(git describe)\n+foreign_desc=$(unset $(git rev-parse --local-env-vars); git -C ../foreign-repo describe)\n+------------\n \n Hooks can get their arguments via the environment, command-line\n arguments, and stdin. See the documentation for each hook below for\n"},{"id":"469943","messageId":"CAPig+cRMdJy9FdL1_rwuMKcA3F3p4g3RF+0mkh12pqN0aUoUiw@mail.gmail.com","threadId":"59051","inReplyTo":"xmqqwn5wuwvs.fsf@gitster.g","subject":"Re: [PATCH] githooks: discuss Git operations in foreign repositories","fromName":"Eric Sunshine","fromEmail":"sunshine@sunshineco.com","sentAt":"2023-01-09T05:03:37Z","receivedAt":"2023-01-09T05:03:55Z","isPatch":true,"sender":{"key":"sunshine@sunshineco.com","avatar":"https://avatars.githubusercontent.com/u/163641?v=4"},"body":"On Sun, Jan 8, 2023 at 11:58 PM Junio C Hamano wrote:\n> \"Eric Sunshine via GitGitGadget\" writes:\n> > +If your hook needs to invoke Git commands in a foreign repository or in a\n> > +different working tree of the same repository, then it should clear local Git\n> > +environment variables, such as `GIT_DIR`, `GIT_WORK_TREE`, etc., which could\n> > +interfere with Git operations in the foreign repository since those variables\n> > +will be referencing the local repository and working tree. For example:\n> > +\n> > +------------\n> > +local_desc=$(git describe)\n> > +foreign_desc=$(unset $(git rev-parse --local-env-vars); git -C ../foreign-repo describe)\n> > +------------\n>\n> It is an excellent idea to add the above, but\n>\n> * I think adding it one paragraph earlier may make it fit better.\n\nThat was my initial choice, as well, and is where I initially inserted\nit, but moved it down a paragraph at the last moment. I'm happy to\nmove it back up again.\n\n> * The paragraph, after which the above gets inserted, can use a bit\n> of enhancement.\n>\n> That is, something like this?\n>\n> repository. An exception are hooks triggered during a push ('pre-receive',\n> 'update', 'post-receive', 'post-update', 'push-to-checkout') which are always\n> -executed in $GIT_DIR.\n> +executed in $GIT_DIR. Environment variables like GIT_DIR and GIT_WORK_TREE\n> +are exported so that the hook can easily learn which repository it is\n> +working with.\n\nYes, good idea, although I might phrase it something like:\n\n Environment variables such as ... are exported so that Git\n commands run by the hook can correctly locate the repository.\n"},{"id":"469945","messageId":"xmqqo7r8uut7.fsf@gitster.g","threadId":"59051","inReplyTo":"CAPig+cRMdJy9FdL1_rwuMKcA3F3p4g3RF+0mkh12pqN0aUoUiw@mail.gmail.com","subject":"Re: [PATCH] githooks: discuss Git operations in foreign repositories","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2023-01-09T05:43:32Z","receivedAt":"2023-01-09T05:43:39Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Eric Sunshine writes:\n\n> Yes, good idea, although I might phrase it something like:\n>\n> Environment variables such as ... are exported so that Git\n> commands run by the hook can correctly locate the repository.\n\nExcellent. Thanks.\n"},{"id":"470003","messageId":"pull.1457.v2.git.1673293508399.gitgitgadget@gmail.com","threadId":"59051","inReplyTo":"pull.1457.git.1673171924727.gitgitgadget@gmail.com","subject":"[PATCH v2] githooks: discuss Git operations in foreign repositories","fromName":"Eric Sunshine via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2023-01-09T19:45:08Z","receivedAt":"2023-01-09T19:48:11Z","isPatch":true,"sender":{"key":"sunshine@sunshineco.com","avatar":"https://avatars.githubusercontent.com/u/163641?v=4"},"body":"From: Eric Sunshine \n\nHook authors are periodically caught off-guard by difficult-to-diagnose\nerrors when their hook invokes Git commands in a repository other than\nthe local one. In particular, Git environment variables, such as GIT_DIR\nand GIT_WORK_TREE, which reference the local repository cause the Git\ncommands to operate on the local repository rather than on the\nrepository which the author intended. This is true whether the\nenvironment variables have been set manually by the user or\nautomatically by Git itself. The same problem crops up when a hook\ninvokes Git commands in a different worktree of the same repository, as\nwell.\n\nRecommended best-practice[1,2,3,4,5,6] for avoiding this problem is for\nthe hook to ensure that Git variables are unset before invoking Git\ncommands in foreign repositories or other worktrees:\n\n unset $(git rev-parse --local-env-vars)\n\nHowever, this advice is not documented anywhere. Rectify this\nshortcoming by mentioning it in githooks.txt documentation.\n\n[1]: https://lore.kernel.org/git/YFuHd1MMlJAvtdzb@coredump.intra.peff.net/\n[2]: https://lore.kernel.org/git/20200228190218.GC1408759@coredump.intra.peff.net/\n[3]: https://lore.kernel.org/git/20190516221702.GA11784@sigill.intra.peff.net/\n[4]: https://lore.kernel.org/git/20190422162127.GC9680@sigill.intra.peff.net/\n[5]: https://lore.kernel.org/git/20180716183942.GB22298@sigill.intra.peff.net/\n[6]: https://lore.kernel.org/git/20150203163235.GA9325@peff.net/\n\nSigned-off-by: Eric Sunshine \n---\n githooks: discuss Git operations in foreign repositories\n \n This is a re-roll of [1]. It incorporates a refined version of Junio's\n suggested improvement[2].\n \n [1]\n https://lore.kernel.org/git/pull.1457.git.1673171924727.gitgitgadget@gmail.com/\n [2] https://lore.kernel.org/git/xmqqwn5wuwvs.fsf@gitster.g/\n\nPublished-As: https://github.com/gitgitgadget/git/releases/tag/pr-1457%2Fsunshineco%2Fhookenv-v2\nFetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-1457/sunshineco/hookenv-v2\nPull-Request: https://github.com/gitgitgadget/git/pull/1457\n\nRange-diff vs v1:\n\n 1: b9a2e23359a ! 1: d58694a4137 githooks: discuss Git operations in foreign repositories\n @@ Commit message\n Signed-off-by: Eric Sunshine \n \n ## Documentation/githooks.txt ##\n -@@ Documentation/githooks.txt: Hooks can get their arguments via the environment, command-line\n - arguments, and stdin. See the documentation for each hook below for\n - details.\n +@@ Documentation/githooks.txt: repository. An exception are hooks triggered during a push ('pre-receive',\n + 'update', 'post-receive', 'post-update', 'push-to-checkout') which are always\n + executed in $GIT_DIR.\n \n -+If your hook needs to invoke Git commands in a foreign repository or in a\n -+different working tree of the same repository, then it should clear local Git\n -+environment variables, such as `GIT_DIR`, `GIT_WORK_TREE`, etc., which could\n -+interfere with Git operations in the foreign repository since those variables\n -+will be referencing the local repository and working tree. For example:\n ++Environment variables, such as `GIT_DIR`, `GIT_WORK_TREE`, etc., are exported\n ++so that Git commands run by the hook can correctly locate the repository. If\n ++your hook needs to invoke Git commands in a foreign repository or in a\n ++different working tree of the same repository, then it should clear these\n ++environment variables so they do not interfere with Git operations at the\n ++foreign location. For example:\n +\n +------------\n +local_desc=$(git describe)\n +foreign_desc=$(unset $(git rev-parse --local-env-vars); git -C ../foreign-repo describe)\n +------------\n +\n - `git init` may copy hooks to the new repository, depending on its\n - configuration. See the \"TEMPLATE DIRECTORY\" section in\n - linkgit:git-init[1] for details. When the rest of this document refers\n + Hooks can get their arguments via the environment, command-line\n + arguments, and stdin. See the documentation for each hook below for\n + details.\n\n\n Documentation/githooks.txt | 12 ++++++++++++\n 1 file changed, 12 insertions(+)\n\ndiff --git a/Documentation/githooks.txt b/Documentation/githooks.txt\nindex a16e62bc8c8..62908602e7b 100644\n--- a/Documentation/githooks.txt\n+++ b/Documentation/githooks.txt\n@@ -27,6 +27,18 @@ repository. An exception are hooks triggered during a push ('pre-receive',\n 'update', 'post-receive', 'post-update', 'push-to-checkout') which are always\n executed in $GIT_DIR.\n \n+Environment variables, such as `GIT_DIR`, `GIT_WORK_TREE`, etc., are exported\n+so that Git commands run by the hook can correctly locate the repository. If\n+your hook needs to invoke Git commands in a foreign repository or in a\n+different working tree of the same repository, then it should clear these\n+environment variables so they do not interfere with Git operations at the\n+foreign location. For example:\n+\n+------------\n+local_desc=$(git describe)\n+foreign_desc=$(unset $(git rev-parse --local-env-vars); git -C ../foreign-repo describe)\n+------------\n+\n Hooks can get their arguments via the environment, command-line\n arguments, and stdin. See the documentation for each hook below for\n details.\n\nbase-commit: a38d39a4c50d1275833aba54c4dbdfce9e2e9ca1\n-- \ngitgitgadget\n"},{"id":"470008","messageId":"CAC-j02NxVbqu+TQvNgbwFtxmXTZBAJtG60BR+yfXcT9ubSn-jA@mail.gmail.com","threadId":"59051","inReplyTo":"pull.1457.v2.git.1673293508399.gitgitgadget@gmail.com","subject":"Re: [PATCH v2] githooks: discuss Git operations in foreign repositories","fromName":"Preston Tunnell Wilson","fromEmail":"prestontunnellwilson@gmail.com","sentAt":"2023-01-09T20:12:07Z","receivedAt":"2023-01-09T20:13:18Z","isPatch":true,"sender":{"key":"prestontunnellwilson@gmail.com","avatar":null},"body":"On Mon, Jan 9, 2023 at 1:45 PM Eric Sunshine via GitGitGadget\n wrote:\n> Documentation/githooks.txt | 12 ++++++++++++\n> 1 file changed, 12 insertions(+)\n>\n> diff --git a/Documentation/githooks.txt b/Documentation/githooks.txt\n> index a16e62bc8c8..62908602e7b 100644\n> --- a/Documentation/githooks.txt\n> +++ b/Documentation/githooks.txt\n> @@ -27,6 +27,18 @@ repository. An exception are hooks triggered during a push ('pre-receive',\n> 'update', 'post-receive', 'post-update', 'push-to-checkout') which are always\n> executed in $GIT_DIR.\n>\n> +Environment variables, such as `GIT_DIR`, `GIT_WORK_TREE`, etc., are exported\n> +so that Git commands run by the hook can correctly locate the repository. If\n> +your hook needs to invoke Git commands in a foreign repository or in a\n> +different working tree of the same repository, then it should clear these\n> +environment variables so they do not interfere with Git operations at the\n> +foreign location. For example:\n> +\n> +------------\n> +local_desc=$(git describe)\n> +foreign_desc=$(unset $(git rev-parse --local-env-vars); git -C ../foreign-repo describe)\n> +------------\n\nThis looks good to me! Thank you! (And I'm sorry about top-posting\n*again*! I think I have the hang of it now.)\n"},{"id":"470014","messageId":"CAPig+cQo3sEJHcQkS6O03Hw5+8NzrMaHwqcR5WwjmVK2s0bcyw@mail.gmail.com","threadId":"59051","inReplyTo":"CAC-j02MV+Gv0D8-fpCOu7JGUimxPLF+OP1dy7bxfs7ArX05BYg@mail.gmail.com","subject":"Re: [PATCH] githooks: discuss Git operations in foreign repositories","fromName":"Eric Sunshine","fromEmail":"sunshine@sunshineco.com","sentAt":"2023-01-09T21:47:12Z","receivedAt":"2023-01-09T21:47:45Z","isPatch":true,"sender":{"key":"sunshine@sunshineco.com","avatar":"https://avatars.githubusercontent.com/u/163641?v=4"},"body":"On Sun, Jan 8, 2023 at 9:54 PM Preston Tunnell Wilson\n wrote:\n> > So, no, I don't think this qualifies for the BUGS section of\n> > git-wortkree, and mentioning this potential gotcha only in\n> > git-worktree but not in any other hook-running command doesn't seem\n> > ideal either. At present, the best place to discuss it seems to be\n> > Documentation/githooks.txt, as this patch does.\n>\n> I agree the best place to put it is in Documentation/githooks.txt. I\n> also agree the BUGS section doesn't make sense, but I'm still\n> wondering if we should call it out in git-worktree.txt in addition to\n> githooks.txt. When I ran into this issue, I tried to compare my setup\n> to that of my coworkers. The difference was that I was using\n> git-worktree, they were not. git-worktree's documentation lists:\n>\n> Within a linked worktree, $GIT_DIR is set to point to this private\n> directory (e.g. /path/main/.git/worktrees/test-next in the example)\n> and $GIT_COMMON_DIR is set to point back to the main worktree’s\n> $GIT_DIR (e.g. /path/main/.git). These settings are made in a .git\n> file located at the top directory of the linked worktree.\n>\n> To me, this is the \"other side of the coin\" of your patch. (Or maybe\n> one of the many other sides of the coin for commands that can run\n> git-hooks.) Mentioning a potential collision between git-hooks and\n> these variables being set could maybe go in the above snippet, maybe\n> in parentheses. It took a lot of working backwards to narrow the issue\n> to the interaction between git-worktree and git-hooks rather than the\n> package manager I was using or the tool the hook was calling. Putting\n> a note in the git-worktree documentation (in addition to the note in\n> git-hooks) might help out someone in the future, but I defer to your\n> judgement. If it doesn't make sense, doesn't fit, or adding it here\n> would detract and make the documentation more confusing, I am happy to\n> leave it out.\n\nI understand your concern, and can relate to the amount of effort it\ntook to narrow down the problem. Nevertheless, even though you\nencountered this problem in relation to git-worktree, it's a more\ngeneral issue which can manifest in other situations. As such, I can't\nthink of a good way to discuss the issue in the git-worktree\ndocumentation that wouldn't feel out of place and make the\ndocumentation more confusing.\n\nI'm not necessarily opposed to someone else giving it a shot if it can\nbe done in a way which doesn't feel out of place and doesn't confuse\ngit-worktree documentation further (especially for those new to the\ndocumentation). I just don't know how to do so myself.\n"},{"id":"470099","messageId":"Y78Hgj4jKga7vNo7@coredump.intra.peff.net","threadId":"59051","inReplyTo":"pull.1457.v2.git.1673293508399.gitgitgadget@gmail.com","subject":"Re: [PATCH v2] githooks: discuss Git operations in foreign repositories","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2023-01-11T19:01:22Z","receivedAt":"2023-01-11T19:01:35Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Mon, Jan 09, 2023 at 07:45:08PM +0000, Eric Sunshine via GitGitGadget wrote:\n\n> Recommended best-practice[1,2,3,4,5,6] for avoiding this problem is for\n> the hook to ensure that Git variables are unset before invoking Git\n> commands in foreign repositories or other worktrees:\n> \n> unset $(git rev-parse --local-env-vars)\n> \n> However, this advice is not documented anywhere. Rectify this\n> shortcoming by mentioning it in githooks.txt documentation.\n> \n> [1]: https://lore.kernel.org/git/YFuHd1MMlJAvtdzb@coredump.intra.peff.net/\n> [2]: https://lore.kernel.org/git/20200228190218.GC1408759@coredump.intra.peff.net/\n> [3]: https://lore.kernel.org/git/20190516221702.GA11784@sigill.intra.peff.net/\n> [4]: https://lore.kernel.org/git/20190422162127.GC9680@sigill.intra.peff.net/\n> [5]: https://lore.kernel.org/git/20180716183942.GB22298@sigill.intra.peff.net/\n> [6]: https://lore.kernel.org/git/20150203163235.GA9325@peff.net/\n\nBoy, I'm like a broken record.\n\nThe patch here looks good to me. The problem is wider than just hooks,\nbut it seems like that's going to be a common place for people to get\ncaught by it. So certainly this is going in the right direction.\n\nThe other place I've run into it is writing a script meant to be run as\nan external command. E.g., running this:\n\n git --git-dir=/some/path my-external-command\n\nmeans that \"my-external-command\" is going to have $GIT_DIR set. If it\nwants to operate on another repository it needs to take care to clear\nthat from the environment.\n\n-Peff\n"}]}