{"thread":{"id":"52656","subject":"[PATCH 0/4] checkout/reset/read-tree: fix --recurse-submodules in linked worktree","startedAt":"2020-01-17T12:23:35Z","lastAt":"2020-01-24T23:47:21Z","messageCount":24,"participants":["Philippe Blain via GitGitGadget","Derrick Stolee","Eric Sunshine","Junio C Hamano","Philippe Blain"],"isPatch":true,"patchVersion":1,"patchTotal":4},"messages":[{"id":"389968","messageId":"pull.523.git.1579263809.gitgitgadget@gmail.com","threadId":"52656","inReplyTo":null,"subject":"[PATCH 0/4] checkout/reset/read-tree: fix --recurse-submodules in linked worktree","fromName":"Philippe Blain via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2020-01-17T12:23:25Z","receivedAt":"2020-01-17T12:23:35Z","isPatch":true,"sender":{"key":"levraiphilippeblain@gmail.com","avatar":"https://avatars.githubusercontent.com/u/44212482?v=4"},"body":"This series fixes the behaviour of git checkout/reset/read-tree\n--recurse-submodules when they are called in a linked worktree (created by \ngit worktree add). \n\nAlthough submodules are cloned in $GIT_COMMON_DIR/worktrees//modules \nupon issuing git submodule update in the linked worktree, any invocation of \ngit checkout/reset/read-tree --recurse-submodules that changes the state of\nthe submodule(s) will incorrectly operate on the repositories of the\nsubmodules in the main worktree, i.e. the ones at $GIT_COMMON_DIR/modules/. \n\nThe fourth patch fixes this behaviour by using get_git_dir() instead of \ngit_common_dir() in submodule.c::submodule_move_head and \nsubmodule.c::submodule_unset_core_worktree to construct the path to the\nsubmodule repository.\n\nThe first 3 patches are clean-up patches on t7410-submodule-checkout-to.sh\n(renamed to t2405-worktree-submodule.sh) to bring it up to date.\n\nNote: in the second test added in the fourth patch I used test_might_fail \nsuch that when the test is run on the current master, only the last test_cmp \nmakes the test fail. If we want to be more strict I'll change that to :\n\ndiff --git a/t/t2405-worktree-submodule.sh b/t/t2405-worktree-submodule.sh\nindex eba17d9e35..31d156cce7 100755\n--- a/t/t2405-worktree-submodule.sh\n+++ b/t/t2405-worktree-submodule.sh\n@@ -70,9 +70,9 @@ test_expect_success 'checkout --recurse-submodules uses $GIT_DIR for submodules\n test_expect_success 'core.worktree is removed in $GIT_DIR/modules//config, not in $GIT_COMMON_DIR/modules//config' '\n git -C main/sub config --get core.worktree > expect &&\n git -C checkout-recurse checkout --recurse-submodules first &&\n- test_might_fail git -C main/.git/worktrees/checkout-recurse/modules/sub config --get core.worktree > linked-config &&\n+ test_expect_code 1 git -C main/.git/worktrees/checkout-recurse/modules/sub config --get core.worktree > linked-config &&\n test_must_be_empty linked-config &&\n- test_might_fail git -C main/sub config --get core.worktree > actual &&\n+ git -C main/sub config --get core.worktree > actual &&\n test_cmp expect actual\n '\n\nCc:Max Kirillov max@max630.net [max@max630.net] Brandon Williams \nbwilliams.eng@gmail.com [bwilliams.eng@gmail.com] Jonathan Tan \njonathantanmy@google.com [jonathantanmy@google.com] Stefan Beller \nstefanbeller@gmail.com [stefanbeller@gmail.com] Nguyễn Thái Ngọc Duy \npclouds@gmail.com [pclouds@gmail.com]\n\nPhilippe Blain (4):\n t7410: rename to t2405-worktree-submodule.sh\n t2405: use git -C and test_commit -C instead of subshells\n t2405: clarify test descriptions and simplify test\n submodule.c: use get_git_dir() instead of get_git_common_dir()\n\n submodule.c | 6 +--\n t/t2405-worktree-submodule.sh | 79 ++++++++++++++++++++++++++++++++\n t/t7410-submodule-checkout-to.sh | 77 -------------------------------\n 3 files changed, 82 insertions(+), 80 deletions(-)\n create mode 100755 t/t2405-worktree-submodule.sh\n delete mode 100755 t/t7410-submodule-checkout-to.sh\n\n\nbase-commit: b4615e40a8125477e18490d868f7b65954372b43\nPublished-As: https://github.com/gitgitgadget/git/releases/tag/pr-523%2Fphil-blain%2Fcheckout-recurse-in-linked-worktree-v1\nFetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-523/phil-blain/checkout-recurse-in-linked-worktree-v1\nPull-Request: https://github.com/gitgitgadget/git/pull/523\n-- \ngitgitgadget\n"},{"id":"389969","messageId":"ff33339690ec167640f989d989cf9b1b14b9e039.1579263809.git.gitgitgadget@gmail.com","threadId":"52656","inReplyTo":"pull.523.git.1579263809.gitgitgadget@gmail.com","subject":"[PATCH 1/4] t7410: rename to t2405-worktree-submodule.sh","fromName":"Philippe Blain via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2020-01-17T12:23:26Z","receivedAt":"2020-01-17T12:23:35Z","isPatch":true,"sender":{"key":"levraiphilippeblain@gmail.com","avatar":"https://avatars.githubusercontent.com/u/44212482?v=4"},"body":"From: Philippe Blain \n\nThis test was added in df56607dff (git-common-dir: make \"modules/\"\nper-working-directory directory, 2014-11-30), back when the 'git worktree' command\ndid not exist and 'git checkout --to' was used to create supplementary worktrees.\n\nSince this file contains tests for the interaction of 'git worktree' with\nsubmodules, rename it to t2405-worktree-submodule.sh, following the naming scheme for\ntests checking the behavior of various commands with submodules.\n\nSigned-off-by: Philippe Blain \n---\n ...410-submodule-checkout-to.sh => t2405-worktree-submodule.sh} | 2 +-\n 1 file changed, 1 insertion(+), 1 deletion(-)\n rename t/{t7410-submodule-checkout-to.sh => t2405-worktree-submodule.sh} (96%)\n\ndiff --git a/t/t7410-submodule-checkout-to.sh b/t/t2405-worktree-submodule.sh\nsimilarity index 96%\nrename from t/t7410-submodule-checkout-to.sh\nrename to t/t2405-worktree-submodule.sh\nindex f1b492ebc4..f2eee328cc 100755\n--- a/t/t7410-submodule-checkout-to.sh\n+++ b/t/t2405-worktree-submodule.sh\n@@ -1,6 +1,6 @@\n #!/bin/sh\n \n-test_description='Combination of submodules and multiple workdirs'\n+test_description='Combination of submodules and multiple worktrees'\n \n . ./test-lib.sh\n \n-- \ngitgitgadget\n\n"},{"id":"389970","messageId":"5060ce3d64419369b2912c395a880fb49a0a3137.1579263809.git.gitgitgadget@gmail.com","threadId":"52656","inReplyTo":"pull.523.git.1579263809.gitgitgadget@gmail.com","subject":"[PATCH 2/4] t2405: use git -C and test_commit -C instead of subshells","fromName":"Philippe Blain via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2020-01-17T12:23:27Z","receivedAt":"2020-01-17T12:23:37Z","isPatch":true,"sender":{"key":"levraiphilippeblain@gmail.com","avatar":"https://avatars.githubusercontent.com/u/44212482?v=4"},"body":"From: Philippe Blain \n\nThe subshells used in the setup phase of this test are unnecessary.\n\nRemove them by using 'git -C' and 'test_commit -C'.\n\nSigned-off-by: Philippe Blain \n---\n t/t2405-worktree-submodule.sh | 40 +++++++++++------------------------\n 1 file changed, 12 insertions(+), 28 deletions(-)\n\ndiff --git a/t/t2405-worktree-submodule.sh b/t/t2405-worktree-submodule.sh\nindex f2eee328cc..b9040c30d4 100755\n--- a/t/t2405-worktree-submodule.sh\n+++ b/t/t2405-worktree-submodule.sh\n@@ -6,32 +6,16 @@ test_description='Combination of submodules and multiple worktrees'\n \n base_path=$(pwd -P)\n \n-test_expect_success 'setup: make origin' '\n-\tmkdir -p origin/sub &&\n-\t(\n-\t\tcd origin/sub && git init &&\n-\t\techo file1 >file1 &&\n-\t\tgit add file1 &&\n-\t\tgit commit -m file1\n-\t) &&\n-\tmkdir -p origin/main &&\n-\t(\n-\t\tcd origin/main && git init &&\n-\t\tgit submodule add ../sub &&\n-\t\tgit commit -m \"add sub\"\n-\t) &&\n-\t(\n-\t\tcd origin/sub &&\n-\t\techo file1updated >file1 &&\n-\t\tgit add file1 &&\n-\t\tgit commit -m \"file1 updated\"\n-\t) &&\n+test_expect_success 'setup: create origin repos' '\n+\tgit init origin/sub &&\n+\ttest_commit -C origin/sub file1 &&\n+\tgit init origin/main &&\n+\tgit -C origin/main submodule add ../sub &&\n+\tgit -C origin/main commit -m \"add sub\" &&\n+\ttest_commit -C origin/sub \"file1-updated\" file1 file1updated &&\n \tgit -C origin/main/sub pull &&\n-\t(\n-\t\tcd origin/main &&\n-\t\tgit add sub &&\n-\t\tgit commit -m \"sub updated\"\n-\t)\n+\tgit -C origin/main add sub &&\n+\tgit -C origin/main commit -m \"sub updated\"\n '\n \n test_expect_success 'setup: clone' '\n@@ -49,7 +33,7 @@ test_expect_success 'checkout main' '\n \n test_expect_failure 'can see submodule diffs just after checkout' '\n \tgit -C default_checkout/main diff --submodule master\"^!\" >out &&\n-\tgrep \"file1 updated\" out\n+\tgrep \"file1-updated\" out\n '\n \n test_expect_success 'checkout main and initialize independent clones' '\n@@ -60,7 +44,7 @@ test_expect_success 'checkout main and initialize independent clones' '\n \n test_expect_success 'can see submodule diffs after independent cloning' '\n \tgit -C fully_cloned_submodule/main diff --submodule master\"^!\" >out &&\n-\tgrep \"file1 updated\" out\n+\tgrep \"file1-updated\" out\n '\n \n test_expect_success 'checkout sub manually' '\n@@ -71,7 +55,7 @@ test_expect_success 'checkout sub manually' '\n \n test_expect_success 'can see submodule diffs after manual checkout of linked submodule' '\n \tgit -C linked_submodule/main diff --submodule master\"^!\" >out &&\n-\tgrep \"file1 updated\" out\n+\tgrep \"file1-updated\" out\n '\n \n test_done\n-- \ngitgitgadget\n\n"},{"id":"389971","messageId":"6e0e664026757315a337dead8079167f4fb29c4b.1579263809.git.gitgitgadget@gmail.com","threadId":"52656","inReplyTo":"pull.523.git.1579263809.gitgitgadget@gmail.com","subject":"[PATCH 3/4] t2405: clarify test descriptions and simplify test","fromName":"Philippe Blain via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2020-01-17T12:23:28Z","receivedAt":"2020-01-17T12:23:39Z","isPatch":true,"sender":{"key":"levraiphilippeblain@gmail.com","avatar":"https://avatars.githubusercontent.com/u/44212482?v=4"},"body":"From: Philippe Blain \n\nWhen 'checkout --to' functionality was moved to 'worktree add', tests were adapted\nin f194b1ef6e (tests: worktree: retrofit \"checkout --to\" tests for \"worktree add\",\n2015-07-06).\n\nThe calls were changed to 'worktree add' in this test (then t7410), but the test\ndescriptions were not updated, keeping 'checkout' instead of using the new\nterminology (linked worktrees).\n\nClarify the tests by using the right terminology. While at it, remove some unnecessary\nleading directories such that all superproject worktrees are directly next to one\nanother in the trash directory.\n\nSigned-off-by: Philippe Blain \n---\n t/t2405-worktree-submodule.sh | 36 ++++++++++++++++-------------------\n 1 file changed, 16 insertions(+), 20 deletions(-)\n\ndiff --git a/t/t2405-worktree-submodule.sh b/t/t2405-worktree-submodule.sh\nindex b9040c30d4..f1952c70dd 100755\n--- a/t/t2405-worktree-submodule.sh\n+++ b/t/t2405-worktree-submodule.sh\n@@ -18,43 +18,39 @@ test_expect_success 'setup: create origin repos' '\n \tgit -C origin/main commit -m \"sub updated\"\n '\n \n-test_expect_success 'setup: clone' '\n-\tmkdir clone &&\n-\tgit -C clone clone --recursive \"$base_path/origin/main\"\n+test_expect_success 'setup: clone superproject to create main worktree' '\n+\tgit clone --recursive \"$base_path/origin/main\" main\n '\n \n rev1_hash_main=$(git --git-dir=origin/main/.git show --pretty=format:%h -q \"HEAD~1\")\n rev1_hash_sub=$(git --git-dir=origin/sub/.git show --pretty=format:%h -q \"HEAD~1\")\n \n-test_expect_success 'checkout main' '\n-\tmkdir default_checkout &&\n-\tgit -C clone/main worktree add \"$base_path/default_checkout/main\" \"$rev1_hash_main\"\n+test_expect_success 'add superproject worktree' '\n+\tgit -C main worktree add \"$base_path/worktree\" \"$rev1_hash_main\"\n '\n \n-test_expect_failure 'can see submodule diffs just after checkout' '\n-\tgit -C default_checkout/main diff --submodule master\"^!\" >out &&\n+test_expect_failure 'submodule is checked out just after worktree add' '\n+\tgit -C worktree diff --submodule master\"^!\" >out &&\n \tgrep \"file1-updated\" out\n '\n \n-test_expect_success 'checkout main and initialize independent clones' '\n-\tmkdir fully_cloned_submodule &&\n-\tgit -C clone/main worktree add \"$base_path/fully_cloned_submodule/main\" \"$rev1_hash_main\" &&\n-\tgit -C fully_cloned_submodule/main submodule update\n+test_expect_success 'add superproject worktree and initialize submodules' '\n+\tgit -C main worktree add \"$base_path/worktree-submodule-update\" \"$rev1_hash_main\" &&\n+\tgit -C worktree-submodule-update submodule update\n '\n \n-test_expect_success 'can see submodule diffs after independent cloning' '\n-\tgit -C fully_cloned_submodule/main diff --submodule master\"^!\" >out &&\n+test_expect_success 'submodule is checked out just after submodule update in linked worktree' '\n+\tgit -C worktree-submodule-update diff --submodule master\"^!\" >out &&\n \tgrep \"file1-updated\" out\n '\n \n-test_expect_success 'checkout sub manually' '\n-\tmkdir linked_submodule &&\n-\tgit -C clone/main worktree add \"$base_path/linked_submodule/main\" \"$rev1_hash_main\" &&\n-\tgit -C clone/main/sub worktree add \"$base_path/linked_submodule/main/sub\" \"$rev1_hash_sub\"\n+test_expect_success 'add superproject worktree and manually add submodule worktree' '\n+\tgit -C main worktree add \"$base_path/linked_submodule\" \"$rev1_hash_main\" &&\n+\tgit -C main/sub worktree add \"$base_path/linked_submodule/sub\" \"$rev1_hash_sub\"\n '\n \n-test_expect_success 'can see submodule diffs after manual checkout of linked submodule' '\n-\tgit -C linked_submodule/main diff --submodule master\"^!\" >out &&\n+test_expect_success 'submodule is checked out after manually adding submodule worktree' '\n+\tgit -C linked_submodule diff --submodule master\"^!\" >out &&\n \tgrep \"file1-updated\" out\n '\n \n-- \ngitgitgadget\n\n"},{"id":"389972","messageId":"72cdb2f95d8c03c7b0324e8132e04e3a10248432.1579263809.git.gitgitgadget@gmail.com","threadId":"52656","inReplyTo":"pull.523.git.1579263809.gitgitgadget@gmail.com","subject":"[PATCH 4/4] submodule.c: use get_git_dir() instead of get_git_common_dir()","fromName":"Philippe Blain via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2020-01-17T12:23:29Z","receivedAt":"2020-01-17T12:23:40Z","isPatch":true,"sender":{"key":"levraiphilippeblain@gmail.com","avatar":"https://avatars.githubusercontent.com/u/44212482?v=4"},"body":"From: Philippe Blain \n\nEver since df56607dff (git-common-dir: make \"modules/\"\nper-working-directory directory, 2014-11-30), submodules in linked worktrees\nare cloned to $GIT_DIR/modules, i.e. $GIT_COMMON_DIR/worktrees//modules.\n\nHowever, this convention was not followed when the worktree updater commands\ncheckout, reset and read-tree learned to recurse into submodules. Specifically,\nsubmodule.c::submodule_move_head, introduced in 6e3c1595c6 (update submodules:\nadd submodule_move_head, 2017-03-14) and submodule.c::submodule_unset_core_worktree,\n(re)introduced in 898c2e65b7 (submodule: unset core.worktree if no working tree\nis present, 2018-12-14) use get_git_common_dir() instead of get_git_dir()\nto get the path of the submodule repository.\n\nThis means that, for example, 'git checkout --recurse-submodules '\nin a linked worktree will correctly checkout , detach the submodule's HEAD\nat the commit recorded in and update the submodule working tree, but the\nsubmodule HEAD that will be moved is the one in $GIT_COMMON_DIR/modules//,\ni.e. the submodule repository of the main superproject working tree.\nIt will also rewrite the gitfile in the submodule working tree of the linked worktree\nto point to $GIT_COMMON_DIR/modules//.\nThis leads to an incorrect (and confusing!) state in the submodule working tree\nof the main superproject worktree.\n\nAdditionnally, if switching to a commit where the submodule is not present,\nsubmodule_unset_core_worktree will be called and will incorrectly remove\n'core.wortree' from the config file of the submodule in the main superproject worktree,\n$GIT_COMMON_DIR/modules//config.\n\nFix this by constructing the path to the submodule repository using get_git_dir()\nin both submodule_move_head and submodule_unset_core_worktree.\n\nSigned-off-by: Philippe Blain \n---\n submodule.c | 6 +++---\n t/t2405-worktree-submodule.sh | 22 ++++++++++++++++++++++\n 2 files changed, 25 insertions(+), 3 deletions(-)\n\ndiff --git a/submodule.c b/submodule.c\nindex 9da7181321..5d19ec48a6 100644\n--- a/submodule.c\n+++ b/submodule.c\n@@ -1811,7 +1811,7 @@ int bad_to_remove_submodule(const char *path, unsigned flags)\n void submodule_unset_core_worktree(const struct submodule *sub)\n {\n \tchar *config_path = xstrfmt(\"%s/modules/%s/config\",\n-\t\t\t\t get_git_common_dir(), sub->name);\n+\t\t\t\t get_git_dir(), sub->name);\n \n \tif (git_config_set_in_file_gently(config_path, \"core.worktree\", NULL))\n \t\twarning(_(\"Could not unset core.worktree setting in submodule '%s'\"),\n@@ -1914,7 +1914,7 @@ int submodule_move_head(const char *path,\n \t\t\t\t\tABSORB_GITDIR_RECURSE_SUBMODULES);\n \t\t} else {\n \t\t\tchar *gitdir = xstrfmt(\"%s/modules/%s\",\n-\t\t\t\t get_git_common_dir(), sub->name);\n+\t\t\t\t get_git_dir(), sub->name);\n \t\t\tconnect_work_tree_and_git_dir(path, gitdir, 0);\n \t\t\tfree(gitdir);\n \n@@ -1924,7 +1924,7 @@ int submodule_move_head(const char *path,\n \n \t\tif (old_head && (flags & SUBMODULE_MOVE_HEAD_FORCE)) {\n \t\t\tchar *gitdir = xstrfmt(\"%s/modules/%s\",\n-\t\t\t\t get_git_common_dir(), sub->name);\n+\t\t\t\t get_git_dir(), sub->name);\n \t\t\tconnect_work_tree_and_git_dir(path, gitdir, 1);\n \t\t\tfree(gitdir);\n \t\t}\ndiff --git a/t/t2405-worktree-submodule.sh b/t/t2405-worktree-submodule.sh\nindex f1952c70dd..eba17d9e35 100755\n--- a/t/t2405-worktree-submodule.sh\n+++ b/t/t2405-worktree-submodule.sh\n@@ -10,6 +10,7 @@ test_expect_success 'setup: create origin repos' '\n \tgit init origin/sub &&\n \ttest_commit -C origin/sub file1 &&\n \tgit init origin/main &&\n+\ttest_commit -C origin/main first &&\n \tgit -C origin/main submodule add ../sub &&\n \tgit -C origin/main commit -m \"add sub\" &&\n \ttest_commit -C origin/sub \"file1-updated\" file1 file1updated &&\n@@ -54,4 +55,25 @@ test_expect_success 'submodule is checked out after manually adding submodule wo\n \tgrep \"file1-updated\" out\n '\n \n+test_expect_success 'checkout --recurse-submodules uses $GIT_DIR for submodules in a linked worktree' '\n+\tgit -C main worktree add \"$base_path/checkout-recurse\" --detach &&\n+\tgit -C checkout-recurse submodule update --init &&\n+\tcat checkout-recurse/sub/.git > expect-gitfile &&\n+\tgit -C main/sub rev-parse HEAD > expect-head-main &&\n+\tgit -C checkout-recurse checkout --recurse-submodules HEAD~1 &&\n+\tcat checkout-recurse/sub/.git > actual-gitfile &&\n+\tgit -C main/sub rev-parse HEAD > actual-head-main &&\n+\ttest_cmp expect-gitfile actual-gitfile &&\n+\ttest_cmp expect-head-main actual-head-main\n+'\n+\n+test_expect_success 'core.worktree is removed in $GIT_DIR/modules//config, not in $GIT_COMMON_DIR/modules//config' '\n+\tgit -C main/sub config --get core.worktree > expect &&\n+\tgit -C checkout-recurse checkout --recurse-submodules first &&\n+\ttest_might_fail git -C main/.git/worktrees/checkout-recurse/modules/sub config --get core.worktree > linked-config &&\n+\ttest_must_be_empty linked-config &&\n+\ttest_might_fail git -C main/sub config --get core.worktree > actual &&\n+\ttest_cmp expect actual\n+'\n+\n test_done\n-- \ngitgitgadget\n"},{"id":"389975","messageId":"b97f7dc2-9cc9-9665-e00e-1cbb8b0996e4@gmail.com","threadId":"52656","inReplyTo":"72cdb2f95d8c03c7b0324e8132e04e3a10248432.1579263809.git.gitgitgadget@gmail.com","subject":"Re: [PATCH 4/4] submodule.c: use get_git_dir() instead of get_git_common_dir()","fromName":"Derrick Stolee","fromEmail":"stolee@gmail.com","sentAt":"2020-01-17T13:24:22Z","receivedAt":"2020-01-17T13:24:26Z","isPatch":true,"sender":{"key":"stolee@gmail.com","avatar":"https://avatars.githubusercontent.com/u/570044?v=4"},"body":"On 1/17/2020 7:23 AM, Philippe Blain via GitGitGadget wrote:\n> From: Philippe Blain \n> \n> Ever since df56607dff (git-common-dir: make \"modules/\"\n> per-working-directory directory, 2014-11-30), submodules in linked worktrees\n> are cloned to $GIT_DIR/modules, i.e. $GIT_COMMON_DIR/worktrees//modules.\n> \n> However, this convention was not followed when the worktree updater commands\n> checkout, reset and read-tree learned to recurse into submodules. Specifically,\n> submodule.c::submodule_move_head, introduced in 6e3c1595c6 (update submodules:\n> add submodule_move_head, 2017-03-14) and submodule.c::submodule_unset_core_worktree,\n> (re)introduced in 898c2e65b7 (submodule: unset core.worktree if no working tree\n> is present, 2018-12-14) use get_git_common_dir() instead of get_git_dir()\n> to get the path of the submodule repository.\n> \n> This means that, for example, 'git checkout --recurse-submodules '\n> in a linked worktree will correctly checkout , detach the submodule's HEAD\n> at the commit recorded in and update the submodule working tree, but the\n> submodule HEAD that will be moved is the one in $GIT_COMMON_DIR/modules//,\n> i.e. the submodule repository of the main superproject working tree.\n> It will also rewrite the gitfile in the submodule working tree of the linked worktree\n> to point to $GIT_COMMON_DIR/modules//.\n> This leads to an incorrect (and confusing!) state in the submodule working tree\n> of the main superproject worktree.\n> \n> Additionnally, if switching to a commit where the submodule is not present,\n\ns/Additionnally/Additionally\n\n> submodule_unset_core_worktree will be called and will incorrectly remove\n> 'core.wortree' from the config file of the submodule in the main superproject worktree,\n> $GIT_COMMON_DIR/modules//config.\n>\n> Fix this by constructing the path to the submodule repository using get_git_dir()\n> in both submodule_move_head and submodule_unset_core_worktree.\n> \n> Signed-off-by: Philippe Blain \n> ---\n> submodule.c | 6 +++---\n> t/t2405-worktree-submodule.sh | 22 ++++++++++++++++++++++\n> 2 files changed, 25 insertions(+), 3 deletions(-)\n> \n> diff --git a/submodule.c b/submodule.c\n> index 9da7181321..5d19ec48a6 100644\n> --- a/submodule.c\n> +++ b/submodule.c\n> @@ -1811,7 +1811,7 @@ int bad_to_remove_submodule(const char *path, unsigned flags)\n> void submodule_unset_core_worktree(const struct submodule *sub)\n> {\n> \tchar *config_path = xstrfmt(\"%s/modules/%s/config\",\n> -\t\t\t\t get_git_common_dir(), sub->name);\n> +\t\t\t\t get_git_dir(), sub->name);\n> \n> \tif (git_config_set_in_file_gently(config_path, \"core.worktree\", NULL))\n> \t\twarning(_(\"Could not unset core.worktree setting in submodule '%s'\"),\n> @@ -1914,7 +1914,7 @@ int submodule_move_head(const char *path,\n> \t\t\t\t\tABSORB_GITDIR_RECURSE_SUBMODULES);\n> \t\t} else {\n> \t\t\tchar *gitdir = xstrfmt(\"%s/modules/%s\",\n> -\t\t\t\t get_git_common_dir(), sub->name);\n> +\t\t\t\t get_git_dir(), sub->name);\n> \t\t\tconnect_work_tree_and_git_dir(path, gitdir, 0);\n> \t\t\tfree(gitdir);\n> \n> @@ -1924,7 +1924,7 @@ int submodule_move_head(const char *path,\n> \n> \t\tif (old_head && (flags & SUBMODULE_MOVE_HEAD_FORCE)) {\n> \t\t\tchar *gitdir = xstrfmt(\"%s/modules/%s\",\n> -\t\t\t\t get_git_common_dir(), sub->name);\n> +\t\t\t\t get_git_dir(), sub->name);\n> \t\t\tconnect_work_tree_and_git_dir(path, gitdir, 1);\n> \t\t\tfree(gitdir);\n> \t\t}\n> diff --git a/t/t2405-worktree-submodule.sh b/t/t2405-worktree-submodule.sh\n> index f1952c70dd..eba17d9e35 100755\n> --- a/t/t2405-worktree-submodule.sh\n> +++ b/t/t2405-worktree-submodule.sh\n> @@ -10,6 +10,7 @@ test_expect_success 'setup: create origin repos' '\n> \tgit init origin/sub &&\n> \ttest_commit -C origin/sub file1 &&\n> \tgit init origin/main &&\n> +\ttest_commit -C origin/main first &&\n> \tgit -C origin/main submodule add ../sub &&\n> \tgit -C origin/main commit -m \"add sub\" &&\n> \ttest_commit -C origin/sub \"file1-updated\" file1 file1updated &&\n> @@ -54,4 +55,25 @@ test_expect_success 'submodule is checked out after manually adding submodule wo\n> \tgrep \"file1-updated\" out\n> '\n> \n> +test_expect_success 'checkout --recurse-submodules uses $GIT_DIR for submodules in a linked worktree' '\n> +\tgit -C main worktree add \"$base_path/checkout-recurse\" --detach &&\n> +\tgit -C checkout-recurse submodule update --init &&\n> +\tcat checkout-recurse/sub/.git > expect-gitfile &&\n\nHere, and the rest of these tests, please drop the space between the \">\" and\nthe output file: \">expect-gitfile\".\n\n> +\tgit -C main/sub rev-parse HEAD > expect-head-main &&\n> +\tgit -C checkout-recurse checkout --recurse-submodules HEAD~1 &&\n> +\tcat checkout-recurse/sub/.git > actual-gitfile &&\n> +\tgit -C main/sub rev-parse HEAD > actual-head-main &&\n> +\ttest_cmp expect-gitfile actual-gitfile &&\n> +\ttest_cmp expect-head-main actual-head-main\n> +'\n> +\n> +test_expect_success 'core.worktree is removed in $GIT_DIR/modules//config, not in $GIT_COMMON_DIR/modules//config' '\n> +\tgit -C main/sub config --get core.worktree > expect &&\n> +\tgit -C checkout-recurse checkout --recurse-submodules first &&\n> +\ttest_might_fail git -C main/.git/worktrees/checkout-recurse/modules/sub config --get core.worktree > linked-config &&\n\nWhy test_might_fail here, and below? Because the config may not exist, which\nwould return an error code? Should we _expect_ that failure with test_must_fail?\n\n> +\ttest_must_be_empty linked-config &&\n> +\ttest_might_fail git -C main/sub config --get core.worktree > actual &&\n> +\ttest_cmp expect actual\n\nThis tests that core.wortkree didn't change throughout the process, but\ncould we instead confirm an exact value by echoing into \"expect\" and\ncomparing both config outputs against that value?\n\nPerhaps it is worth checking the success of the command that was failing\nin submodules that still had core.worktree=true before 898c2e6? For your\ntest, it would be:\n\n\tgit -C main/.git/worktrees/checkout-recurse/modules/sub log\n\nThanks,\n-Stolee\n"},{"id":"389976","messageId":"e5863bce-6038-91cc-36f6-98b1df20f33a@gmail.com","threadId":"52656","inReplyTo":"pull.523.git.1579263809.gitgitgadget@gmail.com","subject":"Re: [PATCH 0/4] checkout/reset/read-tree: fix --recurse-submodules in linked worktree","fromName":"Derrick Stolee","fromEmail":"stolee@gmail.com","sentAt":"2020-01-17T13:24:52Z","receivedAt":"2020-01-17T13:24:56Z","isPatch":true,"sender":{"key":"stolee@gmail.com","avatar":"https://avatars.githubusercontent.com/u/570044?v=4"},"body":"On 1/17/2020 7:23 AM, Philippe Blain via GitGitGadget wrote:\n> The first 3 patches are clean-up patches on t7410-submodule-checkout-to.sh\n> (renamed to t2405-worktree-submodule.sh) to bring it up to date.\n\nThese cleanups make sense to me.\n\nThanks,\n-Stolee\n"},{"id":"389982","messageId":"CAPig+cQgh+gmrDwSqJz1hHViBxTP6UDkuHjKTRLbackcxeQE9A@mail.gmail.com","threadId":"52656","inReplyTo":"5060ce3d64419369b2912c395a880fb49a0a3137.1579263809.git.gitgitgadget@gmail.com","subject":"Re: [PATCH 2/4] t2405: use git -C and test_commit -C instead of subshells","fromName":"Eric Sunshine","fromEmail":"sunshine@sunshineco.com","sentAt":"2020-01-17T13:45:12Z","receivedAt":"2020-01-17T13:45:26Z","isPatch":true,"sender":{"key":"sunshine@sunshineco.com","avatar":"https://avatars.githubusercontent.com/u/163641?v=4"},"body":"On Fri, Jan 17, 2020 at 7:24 AM Philippe Blain via GitGitGadget\n wrote:\n> The subshells used in the setup phase of this test are unnecessary.\n> Remove them by using 'git -C' and 'test_commit -C'.\n\nThe subshells may not be necessary, but the code feels cleaner before\nthis patch is applied since all the added \"-C foo/bar\" noise hurts\nreadability. So, I'm \"meh\" on this patch and wouldn't complain if it\nwas dropped (though I don't insist upon it).\n\n> Signed-off-by: Philippe Blain \n> ---\n> diff --git a/t/t2405-worktree-submodule.sh b/t/t2405-worktree-submodule.sh\n> @@ -6,32 +6,16 @@ test_description='Combination of submodules and multiple worktrees'\n> - git commit -m \"file1 updated\"\n> + git -C origin/main commit -m \"add sub\" &&\n> + test_commit -C origin/sub \"file1-updated\" file1 file1updated &&\n> @@ -49,7 +33,7 @@ test_expect_success 'checkout main' '\n> - grep \"file1 updated\" out\n> + grep \"file1-updated\" out\n\nWhy this change? Is it because test_commit() mishandles the whitespace\nin the commit message? If so, it might deserve mention in the commit\nmessage of this patch. (Even better would be to fix test_commit(), if\nthat is the case.)\n"},{"id":"389983","messageId":"CAPig+cR82mrG=eiOMtyd3sAAqRRL2FD_VG2+W_j15uhWyx30gA@mail.gmail.com","threadId":"52656","inReplyTo":"6e0e664026757315a337dead8079167f4fb29c4b.1579263809.git.gitgitgadget@gmail.com","subject":"Re: [PATCH 3/4] t2405: clarify test descriptions and simplify test","fromName":"Eric Sunshine","fromEmail":"sunshine@sunshineco.com","sentAt":"2020-01-17T13:56:04Z","receivedAt":"2020-01-17T13:56:18Z","isPatch":true,"sender":{"key":"sunshine@sunshineco.com","avatar":"https://avatars.githubusercontent.com/u/163641?v=4"},"body":"On Fri, Jan 17, 2020 at 7:25 AM Philippe Blain via GitGitGadget\n wrote:\n> When 'checkout --to' functionality was moved to 'worktree add', tests were adapted\n> in f194b1ef6e (tests: worktree: retrofit \"checkout --to\" tests for \"worktree add\",\n> 2015-07-06).\n>\n> The calls were changed to 'worktree add' in this test (then t7410), but the test\n> descriptions were not updated, keeping 'checkout' instead of using the new\n> terminology (linked worktrees).\n>\n> Clarify the tests by using the right terminology. While at it, remove some unnecessary\n> leading directories such that all superproject worktrees are directly next to one\n> another in the trash directory.\n\nThe unanswered questions which popped into my head when reading the\n\"While at it...\" include:\n\n Why is it desirable for the worktrees to live in this new location\n rather than their original locations?\n\n Is it safe to relocate the worktrees like this without losing some\n important aspect of the test?\n\n Why were the worktrees located as they were originally? Was there\n some hidden or not-so-obvious reason that we're overlooking? (I\n guess this is really the same as question #2.)\n"},{"id":"390012","messageId":"xmqq5zh9esdl.fsf@gitster-ct.c.googlers.com","threadId":"52656","inReplyTo":"CAPig+cQgh+gmrDwSqJz1hHViBxTP6UDkuHjKTRLbackcxeQE9A@mail.gmail.com","subject":"Re: [PATCH 2/4] t2405: use git -C and test_commit -C instead of subshells","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2020-01-17T20:32:22Z","receivedAt":"2020-01-17T20:32:27Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Eric Sunshine writes:\n\n> On Fri, Jan 17, 2020 at 7:24 AM Philippe Blain via GitGitGadget\n> wrote:\n>> The subshells used in the setup phase of this test are unnecessary.\n>> Remove them by using 'git -C' and 'test_commit -C'.\n>\n> The subshells may not be necessary, but the code feels cleaner before\n> this patch is applied since all the added \"-C foo/bar\" noise hurts\n> readability. So, I'm \"meh\" on this patch and wouldn't complain if it\n> was dropped (though I don't insist upon it).\n\nI dunno. Each of these subshells did not do much after going into\nits subdirectory, so repetition of \"-C foo/bar\" did not bother me\nthat much.\n\n>> Signed-off-by: Philippe Blain \n>> ---\n>> diff --git a/t/t2405-worktree-submodule.sh b/t/t2405-worktree-submodule.sh\n>> @@ -6,32 +6,16 @@ test_description='Combination of submodules and multiple worktrees'\n>> - git commit -m \"file1 updated\"\n>> + git -C origin/main commit -m \"add sub\" &&\n>> + test_commit -C origin/sub \"file1-updated\" file1 file1updated &&\n>> @@ -49,7 +33,7 @@ test_expect_success 'checkout main' '\n>> - grep \"file1 updated\" out\n>> + grep \"file1-updated\" out\n>\n> Why this change? Is it because test_commit() mishandles the whitespace\n> in the commit message? If so, it might deserve mention in the commit\n> message of this patch. (Even better would be to fix test_commit(), if\n> that is the case.)\n\nFWIW I had the same reaction on that dash in \"file1 updated\".\n"},{"id":"390062","messageId":"30360E66-65B3-4669-B661-237813044041@gmail.com","threadId":"52656","inReplyTo":"CAPig+cQgh+gmrDwSqJz1hHViBxTP6UDkuHjKTRLbackcxeQE9A@mail.gmail.com","subject":"Re: [PATCH 2/4] t2405: use git -C and test_commit -C instead of subshells","fromName":"Philippe Blain","fromEmail":"levraiphilippeblain@gmail.com","sentAt":"2020-01-18T19:50:43Z","receivedAt":"2020-01-18T19:50:48Z","isPatch":true,"sender":{"key":"levraiphilippeblain@gmail.com","avatar":"https://avatars.githubusercontent.com/u/44212482?v=4"},"body":"Hi Eric,\n\n>> - grep \"file1 updated\" out\n>> + grep \"file1-updated\" out\n> \n> Why this change? Is it because test_commit() mishandles the whitespace\n> in the commit message? If so, it might deserve mention in the commit\n> message of this patch. (Even better would be to fix test_commit(), if\n> that is the case.)\n\nThe only reason is that I didn’t notice that test_commit accepts a argument, which defaults to if unset. So when I changed the test to use test_commit and ran it I got errors from ‘git tag’ saying \"file1 updated\" is not a valid tag name, so I added a dash.\n\nI’ll correct that in v2.\n\nThanks,\nPhilippe."},{"id":"390063","messageId":"D28196FE-67A1-4D03-8EFD-34ECC5633062@gmail.com","threadId":"52656","inReplyTo":"b97f7dc2-9cc9-9665-e00e-1cbb8b0996e4@gmail.com","subject":"Re: [PATCH 4/4] submodule.c: use get_git_dir() instead of get_git_common_dir()","fromName":"Philippe Blain","fromEmail":"levraiphilippeblain@gmail.com","sentAt":"2020-01-18T21:09:47Z","receivedAt":"2020-01-18T21:09:51Z","isPatch":true,"sender":{"key":"levraiphilippeblain@gmail.com","avatar":"https://avatars.githubusercontent.com/u/44212482?v=4"},"body":"Hi Stolee,\n\n> Le 17 janv. 2020 à 08:24, Derrick Stolee a écrit :\n> \n>> Additionnally, if switching to a commit where the submodule is not present,\n> \n> s/Additionnally/Additionally\nfixed.\n> \n>> +\tcat checkout-recurse/sub/.git > expect-gitfile &&\n> \n> Here, and the rest of these tests, please drop the space between the \">\" and\n> the output file: \">expect-gitfile ».\nfixed.\n> \n>> +\tgit -C main/sub rev-parse HEAD > expect-head-main &&\n>> +\tgit -C checkout-recurse checkout --recurse-submodules HEAD~1 &&\n>> +\tcat checkout-recurse/sub/.git > actual-gitfile &&\n>> +\tgit -C main/sub rev-parse HEAD > actual-head-main &&\n>> +\ttest_cmp expect-gitfile actual-gitfile &&\n>> +\ttest_cmp expect-head-main actual-head-main\n>> +'\n>> +\n>> +test_expect_success 'core.worktree is removed in $GIT_DIR/modules//config, not in $GIT_COMMON_DIR/modules//config' '\n>> +\tgit -C main/sub config --get core.worktree > expect &&\n>> +\tgit -C checkout-recurse checkout --recurse-submodules first &&\n>> +\ttest_might_fail git -C main/.git/worktrees/checkout-recurse/modules/sub config --get core.worktree > linked-config &&\n> \n> Why test_might_fail here, and below? Because the config may not exist, which\n> would return an error code? Should we _expect_ that failure with test_must_fail?\nI expected that question and answered in the cover letter (since Gitgitgadet can’t (yet, I hope) do in-patch \"timely commentaries\", [1]), but here is my answer:\nI used test_might_fail such that when the test is run on the current master, only the last test_cmp makes the test fail. If we want to be more strict I'll change that to :\n\ndiff --git a/t/t2405-worktree-submodule.sh b/t/t2405-worktree-submodule.sh\n\nindex eba17d9e35..31d156cce7 100755\n\n--- a/t/t2405-worktree-submodule.sh\n+++ b/t/t2405-worktree-submodule.sh\n@@ -70,9 +70,9 @@\ntest_expect_success 'checkout --recurse-submodules uses $GIT_DIR for submodules\ntest_expect_success 'core.worktree is removed in $GIT_DIR/modules//config, not in $GIT_COMMON_DIR/modules//config' '\n\tgit -C main/sub config --get core.worktree > expect &&\n\tgit -C checkout-recurse checkout --recurse-submodules first &&\n\n-\ttest_might_fail git -C main/.git/worktrees/checkout-recurse/modules/sub config --get core.worktree >linked-config &&\n+\ttest_expect_code 1 git -C main/.git/worktrees/checkout-recurse/modules/sub config --get core.worktree >linked-config &&\n\n\ttest_must_be_empty linked-config &&\n\n-\ttest_might_fail git -C main/sub config --get core.worktree >actual &&\n+\tgit -C main/sub config --get core.worktree >actual &&\n\n\ttest_cmp expect actual\n‘\n[1] https://github.com/gitgitgadget/gitgitgadget/issues/173\n\n> \n>> +\ttest_must_be_empty linked-config &&\n>> +\ttest_might_fail git -C main/sub config --get core.worktree > actual &&\n>> +\ttest_cmp expect actual\n> \n> This tests that core.wortkree didn't change throughout the process, but\n> could we instead confirm an exact value by echoing into \"expect\" and\n> comparing both config outputs against that value?\nGood idea, thanks. I’ll harden the test in v2.\n> \n> Perhaps it is worth checking the success of the command that was failing\n> in submodules that still had core.worktree=true before 898c2e6? For your\n> test, it would be:\n> \n> \tgit -C main/.git/worktrees/checkout-recurse/modules/sub log\nI’ll also add that.\n> \n> Thanks,\n> -Stolee\nThanks for the review!\nPhilippe.\np.s. Sorry for the re-send, I forgot to CC the list.\n\n"},{"id":"390065","messageId":"6C3CA640-EA4F-4D24-BCE3-FCEBB818026C@gmail.com","threadId":"52656","inReplyTo":"CAPig+cR82mrG=eiOMtyd3sAAqRRL2FD_VG2+W_j15uhWyx30gA@mail.gmail.com","subject":"Re: [PATCH 3/4] t2405: clarify test descriptions and simplify test","fromName":"Philippe Blain","fromEmail":"levraiphilippeblain@gmail.com","sentAt":"2020-01-19T00:21:14Z","receivedAt":"2020-01-19T00:21:18Z","isPatch":true,"sender":{"key":"levraiphilippeblain@gmail.com","avatar":"https://avatars.githubusercontent.com/u/44212482?v=4"},"body":"Hi Eric,\n> Le 17 janv. 2020 à 08:56, Eric Sunshine a écrit :\n> \n>> While at it, remove some unnecessary\n>> leading directories such that all superproject worktrees are directly next to one\n>> another in the trash directory.\n> \n> The unanswered questions which popped into my head when reading the\n> \"While at it...\" include:\n> \n> Why is it desirable for the worktrees to live in this new location\n> rather than their original locations?\nI thought it was desirable because the leading directories don’t serve any purpose apart from carrying information about what they are testing, which the worktree directory itself can do instead of all of them being called \"main\". \n> \n> Is it safe to relocate the worktrees like this without losing some\n> important aspect of the test?\nAfter analyzing the test to see what was being tested, making the change and making sure the test behaved the same way, I concluded that it was.\n> \n> Why were the worktrees located as they were originally? Was there\n> some hidden or not-so-obvious reason that we're overlooking? (I\n> guess this is really the same as question #2.)\nI don’t think there was a reason. The worktrees directories were structured that way since the test was introduced in df56607dff (git-common-dir: make \"modules/\" per-working-directory directory, 2014-11-30). Maybe one reason was for every superproject worktree to be in a directory called \"main\", just as every submodule is in a directory called \"sub\"…\n\nI can explain more this reasoning in the commit message if necessary.\n\nPhilippe."},{"id":"390066","messageId":"CAPig+cR_51+yWwvkyZFDAcmM3yV_vrtV-AoBqk2CBrmAo0r-AQ@mail.gmail.com","threadId":"52656","inReplyTo":"6C3CA640-EA4F-4D24-BCE3-FCEBB818026C@gmail.com","subject":"Re: [PATCH 3/4] t2405: clarify test descriptions and simplify test","fromName":"Eric Sunshine","fromEmail":"sunshine@sunshineco.com","sentAt":"2020-01-19T01:41:02Z","receivedAt":"2020-01-19T01:41:16Z","isPatch":true,"sender":{"key":"sunshine@sunshineco.com","avatar":"https://avatars.githubusercontent.com/u/163641?v=4"},"body":"On Sat, Jan 18, 2020 at 7:21 PM Philippe Blain\n wrote:\n> > Le 17 janv. 2020 à 08:56, Eric Sunshine a écrit :\n> > The unanswered questions which popped into my head when reading the\n> > \"While at it...\" include:\n> > [...]\n> I can explain more this reasoning in the commit message if necessary.\n\nYes, please.\n"},{"id":"390168","messageId":"1a4eae1ef5eb9749fe3e64c4c13e52c3269bc4be.1579618877.git.gitgitgadget@gmail.com","threadId":"52656","inReplyTo":"pull.523.v2.git.1579618877.gitgitgadget@gmail.com","subject":"[PATCH v2 1/4] t7410: rename to t2405-worktree-submodule.sh","fromName":"Philippe Blain via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2020-01-21T15:01:14Z","receivedAt":"2020-01-21T15:01:24Z","isPatch":true,"sender":{"key":"levraiphilippeblain@gmail.com","avatar":"https://avatars.githubusercontent.com/u/44212482?v=4"},"body":"From: Philippe Blain \n\nThis test was added in df56607dff (git-common-dir: make \"modules/\"\nper-working-directory directory, 2014-11-30), back when the 'git worktree' command\ndid not exist and 'git checkout --to' was used to create supplementary worktrees.\n\nSince this file contains tests for the interaction of 'git worktree' with\nsubmodules, rename it to t2405-worktree-submodule.sh, following the naming scheme for\ntests checking the behavior of various commands with submodules.\n\nSigned-off-by: Philippe Blain \n---\n ...410-submodule-checkout-to.sh => t2405-worktree-submodule.sh} | 2 +-\n 1 file changed, 1 insertion(+), 1 deletion(-)\n rename t/{t7410-submodule-checkout-to.sh => t2405-worktree-submodule.sh} (96%)\n\ndiff --git a/t/t7410-submodule-checkout-to.sh b/t/t2405-worktree-submodule.sh\nsimilarity index 96%\nrename from t/t7410-submodule-checkout-to.sh\nrename to t/t2405-worktree-submodule.sh\nindex f1b492ebc4..f2eee328cc 100755\n--- a/t/t7410-submodule-checkout-to.sh\n+++ b/t/t2405-worktree-submodule.sh\n@@ -1,6 +1,6 @@\n #!/bin/sh\n \n-test_description='Combination of submodules and multiple workdirs'\n+test_description='Combination of submodules and multiple worktrees'\n \n . ./test-lib.sh\n \n-- \ngitgitgadget\n\n"},{"id":"390169","messageId":"f06d2c4aa5dc739d42ec502a3c77617aae417611.1579618877.git.gitgitgadget@gmail.com","threadId":"52656","inReplyTo":"pull.523.v2.git.1579618877.gitgitgadget@gmail.com","subject":"[PATCH v2 2/4] t2405: use git -C and test_commit -C instead of subshells","fromName":"Philippe Blain via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2020-01-21T15:01:15Z","receivedAt":"2020-01-21T15:01:25Z","isPatch":true,"sender":{"key":"levraiphilippeblain@gmail.com","avatar":"https://avatars.githubusercontent.com/u/44212482?v=4"},"body":"From: Philippe Blain \n\nThe subshells used in the setup phase of this test are unnecessary.\n\nRemove them by using 'git -C' and 'test_commit -C'.\n\nSigned-off-by: Philippe Blain \n---\n t/t2405-worktree-submodule.sh | 34 +++++++++-------------------------\n 1 file changed, 9 insertions(+), 25 deletions(-)\n\ndiff --git a/t/t2405-worktree-submodule.sh b/t/t2405-worktree-submodule.sh\nindex f2eee328cc..c4e555776a 100755\n--- a/t/t2405-worktree-submodule.sh\n+++ b/t/t2405-worktree-submodule.sh\n@@ -6,32 +6,16 @@ test_description='Combination of submodules and multiple worktrees'\n \n base_path=$(pwd -P)\n \n-test_expect_success 'setup: make origin' '\n-\tmkdir -p origin/sub &&\n-\t(\n-\t\tcd origin/sub && git init &&\n-\t\techo file1 >file1 &&\n-\t\tgit add file1 &&\n-\t\tgit commit -m file1\n-\t) &&\n-\tmkdir -p origin/main &&\n-\t(\n-\t\tcd origin/main && git init &&\n-\t\tgit submodule add ../sub &&\n-\t\tgit commit -m \"add sub\"\n-\t) &&\n-\t(\n-\t\tcd origin/sub &&\n-\t\techo file1updated >file1 &&\n-\t\tgit add file1 &&\n-\t\tgit commit -m \"file1 updated\"\n-\t) &&\n+test_expect_success 'setup: create origin repos' '\n+\tgit init origin/sub &&\n+\ttest_commit -C origin/sub file1 &&\n+\tgit init origin/main &&\n+\tgit -C origin/main submodule add ../sub &&\n+\tgit -C origin/main commit -m \"add sub\" &&\n+\ttest_commit -C origin/sub \"file1 updated\" file1 file1updated file1updated &&\n \tgit -C origin/main/sub pull &&\n-\t(\n-\t\tcd origin/main &&\n-\t\tgit add sub &&\n-\t\tgit commit -m \"sub updated\"\n-\t)\n+\tgit -C origin/main add sub &&\n+\tgit -C origin/main commit -m \"sub updated\"\n '\n \n test_expect_success 'setup: clone' '\n-- \ngitgitgadget\n\n"},{"id":"390171","messageId":"10727275a2a7188eec374a82438eeda09f3b82b8.1579618877.git.gitgitgadget@gmail.com","threadId":"52656","inReplyTo":"pull.523.v2.git.1579618877.gitgitgadget@gmail.com","subject":"[PATCH v2 3/4] t2405: clarify test descriptions and simplify test","fromName":"Philippe Blain via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2020-01-21T15:01:16Z","receivedAt":"2020-01-21T15:01:26Z","isPatch":true,"sender":{"key":"levraiphilippeblain@gmail.com","avatar":"https://avatars.githubusercontent.com/u/44212482?v=4"},"body":"From: Philippe Blain \n\nWhen 'checkout --to' functionality was moved to 'worktree add', tests were adapted\nin f194b1ef6e (tests: worktree: retrofit \"checkout --to\" tests for \"worktree add\",\n2015-07-06).\n\nThe calls were changed to 'worktree add' in this test (then t7410), but the test\ndescriptions were not updated, keeping 'checkout' instead of using the new\nterminology (linked worktrees).\n\nAlso, in the test each worktree is created in\n$TRASH_DIRECTORY//main, where the name of \ncarries some information about what behavior each test verifies. This directory\nstructure is not mandatory for the tests; the worktrees can live next to one\nanother in the trash directory.\n\nClarify the tests by using the right terminology, and remove the unnecessary\nleading directories such that all superproject worktrees are directly next to one\nanother in the trash directory.\n\nSigned-off-by: Philippe Blain \n---\n t/t2405-worktree-submodule.sh | 36 ++++++++++++++++-------------------\n 1 file changed, 16 insertions(+), 20 deletions(-)\n\ndiff --git a/t/t2405-worktree-submodule.sh b/t/t2405-worktree-submodule.sh\nindex c4e555776a..d0830058fd 100755\n--- a/t/t2405-worktree-submodule.sh\n+++ b/t/t2405-worktree-submodule.sh\n@@ -18,43 +18,39 @@ test_expect_success 'setup: create origin repos' '\n \tgit -C origin/main commit -m \"sub updated\"\n '\n \n-test_expect_success 'setup: clone' '\n-\tmkdir clone &&\n-\tgit -C clone clone --recursive \"$base_path/origin/main\"\n+test_expect_success 'setup: clone superproject to create main worktree' '\n+\tgit clone --recursive \"$base_path/origin/main\" main\n '\n \n rev1_hash_main=$(git --git-dir=origin/main/.git show --pretty=format:%h -q \"HEAD~1\")\n rev1_hash_sub=$(git --git-dir=origin/sub/.git show --pretty=format:%h -q \"HEAD~1\")\n \n-test_expect_success 'checkout main' '\n-\tmkdir default_checkout &&\n-\tgit -C clone/main worktree add \"$base_path/default_checkout/main\" \"$rev1_hash_main\"\n+test_expect_success 'add superproject worktree' '\n+\tgit -C main worktree add \"$base_path/worktree\" \"$rev1_hash_main\"\n '\n \n-test_expect_failure 'can see submodule diffs just after checkout' '\n-\tgit -C default_checkout/main diff --submodule master\"^!\" >out &&\n+test_expect_failure 'submodule is checked out just after worktree add' '\n+\tgit -C worktree diff --submodule master\"^!\" >out &&\n \tgrep \"file1 updated\" out\n '\n \n-test_expect_success 'checkout main and initialize independent clones' '\n-\tmkdir fully_cloned_submodule &&\n-\tgit -C clone/main worktree add \"$base_path/fully_cloned_submodule/main\" \"$rev1_hash_main\" &&\n-\tgit -C fully_cloned_submodule/main submodule update\n+test_expect_success 'add superproject worktree and initialize submodules' '\n+\tgit -C main worktree add \"$base_path/worktree-submodule-update\" \"$rev1_hash_main\" &&\n+\tgit -C worktree-submodule-update submodule update\n '\n \n-test_expect_success 'can see submodule diffs after independent cloning' '\n-\tgit -C fully_cloned_submodule/main diff --submodule master\"^!\" >out &&\n+test_expect_success 'submodule is checked out just after submodule update in linked worktree' '\n+\tgit -C worktree-submodule-update diff --submodule master\"^!\" >out &&\n \tgrep \"file1 updated\" out\n '\n \n-test_expect_success 'checkout sub manually' '\n-\tmkdir linked_submodule &&\n-\tgit -C clone/main worktree add \"$base_path/linked_submodule/main\" \"$rev1_hash_main\" &&\n-\tgit -C clone/main/sub worktree add \"$base_path/linked_submodule/main/sub\" \"$rev1_hash_sub\"\n+test_expect_success 'add superproject worktree and manually add submodule worktree' '\n+\tgit -C main worktree add \"$base_path/linked_submodule\" \"$rev1_hash_main\" &&\n+\tgit -C main/sub worktree add \"$base_path/linked_submodule/sub\" \"$rev1_hash_sub\"\n '\n \n-test_expect_success 'can see submodule diffs after manual checkout of linked submodule' '\n-\tgit -C linked_submodule/main diff --submodule master\"^!\" >out &&\n+test_expect_success 'submodule is checked out after manually adding submodule worktree' '\n+\tgit -C linked_submodule diff --submodule master\"^!\" >out &&\n \tgrep \"file1 updated\" out\n '\n \n-- \ngitgitgadget\n\n"},{"id":"390172","messageId":"614bccd31b8edb6cf7c0ae53b70680e0a18bdd7e.1579618877.git.gitgitgadget@gmail.com","threadId":"52656","inReplyTo":"pull.523.v2.git.1579618877.gitgitgadget@gmail.com","subject":"[PATCH v2 4/4] submodule.c: use get_git_dir() instead of get_git_common_dir()","fromName":"Philippe Blain via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2020-01-21T15:01:17Z","receivedAt":"2020-01-21T15:01:27Z","isPatch":true,"sender":{"key":"levraiphilippeblain@gmail.com","avatar":"https://avatars.githubusercontent.com/u/44212482?v=4"},"body":"From: Philippe Blain \n\nEver since df56607dff (git-common-dir: make \"modules/\"\nper-working-directory directory, 2014-11-30), submodules in linked worktrees\nare cloned to $GIT_DIR/modules, i.e. $GIT_COMMON_DIR/worktrees//modules.\n\nHowever, this convention was not followed when the worktree updater commands\ncheckout, reset and read-tree learned to recurse into submodules. Specifically,\nsubmodule.c::submodule_move_head, introduced in 6e3c1595c6 (update submodules:\nadd submodule_move_head, 2017-03-14) and submodule.c::submodule_unset_core_worktree,\n(re)introduced in 898c2e65b7 (submodule: unset core.worktree if no working tree\nis present, 2018-12-14) use get_git_common_dir() instead of get_git_dir()\nto get the path of the submodule repository.\n\nThis means that, for example, 'git checkout --recurse-submodules '\nin a linked worktree will correctly checkout , detach the submodule's HEAD\nat the commit recorded in and update the submodule working tree, but the\nsubmodule HEAD that will be moved is the one in $GIT_COMMON_DIR/modules//,\ni.e. the submodule repository of the main superproject working tree.\nIt will also rewrite the gitfile in the submodule working tree of the linked worktree\nto point to $GIT_COMMON_DIR/modules//.\nThis leads to an incorrect (and confusing!) state in the submodule working tree\nof the main superproject worktree.\n\nAdditionally, if switching to a commit where the submodule is not present,\nsubmodule_unset_core_worktree will be called and will incorrectly remove\n'core.wortree' from the config file of the submodule in the main superproject worktree,\n$GIT_COMMON_DIR/modules//config.\n\nFix this by constructing the path to the submodule repository using get_git_dir()\nin both submodule_move_head and submodule_unset_core_worktree.\n\nSigned-off-by: Philippe Blain \n---\n submodule.c | 6 +++---\n t/t2405-worktree-submodule.sh | 33 +++++++++++++++++++++++++++++++++\n 2 files changed, 36 insertions(+), 3 deletions(-)\n\ndiff --git a/submodule.c b/submodule.c\nindex 9da7181321..5d19ec48a6 100644\n--- a/submodule.c\n+++ b/submodule.c\n@@ -1811,7 +1811,7 @@ int bad_to_remove_submodule(const char *path, unsigned flags)\n void submodule_unset_core_worktree(const struct submodule *sub)\n {\n \tchar *config_path = xstrfmt(\"%s/modules/%s/config\",\n-\t\t\t\t get_git_common_dir(), sub->name);\n+\t\t\t\t get_git_dir(), sub->name);\n \n \tif (git_config_set_in_file_gently(config_path, \"core.worktree\", NULL))\n \t\twarning(_(\"Could not unset core.worktree setting in submodule '%s'\"),\n@@ -1914,7 +1914,7 @@ int submodule_move_head(const char *path,\n \t\t\t\t\tABSORB_GITDIR_RECURSE_SUBMODULES);\n \t\t} else {\n \t\t\tchar *gitdir = xstrfmt(\"%s/modules/%s\",\n-\t\t\t\t get_git_common_dir(), sub->name);\n+\t\t\t\t get_git_dir(), sub->name);\n \t\t\tconnect_work_tree_and_git_dir(path, gitdir, 0);\n \t\t\tfree(gitdir);\n \n@@ -1924,7 +1924,7 @@ int submodule_move_head(const char *path,\n \n \t\tif (old_head && (flags & SUBMODULE_MOVE_HEAD_FORCE)) {\n \t\t\tchar *gitdir = xstrfmt(\"%s/modules/%s\",\n-\t\t\t\t get_git_common_dir(), sub->name);\n+\t\t\t\t get_git_dir(), sub->name);\n \t\t\tconnect_work_tree_and_git_dir(path, gitdir, 1);\n \t\t\tfree(gitdir);\n \t\t}\ndiff --git a/t/t2405-worktree-submodule.sh b/t/t2405-worktree-submodule.sh\nindex d0830058fd..e1b2bfd87e 100755\n--- a/t/t2405-worktree-submodule.sh\n+++ b/t/t2405-worktree-submodule.sh\n@@ -10,6 +10,7 @@ test_expect_success 'setup: create origin repos' '\n \tgit init origin/sub &&\n \ttest_commit -C origin/sub file1 &&\n \tgit init origin/main &&\n+\ttest_commit -C origin/main first &&\n \tgit -C origin/main submodule add ../sub &&\n \tgit -C origin/main commit -m \"add sub\" &&\n \ttest_commit -C origin/sub \"file1 updated\" file1 file1updated file1updated &&\n@@ -54,4 +55,36 @@ test_expect_success 'submodule is checked out after manually adding submodule wo\n \tgrep \"file1 updated\" out\n '\n \n+test_expect_success 'checkout --recurse-submodules uses $GIT_DIR for submodules in a linked worktree' '\n+\tgit -C main worktree add \"$base_path/checkout-recurse\" --detach &&\n+\tgit -C checkout-recurse submodule update --init &&\n+\techo \"gitdir: ../../main/.git/worktrees/checkout-recurse/modules/sub\" >expect-gitfile &&\n+\tcat checkout-recurse/sub/.git >actual-gitfile &&\n+\ttest_cmp expect-gitfile actual-gitfile &&\n+\tgit -C main/sub rev-parse HEAD >expect-head-main &&\n+\tgit -C checkout-recurse checkout --recurse-submodules HEAD~1 &&\n+\tcat checkout-recurse/sub/.git >actual-gitfile &&\n+\tgit -C main/sub rev-parse HEAD >actual-head-main &&\n+\ttest_cmp expect-gitfile actual-gitfile &&\n+\ttest_cmp expect-head-main actual-head-main\n+'\n+\n+test_expect_success 'core.worktree is removed in $GIT_DIR/modules//config, not in $GIT_COMMON_DIR/modules//config' '\n+\techo \"../../../sub\" >expect-main &&\n+\tgit -C main/sub config --get core.worktree >actual-main &&\n+\ttest_cmp expect-main actual-main &&\n+\techo \"../../../../../../checkout-recurse/sub\" >expect-linked &&\n+\tgit -C checkout-recurse/sub config --get core.worktree >actual-linked &&\n+\ttest_cmp expect-linked actual-linked &&\n+\tgit -C checkout-recurse checkout --recurse-submodules first &&\n+\ttest_expect_code 1 git -C main/.git/worktrees/checkout-recurse/modules/sub config --get core.worktree >linked-config &&\n+\ttest_must_be_empty linked-config &&\n+\tgit -C main/sub config --get core.worktree >actual-main &&\n+\ttest_cmp expect-main actual-main\n+'\n+\n+test_expect_success 'unsetting core.worktree does not prevent running commands directly against the submodule repository' '\n+\tgit -C main/.git/worktrees/checkout-recurse/modules/sub log\n+'\n+\n test_done\n-- \ngitgitgadget\n"},{"id":"390170","messageId":"pull.523.v2.git.1579618877.gitgitgadget@gmail.com","threadId":"52656","inReplyTo":"pull.523.git.1579263809.gitgitgadget@gmail.com","subject":"[PATCH v2 0/4] checkout/reset/read-tree: fix --recurse-submodules in linked worktree","fromName":"Philippe Blain via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2020-01-21T15:01:13Z","receivedAt":"2020-01-21T15:01:28Z","isPatch":true,"sender":{"key":"levraiphilippeblain@gmail.com","avatar":"https://avatars.githubusercontent.com/u/44212482?v=4"},"body":"Changes since v1:\n\n * revert the addition of a dash in \"file1 updated\" by correctly using the\n 'tag' argument of test_commit\n * improve the commit message for the 3rd patch, as suggested by Eric\n * fix spacing when redirecting into 'expect' and 'actual'\n * harden the tests by echoing expected values into expect, as suggested by\n Stolee (I did that in both tests)\n * remove test_might_fail and use test_expect_code\n * add the 'git log' test suggested by Stolee\n\nv1: This series fixes the behaviour of git checkout/reset/read-tree\n--recurse-submodules when they are called in a linked worktree (created by \ngit worktree add). \n\nAlthough submodules are cloned in $GIT_COMMON_DIR/worktrees//modules \nupon issuing git submodule update in the linked worktree, any invocation of \ngit checkout/reset/read-tree --recurse-submodules that changes the state of\nthe submodule(s) will incorrectly operate on the repositories of the\nsubmodules in the main worktree, i.e. the ones at $GIT_COMMON_DIR/modules/. \n\nThe fourth patch fixes this behaviour by using get_git_dir() instead of \ngit_common_dir() in submodule.c::submodule_move_head and \nsubmodule.c::submodule_unset_core_worktree to construct the path to the\nsubmodule repository.\n\nThe first 3 patches are clean-up patches on t7410-submodule-checkout-to.sh\n(renamed to t2405-worktree-submodule.sh) to bring it up to date.\n\nCc:Max Kirillov max@max630.net [max@max630.net] Brandon Williams \nbwilliams.eng@gmail.com [bwilliams.eng@gmail.com] Jonathan Tan \njonathantanmy@google.com [jonathantanmy@google.com] Stefan Beller \nstefanbeller@gmail.com [stefanbeller@gmail.com] Nguyễn Thái Ngọc Duy \npclouds@gmail.com [pclouds@gmail.com] Eric Sunshine sunshine@sunshineco.com\n[sunshine@sunshineco.com] Derrick Stolee stolee@gmail.com [stolee@gmail.com]\n\nPhilippe Blain (4):\n t7410: rename to t2405-worktree-submodule.sh\n t2405: use git -C and test_commit -C instead of subshells\n t2405: clarify test descriptions and simplify test\n submodule.c: use get_git_dir() instead of get_git_common_dir()\n\n submodule.c | 6 +--\n t/t2405-worktree-submodule.sh | 90 ++++++++++++++++++++++++++++++++\n t/t7410-submodule-checkout-to.sh | 77 ---------------------------\n 3 files changed, 93 insertions(+), 80 deletions(-)\n create mode 100755 t/t2405-worktree-submodule.sh\n delete mode 100755 t/t7410-submodule-checkout-to.sh\n\n\nbase-commit: b4615e40a8125477e18490d868f7b65954372b43\nPublished-As: https://github.com/gitgitgadget/git/releases/tag/pr-523%2Fphil-blain%2Fcheckout-recurse-in-linked-worktree-v2\nFetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-523/phil-blain/checkout-recurse-in-linked-worktree-v2\nPull-Request: https://github.com/gitgitgadget/git/pull/523\n\nRange-diff vs v1:\n\n 1: ff33339690 = 1: 1a4eae1ef5 t7410: rename to t2405-worktree-submodule.sh\n 2: 5060ce3d64 ! 2: f06d2c4aa5 t2405: use git -C and test_commit -C instead of subshells\n @@ -41,7 +41,7 @@\n +\tgit init origin/main &&\n +\tgit -C origin/main submodule add ../sub &&\n +\tgit -C origin/main commit -m \"add sub\" &&\n -+\ttest_commit -C origin/sub \"file1-updated\" file1 file1updated &&\n ++\ttest_commit -C origin/sub \"file1 updated\" file1 file1updated file1updated &&\n \tgit -C origin/main/sub pull &&\n -\t(\n -\t\tcd origin/main &&\n @@ -53,30 +53,3 @@\n '\n \n test_expect_success 'setup: clone' '\n -@@\n - \n - test_expect_failure 'can see submodule diffs just after checkout' '\n - \tgit -C default_checkout/main diff --submodule master\"^!\" >out &&\n --\tgrep \"file1 updated\" out\n -+\tgrep \"file1-updated\" out\n - '\n - \n - test_expect_success 'checkout main and initialize independent clones' '\n -@@\n - \n - test_expect_success 'can see submodule diffs after independent cloning' '\n - \tgit -C fully_cloned_submodule/main diff --submodule master\"^!\" >out &&\n --\tgrep \"file1 updated\" out\n -+\tgrep \"file1-updated\" out\n - '\n - \n - test_expect_success 'checkout sub manually' '\n -@@\n - \n - test_expect_success 'can see submodule diffs after manual checkout of linked submodule' '\n - \tgit -C linked_submodule/main diff --submodule master\"^!\" >out &&\n --\tgrep \"file1 updated\" out\n -+\tgrep \"file1-updated\" out\n - '\n - \n - test_done\n 3: 6e0e664026 ! 3: 10727275a2 t2405: clarify test descriptions and simplify test\n @@ -10,7 +10,13 @@\n descriptions were not updated, keeping 'checkout' instead of using the new\n terminology (linked worktrees).\n \n - Clarify the tests by using the right terminology. While at it, remove some unnecessary\n + Also, in the test each worktree is created in\n + $TRASH_DIRECTORY//main, where the name of \n + carries some information about what behavior each test verifies. This directory\n + structure is not mandatory for the tests; the worktrees can live next to one\n + another in the trash directory.\n +\n + Clarify the tests by using the right terminology, and remove the unnecessary\n leading directories such that all superproject worktrees are directly next to one\n another in the trash directory.\n \n @@ -44,7 +50,7 @@\n -\tgit -C default_checkout/main diff --submodule master\"^!\" >out &&\n +test_expect_failure 'submodule is checked out just after worktree add' '\n +\tgit -C worktree diff --submodule master\"^!\" >out &&\n - \tgrep \"file1-updated\" out\n + \tgrep \"file1 updated\" out\n '\n \n -test_expect_success 'checkout main and initialize independent clones' '\n @@ -60,7 +66,7 @@\n -\tgit -C fully_cloned_submodule/main diff --submodule master\"^!\" >out &&\n +test_expect_success 'submodule is checked out just after submodule update in linked worktree' '\n +\tgit -C worktree-submodule-update diff --submodule master\"^!\" >out &&\n - \tgrep \"file1-updated\" out\n + \tgrep \"file1 updated\" out\n '\n \n -test_expect_success 'checkout sub manually' '\n @@ -76,6 +82,6 @@\n -\tgit -C linked_submodule/main diff --submodule master\"^!\" >out &&\n +test_expect_success 'submodule is checked out after manually adding submodule worktree' '\n +\tgit -C linked_submodule diff --submodule master\"^!\" >out &&\n - \tgrep \"file1-updated\" out\n + \tgrep \"file1 updated\" out\n '\n \n 4: 72cdb2f95d ! 4: 614bccd31b submodule.c: use get_git_dir() instead of get_git_common_dir()\n @@ -24,7 +24,7 @@\n This leads to an incorrect (and confusing!) state in the submodule working tree\n of the main superproject worktree.\n \n - Additionnally, if switching to a commit where the submodule is not present,\n + Additionally, if switching to a commit where the submodule is not present,\n submodule_unset_core_worktree will be called and will incorrectly remove\n 'core.wortree' from the config file of the submodule in the main superproject worktree,\n $GIT_COMMON_DIR/modules//config.\n @@ -75,30 +75,41 @@\n +\ttest_commit -C origin/main first &&\n \tgit -C origin/main submodule add ../sub &&\n \tgit -C origin/main commit -m \"add sub\" &&\n - \ttest_commit -C origin/sub \"file1-updated\" file1 file1updated &&\n + \ttest_commit -C origin/sub \"file1 updated\" file1 file1updated file1updated &&\n @@\n - \tgrep \"file1-updated\" out\n + \tgrep \"file1 updated\" out\n '\n \n +test_expect_success 'checkout --recurse-submodules uses $GIT_DIR for submodules in a linked worktree' '\n +\tgit -C main worktree add \"$base_path/checkout-recurse\" --detach &&\n +\tgit -C checkout-recurse submodule update --init &&\n -+\tcat checkout-recurse/sub/.git > expect-gitfile &&\n -+\tgit -C main/sub rev-parse HEAD > expect-head-main &&\n ++\techo \"gitdir: ../../main/.git/worktrees/checkout-recurse/modules/sub\" >expect-gitfile &&\n ++\tcat checkout-recurse/sub/.git >actual-gitfile &&\n ++\ttest_cmp expect-gitfile actual-gitfile &&\n ++\tgit -C main/sub rev-parse HEAD >expect-head-main &&\n +\tgit -C checkout-recurse checkout --recurse-submodules HEAD~1 &&\n -+\tcat checkout-recurse/sub/.git > actual-gitfile &&\n -+\tgit -C main/sub rev-parse HEAD > actual-head-main &&\n ++\tcat checkout-recurse/sub/.git >actual-gitfile &&\n ++\tgit -C main/sub rev-parse HEAD >actual-head-main &&\n +\ttest_cmp expect-gitfile actual-gitfile &&\n +\ttest_cmp expect-head-main actual-head-main\n +'\n +\n +test_expect_success 'core.worktree is removed in $GIT_DIR/modules//config, not in $GIT_COMMON_DIR/modules//config' '\n -+\tgit -C main/sub config --get core.worktree > expect &&\n ++\techo \"../../../sub\" >expect-main &&\n ++\tgit -C main/sub config --get core.worktree >actual-main &&\n ++\ttest_cmp expect-main actual-main &&\n ++\techo \"../../../../../../checkout-recurse/sub\" >expect-linked &&\n ++\tgit -C checkout-recurse/sub config --get core.worktree >actual-linked &&\n ++\ttest_cmp expect-linked actual-linked &&\n +\tgit -C checkout-recurse checkout --recurse-submodules first &&\n -+\ttest_might_fail git -C main/.git/worktrees/checkout-recurse/modules/sub config --get core.worktree > linked-config &&\n ++\ttest_expect_code 1 git -C main/.git/worktrees/checkout-recurse/modules/sub config --get core.worktree >linked-config &&\n +\ttest_must_be_empty linked-config &&\n -+\ttest_might_fail git -C main/sub config --get core.worktree > actual &&\n -+\ttest_cmp expect actual\n ++\tgit -C main/sub config --get core.worktree >actual-main &&\n ++\ttest_cmp expect-main actual-main\n ++'\n ++\n ++test_expect_success 'unsetting core.worktree does not prevent running commands directly against the submodule repository' '\n ++\tgit -C main/.git/worktrees/checkout-recurse/modules/sub log\n +'\n +\n test_done\n\n-- \ngitgitgadget\n"},{"id":"390231","messageId":"F2F66C31-851E-42D0-BAF7-2DE8BF0F3D72@gmail.com","threadId":"52656","inReplyTo":"pull.523.v2.git.1579618877.gitgitgadget@gmail.com","subject":"Re: [PATCH v2 0/4] checkout/reset/read-tree: fix --recurse-submodules in linked worktree","fromName":"Philippe Blain","fromEmail":"levraiphilippeblain@gmail.com","sentAt":"2020-01-22T12:55:30Z","receivedAt":"2020-01-22T12:55:36Z","isPatch":true,"sender":{"key":"levraiphilippeblain@gmail.com","avatar":"https://avatars.githubusercontent.com/u/44212482?v=4"},"body":"Hello,\n\nHere is v2 of my series. I realized that Gitgitgadget failed to parse the Cc: footer in my PR description and thus both v1 and v2 got sent to the list without any Cc:s.\n\nCheers,\n\nPhilippe.\n\n> Le 21 janv. 2020 à 10:01, Philippe Blain via GitGitGadget a écrit :\n> \n> Changes since v1:\n> \n> * revert the addition of a dash in \"file1 updated\" by correctly using the\n> 'tag' argument of test_commit\n> * improve the commit message for the 3rd patch, as suggested by Eric\n> * fix spacing when redirecting into 'expect' and 'actual'\n> * harden the tests by echoing expected values into expect, as suggested by\n> Stolee (I did that in both tests)\n> * remove test_might_fail and use test_expect_code\n> * add the 'git log' test suggested by Stolee\n> \n> v1: This series fixes the behaviour of git checkout/reset/read-tree\n> --recurse-submodules when they are called in a linked worktree (created by \n> git worktree add). \n> \n> Although submodules are cloned in $GIT_COMMON_DIR/worktrees//modules \n> upon issuing git submodule update in the linked worktree, any invocation of \n> git checkout/reset/read-tree --recurse-submodules that changes the state of\n> the submodule(s) will incorrectly operate on the repositories of the\n> submodules in the main worktree, i.e. the ones at $GIT_COMMON_DIR/modules/. \n> \n> The fourth patch fixes this behaviour by using get_git_dir() instead of \n> git_common_dir() in submodule.c::submodule_move_head and \n> submodule.c::submodule_unset_core_worktree to construct the path to the\n> submodule repository.\n> \n> The first 3 patches are clean-up patches on t7410-submodule-checkout-to.sh\n> (renamed to t2405-worktree-submodule.sh) to bring it up to date.\n> \n> Cc:Max Kirillov max@max630.net [max@max630.net] Brandon Williams \n> bwilliams.eng@gmail.com [bwilliams.eng@gmail.com] Jonathan Tan \n> jonathantanmy@google.com [jonathantanmy@google.com] Stefan Beller \n> stefanbeller@gmail.com [stefanbeller@gmail.com] Nguyễn Thái Ngọc Duy \n> pclouds@gmail.com [pclouds@gmail.com] Eric Sunshine sunshine@sunshineco.com\n> [sunshine@sunshineco.com] Derrick Stolee stolee@gmail.com [stolee@gmail.com]\n> \n> Philippe Blain (4):\n> t7410: rename to t2405-worktree-submodule.sh\n> t2405: use git -C and test_commit -C instead of subshells\n> t2405: clarify test descriptions and simplify test\n> submodule.c: use get_git_dir() instead of get_git_common_dir()\n> \n> submodule.c | 6 +--\n> t/t2405-worktree-submodule.sh | 90 ++++++++++++++++++++++++++++++++\n> t/t7410-submodule-checkout-to.sh | 77 ---------------------------\n> 3 files changed, 93 insertions(+), 80 deletions(-)\n> create mode 100755 t/t2405-worktree-submodule.sh\n> delete mode 100755 t/t7410-submodule-checkout-to.sh\n> \n> \n> base-commit: b4615e40a8125477e18490d868f7b65954372b43\n> Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-523%2Fphil-blain%2Fcheckout-recurse-in-linked-worktree-v2\n> Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-523/phil-blain/checkout-recurse-in-linked-worktree-v2\n> Pull-Request: https://github.com/gitgitgadget/git/pull/523\n> \n> Range-diff vs v1:\n> \n> 1: ff33339690 = 1: 1a4eae1ef5 t7410: rename to t2405-worktree-submodule.sh\n> 2: 5060ce3d64 ! 2: f06d2c4aa5 t2405: use git -C and test_commit -C instead of subshells\n> @@ -41,7 +41,7 @@\n> +\tgit init origin/main &&\n> +\tgit -C origin/main submodule add ../sub &&\n> +\tgit -C origin/main commit -m \"add sub\" &&\n> -+\ttest_commit -C origin/sub \"file1-updated\" file1 file1updated &&\n> ++\ttest_commit -C origin/sub \"file1 updated\" file1 file1updated file1updated &&\n> \tgit -C origin/main/sub pull &&\n> -\t(\n> -\t\tcd origin/main &&\n> @@ -53,30 +53,3 @@\n> '\n> \n> test_expect_success 'setup: clone' '\n> -@@\n> - \n> - test_expect_failure 'can see submodule diffs just after checkout' '\n> - \tgit -C default_checkout/main diff --submodule master\"^!\" >out &&\n> --\tgrep \"file1 updated\" out\n> -+\tgrep \"file1-updated\" out\n> - '\n> - \n> - test_expect_success 'checkout main and initialize independent clones' '\n> -@@\n> - \n> - test_expect_success 'can see submodule diffs after independent cloning' '\n> - \tgit -C fully_cloned_submodule/main diff --submodule master\"^!\" >out &&\n> --\tgrep \"file1 updated\" out\n> -+\tgrep \"file1-updated\" out\n> - '\n> - \n> - test_expect_success 'checkout sub manually' '\n> -@@\n> - \n> - test_expect_success 'can see submodule diffs after manual checkout of linked submodule' '\n> - \tgit -C linked_submodule/main diff --submodule master\"^!\" >out &&\n> --\tgrep \"file1 updated\" out\n> -+\tgrep \"file1-updated\" out\n> - '\n> - \n> - test_done\n> 3: 6e0e664026 ! 3: 10727275a2 t2405: clarify test descriptions and simplify test\n> @@ -10,7 +10,13 @@\n> descriptions were not updated, keeping 'checkout' instead of using the new\n> terminology (linked worktrees).\n> \n> - Clarify the tests by using the right terminology. While at it, remove some unnecessary\n> + Also, in the test each worktree is created in\n> + $TRASH_DIRECTORY//main, where the name of \n> + carries some information about what behavior each test verifies. This directory\n> + structure is not mandatory for the tests; the worktrees can live next to one\n> + another in the trash directory.\n> +\n> + Clarify the tests by using the right terminology, and remove the unnecessary\n> leading directories such that all superproject worktrees are directly next to one\n> another in the trash directory.\n> \n> @@ -44,7 +50,7 @@\n> -\tgit -C default_checkout/main diff --submodule master\"^!\" >out &&\n> +test_expect_failure 'submodule is checked out just after worktree add' '\n> +\tgit -C worktree diff --submodule master\"^!\" >out &&\n> - \tgrep \"file1-updated\" out\n> + \tgrep \"file1 updated\" out\n> '\n> \n> -test_expect_success 'checkout main and initialize independent clones' '\n> @@ -60,7 +66,7 @@\n> -\tgit -C fully_cloned_submodule/main diff --submodule master\"^!\" >out &&\n> +test_expect_success 'submodule is checked out just after submodule update in linked worktree' '\n> +\tgit -C worktree-submodule-update diff --submodule master\"^!\" >out &&\n> - \tgrep \"file1-updated\" out\n> + \tgrep \"file1 updated\" out\n> '\n> \n> -test_expect_success 'checkout sub manually' '\n> @@ -76,6 +82,6 @@\n> -\tgit -C linked_submodule/main diff --submodule master\"^!\" >out &&\n> +test_expect_success 'submodule is checked out after manually adding submodule worktree' '\n> +\tgit -C linked_submodule diff --submodule master\"^!\" >out &&\n> - \tgrep \"file1-updated\" out\n> + \tgrep \"file1 updated\" out\n> '\n> \n> 4: 72cdb2f95d ! 4: 614bccd31b submodule.c: use get_git_dir() instead of get_git_common_dir()\n> @@ -24,7 +24,7 @@\n> This leads to an incorrect (and confusing!) state in the submodule working tree\n> of the main superproject worktree.\n> \n> - Additionnally, if switching to a commit where the submodule is not present,\n> + Additionally, if switching to a commit where the submodule is not present,\n> submodule_unset_core_worktree will be called and will incorrectly remove\n> 'core.wortree' from the config file of the submodule in the main superproject worktree,\n> $GIT_COMMON_DIR/modules//config.\n> @@ -75,30 +75,41 @@\n> +\ttest_commit -C origin/main first &&\n> \tgit -C origin/main submodule add ../sub &&\n> \tgit -C origin/main commit -m \"add sub\" &&\n> - \ttest_commit -C origin/sub \"file1-updated\" file1 file1updated &&\n> + \ttest_commit -C origin/sub \"file1 updated\" file1 file1updated file1updated &&\n> @@\n> - \tgrep \"file1-updated\" out\n> + \tgrep \"file1 updated\" out\n> '\n> \n> +test_expect_success 'checkout --recurse-submodules uses $GIT_DIR for submodules in a linked worktree' '\n> +\tgit -C main worktree add \"$base_path/checkout-recurse\" --detach &&\n> +\tgit -C checkout-recurse submodule update --init &&\n> -+\tcat checkout-recurse/sub/.git > expect-gitfile &&\n> -+\tgit -C main/sub rev-parse HEAD > expect-head-main &&\n> ++\techo \"gitdir: ../../main/.git/worktrees/checkout-recurse/modules/sub\" >expect-gitfile &&\n> ++\tcat checkout-recurse/sub/.git >actual-gitfile &&\n> ++\ttest_cmp expect-gitfile actual-gitfile &&\n> ++\tgit -C main/sub rev-parse HEAD >expect-head-main &&\n> +\tgit -C checkout-recurse checkout --recurse-submodules HEAD~1 &&\n> -+\tcat checkout-recurse/sub/.git > actual-gitfile &&\n> -+\tgit -C main/sub rev-parse HEAD > actual-head-main &&\n> ++\tcat checkout-recurse/sub/.git >actual-gitfile &&\n> ++\tgit -C main/sub rev-parse HEAD >actual-head-main &&\n> +\ttest_cmp expect-gitfile actual-gitfile &&\n> +\ttest_cmp expect-head-main actual-head-main\n> +'\n> +\n> +test_expect_success 'core.worktree is removed in $GIT_DIR/modules//config, not in $GIT_COMMON_DIR/modules//config' '\n> -+\tgit -C main/sub config --get core.worktree > expect &&\n> ++\techo \"../../../sub\" >expect-main &&\n> ++\tgit -C main/sub config --get core.worktree >actual-main &&\n> ++\ttest_cmp expect-main actual-main &&\n> ++\techo \"../../../../../../checkout-recurse/sub\" >expect-linked &&\n> ++\tgit -C checkout-recurse/sub config --get core.worktree >actual-linked &&\n> ++\ttest_cmp expect-linked actual-linked &&\n> +\tgit -C checkout-recurse checkout --recurse-submodules first &&\n> -+\ttest_might_fail git -C main/.git/worktrees/checkout-recurse/modules/sub config --get core.worktree > linked-config &&\n> ++\ttest_expect_code 1 git -C main/.git/worktrees/checkout-recurse/modules/sub config --get core.worktree >linked-config &&\n> +\ttest_must_be_empty linked-config &&\n> -+\ttest_might_fail git -C main/sub config --get core.worktree > actual &&\n> -+\ttest_cmp expect actual\n> ++\tgit -C main/sub config --get core.worktree >actual-main &&\n> ++\ttest_cmp expect-main actual-main\n> ++'\n> ++\n> ++test_expect_success 'unsetting core.worktree does not prevent running commands directly against the submodule repository' '\n> ++\tgit -C main/.git/worktrees/checkout-recurse/modules/sub log\n> +'\n> +\n> test_done\n> \n> -- \n> gitgitgadget\n\n"},{"id":"390245","messageId":"xmqqwo9j5ei3.fsf@gitster-ct.c.googlers.com","threadId":"52656","inReplyTo":"pull.523.v2.git.1579618877.gitgitgadget@gmail.com","subject":"Re: [PATCH v2 0/4] checkout/reset/read-tree: fix --recurse-submodules in linked worktree","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2020-01-22T22:10:28Z","receivedAt":"2020-01-22T22:10:34Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"\"Philippe Blain via GitGitGadget\" writes:\n\n> Changes since v1:\n>\n> * revert the addition of a dash in \"file1 updated\" by correctly using the\n> 'tag' argument of test_commit\n> * improve the commit message for the 3rd patch, as suggested by Eric\n> * fix spacing when redirecting into 'expect' and 'actual'\n> * harden the tests by echoing expected values into expect, as suggested by\n> Stolee (I did that in both tests)\n> * remove test_might_fail and use test_expect_code\n> * add the 'git log' test suggested by Stolee\n>\n> v1: This series fixes the behaviour of git checkout/reset/read-tree\n> --recurse-submodules when they are called in a linked worktree (created by \n> git worktree add). \n>\n> Although submodules are cloned in $GIT_COMMON_DIR/worktrees//modules \n> upon issuing git submodule update in the linked worktree, any invocation of \n> git checkout/reset/read-tree --recurse-submodules that changes the state of\n> the submodule(s) will incorrectly operate on the repositories of the\n> submodules in the main worktree, i.e. the ones at $GIT_COMMON_DIR/modules/. \n>\n> The fourth patch fixes this behaviour by using get_git_dir() instead of \n> git_common_dir() in submodule.c::submodule_move_head and \n> submodule.c::submodule_unset_core_worktree to construct the path to the\n> submodule repository.\n>\n> The first 3 patches are clean-up patches on t7410-submodule-checkout-to.sh\n> (renamed to t2405-worktree-submodule.sh) to bring it up to date.\n\nI've read these four patches a few times and did not spot any\nremaining issues. I however is hardly an expert on (nor a big fan\nof) the \"--recurse-submodule\" feature, so a proper review by those\nwho are more familiar with it is surely appreciated.\n\nThanks.\n\n> Cc:Max Kirillov max@max630.net [max@max630.net] Brandon Williams \n> bwilliams.eng@gmail.com [bwilliams.eng@gmail.com] Jonathan Tan \n> jonathantanmy@google.com [jonathantanmy@google.com] Stefan Beller \n> stefanbeller@gmail.com [stefanbeller@gmail.com] Nguyễn Thái Ngọc Duy \n> pclouds@gmail.com [pclouds@gmail.com] Eric Sunshine sunshine@sunshineco.com\n> [sunshine@sunshineco.com] Derrick Stolee stolee@gmail.com [stolee@gmail.com]\n\nThis is somewhat unreadable wall of names X-< Is it a funny\nrendering of what is originally in some mark-up format (perhaps\nHTML???)\n\n"},{"id":"390247","messageId":"69FFF66E-9273-488F-A747-2B4EBBDD7A7B@gmail.com","threadId":"52656","inReplyTo":"xmqqwo9j5ei3.fsf@gitster-ct.c.googlers.com","subject":"Re: [PATCH v2 0/4] checkout/reset/read-tree: fix --recurse-submodules in linked worktree","fromName":"Philippe Blain","fromEmail":"levraiphilippeblain@gmail.com","sentAt":"2020-01-22T22:25:21Z","receivedAt":"2020-01-22T22:25:26Z","isPatch":true,"sender":{"key":"levraiphilippeblain@gmail.com","avatar":"https://avatars.githubusercontent.com/u/44212482?v=4"},"body":"Hi Junio, \n\n> Le 22 janv. 2020 à 17:10, Junio C Hamano a écrit :\n> \n> \"Philippe Blain via GitGitGadget\" writes:\n\n>> Cc:Max Kirillov max@max630.net [max@max630.net] Brandon Williams \n>> bwilliams.eng@gmail.com [bwilliams.eng@gmail.com] Jonathan Tan \n>> jonathantanmy@google.com [jonathantanmy@google.com] Stefan Beller \n>> stefanbeller@gmail.com [stefanbeller@gmail.com] Nguyễn Thái Ngọc Duy \n>> pclouds@gmail.com [pclouds@gmail.com] Eric Sunshine sunshine@sunshineco.com\n>> [sunshine@sunshineco.com] Derrick Stolee stolee@gmail.com [stolee@gmail.com]\n> \n> This is somewhat unreadable wall of names X-< Is it a funny\n> rendering of what is originally in some mark-up format (perhaps\n> HTML???)\n\nYes, Gitgitgadget unfortunately failed to convert this wall of text into a proper CC: list because there was no space between the \"Cc:\" and \"Max\". \nI’ll try to submit a PR for that in GGG. \n\nCheers,\nPhilippe. "},{"id":"390423","messageId":"37804466-7DC9-4161-9B98-D2175233831D@gmail.com","threadId":"52656","inReplyTo":"69FFF66E-9273-488F-A747-2B4EBBDD7A7B@gmail.com","subject":"Re: [PATCH v2 0/4] checkout/reset/read-tree: fix --recurse-submodules in linked worktree","fromName":"Philippe Blain","fromEmail":"levraiphilippeblain@gmail.com","sentAt":"2020-01-24T23:00:39Z","receivedAt":"2020-01-24T23:00:45Z","isPatch":true,"sender":{"key":"levraiphilippeblain@gmail.com","avatar":"https://avatars.githubusercontent.com/u/44212482?v=4"},"body":"\n> Le 22 janv. 2020 à 17:25, Philippe Blain a écrit :\n> \n> Hi Junio, \n> \n>> Le 22 janv. 2020 à 17:10, Junio C Hamano a écrit :\n>> \n>> \"Philippe Blain via GitGitGadget\" writes:\n> \n>>> Cc:Max Kirillov max@max630.net [max@max630.net] Brandon Williams \n>>> bwilliams.eng@gmail.com [bwilliams.eng@gmail.com] Jonathan Tan \n>>> jonathantanmy@google.com [jonathantanmy@google.com] Stefan Beller \n>>> stefanbeller@gmail.com [stefanbeller@gmail.com] Nguyễn Thái Ngọc Duy \n>>> pclouds@gmail.com [pclouds@gmail.com] Eric Sunshine sunshine@sunshineco.com\n>>> [sunshine@sunshineco.com] Derrick Stolee stolee@gmail.com [stolee@gmail.com]\n>> \n>> This is somewhat unreadable wall of names X-< Is it a funny\n>> rendering of what is originally in some mark-up format (perhaps\n>> HTML???)\n> \n> Yes, Gitgitgadget unfortunately failed to convert this wall of text into a proper CC: list because there was no space between the \"Cc:\" and \"Max\". \n> I’ll try to submit a PR for that in GGG. \nFor the record, https://github.com/gitgitgadget/gitgitgadget/pull/198 has been merged so this should not happen again.\n\n"},{"id":"390425","messageId":"xmqqd0b8ct8b.fsf@gitster-ct.c.googlers.com","threadId":"52656","inReplyTo":"37804466-7DC9-4161-9B98-D2175233831D@gmail.com","subject":"Re: [PATCH v2 0/4] checkout/reset/read-tree: fix --recurse-submodules in linked worktree","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2020-01-24T23:47:16Z","receivedAt":"2020-01-24T23:47:21Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Philippe Blain writes:\n\n>>>> Cc:Max Kirillov max@max630.net [max@max630.net] Brandon Williams \n>>>> bwilliams.eng@gmail.com [bwilliams.eng@gmail.com] Jonathan Tan \n>>>> jonathantanmy@google.com [jonathantanmy@google.com] Stefan Beller \n>>>> stefanbeller@gmail.com [stefanbeller@gmail.com] Nguyễn Thái Ngọc Duy \n>>>> pclouds@gmail.com [pclouds@gmail.com] Eric Sunshine sunshine@sunshineco.com\n>>>> [sunshine@sunshineco.com] Derrick Stolee stolee@gmail.com [stolee@gmail.com]\n>>> \n>>> This is somewhat unreadable wall of names X-< Is it a funny\n>>> rendering of what is originally in some mark-up format (perhaps\n>>> HTML???)\n>> \n>> Yes, Gitgitgadget unfortunately failed to convert this wall of text into a proper CC: list because there was no space between the \"Cc:\" and \"Max\". \n>> I’ll try to submit a PR for that in GGG. \n> For the record, https://github.com/gitgitgadget/gitgitgadget/pull/198 has been merged so this should not happen again.\n\nThat's good to know. As long as the resulting e-mail addresses are\nsensible (e.g. if Markdown markups are not cleansed enough and the\nduplicated addresses inside [] seep through, the result would still\nnot be good), it is OK.\n\nThanks.\n"}]}