{"thread":{"id":"51916","subject":"[PATCH 1/1] stash apply: report status correctly even in a worktree's subdirectory","startedAt":"2019-09-25T12:45:28Z","lastAt":"2019-10-04T12:31:03Z","messageCount":6,"participants":["Johannes Schindelin via GitGitGadget","Junio C Hamano","Johannes Schindelin"],"isPatch":true,"patchVersion":1,"patchTotal":1},"messages":[{"id":"382916","messageId":"a687c16b824ec9ab45d1e2c39a5470ed89153fc0.1569415522.git.gitgitgadget@gmail.com","threadId":"51916","inReplyTo":"pull.354.git.gitgitgadget@gmail.com","subject":"[PATCH 1/1] stash apply: report status correctly even in a worktree's subdirectory","fromName":"Johannes Schindelin via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2019-09-25T12:45:24Z","receivedAt":"2019-09-25T12:45:28Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"From: Johannes Schindelin <johannes.schindelin@gmx.de>\n\nWhen Git wants to spawn a child Git process inside a worktree's\nsubdirectory, we need to take care of specifying the work tree's\ntop-level directory explicitly because it cannot be discovered: the\ncurrent directory is _not_ the top-level directory of the work tree, and\nneither is it inside the parent directory of `GIT_DIR`.\n\nThis fixes the problem where `git stash apply` would report pretty much\neverything deleted or untracked when run inside a worktree's\nsubdirectory.\n\nSigned-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>\n---\n builtin/stash.c              |  2 ++\n t/t3908-stash-in-worktree.sh | 27 +++++++++++++++++++++++++++\n 2 files changed, 29 insertions(+)\n create mode 100755 t/t3908-stash-in-worktree.sh\n\ndiff --git a/builtin/stash.c b/builtin/stash.c\nindex b5a301f24d..a1e2e7ae7e 100644\n--- a/builtin/stash.c\n+++ b/builtin/stash.c\n@@ -497,6 +497,8 @@ static int do_apply_stash(const char *prefix, struct stash_info *info,\n \t\t */\n \t\tcp.git_cmd = 1;\n \t\tcp.dir = prefix;\n+\t\targv_array_pushf(&cp.env_array, GIT_WORK_TREE_ENVIRONMENT\"=%s\",\n+\t\t\t\t absolute_path(get_git_work_tree()));\n \t\targv_array_push(&cp.args, \"status\");\n \t\trun_command(&cp);\n \t}\ndiff --git a/t/t3908-stash-in-worktree.sh b/t/t3908-stash-in-worktree.sh\nnew file mode 100755\nindex 0000000000..2b2b366ef9\n--- /dev/null\n+++ b/t/t3908-stash-in-worktree.sh\n@@ -0,0 +1,27 @@\n+#!/bin/sh\n+#\n+# Copyright (c) 2019 Johannes E Schindelin\n+#\n+\n+test_description='Test git stash in a worktree'\n+\n+. ./test-lib.sh\n+\n+test_expect_success 'setup' '\n+\ttest_commit initial &&\n+\tgit worktree add wt &&\n+\ttest_commit -C wt in-worktree\n+'\n+\n+test_expect_success 'apply in subdirectory' '\n+\tmkdir wt/subdir &&\n+\t(\n+\t\tcd wt/subdir &&\n+\t\techo modified >../initial.t &&\n+\t\tgit stash &&\n+\t\tgit stash apply >out\n+\t) &&\n+\tgrep \"\\.\\.\\/initial\\.t\" wt/subdir/out\n+'\n+\n+test_done\n-- \ngitgitgadget\n"},{"id":"382917","messageId":"pull.354.git.gitgitgadget@gmail.com","threadId":"51916","inReplyTo":null,"subject":"[PATCH 0/1] stash apply: be prepared to run in a worktree's subdirectory","fromName":"Johannes Schindelin via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2019-09-25T12:45:23Z","receivedAt":"2019-09-25T12:45:29Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"I saw this issue a couple times in my setup, and always wondered why nobody\nelse seemed to be hit by this. When I finally found/made some time to\ninvestigate, I found out that it really requires a specific setup: I have\nmany worktrees connected to my main git.git clone, often run inside t/ and I\ndo stash quite often (now that git stash's performance is a joy on Windows).\n\nJohannes Schindelin (1):\n  stash apply: report status correctly even in a worktree's subdirectory\n\n builtin/stash.c              |  2 ++\n t/t3908-stash-in-worktree.sh | 27 +++++++++++++++++++++++++++\n 2 files changed, 29 insertions(+)\n create mode 100755 t/t3908-stash-in-worktree.sh\n\n\nbase-commit: 4c86140027f4a0d2caaa3ab4bd8bfc5ce3c11c8a\nPublished-As: https://github.com/gitgitgadget/git/releases/tag/pr-354%2Fdscho%2Fapply-stash-in-subdirectory-v1\nFetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-354/dscho/apply-stash-in-subdirectory-v1\nPull-Request: https://github.com/gitgitgadget/git/pull/354\n-- \ngitgitgadget\n"},{"id":"383368","messageId":"xmqqk19lbha6.fsf@gitster-ct.c.googlers.com","threadId":"51916","inReplyTo":"a687c16b824ec9ab45d1e2c39a5470ed89153fc0.1569415522.git.gitgitgadget@gmail.com","subject":"Re: [PATCH 1/1] stash apply: report status correctly even in a worktree's subdirectory","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2019-10-03T22:22:57Z","receivedAt":"2019-10-03T22:23:01Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"\"Johannes Schindelin via GitGitGadget\" <gitgitgadget@gmail.com>\nwrites:\n\n> diff --git a/builtin/stash.c b/builtin/stash.c\n> index b5a301f24d..a1e2e7ae7e 100644\n> --- a/builtin/stash.c\n> +++ b/builtin/stash.c\n> @@ -497,6 +497,8 @@ static int do_apply_stash(const char *prefix, struct stash_info *info,\n>  \t\t */\n>  \t\tcp.git_cmd = 1;\n>  \t\tcp.dir = prefix;\n> +\t\targv_array_pushf(&cp.env_array, GIT_WORK_TREE_ENVIRONMENT\"=%s\",\n> +\t\t\t\t absolute_path(get_git_work_tree()));\n>  \t\targv_array_push(&cp.args, \"status\");\n>  \t\trun_command(&cp);\n>  \t}\n\nNicely spotted.  Exporting GIT_WORK_TREE alone without GIT_DIR feels\na bit disturbing, at least to me, though.\n\nI wondered if this misbehaves when the end user has GIT_WORK_TREE\nenvironment exported, but in such a case, get_git_work_tree() would\nreturn that directory, and by re-exporting it to the child process,\nwe would honor the end user's intention, so all is good, I think.\n\nThanks.\n\n> diff --git a/t/t3908-stash-in-worktree.sh b/t/t3908-stash-in-worktree.sh\n> new file mode 100755\n> index 0000000000..2b2b366ef9\n> --- /dev/null\n> +++ b/t/t3908-stash-in-worktree.sh\n> @@ -0,0 +1,27 @@\n> +#!/bin/sh\n> +#\n> +# Copyright (c) 2019 Johannes E Schindelin\n> +#\n> +\n> +test_description='Test git stash in a worktree'\n> +\n> +. ./test-lib.sh\n> +\n> +test_expect_success 'setup' '\n> +\ttest_commit initial &&\n> +\tgit worktree add wt &&\n> +\ttest_commit -C wt in-worktree\n> +'\n> +\n> +test_expect_success 'apply in subdirectory' '\n> +\tmkdir wt/subdir &&\n> +\t(\n> +\t\tcd wt/subdir &&\n> +\t\techo modified >../initial.t &&\n> +\t\tgit stash &&\n> +\t\tgit stash apply >out\n> +\t) &&\n> +\tgrep \"\\.\\.\\/initial\\.t\" wt/subdir/out\n> +'\n> +\n> +test_done\n"},{"id":"383404","messageId":"nycvar.QRO.7.76.6.1910041118380.46@tvgsbejvaqbjf.bet","threadId":"51916","inReplyTo":"xmqqk19lbha6.fsf@gitster-ct.c.googlers.com","subject":"Re: [PATCH 1/1] stash apply: report status correctly even in a worktree's subdirectory","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2019-10-04T09:19:59Z","receivedAt":"2019-10-04T09:20:20Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi Junio,\n\nOn Fri, 4 Oct 2019, Junio C Hamano wrote:\n\n> \"Johannes Schindelin via GitGitGadget\" <gitgitgadget@gmail.com>\n> writes:\n>\n> > diff --git a/builtin/stash.c b/builtin/stash.c\n> > index b5a301f24d..a1e2e7ae7e 100644\n> > --- a/builtin/stash.c\n> > +++ b/builtin/stash.c\n> > @@ -497,6 +497,8 @@ static int do_apply_stash(const char *prefix, struct stash_info *info,\n> >  \t\t */\n> >  \t\tcp.git_cmd = 1;\n> >  \t\tcp.dir = prefix;\n> > +\t\targv_array_pushf(&cp.env_array, GIT_WORK_TREE_ENVIRONMENT\"=%s\",\n> > +\t\t\t\t absolute_path(get_git_work_tree()));\n> >  \t\targv_array_push(&cp.args, \"status\");\n> >  \t\trun_command(&cp);\n> >  \t}\n>\n> Nicely spotted.  Exporting GIT_WORK_TREE alone without GIT_DIR feels\n> a bit disturbing, at least to me, though.\n\nYou're absolutely right! Of course, the problem this patch fixes is\n_because_ `GIT_DIR` is defined, but I should make sure that I don't\nintroduce the reverse problem ;-)\n\n> I wondered if this misbehaves when the end user has GIT_WORK_TREE\n> environment exported, but in such a case, get_git_work_tree() would\n> return that directory, and by re-exporting it to the child process,\n> we would honor the end user's intention, so all is good, I think.\n\nYes, that'd my understanding, too. It won't hurt to re-define that\nvariable, but it hurts not to define it at all.\n\nCiao,\nDscho\n\n>\n> Thanks.\n>\n> > diff --git a/t/t3908-stash-in-worktree.sh b/t/t3908-stash-in-worktree.sh\n> > new file mode 100755\n> > index 0000000000..2b2b366ef9\n> > --- /dev/null\n> > +++ b/t/t3908-stash-in-worktree.sh\n> > @@ -0,0 +1,27 @@\n> > +#!/bin/sh\n> > +#\n> > +# Copyright (c) 2019 Johannes E Schindelin\n> > +#\n> > +\n> > +test_description='Test git stash in a worktree'\n> > +\n> > +. ./test-lib.sh\n> > +\n> > +test_expect_success 'setup' '\n> > +\ttest_commit initial &&\n> > +\tgit worktree add wt &&\n> > +\ttest_commit -C wt in-worktree\n> > +'\n> > +\n> > +test_expect_success 'apply in subdirectory' '\n> > +\tmkdir wt/subdir &&\n> > +\t(\n> > +\t\tcd wt/subdir &&\n> > +\t\techo modified >../initial.t &&\n> > +\t\tgit stash &&\n> > +\t\tgit stash apply >out\n> > +\t) &&\n> > +\tgrep \"\\.\\.\\/initial\\.t\" wt/subdir/out\n> > +'\n> > +\n> > +test_done\n>\n"},{"id":"383417","messageId":"pull.354.v2.git.gitgitgadget@gmail.com","threadId":"51916","inReplyTo":"pull.354.git.gitgitgadget@gmail.com","subject":"[PATCH v2 0/1] stash apply: be prepared to run in a worktree's subdirectory","fromName":"Johannes Schindelin via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2019-10-04T12:30:58Z","receivedAt":"2019-10-04T12:31:02Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"I saw this issue a couple times in my setup, and always wondered why nobody\nelse seemed to be hit by this. When I finally found/made some time to\ninvestigate, I found out that it really requires a specific setup: I have\nmany worktrees connected to my main git.git clone, often run inside t/ and I\ndo stash quite often (now that git stash's performance is a joy on Windows).\n\nChanges since v1:\n\n * We now make sure to also set GIT_DIR.\n\nJohannes Schindelin (1):\n  stash apply: report status correctly even in a worktree's subdirectory\n\n builtin/stash.c              |  4 ++++\n t/t3908-stash-in-worktree.sh | 27 +++++++++++++++++++++++++++\n 2 files changed, 31 insertions(+)\n create mode 100755 t/t3908-stash-in-worktree.sh\n\n\nbase-commit: 4c86140027f4a0d2caaa3ab4bd8bfc5ce3c11c8a\nPublished-As: https://github.com/gitgitgadget/git/releases/tag/pr-354%2Fdscho%2Fapply-stash-in-subdirectory-v2\nFetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-354/dscho/apply-stash-in-subdirectory-v2\nPull-Request: https://github.com/gitgitgadget/git/pull/354\n\nRange-diff vs v1:\n\n 1:  a687c16b82 ! 1:  4e19436fbf stash apply: report status correctly even in a worktree's subdirectory\n     @@ -3,15 +3,20 @@\n          stash apply: report status correctly even in a worktree's subdirectory\n      \n          When Git wants to spawn a child Git process inside a worktree's\n     -    subdirectory, we need to take care of specifying the work tree's\n     -    top-level directory explicitly because it cannot be discovered: the\n     -    current directory is _not_ the top-level directory of the work tree, and\n     -    neither is it inside the parent directory of `GIT_DIR`.\n     +    subdirectory while `GIT_DIR` is set, we need to take care of specifying\n     +    the work tree's top-level directory explicitly because it cannot be\n     +    discovered: the current directory is _not_ the top-level directory of\n     +    the work tree, and neither is it inside the parent directory of\n     +    `GIT_DIR`.\n      \n          This fixes the problem where `git stash apply` would report pretty much\n          everything deleted or untracked when run inside a worktree's\n          subdirectory.\n      \n     +    To make sure that we do not introduce the \"reverse problem\", i.e. when\n     +    `GIT_WORK_TREE` is defined but `GIT_DIR` is not, we simply make sure\n     +    that both are set.\n     +\n          Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>\n      \n       diff --git a/builtin/stash.c b/builtin/stash.c\n     @@ -23,6 +28,8 @@\n       \t\tcp.dir = prefix;\n      +\t\targv_array_pushf(&cp.env_array, GIT_WORK_TREE_ENVIRONMENT\"=%s\",\n      +\t\t\t\t absolute_path(get_git_work_tree()));\n     ++\t\targv_array_pushf(&cp.env_array, GIT_DIR_ENVIRONMENT\"=%s\",\n     ++\t\t\t\t absolute_path(get_git_dir()));\n       \t\targv_array_push(&cp.args, \"status\");\n       \t\trun_command(&cp);\n       \t}\n\n-- \ngitgitgadget\n"},{"id":"383418","messageId":"4e19436fbfbc923df6ab26477edfbacf9eab0ead.1570192257.git.gitgitgadget@gmail.com","threadId":"51916","inReplyTo":"pull.354.v2.git.gitgitgadget@gmail.com","subject":"[PATCH v2 1/1] stash apply: report status correctly even in a worktree's subdirectory","fromName":"Johannes Schindelin via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2019-10-04T12:30:59Z","receivedAt":"2019-10-04T12:31:03Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"From: Johannes Schindelin <johannes.schindelin@gmx.de>\n\nWhen Git wants to spawn a child Git process inside a worktree's\nsubdirectory while `GIT_DIR` is set, we need to take care of specifying\nthe work tree's top-level directory explicitly because it cannot be\ndiscovered: the current directory is _not_ the top-level directory of\nthe work tree, and neither is it inside the parent directory of\n`GIT_DIR`.\n\nThis fixes the problem where `git stash apply` would report pretty much\neverything deleted or untracked when run inside a worktree's\nsubdirectory.\n\nTo make sure that we do not introduce the \"reverse problem\", i.e. when\n`GIT_WORK_TREE` is defined but `GIT_DIR` is not, we simply make sure\nthat both are set.\n\nSigned-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>\n---\n builtin/stash.c              |  4 ++++\n t/t3908-stash-in-worktree.sh | 27 +++++++++++++++++++++++++++\n 2 files changed, 31 insertions(+)\n create mode 100755 t/t3908-stash-in-worktree.sh\n\ndiff --git a/builtin/stash.c b/builtin/stash.c\nindex b5a301f24d..b108f099fa 100644\n--- a/builtin/stash.c\n+++ b/builtin/stash.c\n@@ -497,6 +497,10 @@ static int do_apply_stash(const char *prefix, struct stash_info *info,\n \t\t */\n \t\tcp.git_cmd = 1;\n \t\tcp.dir = prefix;\n+\t\targv_array_pushf(&cp.env_array, GIT_WORK_TREE_ENVIRONMENT\"=%s\",\n+\t\t\t\t absolute_path(get_git_work_tree()));\n+\t\targv_array_pushf(&cp.env_array, GIT_DIR_ENVIRONMENT\"=%s\",\n+\t\t\t\t absolute_path(get_git_dir()));\n \t\targv_array_push(&cp.args, \"status\");\n \t\trun_command(&cp);\n \t}\ndiff --git a/t/t3908-stash-in-worktree.sh b/t/t3908-stash-in-worktree.sh\nnew file mode 100755\nindex 0000000000..2b2b366ef9\n--- /dev/null\n+++ b/t/t3908-stash-in-worktree.sh\n@@ -0,0 +1,27 @@\n+#!/bin/sh\n+#\n+# Copyright (c) 2019 Johannes E Schindelin\n+#\n+\n+test_description='Test git stash in a worktree'\n+\n+. ./test-lib.sh\n+\n+test_expect_success 'setup' '\n+\ttest_commit initial &&\n+\tgit worktree add wt &&\n+\ttest_commit -C wt in-worktree\n+'\n+\n+test_expect_success 'apply in subdirectory' '\n+\tmkdir wt/subdir &&\n+\t(\n+\t\tcd wt/subdir &&\n+\t\techo modified >../initial.t &&\n+\t\tgit stash &&\n+\t\tgit stash apply >out\n+\t) &&\n+\tgrep \"\\.\\.\\/initial\\.t\" wt/subdir/out\n+'\n+\n+test_done\n-- \ngitgitgadget\n"}]}