{"thread":{"id":"51094","subject":"Running 'git worktree add' in 'pre-commit' hook","startedAt":"2019-05-14T14:52:59Z","lastAt":"2019-05-17T01:19:45Z","messageCount":17,"participants":["Cosmin Polifronie","Bryan Turner","Duy Nguyen","Eric Sunshine","Jeff King"],"isPatch":false,"patchVersion":null,"patchTotal":null},"messages":[{"id":"375542","messageId":"CAJFQqN+Z9eX6onaj8vVSqpvf-nOC7-Y0Un4NLUie6x6bGfmvZA@mail.gmail.com","threadId":"51094","inReplyTo":null,"subject":"Running 'git worktree add' in 'pre-commit' hook","fromName":"Cosmin Polifronie","fromEmail":"oppturbv@gmail.com","sentAt":"2019-05-14T14:52:45Z","receivedAt":"2019-05-14T14:52:59Z","isPatch":false,"sender":{"key":"oppturbv@gmail.com","avatar":null},"body":"Hello! I am trying to run 'git worktree add <path> HEAD' in the\n'pre-commit' hook, more specifically in a Python script that is being\ncalled from the hook. When doing so, I am greeted with the following\nerror:\n\nOn Windows 10:\nPreparing worktree (detached HEAD cbfef18)\nfatal: Unable to create 'C:/Users/meh/Desktop/abc/.git/index.lock': No\nsuch file or directory\n\nOn Arch Linux:\nPreparing worktree (detached HEAD cbfef18)\nfatal: Unable to create '/home/cosmin/Downloads/abc/.git/index.lock':\nNot a directory\n\nIs it forbidden to call this command from a hook? If yes, what kind of\nalternatives do I have? I need to make a copy of the repo in its HEAD\nstate, process it and then decide if I will pass the current commit or\nnot.\n\nThanks! :)\n"},{"id":"375574","messageId":"CAGyf7-GtnCqfqPWVpBAELB4xAE6syn7+DfKXgfhL0UoMfvbn4A@mail.gmail.com","threadId":"51094","inReplyTo":"CAJFQqN+Z9eX6onaj8vVSqpvf-nOC7-Y0Un4NLUie6x6bGfmvZA@mail.gmail.com","subject":"Re: Running 'git worktree add' in 'pre-commit' hook","fromName":"Bryan Turner","fromEmail":"bturner@atlassian.com","sentAt":"2019-05-15T00:46:43Z","receivedAt":"2019-05-15T00:46:57Z","isPatch":false,"sender":{"key":"bturner@atlassian.com","avatar":"https://gravatar.com/avatar/16bcf3167981c1ef7c804e502642366d888a35b0d0b0a4ca01fdc442aa1acb1e?d=mp&s=160"},"body":"On Tue, May 14, 2019 at 7:53 AM Cosmin Polifronie <oppturbv@gmail.com> wrote:\n>\n> Hello! I am trying to run 'git worktree add <path> HEAD' in the\n> 'pre-commit' hook, more specifically in a Python script that is being\n> called from the hook. When doing so, I am greeted with the following\n> error:\n>\n> On Windows 10:\n> Preparing worktree (detached HEAD cbfef18)\n> fatal: Unable to create 'C:/Users/meh/Desktop/abc/.git/index.lock': No\n> such file or directory\n>\n> On Arch Linux:\n> Preparing worktree (detached HEAD cbfef18)\n> fatal: Unable to create '/home/cosmin/Downloads/abc/.git/index.lock':\n> Not a directory\n>\n> Is it forbidden to call this command from a hook? If yes, what kind of\n> alternatives do I have? I need to make a copy of the repo in its HEAD\n> state, process it and then decide if I will pass the current commit or\n> not.\n\nI can't speak to whether `git worktree add` should succeed or fail\ninside a `pre-commit` hook, but...\n\nWhy do you need a new work tree, versus whatever working copy you're\nrunning `git commit` in? Is there a reason whatever validation needs\nto be done can't be done in the existing working copy? `HEAD` is the\n_previous, existing commit_, not the new, currently-being-created\ncommit, so your validation in the new work tree, if you actually\nmanaged to create one, would be applied to the _latest existing\ncommit_, not the new changes you're trying to commit. Even trying to\ncopy the changes over wouldn't necessarily result in the same state,\nbecause there may be unstaged changes.\n\nWhat type of validation are you trying to do? I think the failure\nyou're running into is an alarm bell indicating what you're trying to\ndo may not make sense. However, without any insight into what \"process\nit and then decide if I will pass the current commit or not\" actually\nlooks like, it's hard to offer you much help.\n\nBryan\n\n>\n> Thanks! :)\n"},{"id":"375627","messageId":"CAJFQqNJsnkdwqk0+kCp6j5aL9BaXd=-JaXC9K5kSBSed+=V3xg@mail.gmail.com","threadId":"51094","inReplyTo":"CAGyf7-GtnCqfqPWVpBAELB4xAE6syn7+DfKXgfhL0UoMfvbn4A@mail.gmail.com","subject":"Re: Running 'git worktree add' in 'pre-commit' hook","fromName":"Cosmin Polifronie","fromEmail":"oppturbv@gmail.com","sentAt":"2019-05-15T12:38:44Z","receivedAt":"2019-05-15T12:38:58Z","isPatch":false,"sender":{"key":"oppturbv@gmail.com","avatar":null},"body":"Hello Bryan,\n\nMy project contains a Gradle build that is dependent on a\nconfiguration found in a file called build.gradle. What I need to do\nis run Gradle with a proper build.gradle file, meaning: if a new\nbuild.gradle is staged, I will use that for my Gradle build, if not I\nwill use the latest approved commit's one (as in a commit that has\nalready passed the pre-commit check). The problem is that build.gradle\ncan be modified and not be staged, and Gradle cannot know that, it\nwill use whatever is available on disk. I want to avoid that.\n\nThis is why I needed a copy of my current repo with the file versions\nalready approved (already staged at a previous date). This is why I am\nusing 'git worktree add'.\n\nNow, I have found another solution using stashes, and those might\nwork, but I am still curious about this situation.\n\nOn Wed, May 15, 2019 at 3:46 AM Bryan Turner <bturner@atlassian.com> wrote:\n>\n> On Tue, May 14, 2019 at 7:53 AM Cosmin Polifronie <oppturbv@gmail.com> wrote:\n> >\n> > Hello! I am trying to run 'git worktree add <path> HEAD' in the\n> > 'pre-commit' hook, more specifically in a Python script that is being\n> > called from the hook. When doing so, I am greeted with the following\n> > error:\n> >\n> > On Windows 10:\n> > Preparing worktree (detached HEAD cbfef18)\n> > fatal: Unable to create 'C:/Users/meh/Desktop/abc/.git/index.lock': No\n> > such file or directory\n> >\n> > On Arch Linux:\n> > Preparing worktree (detached HEAD cbfef18)\n> > fatal: Unable to create '/home/cosmin/Downloads/abc/.git/index.lock':\n> > Not a directory\n> >\n> > Is it forbidden to call this command from a hook? If yes, what kind of\n> > alternatives do I have? I need to make a copy of the repo in its HEAD\n> > state, process it and then decide if I will pass the current commit or\n> > not.\n>\n> I can't speak to whether `git worktree add` should succeed or fail\n> inside a `pre-commit` hook, but...\n>\n> Why do you need a new work tree, versus whatever working copy you're\n> running `git commit` in? Is there a reason whatever validation needs\n> to be done can't be done in the existing working copy? `HEAD` is the\n> _previous, existing commit_, not the new, currently-being-created\n> commit, so your validation in the new work tree, if you actually\n> managed to create one, would be applied to the _latest existing\n> commit_, not the new changes you're trying to commit. Even trying to\n> copy the changes over wouldn't necessarily result in the same state,\n> because there may be unstaged changes.\n>\n> What type of validation are you trying to do? I think the failure\n> you're running into is an alarm bell indicating what you're trying to\n> do may not make sense. However, without any insight into what \"process\n> it and then decide if I will pass the current commit or not\" actually\n> looks like, it's hard to offer you much help.\n>\n> Bryan\n>\n> >\n> > Thanks! :)\n"},{"id":"375715","messageId":"CACsJy8DnkjuZD-9pbhAsFo16jHKt8U831LLxb3-nCQP5_FOmtA@mail.gmail.com","threadId":"51094","inReplyTo":"CAJFQqN+Z9eX6onaj8vVSqpvf-nOC7-Y0Un4NLUie6x6bGfmvZA@mail.gmail.com","subject":"Re: Running 'git worktree add' in 'pre-commit' hook","fromName":"Duy Nguyen","fromEmail":"pclouds@gmail.com","sentAt":"2019-05-16T11:25:24Z","receivedAt":"2019-05-16T11:25:52Z","isPatch":false,"sender":{"key":"pclouds@gmail.com","avatar":"https://avatars.githubusercontent.com/u/720?v=4"},"body":"On Tue, May 14, 2019 at 9:53 PM Cosmin Polifronie <oppturbv@gmail.com> wrote:\n>\n> Hello! I am trying to run 'git worktree add <path> HEAD' in the\n> 'pre-commit' hook, more specifically in a Python script that is being\n> called from the hook. When doing so, I am greeted with the following\n> error:\n>\n> On Windows 10:\n> Preparing worktree (detached HEAD cbfef18)\n> fatal: Unable to create 'C:/Users/meh/Desktop/abc/.git/index.lock': No\n> such file or directory\n>\n> On Arch Linux:\n> Preparing worktree (detached HEAD cbfef18)\n> fatal: Unable to create '/home/cosmin/Downloads/abc/.git/index.lock':\n> Not a directory\n>\n> Is it forbidden to call this command from a hook?\n\npre-commit hook sets GIT_INDEX_FILE to this \"index.lock\" so you have\nthe latest index content (which is not the same as from\n$GIT_DIR/index). This variable will interfere with any commands that\nwork on a different worktree.\n\nSo you probably can still make it work by backing up $GIT_INDEX_FILE\n(in case you need it), then unset it before you use \"git worktree\" (or\ncd to it if you keep a permanent separate worktree for pre-commit\nactivities). To make sure you don't have similar problems, you\nprobably should do \"env | grep GIT\" from the hook and see if any other\nvariables are set.\n\n> If yes, what kind of\n> alternatives do I have? I need to make a copy of the repo in its HEAD\n> state, process it and then decide if I will pass the current commit or\n> not.\n>\n> Thanks! :)\n\n\n\n-- \nDuy\n"},{"id":"375717","messageId":"CAPig+cTQrF1W4LEgOiEoKn4f69OEygFat=4V8aTzf0Rxom2hEw@mail.gmail.com","threadId":"51094","inReplyTo":"CACsJy8DnkjuZD-9pbhAsFo16jHKt8U831LLxb3-nCQP5_FOmtA@mail.gmail.com","subject":"Re: Running 'git worktree add' in 'pre-commit' hook","fromName":"Eric Sunshine","fromEmail":"sunshine@sunshineco.com","sentAt":"2019-05-16T11:33:02Z","receivedAt":"2019-05-16T11:33:16Z","isPatch":false,"sender":{"key":"sunshine@sunshineco.com","avatar":"https://avatars.githubusercontent.com/u/163641?v=4"},"body":"On Thu, May 16, 2019 at 7:25 AM Duy Nguyen <pclouds@gmail.com> wrote:\n> pre-commit hook sets GIT_INDEX_FILE to this \"index.lock\" so you have\n> the latest index content (which is not the same as from\n> $GIT_DIR/index). This variable will interfere with any commands that\n> work on a different worktree.\n>\n> So you probably can still make it work by backing up $GIT_INDEX_FILE\n> (in case you need it), then unset it before you use \"git worktree\" (or\n> cd to it if you keep a permanent separate worktree for pre-commit\n> activities). To make sure you don't have similar problems, you\n> probably should do \"env | grep GIT\" from the hook and see if any other\n> variables are set.\n\nI researched this also and concluded that it's a bug in git-commit.\nYou run afoul of it in other situations, as well. For instance, say\nyou have your index file in a non-standard location:\n\n    $ export GIT_INDEX_FILE=../storage/index\n    $ git worktree add --detach other\n    Preparing worktree (detached HEAD c9156d2)\n    fatal: Unable to create '/.../foo/other/../storage/index.lock': No\nsuch file or directory\n    $\n\nI think the correct fix is for git-commit to assign GIT_INDEX_FILE\nwith the absolute path to the index file, not a relative path.\n"},{"id":"375718","messageId":"CAPig+cS1iypB9oZ+O=KsJ7CHyWUEOKzr8djRthrXMdC+uNHVKA@mail.gmail.com","threadId":"51094","inReplyTo":"CAPig+cTQrF1W4LEgOiEoKn4f69OEygFat=4V8aTzf0Rxom2hEw@mail.gmail.com","subject":"Re: Running 'git worktree add' in 'pre-commit' hook","fromName":"Eric Sunshine","fromEmail":"sunshine@sunshineco.com","sentAt":"2019-05-16T11:39:04Z","receivedAt":"2019-05-16T11:39:18Z","isPatch":false,"sender":{"key":"sunshine@sunshineco.com","avatar":"https://avatars.githubusercontent.com/u/163641?v=4"},"body":"On Thu, May 16, 2019 at 7:33 AM Eric Sunshine <sunshine@sunshineco.com> wrote:\n> I researched this also and concluded that it's a bug in git-commit.\n> You run afoul of it in other situations, as well. For instance, say\n> you have your index file in a non-standard location:\n>\n>     $ export GIT_INDEX_FILE=../storage/index\n>     $ git worktree add --detach other\n>     Preparing worktree (detached HEAD c9156d2)\n>     fatal: Unable to create '/.../foo/other/../storage/index.lock': No\n> such file or directory\n>     $\n>\n> I think the correct fix is for git-commit to assign GIT_INDEX_FILE\n> with the absolute path to the index file, not a relative path.\n\nThinking more clearly on it, a better fix might be for git-worktree to\ndeal with this itself, converting such a path to absolute before\ncd'ing to the new worktree directory (or something -- I don't have the\ncode in front of me to provide a more concrete suggestion).\n"},{"id":"375719","messageId":"CACsJy8BXxVggL7CWA5Q0d7U4B+encfZ85NshxLJ=VvHhf_LUYg@mail.gmail.com","threadId":"51094","inReplyTo":"CAPig+cTQrF1W4LEgOiEoKn4f69OEygFat=4V8aTzf0Rxom2hEw@mail.gmail.com","subject":"Re: Running 'git worktree add' in 'pre-commit' hook","fromName":"Duy Nguyen","fromEmail":"pclouds@gmail.com","sentAt":"2019-05-16T11:41:51Z","receivedAt":"2019-05-16T11:42:21Z","isPatch":false,"sender":{"key":"pclouds@gmail.com","avatar":"https://avatars.githubusercontent.com/u/720?v=4"},"body":"On Thu, May 16, 2019 at 6:33 PM Eric Sunshine <sunshine@sunshineco.com> wrote:\n> I researched this also and concluded that it's a bug in git-commit.\n> You run afoul of it in other situations, as well. For instance, say\n> you have your index file in a non-standard location:\n>\n>     $ export GIT_INDEX_FILE=../storage/index\n>     $ git worktree add --detach other\n>     Preparing worktree (detached HEAD c9156d2)\n>     fatal: Unable to create '/.../foo/other/../storage/index.lock': No\n> such file or directory\n>     $\n>\n> I think the correct fix is for git-commit to assign GIT_INDEX_FILE\n> with the absolute path to the index file, not a relative path.\n\nOh if it's relative $GIT_INDEX_FILE then I think its our environment.c\ncode that does not work so well when we chdir() away. I vaguely recall\nsomething about this when discussing Jeff's chdir-notify series. But\nit turns out that's about chdir() in run-command.c [1]. But the idea\nis still the same, all variables are supposed to be relative to $CWD.\nWhenever you move $CWD you should reparent all of them, including\n$GIT_INDEX_FILE.\n\n[1] https://public-inbox.org/git/CACsJy8CdqpNOw+zdMyugX-902Z=gLNij5_xcmE4jGLRBTqiO1g@mail.gmail.com/\n-- \nDuy\n"},{"id":"375720","messageId":"CACsJy8AgzSZ_Gj9JW2yTdgkGQNCMMLy8i+pH_+A93+V95anQWQ@mail.gmail.com","threadId":"51094","inReplyTo":"CAPig+cS1iypB9oZ+O=KsJ7CHyWUEOKzr8djRthrXMdC+uNHVKA@mail.gmail.com","subject":"Re: Running 'git worktree add' in 'pre-commit' hook","fromName":"Duy Nguyen","fromEmail":"pclouds@gmail.com","sentAt":"2019-05-16T11:44:38Z","receivedAt":"2019-05-16T11:45:07Z","isPatch":false,"sender":{"key":"pclouds@gmail.com","avatar":"https://avatars.githubusercontent.com/u/720?v=4"},"body":"On Thu, May 16, 2019 at 6:39 PM Eric Sunshine <sunshine@sunshineco.com> wrote:\n>\n> On Thu, May 16, 2019 at 7:33 AM Eric Sunshine <sunshine@sunshineco.com> wrote:\n> > I researched this also and concluded that it's a bug in git-commit.\n> > You run afoul of it in other situations, as well. For instance, say\n> > you have your index file in a non-standard location:\n> >\n> >     $ export GIT_INDEX_FILE=../storage/index\n> >     $ git worktree add --detach other\n> >     Preparing worktree (detached HEAD c9156d2)\n> >     fatal: Unable to create '/.../foo/other/../storage/index.lock': No\n> > such file or directory\n> >     $\n> >\n> > I think the correct fix is for git-commit to assign GIT_INDEX_FILE\n> > with the absolute path to the index file, not a relative path.\n>\n> Thinking more clearly on it, a better fix might be for git-worktree to\n> deal with this itself, converting such a path to absolute before\n> cd'ing to the new worktree directory (or something -- I don't have the\n> code in front of me to provide a more concrete suggestion).\n\nOur mails crossed. But yeah it's basically the same thing.\n\nAlthough I still doubt a good $GIT_INDEX_FILE would fix the problem.\nEven if you manage to create a new worktree, when you cd there and\nstart using it, this $GIT_INDEX_FILE will still mess things up (unless\nyou do mean to ignore the worktree's index file and update one\nprovided by pre-commit hook).\n-- \nDuy\n"},{"id":"375721","messageId":"CAPig+cQ5T1yj0t1gehXYKN7=rEJ15feOUEJEtAVK565=VwdUrg@mail.gmail.com","threadId":"51094","inReplyTo":"CACsJy8DnkjuZD-9pbhAsFo16jHKt8U831LLxb3-nCQP5_FOmtA@mail.gmail.com","subject":"Re: Running 'git worktree add' in 'pre-commit' hook","fromName":"Eric Sunshine","fromEmail":"sunshine@sunshineco.com","sentAt":"2019-05-16T11:59:12Z","receivedAt":"2019-05-16T11:59:26Z","isPatch":false,"sender":{"key":"sunshine@sunshineco.com","avatar":"https://avatars.githubusercontent.com/u/163641?v=4"},"body":"On Thu, May 16, 2019 at 7:25 AM Duy Nguyen <pclouds@gmail.com> wrote:\n> pre-commit hook sets GIT_INDEX_FILE to this \"index.lock\" so you have\n> the latest index content (which is not the same as from\n> $GIT_DIR/index). This variable will interfere with any commands that\n> work on a different worktree.\n\nI think that this is not quite accurate. The problem isn't that\ngit-commit is pointing GIT_INDEX_FILE at a temporary index; that works\nfine. The problem is when it is using the normal .git/index file, in\nwhich case the value it assigns to GIT_INDEX_FILE is relative.\n\nFor instance, with pre-commit hook:\n\n    #!/bin/sh\n    echo \"GIT_INDEX_FILE=$GIT_INDEX_FILE\"\n    git worktree add --detach other\n\n\"git-worktree add\" with a normal git-commit breaks due to the relative\nlocation specified by GIT_INDEX_FILE:\n\n    $ git add file\n    $ git commit -m foo\n    GIT_INDEX_FILE=.git/index\n    Preparing worktree (detached HEAD 0857fef)\n    fatal: Unable to create '/.../other/.git/index.lock': Not a directory\n    $\n\nwhereas, the \"git commit <file>\" form succeeds just fine because it\nassigns an absolute path for the temporary index:\n\n    $ git commit -m foo file\n    GIT_INDEX_FILE=/.../.git/next-index-23675.lock\n    Preparing worktree (detached HEAD 0857fef)\n    HEAD is now at 0857fef first\n    [master 2fa5413] foo\n    $\n"},{"id":"375722","messageId":"CAPig+cTkiBhZ70SC=xXQP_RfkDagCRLFxpcZYLidQrz9_v-_Bg@mail.gmail.com","threadId":"51094","inReplyTo":"CACsJy8AgzSZ_Gj9JW2yTdgkGQNCMMLy8i+pH_+A93+V95anQWQ@mail.gmail.com","subject":"Re: Running 'git worktree add' in 'pre-commit' hook","fromName":"Eric Sunshine","fromEmail":"sunshine@sunshineco.com","sentAt":"2019-05-16T12:05:56Z","receivedAt":"2019-05-16T12:06:10Z","isPatch":false,"sender":{"key":"sunshine@sunshineco.com","avatar":"https://avatars.githubusercontent.com/u/163641?v=4"},"body":"On Thu, May 16, 2019 at 7:45 AM Duy Nguyen <pclouds@gmail.com> wrote:\n> On Thu, May 16, 2019 at 6:39 PM Eric Sunshine <sunshine@sunshineco.com> wrote:\n> > Thinking more clearly on it, a better fix might be for git-worktree to\n> > deal with this itself, converting such a path to absolute before\n> > cd'ing to the new worktree directory (or something -- I don't have the\n> > code in front of me to provide a more concrete suggestion).\n>\n> Our mails crossed. But yeah it's basically the same thing.\n>\n> Although I still doubt a good $GIT_INDEX_FILE would fix the problem.\n> Even if you manage to create a new worktree, when you cd there and\n> start using it, this $GIT_INDEX_FILE will still mess things up (unless\n> you do mean to ignore the worktree's index file and update one\n> provided by pre-commit hook).\n\nI was forgetting that each worktree has its own index. Your advice\nthat his hook should sanitize GIT_INDEX_FILE (and possibly other GIT_*\nvariables) is probably the only sane approach. I don't think it's\nsomething we can handle in some automated fashion since we don't know\nwhence GIT_INDEX_FILE arose.\n"},{"id":"375724","messageId":"CAPig+cTCdBkru-6tJVeYxQSaosv7pb8R5ge_aedzi5C7w39mQg@mail.gmail.com","threadId":"51094","inReplyTo":"CACsJy8BXxVggL7CWA5Q0d7U4B+encfZ85NshxLJ=VvHhf_LUYg@mail.gmail.com","subject":"Re: Running 'git worktree add' in 'pre-commit' hook","fromName":"Eric Sunshine","fromEmail":"sunshine@sunshineco.com","sentAt":"2019-05-16T12:14:50Z","receivedAt":"2019-05-16T12:15:04Z","isPatch":false,"sender":{"key":"sunshine@sunshineco.com","avatar":"https://avatars.githubusercontent.com/u/163641?v=4"},"body":"On Thu, May 16, 2019 at 7:42 AM Duy Nguyen <pclouds@gmail.com> wrote:\n> On Thu, May 16, 2019 at 6:33 PM Eric Sunshine <sunshine@sunshineco.com> wrote:\n> > You run afoul of it in other situations, as well. For instance, say\n> > you have your index file in a non-standard location:\n> >\n> >     $ export GIT_INDEX_FILE=../storage/index\n> >     $ git worktree add --detach other\n> >     Preparing worktree (detached HEAD c9156d2)\n> >     fatal: Unable to create '/.../foo/other/../storage/index.lock': No\n> > such file or directory\n> >     $\n> >\n> Oh if it's relative $GIT_INDEX_FILE then I think its our environment.c\n> code that does not work so well when we chdir() away. I vaguely recall\n> something about this when discussing Jeff's chdir-notify series. But\n> it turns out that's about chdir() in run-command.c [1]. But the idea\n> is still the same, all variables are supposed to be relative to $CWD.\n> Whenever you move $CWD you should reparent all of them, including\n> $GIT_INDEX_FILE.\n\nbuiltin/worktree.c:add_worktree() is already assigning\nnew-worktree-specific values for GIT_DIR and GIT_WORK_TREE, so for the\nspecific example I showed above, the correct fix would be for\nadd_worktree() to remove GIT_INDEX_FILE from the environment before\ninvoking other Git commands, correct?\n"},{"id":"375725","messageId":"CACsJy8BP6af7YBGQXc_a_dEYeFgbpP3WaGAb0Z-NvTRRCgeTMw@mail.gmail.com","threadId":"51094","inReplyTo":"CAPig+cTCdBkru-6tJVeYxQSaosv7pb8R5ge_aedzi5C7w39mQg@mail.gmail.com","subject":"Re: Running 'git worktree add' in 'pre-commit' hook","fromName":"Duy Nguyen","fromEmail":"pclouds@gmail.com","sentAt":"2019-05-16T12:28:58Z","receivedAt":"2019-05-16T12:29:27Z","isPatch":false,"sender":{"key":"pclouds@gmail.com","avatar":"https://avatars.githubusercontent.com/u/720?v=4"},"body":"On Thu, May 16, 2019 at 7:15 PM Eric Sunshine <sunshine@sunshineco.com> wrote:\n>\n> On Thu, May 16, 2019 at 7:42 AM Duy Nguyen <pclouds@gmail.com> wrote:\n> > On Thu, May 16, 2019 at 6:33 PM Eric Sunshine <sunshine@sunshineco.com> wrote:\n> > > You run afoul of it in other situations, as well. For instance, say\n> > > you have your index file in a non-standard location:\n> > >\n> > >     $ export GIT_INDEX_FILE=../storage/index\n> > >     $ git worktree add --detach other\n> > >     Preparing worktree (detached HEAD c9156d2)\n> > >     fatal: Unable to create '/.../foo/other/../storage/index.lock': No\n> > > such file or directory\n> > >     $\n> > >\n> > Oh if it's relative $GIT_INDEX_FILE then I think its our environment.c\n> > code that does not work so well when we chdir() away. I vaguely recall\n> > something about this when discussing Jeff's chdir-notify series. But\n> > it turns out that's about chdir() in run-command.c [1]. But the idea\n> > is still the same, all variables are supposed to be relative to $CWD.\n> > Whenever you move $CWD you should reparent all of them, including\n> > $GIT_INDEX_FILE.\n>\n> builtin/worktree.c:add_worktree() is already assigning\n> new-worktree-specific values for GIT_DIR and GIT_WORK_TREE, so for the\n> specific example I showed above, the correct fix would be for\n> add_worktree() to remove GIT_INDEX_FILE from the environment before\n> invoking other Git commands, correct?\n\nI'll work on the assumption that all $GIT_* are for the _current_\nworktree, not the one we're creating. If so, yes we need to sanitize\n$GIT_*, maybe do it the same way submodules do (I think\nprepare_submodule_repo_env() is the right one).\n-- \nDuy\n"},{"id":"375751","messageId":"20190516221702.GA11784@sigill.intra.peff.net","threadId":"51094","inReplyTo":"CACsJy8DnkjuZD-9pbhAsFo16jHKt8U831LLxb3-nCQP5_FOmtA@mail.gmail.com","subject":"Re: Running 'git worktree add' in 'pre-commit' hook","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2019-05-16T22:17:02Z","receivedAt":"2019-05-16T22:23:34Z","isPatch":false,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Thu, May 16, 2019 at 06:25:24PM +0700, Duy Nguyen wrote:\n\n> > Is it forbidden to call this command from a hook?\n> \n> pre-commit hook sets GIT_INDEX_FILE to this \"index.lock\" so you have\n> the latest index content (which is not the same as from\n> $GIT_DIR/index). This variable will interfere with any commands that\n> work on a different worktree.\n> \n> So you probably can still make it work by backing up $GIT_INDEX_FILE\n> (in case you need it), then unset it before you use \"git worktree\" (or\n> cd to it if you keep a permanent separate worktree for pre-commit\n> activities). To make sure you don't have similar problems, you\n> probably should do \"env | grep GIT\" from the hook and see if any other\n> variables are set.\n\nIf you're entering another repo from a hook, you're supposed to use:\n\n  unset $(git rev-parse --local-env-vars)\n\nI wondered if we'd need another similar mechanism for entering the\nworktree of another repo, that would maybe clear fewer variables. But I\nthink it's actually the same: we really want to clear everything and let\nour \"cd\" pick up the new repository path.\n\nThe case of actually _adding_ a new work tree (before we enter it) is\nweirder, though. We definitely want to stay in the same repository, and\nclearing all of that would not make sense. I do wonder if worktree-add\nshould be handling GIT_INDEX_FILE (ignoring it when we want to be\ndealing with the index of the new worktree we added, and handling any\nrelative fixups if we chdir inside the worktree code).\n\n-Peff\n"},{"id":"375764","messageId":"CAPig+cQQ5svrDKRPenL2+bJHGjddUPnAXCFSsQN+WBHqzGpDow@mail.gmail.com","threadId":"51094","inReplyTo":"20190516221702.GA11784@sigill.intra.peff.net","subject":"Re: Running 'git worktree add' in 'pre-commit' hook","fromName":"Eric Sunshine","fromEmail":"sunshine@sunshineco.com","sentAt":"2019-05-16T23:16:54Z","receivedAt":"2019-05-16T23:17:08Z","isPatch":false,"sender":{"key":"sunshine@sunshineco.com","avatar":"https://avatars.githubusercontent.com/u/163641?v=4"},"body":"On Thu, May 16, 2019 at 6:17 PM Jeff King <peff@peff.net> wrote:\n> On Thu, May 16, 2019 at 06:25:24PM +0700, Duy Nguyen wrote:\n> > So you probably can still make it work by backing up $GIT_INDEX_FILE\n> > (in case you need it), then unset it before you use \"git worktree\" (or\n> > cd to it if you keep a permanent separate worktree for pre-commit\n> > activities). [...]\n>\n> The case of actually _adding_ a new work tree (before we enter it) is\n> weirder, though. We definitely want to stay in the same repository, and\n> clearing all of that would not make sense. I do wonder if worktree-add\n> should be handling GIT_INDEX_FILE (ignoring it when we want to be\n> dealing with the index of the new worktree we added, and handling any\n> relative fixups if we chdir inside the worktree code).\n\nIgnoring GIT_INDEX_FILE was indeed the conclusion reached earlier in\nthis thread. Addressing your other point, \"git worktree add\" does\nchdir() into the new worktree if a post-checkout hook exists since\nthat hook needs to run in the new worktree, not in the worktree in\nwhich the \"git worktree add\" command itself was invoked. For the hook\ninvocation, it already sanitizes the environment of GIT_DIR and\nGIT_WORK_TREE, and GIT_INDEX_FILE ought to be cleaned too. Is there\nany existing code in Git for doing the relative fixups you mention for\nother Git environment variables?\n"},{"id":"375769","messageId":"20190517001926.GA13525@sigill.intra.peff.net","threadId":"51094","inReplyTo":"CAPig+cQQ5svrDKRPenL2+bJHGjddUPnAXCFSsQN+WBHqzGpDow@mail.gmail.com","subject":"Re: Running 'git worktree add' in 'pre-commit' hook","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2019-05-17T00:19:27Z","receivedAt":"2019-05-17T00:19:30Z","isPatch":false,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Thu, May 16, 2019 at 07:16:54PM -0400, Eric Sunshine wrote:\n\n> On Thu, May 16, 2019 at 6:17 PM Jeff King <peff@peff.net> wrote:\n> > On Thu, May 16, 2019 at 06:25:24PM +0700, Duy Nguyen wrote:\n> > > So you probably can still make it work by backing up $GIT_INDEX_FILE\n> > > (in case you need it), then unset it before you use \"git worktree\" (or\n> > > cd to it if you keep a permanent separate worktree for pre-commit\n> > > activities). [...]\n> >\n> > The case of actually _adding_ a new work tree (before we enter it) is\n> > weirder, though. We definitely want to stay in the same repository, and\n> > clearing all of that would not make sense. I do wonder if worktree-add\n> > should be handling GIT_INDEX_FILE (ignoring it when we want to be\n> > dealing with the index of the new worktree we added, and handling any\n> > relative fixups if we chdir inside the worktree code).\n> \n> Ignoring GIT_INDEX_FILE was indeed the conclusion reached earlier in\n> this thread. Addressing your other point, \"git worktree add\" does\n> chdir() into the new worktree if a post-checkout hook exists since\n> that hook needs to run in the new worktree, not in the worktree in\n> which the \"git worktree add\" command itself was invoked. For the hook\n> invocation, it already sanitizes the environment of GIT_DIR and\n> GIT_WORK_TREE, and GIT_INDEX_FILE ought to be cleaned too. Is there\n> any existing code in Git for doing the relative fixups you mention for\n> other Git environment variables?\n\nYou can assign local_repo_env to child_process.env (or push it\nindividually to env_array if you have to mix with other variables). See\ngit_connect() for an example.\n\n-Peff\n"},{"id":"375778","messageId":"CAPig+cQpmD9+frJWNfcts1+RFBTjrHUHk_eR4Y94GAnGZC=7hQ@mail.gmail.com","threadId":"51094","inReplyTo":"20190517001926.GA13525@sigill.intra.peff.net","subject":"Re: Running 'git worktree add' in 'pre-commit' hook","fromName":"Eric Sunshine","fromEmail":"sunshine@sunshineco.com","sentAt":"2019-05-17T01:02:06Z","receivedAt":"2019-05-17T01:02:21Z","isPatch":false,"sender":{"key":"sunshine@sunshineco.com","avatar":"https://avatars.githubusercontent.com/u/163641?v=4"},"body":"On Thu, May 16, 2019 at 8:19 PM Jeff King <peff@peff.net> wrote:\n> On Thu, May 16, 2019 at 07:16:54PM -0400, Eric Sunshine wrote:\n> > Is there\n> > any existing code in Git for doing the relative fixups you mention for\n> > other Git environment variables?\n>\n> You can assign local_repo_env to child_process.env (or push it\n> individually to env_array if you have to mix with other variables). See\n> git_connect() for an example.\n\nI wasn't aware of 'local_repo_env', but can see how it could be\nhelpful as a basis for building machinery to adjust relative paths\ncontained in those environment variables prior to a chdir(). My\noriginal question was if such machinery already exists, but I'm\nguessing from your response that it doesn't.\n"},{"id":"375780","messageId":"20190517011941.GA30372@sigill.intra.peff.net","threadId":"51094","inReplyTo":"CAPig+cQpmD9+frJWNfcts1+RFBTjrHUHk_eR4Y94GAnGZC=7hQ@mail.gmail.com","subject":"Re: Running 'git worktree add' in 'pre-commit' hook","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2019-05-17T01:19:42Z","receivedAt":"2019-05-17T01:19:45Z","isPatch":false,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Thu, May 16, 2019 at 09:02:06PM -0400, Eric Sunshine wrote:\n\n> On Thu, May 16, 2019 at 8:19 PM Jeff King <peff@peff.net> wrote:\n> > On Thu, May 16, 2019 at 07:16:54PM -0400, Eric Sunshine wrote:\n> > > Is there\n> > > any existing code in Git for doing the relative fixups you mention for\n> > > other Git environment variables?\n> >\n> > You can assign local_repo_env to child_process.env (or push it\n> > individually to env_array if you have to mix with other variables). See\n> > git_connect() for an example.\n> \n> I wasn't aware of 'local_repo_env', but can see how it could be\n> helpful as a basis for building machinery to adjust relative paths\n> contained in those environment variables prior to a chdir(). My\n> original question was if such machinery already exists, but I'm\n> guessing from your response that it doesn't.\n\nOh, I see. I totally missed that you said \"relative fixups\" and not\n\"clearing\". :)\n\nThere's the chdir_notify system. Any variables which need updating need\nto register themselves, and then the chdir in the worktree code needs to\nbe done with chdir_notify(). See chdir-notify.h for details.\n\n-Peff\n"}]}