{"thread":{"id":"48866","subject":"rev-parse --show-toplevel broken during exec'ed rebase?","startedAt":"2018-07-12T02:51:03Z","lastAt":"2018-07-19T22:35:01Z","messageCount":22,"participants":["Vitali Lovich","Johannes Schindelin","Junio C Hamano","brian m. carlson","Jeff King","SZEDER Gábor"],"isPatch":false,"patchVersion":null,"patchTotal":null},"messages":[{"id":"352318","messageId":"CAF8PYMjub1KK_ZK0hijTNBACSa6SaFyu1dJgrXd5fJU5Hc_dhg@mail.gmail.com","threadId":"48866","inReplyTo":null,"subject":"rev-parse --show-toplevel broken during exec'ed rebase?","fromName":"Vitali Lovich","fromEmail":"vlovich@gmail.com","sentAt":"2018-07-12T02:50:47Z","receivedAt":"2018-07-12T02:51:03Z","isPatch":false,"sender":{"key":"vlovich@gmail.com","avatar":null},"body":"Typically git rev-parse --show-toplevel prints the folder containing\nthe .git folder regardless what subdirectory one is in. One exception\nI have found is that if one is within the context of git rebase --exec\nthen show-toplevel always just prints the current directory it's\nrunning from.\n\nRepro (starting with cwd within git project):\n> (cd xdiff; git rev-parse --show-toplevel)\n... path to git repository\n> git rebase -i 18404434bf406f6a6f892ed73320c5cf9cc187dd\n# Stop at some commit for editing\n> (cd xdiff; git rev-parse --show-toplevel)\n... path to git repository\n> git rebase 18404434bf406f6a6f892ed73320c5cf9cc187dd -x \"(cd xdiff; git rev-parse --show-toplevel)\"\n... path to git repository/xdiff !!!\n\nThis seems like incorrect behaviour to me since it's a weird\ninconsistency (even with other rebase contexts) & the documentation\nsays \"Show the absolute path of the top-level directory.\" with no\ncaveats.\n\nThanks,\nVital\n"},{"id":"352319","messageId":"CAF8PYMikJZF4Qk4VHhAUPB80LG=gjZBVNBu0yGKCLLPpdG_3sg@mail.gmail.com","threadId":"48866","inReplyTo":"CAF8PYMjub1KK_ZK0hijTNBACSa6SaFyu1dJgrXd5fJU5Hc_dhg@mail.gmail.com","subject":"Re: rev-parse --show-toplevel broken during exec'ed rebase?","fromName":"Vitali Lovich","fromEmail":"vlovich@gmail.com","sentAt":"2018-07-12T02:53:53Z","receivedAt":"2018-07-12T02:54:08Z","isPatch":false,"sender":{"key":"vlovich@gmail.com","avatar":null},"body":"Sorry. Forgot to include the git versions I tested with (2.13.1,\n2.17.0, 2.18.0)\nOn Wed, Jul 11, 2018 at 7:50 PM Vitali Lovich wrote:\n>\n> Typically git rev-parse --show-toplevel prints the folder containing\n> the .git folder regardless what subdirectory one is in. One exception\n> I have found is that if one is within the context of git rebase --exec\n> then show-toplevel always just prints the current directory it's\n> running from.\n>\n> Repro (starting with cwd within git project):\n> > (cd xdiff; git rev-parse --show-toplevel)\n> ... path to git repository\n> > git rebase -i 18404434bf406f6a6f892ed73320c5cf9cc187dd\n> # Stop at some commit for editing\n> > (cd xdiff; git rev-parse --show-toplevel)\n> ... path to git repository\n> > git rebase 18404434bf406f6a6f892ed73320c5cf9cc187dd -x \"(cd xdiff; git rev-parse --show-toplevel)\"\n> ... path to git repository/xdiff !!!\n>\n> This seems like incorrect behaviour to me since it's a weird\n> inconsistency (even with other rebase contexts) & the documentation\n> says \"Show the absolute path of the top-level directory.\" with no\n> caveats.\n>\n> Thanks,\n> Vital\n"},{"id":"352343","messageId":"nycvar.QRO.7.76.6.1807121647310.75@tvgsbejvaqbjf.bet","threadId":"48866","inReplyTo":"CAF8PYMikJZF4Qk4VHhAUPB80LG=gjZBVNBu0yGKCLLPpdG_3sg@mail.gmail.com","subject":"Re: rev-parse --show-toplevel broken during exec'ed rebase?","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2018-07-12T14:49:06Z","receivedAt":"2018-07-12T14:49:26Z","isPatch":false,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi Vitali,\n\n[please avoid top-posting on this mailing list]\n\nOn Wed, 11 Jul 2018, Vitali Lovich wrote:\n\n> On Wed, Jul 11, 2018 at 7:50 PM Vitali Lovich wrote:\n> >\n> > Typically git rev-parse --show-toplevel prints the folder containing\n> > the .git folder regardless what subdirectory one is in. One exception\n> > I have found is that if one is within the context of git rebase --exec\n> > then show-toplevel always just prints the current directory it's\n> > running from.\n> >\n> > Repro (starting with cwd within git project):\n> > > (cd xdiff; git rev-parse --show-toplevel)\n> > ... path to git repository\n> > > git rebase -i 18404434bf406f6a6f892ed73320c5cf9cc187dd\n> > # Stop at some commit for editing\n> > > (cd xdiff; git rev-parse --show-toplevel)\n> > ... path to git repository\n> > > git rebase 18404434bf406f6a6f892ed73320c5cf9cc187dd -x \"(cd xdiff; git rev-parse --show-toplevel)\"\n> > ... path to git repository/xdiff !!!\n> >\n> > This seems like incorrect behaviour to me since it's a weird\n> > inconsistency (even with other rebase contexts) & the documentation\n> > says \"Show the absolute path of the top-level directory.\" with no\n> > caveats.\n>\n> Sorry. Forgot to include the git versions I tested with (2.13.1,\n> 2.17.0, 2.18.0)\n\nThis is actually not so much a bug in `rebase` as in `rev-parse\n--show-top-level`:\n\n\t$ GIT_DIR=$PWD/.git git -C xdiff rev-parse --show-toplevel\n\tC:/git-sdk-64/usr/src/git/xdiff\n\nCiao,\nJohannes\n"},{"id":"352344","messageId":"nycvar.QRO.7.76.6.1807121657120.75@tvgsbejvaqbjf.bet","threadId":"48866","inReplyTo":"nycvar.QRO.7.76.6.1807121647310.75@tvgsbejvaqbjf.bet","subject":"Re: rev-parse --show-toplevel broken during exec'ed rebase?","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2018-07-12T15:02:10Z","receivedAt":"2018-07-12T15:02:29Z","isPatch":false,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi,\n\nOn Thu, 12 Jul 2018, Johannes Schindelin wrote:\n\n> On Wed, 11 Jul 2018, Vitali Lovich wrote:\n> \n> > On Wed, Jul 11, 2018 at 7:50 PM Vitali Lovich wrote:\n> > >\n> > > Typically git rev-parse --show-toplevel prints the folder containing\n> > > the .git folder regardless what subdirectory one is in. One exception\n> > > I have found is that if one is within the context of git rebase --exec\n> > > then show-toplevel always just prints the current directory it's\n> > > running from.\n> > >\n> > > Repro (starting with cwd within git project):\n> > > > (cd xdiff; git rev-parse --show-toplevel)\n> > > ... path to git repository\n> > > > git rebase -i 18404434bf406f6a6f892ed73320c5cf9cc187dd\n> > > # Stop at some commit for editing\n> > > > (cd xdiff; git rev-parse --show-toplevel)\n> > > ... path to git repository\n> > > > git rebase 18404434bf406f6a6f892ed73320c5cf9cc187dd -x \"(cd xdiff; git rev-parse --show-toplevel)\"\n> > > ... path to git repository/xdiff !!!\n> > >\n> > > This seems like incorrect behaviour to me since it's a weird\n> > > inconsistency (even with other rebase contexts) & the documentation\n> > > says \"Show the absolute path of the top-level directory.\" with no\n> > > caveats.\n> >\n> > Sorry. Forgot to include the git versions I tested with (2.13.1,\n> > 2.17.0, 2.18.0)\n> \n> This is actually not so much a bug in `rebase` as in `rev-parse\n> --show-top-level`:\n> \n> \t$ GIT_DIR=$PWD/.git git -C xdiff rev-parse --show-toplevel\n> \tC:/git-sdk-64/usr/src/git/xdiff\n\nAnd the reason for this behavior is that setting `GIT_DIR` explicitly\nmakes Git *always* consider the current working directory to be the\ntop-level directory:\n\n\thttps://github.com/git/git/blob/v2.18.0/setup.c#L1061-L1063\n\nI wonder whether changing the line\n(https://github.com/git/git/blob/v2.18.0/sequencer.c#L2635)\n\n argv_array_pushf(&child_env, \"GIT_DIR=%s\", absolute_path(get_git_dir()));\n\nto\n\n argv_array_push(&child_env, \"GIT_DIR\");\n\nbreaks anything (and whether it fixes the bug you demonstrated via\n`rev-parse --show-toplevel`).\n\nCiao,\nJohannes\n"},{"id":"352346","messageId":"xmqq7em01ns9.fsf@gitster-ct.c.googlers.com","threadId":"48866","inReplyTo":"CAF8PYMjub1KK_ZK0hijTNBACSa6SaFyu1dJgrXd5fJU5Hc_dhg@mail.gmail.com","subject":"Re: rev-parse --show-toplevel broken during exec'ed rebase?","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2018-07-12T15:23:02Z","receivedAt":"2018-07-12T15:23:06Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Vitali Lovich writes:\n\n> Repro (starting with cwd within git project):\n>> (cd xdiff; git rev-parse --show-toplevel)\n> ... path to git repository\n>> git rebase -i 18404434bf406f6a6f892ed73320c5cf9cc187dd\n> # Stop at some commit for editing\n>> (cd xdiff; git rev-parse --show-toplevel)\n> ... path to git repository\n>> git rebase 18404434bf406f6a6f892ed73320c5cf9cc187dd -x \"(cd xdiff; git rev-parse --show-toplevel)\"\n> ... path to git repository/xdiff !!!\n>\n> This seems like incorrect behaviour to me since it's a weird\n> inconsistency (even with other rebase contexts) & the documentation\n> says \"Show the absolute path of the top-level directory.\" with no\n> caveats.\n\nDoes it reproduce with older rebase (say from v2.10 days), too?\n\nI suspect that the above is because \"git rebase\" is exporting\nGIT_DIR without exporting GIT_WORK_TREE. When the former is given\nwithout the latter, that tells Git that you are at the top-level of\nthe working tree (if that is not the case, you also export the\nlatter to tell Git where the top-level is). \n\nI suspect that \"git rebase\" before the ongoing piecemeal rewrite to\nC started (or scripted Porcelain in general) refrained from\nexporting GIT_DIR to the environment, and if my suspicion is correct,\nolder \"git rebase\" would behave differently to your test case.\n"},{"id":"352372","messageId":"CAF8PYMhGWi-Tkzde_G_ifcz=wmaAO3Ppwfy3kbiypgX-C_suvA@mail.gmail.com","threadId":"48866","inReplyTo":"xmqq7em01ns9.fsf@gitster-ct.c.googlers.com","subject":"Re: rev-parse --show-toplevel broken during exec'ed rebase?","fromName":"Vitali Lovich","fromEmail":"vlovich@gmail.com","sentAt":"2018-07-12T17:39:42Z","receivedAt":"2018-07-12T17:39:57Z","isPatch":false,"sender":{"key":"vlovich@gmail.com","avatar":null},"body":"On Thu, Jul 12, 2018 at 8:23 AM Junio C Hamano wrote:\n>\n> Vitali Lovich writes:\n>\n> > Repro (starting with cwd within git project):\n> >> (cd xdiff; git rev-parse --show-toplevel)\n> > ... path to git repository\n> >> git rebase -i 18404434bf406f6a6f892ed73320c5cf9cc187dd\n> > # Stop at some commit for editing\n> >> (cd xdiff; git rev-parse --show-toplevel)\n> > ... path to git repository\n> >> git rebase 18404434bf406f6a6f892ed73320c5cf9cc187dd -x \"(cd xdiff; git rev-parse --show-toplevel)\"\n> > ... path to git repository/xdiff !!!\n> >\n> > This seems like incorrect behaviour to me since it's a weird\n> > inconsistency (even with other rebase contexts) & the documentation\n> > says \"Show the absolute path of the top-level directory.\" with no\n> > caveats.\n>\n> Does it reproduce with older rebase (say from v2.10 days), too?\n>\n> I suspect that the above is because \"git rebase\" is exporting\n> GIT_DIR without exporting GIT_WORK_TREE. When the former is given\n> without the latter, that tells Git that you are at the top-level of\n> the working tree (if that is not the case, you also export the\n> latter to tell Git where the top-level is).\n>\n> I suspect that \"git rebase\" before the ongoing piecemeal rewrite to\n> C started (or scripted Porcelain in general) refrained from\n> exporting GIT_DIR to the environment, and if my suspicion is correct,\n> older \"git rebase\" would behave differently to your test case.\n\nUnfortunately I don't have an older git version handy to test this out.\n"},{"id":"352519","messageId":"20180713184732.GC968742@genre.crustytoothpaste.net","threadId":"48866","inReplyTo":"xmqq7em01ns9.fsf@gitster-ct.c.googlers.com","subject":"Re: rev-parse --show-toplevel broken during exec'ed rebase?","fromName":"brian m. carlson","fromEmail":"sandals@crustytoothpaste.net","sentAt":"2018-07-13T18:47:32Z","receivedAt":"2018-07-13T18:47:40Z","isPatch":false,"sender":{"key":"sandals@crustytoothpaste.net","avatar":"https://avatars.githubusercontent.com/u/497054?v=4"},"body":"On Thu, Jul 12, 2018 at 08:23:02AM -0700, Junio C Hamano wrote:\n> Vitali Lovich writes:\n> \n> > Repro (starting with cwd within git project):\n> >> (cd xdiff; git rev-parse --show-toplevel)\n> > ... path to git repository\n> >> git rebase -i 18404434bf406f6a6f892ed73320c5cf9cc187dd\n> > # Stop at some commit for editing\n> >> (cd xdiff; git rev-parse --show-toplevel)\n> > ... path to git repository\n> >> git rebase 18404434bf406f6a6f892ed73320c5cf9cc187dd -x \"(cd xdiff; git rev-parse --show-toplevel)\"\n> > ... path to git repository/xdiff !!!\n> >\n> > This seems like incorrect behaviour to me since it's a weird\n> > inconsistency (even with other rebase contexts) & the documentation\n> > says \"Show the absolute path of the top-level directory.\" with no\n> > caveats.\n> \n> Does it reproduce with older rebase (say from v2.10 days), too?\n\nIt appears to work correctly on the CentOS SCL Git 2.9.3. We print the\ntop level of the repository there.\n-- \nbrian m. carlson: Houston, Texas, US\nOpenPGP: https://keybase.io/bk2204\n"},{"id":"352534","messageId":"20180713201949.GA17670@sigill.intra.peff.net","threadId":"48866","inReplyTo":"20180713184732.GC968742@genre.crustytoothpaste.net","subject":"Re: rev-parse --show-toplevel broken during exec'ed rebase?","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2018-07-13T20:19:49Z","receivedAt":"2018-07-13T20:19:52Z","isPatch":false,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Fri, Jul 13, 2018 at 06:47:32PM +0000, brian m. carlson wrote:\n\n> > >> git rebase 18404434bf406f6a6f892ed73320c5cf9cc187dd -x \"(cd xdiff; git rev-parse --show-toplevel)\"\n> > > ... path to git repository/xdiff !!!\n> > >\n> > > This seems like incorrect behaviour to me since it's a weird\n> > > inconsistency (even with other rebase contexts) & the documentation\n> > > says \"Show the absolute path of the top-level directory.\" with no\n> > > caveats.\n> > \n> > Does it reproduce with older rebase (say from v2.10 days), too?\n> \n> It appears to work correctly on the CentOS SCL Git 2.9.3. We print the\n> top level of the repository there.\n\nBisecting is tricky, because there are actually three distinct\nbehaviors.\n\nThe command above prints the correct top-level until 18633e1a22 (rebase\n-i: use the rebase--helper builtin, 2017-02-09). After that, rev-parse\nprints \"Not a git repository\".\n\nThat goes on until 09d7b6c6fa (sequencer: pass absolute GIT_DIR to exec\ncommands, 2017-10-31).\n\nNone of which is too surprising. The root of the bug is in the\nconversion to rebase--helper, I think, when presumably we started\nsetting GIT_DIR at all (but I didn't dig further). Then 09d7b6c6fa fixed\n_one_ fallout of that, which was relative paths, but didn't help the\nsubdirectory case.\n\nJust reading over this thread, I suspect the simplest fix is to pass\nGIT_DIR and GIT_WORK_TREE together, which is almost always the right\nthing to do.\n\n-Peff\n"},{"id":"352542","messageId":"20180713230548.GC1042117@genre.crustytoothpaste.net","threadId":"48866","inReplyTo":"20180713201949.GA17670@sigill.intra.peff.net","subject":"Re: rev-parse --show-toplevel broken during exec'ed rebase?","fromName":"brian m. carlson","fromEmail":"sandals@crustytoothpaste.net","sentAt":"2018-07-13T23:05:49Z","receivedAt":"2018-07-13T23:05:57Z","isPatch":false,"sender":{"key":"sandals@crustytoothpaste.net","avatar":"https://avatars.githubusercontent.com/u/497054?v=4"},"body":"On Fri, Jul 13, 2018 at 04:19:49PM -0400, Jeff King wrote:\n> Just reading over this thread, I suspect the simplest fix is to pass\n> GIT_DIR and GIT_WORK_TREE together, which is almost always the right\n> thing to do.\n\nI agree. I'll write up a patch.\n-- \nbrian m. carlson: Houston, Texas, US\nOpenPGP: https://keybase.io/bk2204\n"},{"id":"352543","messageId":"20180714003505.67650-1-sandals@crustytoothpaste.net","threadId":"48866","inReplyTo":"20180713230548.GC1042117@genre.crustytoothpaste.net","subject":"[PATCH] sequencer: pass absolute GIT_WORK_TREE to exec commands","fromName":"brian m. carlson","fromEmail":"sandals@crustytoothpaste.net","sentAt":"2018-07-14T00:35:05Z","receivedAt":"2018-07-14T00:35:16Z","isPatch":true,"sender":{"key":"sandals@crustytoothpaste.net","avatar":"https://avatars.githubusercontent.com/u/497054?v=4"},"body":"The sequencer currently passes GIT_DIR, but not GIT_WORK_TREE, to exec\ncommands. In that configuration, we assume that whatever directory\nwe're in is the top level of the work tree, and git rev-parse\n--show-toplevel responds accordingly. However, when we're in a\nsubdirectory, that isn't correct: we respond with the subdirectory as\nthe top level, resulting in unexpected behavior.\n\nEnsure that we pass GIT_WORK_TREE as well as GIT_DIR so that git\noperations within subdirectories work correctly.\n\nSigned-off-by: brian m. carlson \n---\nDscho, is this test going to cause a problem on Windows with the forward\nslash in the grep statement?\n\n sequencer.c | 2 ++\n t/t3404-rebase-interactive.sh | 9 +++++++++\n 2 files changed, 11 insertions(+)\n\ndiff --git a/sequencer.c b/sequencer.c\nindex 5354d4d51e..c8e16f9168 100644\n--- a/sequencer.c\n+++ b/sequencer.c\n@@ -2636,6 +2636,8 @@ static int do_exec(const char *command_line)\n \tfprintf(stderr, \"Executing: %s\\n\", command_line);\n \tchild_argv[0] = command_line;\n \targv_array_pushf(&child_env, \"GIT_DIR=%s\", absolute_path(get_git_dir()));\n+\targv_array_pushf(&child_env, \"GIT_WORK_TREE=%s\",\n+\t\t\t absolute_path(get_git_work_tree()));\n \tstatus = run_command_v_opt_cd_env(child_argv, RUN_USING_SHELL, NULL,\n \t\t\t\t\t child_env.argv);\n \ndiff --git a/t/t3404-rebase-interactive.sh b/t/t3404-rebase-interactive.sh\nindex 352a52e59d..d03055d149 100755\n--- a/t/t3404-rebase-interactive.sh\n+++ b/t/t3404-rebase-interactive.sh\n@@ -119,6 +119,15 @@ test_expect_success 'rebase -i with exec allows git commands in subdirs' '\n \t)\n '\n \n+test_expect_success 'rebase -i sets work tree properly' '\n+\ttest_when_finished \"rm -rf subdir\" &&\n+\ttest_when_finished \"test_might_fail git rebase --abort\" &&\n+\tmkdir subdir &&\n+\tgit rebase -x \"(cd subdir && git rev-parse --show-toplevel)\" HEAD^ \\\n+\t\t>actual &&\n+\t! grep \"/subdir$\" actual\n+'\n+\n test_expect_success 'rebase -i with the exec command checks tree cleanness' '\n \tgit checkout master &&\n \tset_fake_editor &&\n"},{"id":"352544","messageId":"20180714005702.GA20399@sigill.intra.peff.net","threadId":"48866","inReplyTo":"20180714003505.67650-1-sandals@crustytoothpaste.net","subject":"Re: [PATCH] sequencer: pass absolute GIT_WORK_TREE to exec commands","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2018-07-14T00:57:03Z","receivedAt":"2018-07-14T00:57:06Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Sat, Jul 14, 2018 at 12:35:05AM +0000, brian m. carlson wrote:\n\n> diff --git a/sequencer.c b/sequencer.c\n> index 5354d4d51e..c8e16f9168 100644\n> --- a/sequencer.c\n> +++ b/sequencer.c\n> @@ -2636,6 +2636,8 @@ static int do_exec(const char *command_line)\n> \tfprintf(stderr, \"Executing: %s\\n\", command_line);\n> \tchild_argv[0] = command_line;\n> \targv_array_pushf(&child_env, \"GIT_DIR=%s\", absolute_path(get_git_dir()));\n> +\targv_array_pushf(&child_env, \"GIT_WORK_TREE=%s\",\n> +\t\t\t absolute_path(get_git_work_tree()));\n> \tstatus = run_command_v_opt_cd_env(child_argv, RUN_USING_SHELL, NULL,\n> \t\t\t\t\t child_env.argv);\n> \n\nAs a general rule of \"always pass GIT_WORK_TREE with GIT_DIR\", you have\nto deal with the case where there _isn't_ a work tree. Are we OK here\nbecause this code is inside sequencer.c, and you cannot do a sequencer\noperation without a work tree?\n\nThis all seemed vaguely familiar, so I dug up 2cd83d10bb (setup:\nsuppress implicit \".\" work-tree for bare repos, 2013-03-08), which\nhandles a similar case in the alias code.\n\nSo I think it would also work to set:\n\n GIT_IMPLICIT_WORK_TREE=0\n\nhere. But if the answer to my \"are we OK\" above is yes, I am fine with\neither that solution or the one you show here (but I think the commit\nmessage should probably mention it).\n\n> diff --git a/t/t3404-rebase-interactive.sh b/t/t3404-rebase-interactive.sh\n> index 352a52e59d..d03055d149 100755\n> --- a/t/t3404-rebase-interactive.sh\n> +++ b/t/t3404-rebase-interactive.sh\n> @@ -119,6 +119,15 @@ test_expect_success 'rebase -i with exec allows git commands in subdirs' '\n> \t)\n> '\n> \n> +test_expect_success 'rebase -i sets work tree properly' '\n> +\ttest_when_finished \"rm -rf subdir\" &&\n> +\ttest_when_finished \"test_might_fail git rebase --abort\" &&\n> +\tmkdir subdir &&\n> +\tgit rebase -x \"(cd subdir && git rev-parse --show-toplevel)\" HEAD^ \\\n> +\t\t>actual &&\n> +\t! grep \"/subdir$\" actual\n> +'\n\nThis test looks good to me.\n\n-Peff\n"},{"id":"352549","messageId":"20180714175555.GD1042117@genre.crustytoothpaste.net","threadId":"48866","inReplyTo":"20180714005702.GA20399@sigill.intra.peff.net","subject":"Re: [PATCH] sequencer: pass absolute GIT_WORK_TREE to exec commands","fromName":"brian m. carlson","fromEmail":"sandals@crustytoothpaste.net","sentAt":"2018-07-14T17:55:55Z","receivedAt":"2018-07-14T17:56:03Z","isPatch":true,"sender":{"key":"sandals@crustytoothpaste.net","avatar":"https://avatars.githubusercontent.com/u/497054?v=4"},"body":"On Fri, Jul 13, 2018 at 08:57:03PM -0400, Jeff King wrote:\n> On Sat, Jul 14, 2018 at 12:35:05AM +0000, brian m. carlson wrote:\n> \n> > diff --git a/sequencer.c b/sequencer.c\n> > index 5354d4d51e..c8e16f9168 100644\n> > --- a/sequencer.c\n> > +++ b/sequencer.c\n> > @@ -2636,6 +2636,8 @@ static int do_exec(const char *command_line)\n> > \tfprintf(stderr, \"Executing: %s\\n\", command_line);\n> > \tchild_argv[0] = command_line;\n> > \targv_array_pushf(&child_env, \"GIT_DIR=%s\", absolute_path(get_git_dir()));\n> > +\targv_array_pushf(&child_env, \"GIT_WORK_TREE=%s\",\n> > +\t\t\t absolute_path(get_git_work_tree()));\n> > \tstatus = run_command_v_opt_cd_env(child_argv, RUN_USING_SHELL, NULL,\n> > \t\t\t\t\t child_env.argv);\n> > \n> \n> As a general rule of \"always pass GIT_WORK_TREE with GIT_DIR\", you have\n> to deal with the case where there _isn't_ a work tree. Are we OK here\n> because this code is inside sequencer.c, and you cannot do a sequencer\n> operation without a work tree?\n\nI believe that's correct. We only call this code path from\nsequencer_pick_commits or sequencer_continue, and those code paths are\nonly called from revert, commit, and rebase--helper; all of those\nrequire a working tree.\n\n> So I think it would also work to set:\n> \n> GIT_IMPLICIT_WORK_TREE=0\n> \n> here. But if the answer to my \"are we OK\" above is yes, I am fine with\n> either that solution or the one you show here (but I think the commit\n> message should probably mention it).\n\nI'll reroll with a mention of that in the commit message. Thanks for a\ncareful review.\n-- \nbrian m. carlson: Houston, Texas, US\nOpenPGP: https://keybase.io/bk2204\n"},{"id":"352553","messageId":"20180714183859.97988-1-sandals@crustytoothpaste.net","threadId":"48866","inReplyTo":"20180714175555.GD1042117@genre.crustytoothpaste.net","subject":"[PATCH v2] sequencer: pass absolute GIT_WORK_TREE to exec commands","fromName":"brian m. carlson","fromEmail":"sandals@crustytoothpaste.net","sentAt":"2018-07-14T18:38:59Z","receivedAt":"2018-07-14T18:39:09Z","isPatch":true,"sender":{"key":"sandals@crustytoothpaste.net","avatar":"https://avatars.githubusercontent.com/u/497054?v=4"},"body":"The sequencer currently passes GIT_DIR, but not GIT_WORK_TREE, to exec\ncommands. In that configuration, we assume that whatever directory\nwe're in is the top level of the work tree, and git rev-parse\n--show-toplevel responds accordingly. However, when we're in a\nsubdirectory, that isn't correct: we respond with the subdirectory as\nthe top level, resulting in unexpected behavior.\n\nEnsure that we pass GIT_WORK_TREE as well as GIT_DIR so that git\noperations within subdirectories work correctly.\n\nNote that we are guaranteed to have a work tree in this case: the\nrelevant sequencer functions are called only from revert, cherry-pick,\nand rebase--helper; all of these commands require a working tree.\n\nSigned-off-by: brian m. carlson \n---\n sequencer.c | 2 ++\n t/t3404-rebase-interactive.sh | 9 +++++++++\n 2 files changed, 11 insertions(+)\n\ndiff --git a/sequencer.c b/sequencer.c\nindex 5354d4d51e..c8e16f9168 100644\n--- a/sequencer.c\n+++ b/sequencer.c\n@@ -2636,6 +2636,8 @@ static int do_exec(const char *command_line)\n \tfprintf(stderr, \"Executing: %s\\n\", command_line);\n \tchild_argv[0] = command_line;\n \targv_array_pushf(&child_env, \"GIT_DIR=%s\", absolute_path(get_git_dir()));\n+\targv_array_pushf(&child_env, \"GIT_WORK_TREE=%s\",\n+\t\t\t absolute_path(get_git_work_tree()));\n \tstatus = run_command_v_opt_cd_env(child_argv, RUN_USING_SHELL, NULL,\n \t\t\t\t\t child_env.argv);\n \ndiff --git a/t/t3404-rebase-interactive.sh b/t/t3404-rebase-interactive.sh\nindex 352a52e59d..d03055d149 100755\n--- a/t/t3404-rebase-interactive.sh\n+++ b/t/t3404-rebase-interactive.sh\n@@ -119,6 +119,15 @@ test_expect_success 'rebase -i with exec allows git commands in subdirs' '\n \t)\n '\n \n+test_expect_success 'rebase -i sets work tree properly' '\n+\ttest_when_finished \"rm -rf subdir\" &&\n+\ttest_when_finished \"test_might_fail git rebase --abort\" &&\n+\tmkdir subdir &&\n+\tgit rebase -x \"(cd subdir && git rev-parse --show-toplevel)\" HEAD^ \\\n+\t\t>actual &&\n+\t! grep \"/subdir$\" actual\n+'\n+\n test_expect_success 'rebase -i with the exec command checks tree cleanness' '\n \tgit checkout master &&\n \tset_fake_editor &&\n"},{"id":"352555","messageId":"nycvar.QRO.7.76.6.1807142304340.75@tvgsbejvaqbjf.bet","threadId":"48866","inReplyTo":"20180714003505.67650-1-sandals@crustytoothpaste.net","subject":"Re: [PATCH] sequencer: pass absolute GIT_WORK_TREE to exec commands","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2018-07-14T21:05:34Z","receivedAt":"2018-07-14T21:05:57Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi Brian,\n\nOn Sat, 14 Jul 2018, brian m. carlson wrote:\n\n> The sequencer currently passes GIT_DIR, but not GIT_WORK_TREE, to exec\n> commands. In that configuration, we assume that whatever directory\n> we're in is the top level of the work tree, and git rev-parse\n> --show-toplevel responds accordingly. However, when we're in a\n> subdirectory, that isn't correct: we respond with the subdirectory as\n> the top level, resulting in unexpected behavior.\n> \n> Ensure that we pass GIT_WORK_TREE as well as GIT_DIR so that git\n> operations within subdirectories work correctly.\n> \n> Signed-off-by: brian m. carlson \n> ---\n> Dscho, is this test going to cause a problem on Windows with the forward\n> slash in the grep statement?\n\nIt passes on Windows. The reason is that you are asking *Git* for a path,\nand Git will always try to use forward slashes (which work on Windows\nunder most circumstances).\n\nCiao,\nDscho\n\n> sequencer.c | 2 ++\n> t/t3404-rebase-interactive.sh | 9 +++++++++\n> 2 files changed, 11 insertions(+)\n> \n> diff --git a/sequencer.c b/sequencer.c\n> index 5354d4d51e..c8e16f9168 100644\n> --- a/sequencer.c\n> +++ b/sequencer.c\n> @@ -2636,6 +2636,8 @@ static int do_exec(const char *command_line)\n> \tfprintf(stderr, \"Executing: %s\\n\", command_line);\n> \tchild_argv[0] = command_line;\n> \targv_array_pushf(&child_env, \"GIT_DIR=%s\", absolute_path(get_git_dir()));\n> +\targv_array_pushf(&child_env, \"GIT_WORK_TREE=%s\",\n> +\t\t\t absolute_path(get_git_work_tree()));\n> \tstatus = run_command_v_opt_cd_env(child_argv, RUN_USING_SHELL, NULL,\n> \t\t\t\t\t child_env.argv);\n> \n> diff --git a/t/t3404-rebase-interactive.sh b/t/t3404-rebase-interactive.sh\n> index 352a52e59d..d03055d149 100755\n> --- a/t/t3404-rebase-interactive.sh\n> +++ b/t/t3404-rebase-interactive.sh\n> @@ -119,6 +119,15 @@ test_expect_success 'rebase -i with exec allows git commands in subdirs' '\n> \t)\n> '\n> \n> +test_expect_success 'rebase -i sets work tree properly' '\n> +\ttest_when_finished \"rm -rf subdir\" &&\n> +\ttest_when_finished \"test_might_fail git rebase --abort\" &&\n> +\tmkdir subdir &&\n> +\tgit rebase -x \"(cd subdir && git rev-parse --show-toplevel)\" HEAD^ \\\n> +\t\t>actual &&\n> +\t! grep \"/subdir$\" actual\n> +'\n> +\n> test_expect_success 'rebase -i with the exec command checks tree cleanness' '\n> \tgit checkout master &&\n> \tset_fake_editor &&\n> \n"},{"id":"352556","messageId":"20180714210917.GI1042117@genre.crustytoothpaste.net","threadId":"48866","inReplyTo":"nycvar.QRO.7.76.6.1807142304340.75@tvgsbejvaqbjf.bet","subject":"Re: [PATCH] sequencer: pass absolute GIT_WORK_TREE to exec commands","fromName":"brian m. carlson","fromEmail":"sandals@crustytoothpaste.net","sentAt":"2018-07-14T21:09:17Z","receivedAt":"2018-07-14T21:09:29Z","isPatch":true,"sender":{"key":"sandals@crustytoothpaste.net","avatar":"https://avatars.githubusercontent.com/u/497054?v=4"},"body":"On Sat, Jul 14, 2018 at 11:05:34PM +0200, Johannes Schindelin wrote:\n> Hi Brian,\n> \n> On Sat, 14 Jul 2018, brian m. carlson wrote:\n> > Dscho, is this test going to cause a problem on Windows with the forward\n> > slash in the grep statement?\n> \n> It passes on Windows. The reason is that you are asking *Git* for a path,\n> and Git will always try to use forward slashes (which work on Windows\n> under most circumstances).\n\nGreat, thanks.\n-- \nbrian m. carlson: Houston, Texas, US\nOpenPGP: https://keybase.io/bk2204\n"},{"id":"352557","messageId":"20180714211950.GA6430@sigill.intra.peff.net","threadId":"48866","inReplyTo":"20180714183859.97988-1-sandals@crustytoothpaste.net","subject":"Re: [PATCH v2] sequencer: pass absolute GIT_WORK_TREE to exec commands","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2018-07-14T21:19:50Z","receivedAt":"2018-07-14T21:20:47Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Sat, Jul 14, 2018 at 06:38:59PM +0000, brian m. carlson wrote:\n\n> The sequencer currently passes GIT_DIR, but not GIT_WORK_TREE, to exec\n> commands. In that configuration, we assume that whatever directory\n> we're in is the top level of the work tree, and git rev-parse\n> --show-toplevel responds accordingly. However, when we're in a\n> subdirectory, that isn't correct: we respond with the subdirectory as\n> the top level, resulting in unexpected behavior.\n> \n> Ensure that we pass GIT_WORK_TREE as well as GIT_DIR so that git\n> operations within subdirectories work correctly.\n> \n> Note that we are guaranteed to have a work tree in this case: the\n> relevant sequencer functions are called only from revert, cherry-pick,\n> and rebase--helper; all of these commands require a working tree.\n\nThanks for digging into this corner case.\n\nIt's possible, of course, that some day we may have a caller without a\nwork-tree, but it seems pretty unlikely for sequencer code. If we did,\nthen:\n\n> diff --git a/sequencer.c b/sequencer.c\n> index 5354d4d51e..c8e16f9168 100644\n> --- a/sequencer.c\n> +++ b/sequencer.c\n> @@ -2636,6 +2636,8 @@ static int do_exec(const char *command_line)\n> \tfprintf(stderr, \"Executing: %s\\n\", command_line);\n> \tchild_argv[0] = command_line;\n> \targv_array_pushf(&child_env, \"GIT_DIR=%s\", absolute_path(get_git_dir()));\n> +\targv_array_pushf(&child_env, \"GIT_WORK_TREE=%s\",\n> +\t\t\t absolute_path(get_git_work_tree()));\n\nI think this would just segfault (feeding NULL to absolute_path). That's\nprobably OK, as it would be a big red flag to whomever is trying to\nteach the sequencer to work in a bare repo. :)\n\nSo this version looks good to me.\n\n-Peff\n"},{"id":"352641","messageId":"xmqqsh4jt5d0.fsf@gitster-ct.c.googlers.com","threadId":"48866","inReplyTo":"20180713201949.GA17670@sigill.intra.peff.net","subject":"Re: rev-parse --show-toplevel broken during exec'ed rebase?","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2018-07-16T18:14:51Z","receivedAt":"2018-07-16T18:14:56Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jeff King writes:\n\n> None of which is too surprising. The root of the bug is in the\n> conversion to rebase--helper, I think, when presumably we started\n> setting GIT_DIR at all (but I didn't dig further). Then 09d7b6c6fa fixed\n> _one_ fallout of that, which was relative paths, but didn't help the\n> subdirectory case.\n>\n> Just reading over this thread, I suspect the simplest fix is to pass\n> GIT_DIR and GIT_WORK_TREE together, which is almost always the right\n> thing to do.\n\nPerhaps. Not exporting GIT_DIR (unless the end-user already did to\nthe environment before starting \"git rebase\"---it would be a bad\nchange to unexport it unconditionally) may probably be a way to make\nrebase--helper conversion more faithful to the original scripted\nPorcelain, but I suspect in practice always giving GIT_DIR and\nGIT_WORK_TREE would work well for many existing hooks.\n"},{"id":"352644","messageId":"20180716183942.GB22298@sigill.intra.peff.net","threadId":"48866","inReplyTo":"xmqqsh4jt5d0.fsf@gitster-ct.c.googlers.com","subject":"Re: rev-parse --show-toplevel broken during exec'ed rebase?","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2018-07-16T18:39:43Z","receivedAt":"2018-07-16T18:39:46Z","isPatch":false,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Mon, Jul 16, 2018 at 11:14:51AM -0700, Junio C Hamano wrote:\n\n> Jeff King writes:\n> \n> > None of which is too surprising. The root of the bug is in the\n> > conversion to rebase--helper, I think, when presumably we started\n> > setting GIT_DIR at all (but I didn't dig further). Then 09d7b6c6fa fixed\n> > _one_ fallout of that, which was relative paths, but didn't help the\n> > subdirectory case.\n> >\n> > Just reading over this thread, I suspect the simplest fix is to pass\n> > GIT_DIR and GIT_WORK_TREE together, which is almost always the right\n> > thing to do.\n> \n> Perhaps. Not exporting GIT_DIR (unless the end-user already did to\n> the environment before starting \"git rebase\"---it would be a bad\n> change to unexport it unconditionally) may probably be a way to make\n> rebase--helper conversion more faithful to the original scripted\n> Porcelain, but I suspect in practice always giving GIT_DIR and\n> GIT_WORK_TREE would work well for many existing hooks.\n\nYeah, that may be an option. I don't remember if this was discussed in\nthis thread or elsewhere, but setting GIT_DIR is a regression for hooks,\netc, which do:\n\n git -C /some/other/repo log\n\nor similar. I'm not sure if that falls under \"actual regression\" or\n\"just how it happened to work in the past\". I'm not even sure it worked\nthat way _consistently_ in the past.\n\nThe best practice if you're switching directories is to do:\n\n unset $(git rev-parse --local-env-vars)\n\nthough I highly doubt that most people bother.\n\n-Peff\n"},{"id":"352683","messageId":"xmqqefg2sx88.fsf@gitster-ct.c.googlers.com","threadId":"48866","inReplyTo":"20180716183942.GB22298@sigill.intra.peff.net","subject":"Re: rev-parse --show-toplevel broken during exec'ed rebase?","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2018-07-16T21:10:31Z","receivedAt":"2018-07-16T21:10:35Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jeff King writes:\n\n> On Mon, Jul 16, 2018 at 11:14:51AM -0700, Junio C Hamano wrote:\n>\n>> Porcelain, but I suspect in practice always giving GIT_DIR and\n>> GIT_WORK_TREE would work well for many existing hooks.\n>\n> Yeah, that may be an option. I don't remember if this was discussed in\n> this thread or elsewhere, but setting GIT_DIR is a regression for hooks,\n> etc, which do:\n>\n> git -C /some/other/repo log\n>\n> or similar. I'm not sure if that falls under \"actual regression\" or\n> \"just how it happened to work in the past\". I'm not even sure it worked\n> that way _consistently_ in the past.\n>\n> The best practice if you're switching directories is to do:\n>\n> unset $(git rev-parse --local-env-vars)\n>\n> though I highly doubt that most people bother.\n\nYeah, that is exactly where my \"in practice\" comes from. I think I\ncaught and killed another potential regression last week while it is\nstill in the draft status during my review ;-)\n"},{"id":"353074","messageId":"nycvar.QRO.7.76.6.1807192225260.71@tvgsbejvaqbjf.bet","threadId":"48866","inReplyTo":"xmqqsh4jt5d0.fsf@gitster-ct.c.googlers.com","subject":"Re: rev-parse --show-toplevel broken during exec'ed rebase?","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2018-07-19T20:28:03Z","receivedAt":"2018-07-19T20:28:40Z","isPatch":false,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi Junio,\n\nOn Mon, 16 Jul 2018, Junio C Hamano wrote:\n\n> Jeff King writes:\n> \n> > None of which is too surprising. The root of the bug is in the\n> > conversion to rebase--helper, I think, when presumably we started\n> > setting GIT_DIR at all (but I didn't dig further). Then 09d7b6c6fa fixed\n> > _one_ fallout of that, which was relative paths, but didn't help the\n> > subdirectory case.\n> >\n> > Just reading over this thread, I suspect the simplest fix is to pass\n> > GIT_DIR and GIT_WORK_TREE together, which is almost always the right\n> > thing to do.\n> \n> Perhaps. Not exporting GIT_DIR (unless the end-user already did to\n> the environment before starting \"git rebase\"---it would be a bad\n> change to unexport it unconditionally) may probably be a way to make\n> rebase--helper conversion more faithful to the original scripted\n> Porcelain, but I suspect in practice always giving GIT_DIR and\n> GIT_WORK_TREE would work well for many existing hooks.\n\nForgetting the code in git-sh-setup, are we?\n\ngit_dir_init() rather specifically set GIT_DIR to the absolute path, and\nsince that variable is already exported, the `exec` commands launched via\n`git-rebase--interactive` all saw it.\n\nThat is the reason why the sequencer.c was taught to set GIT_DIR to an\nabsolute path rathern than not setting it: for backwards compatibility.\n\nCiao,\nDscho\n"},{"id":"353093","messageId":"20180719220929.6550-1-szeder.dev@gmail.com","threadId":"48866","inReplyTo":"nycvar.QRO.7.76.6.1807192225260.71@tvgsbejvaqbjf.bet","subject":"Re: rev-parse --show-toplevel broken during exec'ed rebase?","fromName":"SZEDER Gábor","fromEmail":"szeder.dev@gmail.com","sentAt":"2018-07-19T22:09:29Z","receivedAt":"2018-07-19T22:11:10Z","isPatch":false,"sender":{"key":"szeder.dev@gmail.com","avatar":"https://avatars.githubusercontent.com/u/116324?v=4"},"body":"> On Mon, 16 Jul 2018, Junio C Hamano wrote:\n> \n> > Jeff King writes:\n> > \n> > > None of which is too surprising. The root of the bug is in the\n> > > conversion to rebase--helper, I think, when presumably we started\n> > > setting GIT_DIR at all (but I didn't dig further). Then 09d7b6c6fa fixed\n> > > _one_ fallout of that, which was relative paths, but didn't help the\n> > > subdirectory case.\n> > >\n> > > Just reading over this thread, I suspect the simplest fix is to pass\n> > > GIT_DIR and GIT_WORK_TREE together, which is almost always the right\n> > > thing to do.\n> > \n> > Perhaps. Not exporting GIT_DIR (unless the end-user already did to\n> > the environment before starting \"git rebase\"---it would be a bad\n> > change to unexport it unconditionally) may probably be a way to make\n> > rebase--helper conversion more faithful to the original scripted\n> > Porcelain, but I suspect in practice always giving GIT_DIR and\n> > GIT_WORK_TREE would work well for many existing hooks.\n> \n> Forgetting the code in git-sh-setup, are we?\n> \n> git_dir_init() rather specifically set GIT_DIR to the absolute path, and\n> since that variable is already exported, the `exec` commands launched via\n> `git-rebase--interactive` all saw it.\n> \n> That is the reason why the sequencer.c was taught to set GIT_DIR to an\n> absolute path rathern than not setting it: for backwards compatibility.\n\nGIT_DIR was not exported to 'exec' commands during an interactive\nrebase prior to 18633e1a22 (rebase -i: use the rebase--helper builtin,\n2017-02-09) (nor was GIT_PREFIX):\n\n $ git log -Sgit_dir_init master git-rebase*.sh\n # Nothing.\n $ git checkout 18633e1a22a6^ && make -j4 prefix=/tmp/BEFORE install\n <....>\n $ git checkout 18633e1a22a6 && make -j4 prefix=/tmp/AFTER install\n <....>\n $ GIT_EDITOR='sed -i -e \"1ix set |grep ^GIT\"' /tmp/BEFORE/bin/git rebase -i HEAD^\n Executing: set |grep ^GIT\n GIT_CHERRY_PICK_HELP=$'\\nWhen you have resolved this problem, run \"git rebase --continue\".\\nIf you prefer to skip this patch, run \"git rebase --skip\" instead.\\nTo check out the original branch and stop rebasing, run \"git rebase --abort\".\\n'\n GIT_EDITOR='sed -i -e \"1ix set |grep ^GIT\"'\n GIT_INTERNAL_GETTEXT_SH_SCHEME=gnu\n GIT_REFLOG_ACTION='rebase -i (start): checkout HEAD^'\n warning: notes ref refs/notes/commits is invalid\n Successfully rebased and updated refs/heads/master.\n $ GIT_EDITOR='sed -i -e \"1ix set |grep ^GIT\"' /tmp/AFTER/bin/git rebase -i HEAD^\n Executing: set |grep ^GIT\n GIT_CHERRY_PICK_HELP='\n GIT_DIR='.git'\n GIT_EDITOR='sed -i -e \"1ix set |grep ^GIT\"'\n GIT_INTERNAL_GETTEXT_SH_SCHEME='gnu'\n GIT_PREFIX=''\n GIT_REFLOG_ACTION='rebase -i (start): checkout HEAD^'\n warning: notes ref refs/notes/commits is invalid\n Successfully rebased and updated refs/heads/master.\n\nAnd then recently came 226c0ddd0d (exec_cmd: RUNTIME_PREFIX on some\nPOSIX systems, 2018-04-10), which then started to export GIT_EXEC_PATH\nto 'exec' commands as well... \n\n\n"},{"id":"353098","messageId":"xmqqva9adfcf.fsf@gitster-ct.c.googlers.com","threadId":"48866","inReplyTo":"20180719220929.6550-1-szeder.dev@gmail.com","subject":"Re: rev-parse --show-toplevel broken during exec'ed rebase?","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2018-07-19T22:34:56Z","receivedAt":"2018-07-19T22:35:01Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"SZEDER Gábor writes:\n\n>> Forgetting the code in git-sh-setup, are we?\n>> \n>> git_dir_init() rather specifically set GIT_DIR to the absolute path, and\n>> since that variable is already exported, the `exec` commands launched via\n>> `git-rebase--interactive` all saw it.\n>> \n>> That is the reason why the sequencer.c was taught to set GIT_DIR to an\n>> absolute path rathern than not setting it: for backwards compatibility.\n>\n> GIT_DIR was not exported to 'exec' commands during an interactive\n> rebase prior to 18633e1a22 (rebase -i: use the rebase--helper builtin,\n> 2017-02-09) (nor was GIT_PREFIX):\n>\n> $ git log -Sgit_dir_init master git-rebase*.sh\n> # Nothing.\n> $ git checkout 18633e1a22a6^ && make -j4 prefix=/tmp/BEFORE install\n> <....>\n> $ git checkout 18633e1a22a6 && make -j4 prefix=/tmp/AFTER install\n> <....>\n> $ GIT_EDITOR='sed -i -e \"1ix set |grep ^GIT\"' /tmp/BEFORE/bin/git rebase -i HEAD^\n> Executing: set |grep ^GIT\n> GIT_CHERRY_PICK_HELP=$'\\nWhen you have resolved this problem, run \"git rebase --continue\".\\nIf you prefer to skip this patch, run \"git rebase --skip\" instead.\\nTo check out the original branch and stop rebasing, run \"git rebase --abort\".\\n'\n> GIT_EDITOR='sed -i -e \"1ix set |grep ^GIT\"'\n> GIT_INTERNAL_GETTEXT_SH_SCHEME=gnu\n> GIT_REFLOG_ACTION='rebase -i (start): checkout HEAD^'\n> warning: notes ref refs/notes/commits is invalid\n> Successfully rebased and updated refs/heads/master.\n> $ GIT_EDITOR='sed -i -e \"1ix set |grep ^GIT\"' /tmp/AFTER/bin/git rebase -i HEAD^\n> Executing: set |grep ^GIT\n> GIT_CHERRY_PICK_HELP='\n> GIT_DIR='.git'\n> GIT_EDITOR='sed -i -e \"1ix set |grep ^GIT\"'\n> GIT_INTERNAL_GETTEXT_SH_SCHEME='gnu'\n> GIT_PREFIX=''\n> GIT_REFLOG_ACTION='rebase -i (start): checkout HEAD^'\n> warning: notes ref refs/notes/commits is invalid\n> Successfully rebased and updated refs/heads/master.\n>\n> And then recently came 226c0ddd0d (exec_cmd: RUNTIME_PREFIX on some\n> POSIX systems, 2018-04-10), which then started to export GIT_EXEC_PATH\n> to 'exec' commands as well... \n\nCorrect. git-sh-setup does assign to GIT_DIR, but unless the\nend-user has it exported, it does not export, which has long been\nvery deliberate. But it does not really matter, as exporting BOTH\nGIT_DIR and GIT_WORK_TREE is a lot easier to understand and probably\nsafe (even though technically a regressing) solution, and fewer and\nfewer things will be relying on git-sh-setup in the future anyway.\n\n"}]}