{"thread":{"id":"47049","subject":"[PATCH] rebase: exec leaks GIT_DIR to environment","startedAt":"2017-10-28T00:02:05Z","lastAt":"2017-10-31T08:13:32Z","messageCount":12,"participants":["Jacob Keller","Johannes Schindelin","Phillip Wood","Junio C Hamano"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"331199","messageId":"20171028000152.2760-1-jacob.e.keller@intel.com","threadId":"47049","inReplyTo":null,"subject":"[PATCH] rebase: exec leaks GIT_DIR to environment","fromName":"Jacob Keller","fromEmail":"jacob.e.keller@intel.com","sentAt":"2017-10-28T00:01:52Z","receivedAt":"2017-10-28T00:02:05Z","isPatch":true,"sender":{"key":"jacob.e.keller@intel.com","avatar":"https://avatars.githubusercontent.com/u/874719?v=4"},"body":"From: Jacob Keller \n\nI noticed a failure with git rebase interactive mode which causes \"exec\"\ncommands to be run with GIT_DIR set. When GIT_DIR is in the environment,\nthen any command which results in running a git command in\na subdirectory will fail because GIT_DIR=\".git\".\n\nThis unfortunately breaks one of my project's Makefiles, which uses\ngit-describe to find the version information, but does so from within\na sub directory.\n\nI'm in the process of running a bisect to find where this got\nintroduced, but I suspect it's part of the rebase--helper changes that\nhappened a while ago.\n\nSigned-off-by: Jacob Keller \n---\n t/t3404-rebase-interactive.sh | 11 +++++++++++\n 1 file changed, 11 insertions(+)\n\ndiff --git a/t/t3404-rebase-interactive.sh b/t/t3404-rebase-interactive.sh\nindex 3704dbb2ecf6..60ab5136f702 100755\n--- a/t/t3404-rebase-interactive.sh\n+++ b/t/t3404-rebase-interactive.sh\n@@ -108,6 +108,17 @@ test_expect_success 'rebase -i with the exec command runs from tree root' '\n \trm -fr subdir\n '\n \n+test_expect_failure 'rebase -i with the exec git commands in subdirs still work' '\n+\ttest_when_finished \"rm -ff subdir\" &&\n+\ttest_when_finished \"git rebase --abort\" &&\n+\tgit checkout master &&\n+\tmkdir subdir && (cd subdir &&\n+\tset_fake_editor &&\n+\tFAKE_LINES=\"1 exec_>cd_subdir_&&_git_rev-parse_--is-inside-work-tree\" \\\n+\t\tgit rebase -i HEAD^\n+\t)\n+'\n+\n test_expect_success 'rebase -i with the exec command checks tree cleanness' '\n \tgit checkout master &&\n \tset_fake_editor &&\n-- \n2.11.1.4.gad8c7cd\n\n"},{"id":"331224","messageId":"alpine.DEB.2.21.1.1710281740070.6482@virtualbox","threadId":"47049","inReplyTo":"20171028000152.2760-1-jacob.e.keller@intel.com","subject":"Re: [PATCH] rebase: exec leaks GIT_DIR to environment","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2017-10-28T16:00:35Z","receivedAt":"2017-10-28T16:00:48Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi Jake,\n\nOn Fri, 27 Oct 2017, Jacob Keller wrote:\n\n> From: Jacob Keller \n> \n> I noticed a failure with git rebase interactive mode which causes \"exec\"\n> commands to be run with GIT_DIR set. When GIT_DIR is in the environment,\n> then any command which results in running a git command in\n> a subdirectory will fail because GIT_DIR=\".git\".\n> \n> This unfortunately breaks one of my project's Makefiles, which uses\n> git-describe to find the version information, but does so from within\n> a sub directory.\n> \n> I'm in the process of running a bisect to find where this got\n> introduced, but I suspect it's part of the rebase--helper changes that\n> happened a while ago.\n\nA safe assumption. I do not know how the shell code managed that GIT_DIR\nreset, though:\n\n-- snip from v2.12.0's git-rebase--interactive.sh --\n x|\"exec\")\n read -r command rest < \"$todo\"\n mark_action_done\n eval_gettextln \"Executing: \\$rest\"\n \"${SHELL:-@SHELL_PATH@}\" -c \"$rest\" # Actual execution\n-- snap --\n\nMaybe you can spot it?\n\nThe fix should be as easy as\n\n-- snip --\ndiff --git a/sequencer.c b/sequencer.c\nindex f2c84c2fa62..018ba8d27e2 100644\n--- a/sequencer.c\n+++ b/sequencer.c\n@@ -1859,11 +1859,13 @@ static int error_failed_squash(struct commit *commit,\n static int do_exec(const char *command_line)\n {\n \tconst char *child_argv[] = { NULL, NULL };\n+\tconst char *child_env[] = { \"GIT_DIR\", NULL };\n \tint dirty, status;\n \n \tfprintf(stderr, \"Executing: %s\\n\", command_line);\n \tchild_argv[0] = command_line;\n-\tstatus = run_command_v_opt(child_argv, RUN_USING_SHELL);\n+\tstatus = run_command_v_opt_cd_env(child_argv, RUN_USING_SHELL, NULL,\n+\t\t\t\t\t child_env);\n \n \t/* force re-reading of the cache */\n \tif (discard_cache() < 0 || read_cache() < 0)\n-- snap --\n\n*However*, your test still fails with this, as\n\n- your added test tries to remove the directory with -ff instead of -rf\n\n- it tries to run `git rebase --abort` afterwards, which fails with my fix\n because there is no rebase in progress\n\n- instead of `cd subdir && ...`, it calls `>cd subdir && ...`, which\n causes it to abort with a \"subdir: not fonud\"\n\nSo I need this, too, to make it all work:\n\n-- snip --\ndiff --git a/t/t3404-rebase-interactive.sh b/t/t3404-rebase-interactive.sh\nindex 60ab5136f70..967caab222a 100755\n--- a/t/t3404-rebase-interactive.sh\n+++ b/t/t3404-rebase-interactive.sh\n@@ -108,13 +108,13 @@ test_expect_success 'rebase -i with the exec command runs from tree root' '\n \trm -fr subdir\n '\n \n-test_expect_failure 'rebase -i with the exec git commands in subdirs still work' '\n-\ttest_when_finished \"rm -ff subdir\" &&\n-\ttest_when_finished \"git rebase --abort\" &&\n+test_expect_success 'rebase -i with the exec git commands in subdirs still work' '\n+\ttest_when_finished \"rm -rf subdir\" &&\n+\ttest_when_finished \"git rebase --abort || :\" &&\n \tgit checkout master &&\n \tmkdir subdir && (cd subdir &&\n \tset_fake_editor &&\n-\tFAKE_LINES=\"1 exec_>cd_subdir_&&_git_rev-parse_--is-inside-work-tree\" \\\n+\tFAKE_LINES=\"1 exec_cd_subdir_&&_git_rev-parse_--is-inside-work-tree\" \\\n \t\tgit rebase -i HEAD^\n \t)\n '\n-- snap --\n\nI only had time to write these two patches, and to verify that t3404\npasses now, but not that anything else passes, neither to write a proper\ncommit message.\n\nMaybe you can take it from there?\n\nCiao,\nDscho\n"},{"id":"331245","messageId":"CA+P7+xqvHCi_aARgr0nXs7U_-P42vf8eiY7zPpG_0bp5GxokXQ@mail.gmail.com","threadId":"47049","inReplyTo":"alpine.DEB.2.21.1.1710281740070.6482@virtualbox","subject":"Re: [PATCH] rebase: exec leaks GIT_DIR to environment","fromName":"Jacob Keller","fromEmail":"jacob.keller@gmail.com","sentAt":"2017-10-28T22:35:52Z","receivedAt":"2017-10-28T22:36:19Z","isPatch":true,"sender":{"key":"jacob.keller@gmail.com","avatar":"https://avatars.githubusercontent.com/u/874719?v=4"},"body":"On Sat, Oct 28, 2017 at 9:00 AM, Johannes Schindelin\n wrote:\n> Hi Jake,\n>\n> On Fri, 27 Oct 2017, Jacob Keller wrote:\n>\n>> From: Jacob Keller \n>>\n>> I noticed a failure with git rebase interactive mode which causes \"exec\"\n>> commands to be run with GIT_DIR set. When GIT_DIR is in the environment,\n>> then any command which results in running a git command in\n>> a subdirectory will fail because GIT_DIR=\".git\".\n>>\n>> This unfortunately breaks one of my project's Makefiles, which uses\n>> git-describe to find the version information, but does so from within\n>> a sub directory.\n>>\n>> I'm in the process of running a bisect to find where this got\n>> introduced, but I suspect it's part of the rebase--helper changes that\n>> happened a while ago.\n>\n> A safe assumption. I do not know how the shell code managed that GIT_DIR\n> reset, though:\n>\n> -- snip from v2.12.0's git-rebase--interactive.sh --\n> x|\"exec\")\n> read -r command rest < \"$todo\"\n> mark_action_done\n> eval_gettextln \"Executing: \\$rest\"\n> \"${SHELL:-@SHELL_PATH@}\" -c \"$rest\" # Actual execution\n> -- snap --\n>\n\n>\n> *However*, your test still fails with this, as\n>\n\nYea I'm not surprised the test failed, I was in a hurry at the end of\nthe workday when I spotted it, but wanted to get something on the\nmailing list before I left.\n\n> - your added test tries to remove the directory with -ff instead of -rf\n>\n> - it tries to run `git rebase --abort` afterwards, which fails with my fix\n> because there is no rebase in progress\n>\n> - instead of `cd subdir && ...`, it calls `>cd subdir && ...`, which\n> causes it to abort with a \"subdir: not fonud\"\n>\n\n>\n> I only had time to write these two patches, and to verify that t3404\n> passes now, but not that anything else passes, neither to write a proper\n> commit message.\n>\n> Maybe you can take it from there?\n>\n\nYep, thanks for spotting the fix!\n\nThanks,\nJake\n\n> Ciao,\n> Dscho\n"},{"id":"331288","messageId":"4150d979-f653-e79b-563a-1dc43f12468d@talktalk.net","threadId":"47049","inReplyTo":"alpine.DEB.2.21.1.1710281740070.6482@virtualbox","subject":"Re: [PATCH] rebase: exec leaks GIT_DIR to environment","fromName":"Phillip Wood","fromEmail":"phillip.wood@talktalk.net","sentAt":"2017-10-29T18:34:29Z","receivedAt":"2017-10-29T18:34:39Z","isPatch":true,"sender":{"key":"phillip.wood@dunelm.org.uk","avatar":null},"body":"On 28/10/17 17:00, Johannes Schindelin wrote:\n> \n> Hi Jake,\n> \n> On Fri, 27 Oct 2017, Jacob Keller wrote:\n> \n>> From: Jacob Keller \n>>\n>> I noticed a failure with git rebase interactive mode which causes \"exec\"\n>> commands to be run with GIT_DIR set. When GIT_DIR is in the environment,\n>> then any command which results in running a git command in\n>> a subdirectory will fail because GIT_DIR=\".git\".\n>>\n>> This unfortunately breaks one of my project's Makefiles, which uses\n>> git-describe to find the version information, but does so from within\n>> a sub directory.\n>>\n>> I'm in the process of running a bisect to find where this got\n>> introduced, but I suspect it's part of the rebase--helper changes that\n>> happened a while ago.\n> \n> A safe assumption. I do not know how the shell code managed that GIT_DIR\n> reset, though:\n> \n> -- snip from v2.12.0's git-rebase--interactive.sh --\n> x|\"exec\")\n> read -r command rest < \"$todo\"\n> mark_action_done\n> eval_gettextln \"Executing: \\$rest\"\n> \"${SHELL:-@SHELL_PATH@}\" -c \"$rest\" # Actual execution\n> -- snap --\n> \n> Maybe you can spot it?\n> \n> The fix should be as easy as\n> \n> -- snip --\n> diff --git a/sequencer.c b/sequencer.c\n> index f2c84c2fa62..018ba8d27e2 100644\n> --- a/sequencer.c\n> +++ b/sequencer.c\n> @@ -1859,11 +1859,13 @@ static int error_failed_squash(struct commit *commit,\n> static int do_exec(const char *command_line)\n> {\n> \tconst char *child_argv[] = { NULL, NULL };\n> +\tconst char *child_env[] = { \"GIT_DIR\", NULL };\n> \tint dirty, status;\n> \n> \tfprintf(stderr, \"Executing: %s\\n\", command_line);\n> \tchild_argv[0] = command_line;\n> -\tstatus = run_command_v_opt(child_argv, RUN_USING_SHELL);\n> +\tstatus = run_command_v_opt_cd_env(child_argv, RUN_USING_SHELL, NULL,\n> +\t\t\t\t\t child_env);\n> \n> \t/* force re-reading of the cache */\n> \tif (discard_cache() < 0 || read_cache() < 0)\n> -- snap --\n\nJust clearing GIT_DIR does not match the behavior of the shell version\n(tested by passing -p to avoid rebase--helper) as that passes GIT_DIR to\nexec commands if it has been explicitly set. I think that users that set\nGIT_DIR on the command line would expect it to be propagated to exec\ncommands.\n\n$ git rebase -px'echo $GIT_DIR' @\n\n Merge commit\n'7c2f1abd64' into phil\nExecuting: echo $GIT_DIR\n\nSuccessfully rebased and updated refs/heads/phil.\n\n$ env GIT_DIR=.git git rebase -px'echo $GIT_DIR' @\n\n Merge commit\n'7c2f1abd64' into phil\nExecuting: echo $GIT_DIR\n/home/phil/Documents/src/git/.git/worktrees/git-next\nSuccessfully rebased and updated refs/heads/phil.\n\n> *However*, your test still fails with this, as\n> \n> - your added test tries to remove the directory with -ff instead of -rf\n> \n> - it tries to run `git rebase --abort` afterwards, which fails with my fix\n> because there is no rebase in progress\n> \n> - instead of `cd subdir && ...`, it calls `>cd subdir && ...`, which\n> causes it to abort with a \"subdir: not fonud\"\n> \n> So I need this, too, to make it all work:\n> \n> -- snip --\n> diff --git a/t/t3404-rebase-interactive.sh b/t/t3404-rebase-interactive.sh\n> index 60ab5136f70..967caab222a 100755\n> --- a/t/t3404-rebase-interactive.sh\n> +++ b/t/t3404-rebase-interactive.sh\n> @@ -108,13 +108,13 @@ test_expect_success 'rebase -i with the exec command runs from tree root' '\n> \trm -fr subdir\n> '\n> \n> -test_expect_failure 'rebase -i with the exec git commands in subdirs still work' '\n> -\ttest_when_finished \"rm -ff subdir\" &&\n> -\ttest_when_finished \"git rebase --abort\" &&\n> +test_expect_success 'rebase -i with the exec git commands in subdirs still work' '\n> +\ttest_when_finished \"rm -rf subdir\" &&\n> +\ttest_when_finished \"git rebase --abort || :\" &&\n> \tgit checkout master &&\n> \tmkdir subdir && (cd subdir &&\n> \tset_fake_editor &&\n> -\tFAKE_LINES=\"1 exec_>cd_subdir_&&_git_rev-parse_--is-inside-work-tree\" \\\n> +\tFAKE_LINES=\"1 exec_cd_subdir_&&_git_rev-parse_--is-inside-work-tree\" \\\n> \t\tgit rebase -i HEAD^\n> \t)\n> '\n> -- snap --\n> \n> I only had time to write these two patches, and to verify that t3404\n> passes now, but not that anything else passes, neither to write a proper\n> commit message.\n> \n> Maybe you can take it from there?\n> \n> Ciao,\n> Dscho\n> \n\n"},{"id":"331308","messageId":"xmqq1sll8j6f.fsf@gitster.mtv.corp.google.com","threadId":"47049","inReplyTo":"4150d979-f653-e79b-563a-1dc43f12468d@talktalk.net","subject":"Re: [PATCH] rebase: exec leaks GIT_DIR to environment","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2017-10-30T02:26:48Z","receivedAt":"2017-10-30T02:26:56Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Phillip Wood writes:\n\n> Just clearing GIT_DIR does not match the behavior of the shell version\n> (tested by passing -p to avoid rebase--helper) as that passes GIT_DIR to\n> exec commands if it has been explicitly set. I think that users that set\n> GIT_DIR on the command line would expect it to be propagated to exec\n> commands.\n>\n> $ git rebase -px'echo $GIT_DIR' @\n>\n> Merge commit\n> '7c2f1abd64' into phil\n> Executing: echo $GIT_DIR\n>\n> Successfully rebased and updated refs/heads/phil.\n>\n> $ env GIT_DIR=.git git rebase -px'echo $GIT_DIR' @\n>\n> Merge commit\n> '7c2f1abd64' into phil\n> Executing: echo $GIT_DIR\n> /home/phil/Documents/src/git/.git/worktrees/git-next\n> Successfully rebased and updated refs/heads/phil.\n\nHmmm, I do not mess with GIT_DIR at all in my workflow, so I am\nhaving a bit of hard time judging if this regression is serious\nenough to be a release blocker. \n\nI'd prefer to avoid reverting the whole js/rebase-i-final topic from\n'master' this late in the game, even though I do not expect we would\nsee the remainder of the system gets broken due to hidden dependency\non the topic, because the changes on the topic are relatively well\nisolated.\n\n\n570676e011\nd1114d87c7\n2f0e14e649\n5f3108b7b6\n"},{"id":"331310","messageId":"CA+P7+xrUeM-TsXrbOxscqaqCJB+wq2NP-TdnxzvA7n4vQXvOxQ@mail.gmail.com","threadId":"47049","inReplyTo":"4150d979-f653-e79b-563a-1dc43f12468d@talktalk.net","subject":"Re: [PATCH] rebase: exec leaks GIT_DIR to environment","fromName":"Jacob Keller","fromEmail":"jacob.keller@gmail.com","sentAt":"2017-10-30T02:51:14Z","receivedAt":"2017-10-30T02:51:41Z","isPatch":true,"sender":{"key":"jacob.keller@gmail.com","avatar":"https://avatars.githubusercontent.com/u/874719?v=4"},"body":"On Sun, Oct 29, 2017 at 11:34 AM, Phillip Wood\n wrote:\n>\n> Just clearing GIT_DIR does not match the behavior of the shell version\n> (tested by passing -p to avoid rebase--helper) as that passes GIT_DIR to\n> exec commands if it has been explicitly set. I think that users that set\n> GIT_DIR on the command line would expect it to be propagated to exec\n> commands.\n>\n> $ git rebase -px'echo $GIT_DIR' @\n>\n> Merge commit\n> '7c2f1abd64' into phil\n> Executing: echo $GIT_DIR\n>\n> Successfully rebased and updated refs/heads/phil.\n>\n> $ env GIT_DIR=.git git rebase -px'echo $GIT_DIR' @\n>\n> Merge commit\n> '7c2f1abd64' into phil\n> Executing: echo $GIT_DIR\n> /home/phil/Documents/src/git/.git/worktrees/git-next\n> Successfully rebased and updated refs/heads/phil.\n>\n\nI'm not really sure what the exact fix is, since we do want to pass in\nGIT_DIR if it was passed to us, but maybe it needs to be an absolute\npath?\n\nThanks,\nJake\n"},{"id":"331319","messageId":"CA+P7+xrXLcTQpPWgzLwt_yZo=QdfetF36jrc_TtXfqMKR2Hh3w@mail.gmail.com","threadId":"47049","inReplyTo":"xmqq1sll8j6f.fsf@gitster.mtv.corp.google.com","subject":"Re: [PATCH] rebase: exec leaks GIT_DIR to environment","fromName":"Jacob Keller","fromEmail":"jacob.keller@gmail.com","sentAt":"2017-10-30T02:53:25Z","receivedAt":"2017-10-30T02:53:51Z","isPatch":true,"sender":{"key":"jacob.keller@gmail.com","avatar":"https://avatars.githubusercontent.com/u/874719?v=4"},"body":"On Sun, Oct 29, 2017 at 7:26 PM, Junio C Hamano wrote:\n> Phillip Wood writes:\n>\n>> Just clearing GIT_DIR does not match the behavior of the shell version\n>> (tested by passing -p to avoid rebase--helper) as that passes GIT_DIR to\n>> exec commands if it has been explicitly set. I think that users that set\n>> GIT_DIR on the command line would expect it to be propagated to exec\n>> commands.\n>>\n>> $ git rebase -px'echo $GIT_DIR' @\n>>\n>> Merge commit\n>> '7c2f1abd64' into phil\n>> Executing: echo $GIT_DIR\n>>\n>> Successfully rebased and updated refs/heads/phil.\n>>\n>> $ env GIT_DIR=.git git rebase -px'echo $GIT_DIR' @\n>>\n>> Merge commit\n>> '7c2f1abd64' into phil\n>> Executing: echo $GIT_DIR\n>> /home/phil/Documents/src/git/.git/worktrees/git-next\n>> Successfully rebased and updated refs/heads/phil.\n>\n> Hmmm, I do not mess with GIT_DIR at all in my workflow, so I am\n> having a bit of hard time judging if this regression is serious\n> enough to be a release blocker.\n>\n\nSo, I don't directly mess with GIT_DIR in my use case either, I just\nhappened to run a build from a sub directory which relies on git\ncommands continuing to work. However, because GIT_DIR was set to a\nrelative path, it broke this Make in the subdirectory, which resulted\nin the problem occurring, but *only* during the exec command, if I ran\nthe command manually after reach rebase step, it worked fine (since\nGIT_DIR was no longer set).\n\nI don't know how big a deal it is, since I didn't notice it for quite some time.\n\nThanks,\nJake\n\n> I'd prefer to avoid reverting the whole js/rebase-i-final topic from\n> 'master' this late in the game, even though I do not expect we would\n> see the remainder of the system gets broken due to hidden dependency\n> on the topic, because the changes on the topic are relatively well\n> isolated.\n>\n>\n> 570676e011\n> d1114d87c7\n> 2f0e14e649\n> 5f3108b7b6\n\nI am pretty confident we can fix it. I think the easiest solution\nwould be to just make sure GIT_DIR is an absolute path when passing it\nto the exec command.\n\nThanks,\nJake\n"},{"id":"331322","messageId":"xmqqo9op71d8.fsf@gitster.mtv.corp.google.com","threadId":"47049","inReplyTo":"CA+P7+xrXLcTQpPWgzLwt_yZo=QdfetF36jrc_TtXfqMKR2Hh3w@mail.gmail.com","subject":"Re: [PATCH] rebase: exec leaks GIT_DIR to environment","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2017-10-30T03:36:51Z","receivedAt":"2017-10-30T03:36:58Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jacob Keller writes:\n\n> I am pretty confident we can fix it....\n\nI am sure we can eventually, but 3 hours is not enough soak time. \n\nI am inclined to leave the fix for 2.15.1/2.16.0 instead of delaying\nthe release by 10 more days.\n"},{"id":"331334","messageId":"CA+P7+xo5UgUPQCYU-LaXn+HZZ1qe++KOevTMh2C1sgnzK0SAQA@mail.gmail.com","threadId":"47049","inReplyTo":"xmqqo9op71d8.fsf@gitster.mtv.corp.google.com","subject":"Re: [PATCH] rebase: exec leaks GIT_DIR to environment","fromName":"Jacob Keller","fromEmail":"jacob.keller@gmail.com","sentAt":"2017-10-30T06:26:55Z","receivedAt":"2017-10-30T06:27:21Z","isPatch":true,"sender":{"key":"jacob.keller@gmail.com","avatar":"https://avatars.githubusercontent.com/u/874719?v=4"},"body":"On Sun, Oct 29, 2017 at 8:36 PM, Junio C Hamano wrote:\n> Jacob Keller writes:\n>\n>> I am pretty confident we can fix it....\n>\n> I am sure we can eventually, but 3 hours is not enough soak time.\n>\n> I am inclined to leave the fix for 2.15.1/2.16.0 instead of delaying\n> the release by 10 more days.\n\nThat's fair. I'm not even sure it was introduced since the last\nrelease (I tried 2.12, but not 2.13 or 2.14 manually). Thus, it likely\nwasn't noticed for at least a release, meaning it's less important (to\nme at least) that we provide a fix immediately, since it went\nunnoticed this long, likely that means few people will be impacted.\n\nAs far as I can tell, you'd only be impacted if you attempt to run a\ngit command from within a sub directory inside of an exec statement.\nIs there some place we can list it as a known bug in case someone else\nruns into it?\n\nRegards,\nJake\n"},{"id":"331336","messageId":"7026e5be-bbf0-9d55-8901-f920a775879b@talktalk.net","threadId":"47049","inReplyTo":"CA+P7+xo5UgUPQCYU-LaXn+HZZ1qe++KOevTMh2C1sgnzK0SAQA@mail.gmail.com","subject":"Re: [PATCH] rebase: exec leaks GIT_DIR to environment","fromName":"Phillip Wood","fromEmail":"phillip.wood@talktalk.net","sentAt":"2017-10-30T10:20:36Z","receivedAt":"2017-10-30T10:20:47Z","isPatch":true,"sender":{"key":"phillip.wood@dunelm.org.uk","avatar":null},"body":"On 30/10/17 06:26, Jacob Keller wrote:\n> On Sun, Oct 29, 2017 at 8:36 PM, Junio C Hamano wrote:\n>> Jacob Keller writes:\n>>\n>>> I am pretty confident we can fix it....\n>>\n>> I am sure we can eventually, but 3 hours is not enough soak time.\n>>\n>> I am inclined to leave the fix for 2.15.1/2.16.0 instead of delaying\n>> the release by 10 more days.\n> \n> That's fair. I'm not even sure it was introduced since the last\n> release (I tried 2.12, but not 2.13 or 2.14 manually). Thus, it likely\n> wasn't noticed for at least a release, meaning it's less important (to\n> me at least) that we provide a fix immediately, since it went\n> unnoticed this long, likely that means few people will be impacted.\n\nIt is in 2.14.3, I haven't bisected but I suspect it was introduced by\n311af5266b sequencer (rebase -i): implement the 'exec' command\n\nRunning\ngit rebase -x'perl -e '\\''$,=$\\=\"\\n\"; print grep { /^GIT_/ } sort keys\n%ENV'\\' @\nShows that the rebase--helper version also sets GIT_PREFIX as well as\nGIT_DIR, I suspect the difference is coming from differences in the\nsetup for builtin commands vs external commands. The shell version and\nthe rebase--helper version set GIT_CHERRY_PICK_HELP, GIT_EDITOR,\nGIT_INTERNAL_GETTEXT_SH_SCHEME, GIT_REFLOG_ACTION\n\nBest Wishes\n\nPhillip\n\n\n"},{"id":"331353","messageId":"alpine.DEB.2.21.1.1710301343000.6482@virtualbox","threadId":"47049","inReplyTo":"7026e5be-bbf0-9d55-8901-f920a775879b@talktalk.net","subject":"Re: [PATCH] rebase: exec leaks GIT_DIR to environment","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2017-10-30T12:46:36Z","receivedAt":"2017-10-30T12:47:10Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi Phillip,\n\nOn Mon, 30 Oct 2017, Phillip Wood wrote:\n\n> On 30/10/17 06:26, Jacob Keller wrote:\n> > On Sun, Oct 29, 2017 at 8:36 PM, Junio C Hamano wrote:\n> >> Jacob Keller writes:\n> >>\n> >>> I am pretty confident we can fix it....\n> >>\n> >> I am sure we can eventually, but 3 hours is not enough soak time.\n> >>\n> >> I am inclined to leave the fix for 2.15.1/2.16.0 instead of delaying\n> >> the release by 10 more days.\n> > \n> > That's fair. I'm not even sure it was introduced since the last\n> > release (I tried 2.12, but not 2.13 or 2.14 manually). Thus, it likely\n> > wasn't noticed for at least a release, meaning it's less important (to\n> > me at least) that we provide a fix immediately, since it went\n> > unnoticed this long, likely that means few people will be impacted.\n> \n> It is in 2.14.3, I haven't bisected but I suspect it was introduced by\n> 311af5266b sequencer (rebase -i): implement the 'exec' command\n> \n> Running\n> git rebase -x'perl -e '\\''$,=$\\=\"\\n\"; print grep { /^GIT_/ } sort keys\n> %ENV'\\' @\n> Shows that the rebase--helper version also sets GIT_PREFIX as well as\n> GIT_DIR, I suspect the difference is coming from differences in the\n> setup for builtin commands vs external commands. The shell version and\n> the rebase--helper version set GIT_CHERRY_PICK_HELP, GIT_EDITOR,\n> GIT_INTERNAL_GETTEXT_SH_SCHEME, GIT_REFLOG_ACTION\n\nIndeed, when you look at git_dir_init in git-sh-setup, you will see that\nUnix shell scripts explicitly get their GIT_DIR turned into an absolute\npath.\n\nSo my suggested patch is wrong, and it should be more along the lines of\n\n\tstruct strbuf buf = STRBUF_INIT;\n\tconst char *child_env[] = { NULL, NULL };\n\n\tstrbuf_addf(&buf, \"GIT_DIR=%s\", absolute_path(get_git_dir()));\n\tchild_env[0] = buf.buf;\n\n\t...\n\n\tstrbuf_release(&buf);\n\nJake, can I still take you up on taking it from here?\n\nCiao,\nDscho\n"},{"id":"331464","messageId":"A0F486F0-DDA3-4E99-9C0F-371387093C51@gmail.com","threadId":"47049","inReplyTo":"alpine.DEB.2.21.1.1710301343000.6482@virtualbox","subject":"Re: [PATCH] rebase: exec leaks GIT_DIR to environment","fromName":"Jacob Keller","fromEmail":"jacob.keller@gmail.com","sentAt":"2017-10-31T08:13:18Z","receivedAt":"2017-10-31T08:13:32Z","isPatch":true,"sender":{"key":"jacob.keller@gmail.com","avatar":"https://avatars.githubusercontent.com/u/874719?v=4"},"body":"\n\nOn October 30, 2017 5:46:36 AM PDT, Johannes Schindelin wrote:\n>Hi Phillip,\n>\n>On Mon, 30 Oct 2017, Phillip Wood wrote:\n>\n>> On 30/10/17 06:26, Jacob Keller wrote:\n>> > On Sun, Oct 29, 2017 at 8:36 PM, Junio C Hamano \n>wrote:\n>> >> Jacob Keller writes:\n>> >>\n>> >>> I am pretty confident we can fix it....\n>> >>\n>> >> I am sure we can eventually, but 3 hours is not enough soak time.\n>> >>\n>> >> I am inclined to leave the fix for 2.15.1/2.16.0 instead of\n>delaying\n>> >> the release by 10 more days.\n>> > \n>> > That's fair. I'm not even sure it was introduced since the last\n>> > release (I tried 2.12, but not 2.13 or 2.14 manually). Thus, it\n>likely\n>> > wasn't noticed for at least a release, meaning it's less important\n>(to\n>> > me at least) that we provide a fix immediately, since it went\n>> > unnoticed this long, likely that means few people will be impacted.\n>> \n>> It is in 2.14.3, I haven't bisected but I suspect it was introduced\n>by\n>> 311af5266b sequencer (rebase -i): implement the 'exec' command\n>> \n>> Running\n>> git rebase -x'perl -e '\\''$,=$\\=\"\\n\"; print grep { /^GIT_/ } sort\n>keys\n>> %ENV'\\' @\n>> Shows that the rebase--helper version also sets GIT_PREFIX as well as\n>> GIT_DIR, I suspect the difference is coming from differences in the\n>> setup for builtin commands vs external commands. The shell version\n>and\n>> the rebase--helper version set GIT_CHERRY_PICK_HELP, GIT_EDITOR,\n>> GIT_INTERNAL_GETTEXT_SH_SCHEME, GIT_REFLOG_ACTION\n>\n>Indeed, when you look at git_dir_init in git-sh-setup, you will see\n>that\n>Unix shell scripts explicitly get their GIT_DIR turned into an absolute\n>path.\n>\n>So my suggested patch is wrong, and it should be more along the lines\n>of\n>\n>\tstruct strbuf buf = STRBUF_INIT;\n>\tconst char *child_env[] = { NULL, NULL };\n>\n>\tstrbuf_addf(&buf, \"GIT_DIR=%s\", absolute_path(get_git_dir()));\n>\tchild_env[0] = buf.buf;\n>\n>\t...\n>\n>\tstrbuf_release(&buf);\n>\n>Jake, can I still take you up on taking it from here?\n>\n>Ciao,\n>Dscho\n\nYes, I will clean this up and submit something tomorrow once I get to a computer. I didn't end up having time to work on Git today.\n\nThanks,\nJake\n\n-- \nSent from my Android device with K-9 Mail. Please excuse my brevity.\n"}]}