{"thread":{"id":"41540","subject":"[PATCH 1/2] Check recursive submodule update to have correct path from subdirectory","startedAt":"2016-02-26T20:51:50Z","lastAt":"2016-02-27T01:58:43Z","messageCount":4,"participants":["Stefan Beller","Duy Nguyen"],"isPatch":true,"patchVersion":1,"patchTotal":2},"messages":[{"id":"279565","messageId":"1456519912-10641-1-git-send-email-sbeller@google.com","threadId":"41540","inReplyTo":null,"subject":"[PATCH 0/2] recursive submodules: paths are hard","fromName":"Stefan Beller","fromEmail":"sbeller@google.com","sentAt":"2016-02-26T20:51:50Z","receivedAt":"2016-02-26T20:51:50Z","isPatch":true,"sender":{"key":"stefanbeller@gmail.com","avatar":"https://avatars.githubusercontent.com/u/455868?v=4"},"body":"I thought getting rid of the extra `prefix` argument in submodule helper\nfunctions was easy up to the point of all but one test passing in the test\nsuite.\n\nIt turned out the implementation of that prefix patch was wrong, only caught\nby tests, so we want to add tests for subtle details with submodule sync and\nupdate which have not been tests yet.\n\nFor the record, I thought the 'prefix' patch was as easy as:\n\n-       git submodule--helper list --prefix \"$wt_prefix\" \"$@\" | {\n+       git -C \"$wt_prefix\" submodule--helper list \"$@\" | {\n\nfor all occurrences of `submodule--helper list`.\n\nThis is not the case as by doing so the recursive functionality of submodules\nis broken in some edge cases.\n\nConsider this sequence:\n\n  mkdir untracked &&\n  cd untracked &&\n  git submodule <command> --recursive\n  \nThe operation is run from within the work tree, so fro a normal submodule\noperation (without --recursive) you expect the pathes to be adapted to be\nprefixed by a `../` to make sense relative to the untracked directory.\n\nIn the case of recursive submodule operations, currently `git submodule`\nusually does\n\n        if test -n \"$recursive\"\n        then\n        (\n                prefix=\"$prefix$sm_path/\"\n                clear_local_git_env\n                cd \"$sm_path\" &&\n                eval cmd_update\n        )\n        \nBy having a change of directory followed by the recursive call to the operation\nwe need to make sure the displayed path is still correctly referenced to where\nthe operation started.\n\nBy passing the prefix separately to git submodule--helper, this works currently\nas the prefix is only used for calculating the displaypath. If it were passed\nby the standard Git machinery, there is going on more, which\nfails us at some point.\n\nI think we may need to enable Git to pass in 'negative' pathes for the prefix,\ni.e. \n  Although operating on this repository, your reference for displaying paths\n  should be '../untracked' for the example above, when the submodule is in the\n  root directory of the superproject.\n\nThis seems currently not possible with the standard way to pass down the prefix.\n\nTL;DR: Most of the test is unrelated to the patch series, the patch series\nadds some tests, which I would have found useful to stop me going the wrong\ndirection.\n\nThanks,\nStefan\n\nStefan Beller (2):\n  Check recursive submodule update to have correct path from\n    subdirectory\n  submodule sync: Test we can pass individual submodules\n\n t/t7403-submodule-sync.sh   | 13 +++++++++----\n t/t7406-submodule-update.sh | 12 ++++++++++++\n 2 files changed, 21 insertions(+), 4 deletions(-)\n\n-- \n2.7.2.368.g934fe14\n"},{"id":"279564","messageId":"1456519912-10641-2-git-send-email-sbeller@google.com","threadId":"41540","inReplyTo":"1456519912-10641-1-git-send-email-sbeller@google.com","subject":"[PATCH 1/2] Check recursive submodule update to have correct path from subdirectory","fromName":"Stefan Beller","fromEmail":"sbeller@google.com","sentAt":"2016-02-26T20:51:51Z","receivedAt":"2016-02-26T20:51:51Z","isPatch":true,"sender":{"key":"stefanbeller@gmail.com","avatar":"https://avatars.githubusercontent.com/u/455868?v=4"},"body":"A similar test exists for `submodule sync` to behave well when being in\nan unrelated subdirectory and performing operations on submodules.\n\nSigned-off-by: Stefan Beller <sbeller@google.com>\n---\n t/t7406-submodule-update.sh | 12 ++++++++++++\n 1 file changed, 12 insertions(+)\n\ndiff --git a/t/t7406-submodule-update.sh b/t/t7406-submodule-update.sh\nindex 68ea31d..628da7f 100755\n--- a/t/t7406-submodule-update.sh\n+++ b/t/t7406-submodule-update.sh\n@@ -774,4 +774,16 @@ test_expect_success 'submodule update --recursive drops module name before recur\n \t test_i18ngrep \"Submodule path .deeper/submodule/subsubmodule.: checked out\" actual\n \t)\n '\n+\n+test_expect_success 'submodule update --recursive works from subdirectory' '\n+\t(cd super2 &&\n+\t (cd deeper/submodule/subsubmodule &&\n+\t  git checkout HEAD^\n+\t ) &&\n+\t mkdir untracked &&\n+\t cd untracked &&\n+\t git submodule update --recursive >actual &&\n+\t test_i18ngrep \"Submodule path .../deeper/submodule/subsubmodule.: checked out\" actual\n+\t)\n+'\n test_done\n-- \n2.7.2.368.g934fe14\n"},{"id":"279566","messageId":"1456519912-10641-3-git-send-email-sbeller@google.com","threadId":"41540","inReplyTo":"1456519912-10641-1-git-send-email-sbeller@google.com","subject":"[PATCH 2/2] submodule sync: Test we can pass individual submodules","fromName":"Stefan Beller","fromEmail":"sbeller@google.com","sentAt":"2016-02-26T20:51:52Z","receivedAt":"2016-02-26T20:51:52Z","isPatch":true,"sender":{"key":"stefanbeller@gmail.com","avatar":"https://avatars.githubusercontent.com/u/455868?v=4"},"body":"The parent patch made changes to the way `git submodule--helper list`\nis called. From experience this is a sensitive topic and lots subtle things\ncan go wrong. As all submodule subcommands except `sync` are setup to\nrun `git submodule--helper list` in the original directory, I suspected\na possible breakage in `sync` not being able to specify an exact submodule\nto run in, so let's add a test for that. Instead of adding a complete new\ntest we can modify an existing test to additionally test the additional\nassertion of having just one submodule work fine.\n\nSigned-off-by: Stefan Beller <sbeller@google.com>\n---\n t/t7403-submodule-sync.sh | 13 +++++++++----\n 1 file changed, 9 insertions(+), 4 deletions(-)\n\ndiff --git a/t/t7403-submodule-sync.sh b/t/t7403-submodule-sync.sh\nindex 79bc135..5dde123 100755\n--- a/t/t7403-submodule-sync.sh\n+++ b/t/t7403-submodule-sync.sh\n@@ -28,6 +28,9 @@ test_expect_success setup '\n \t\tgit submodule add ../submodule submodule &&\n \t\ttest_tick &&\n \t\tgit commit -m \"submodule\"\n+\t\tgit submodule add ../submodule submodule2 &&\n+\t\ttest_tick &&\n+\t\tgit commit -m \"second submodule\"\n \t) &&\n \tgit clone super super-clone &&\n \t(\n@@ -149,15 +152,16 @@ test_expect_success 'reset submodule URLs' '\n \treset_submodule_urls super-clone\n '\n \n-test_expect_success '\"git submodule sync\" should update submodule URLs - subdirectory' '\n+test_expect_success '\"git submodule sync\" should update specified submodule URLs - subdirectory' '\n \t(\n \t\tcd super-clone &&\n \t\tgit pull --no-recurse-submodules &&\n \t\tmkdir -p sub &&\n \t\tcd sub &&\n-\t\tgit submodule sync >../../output\n+\t\tgit submodule sync ../submodule >../../output\n \t) &&\n \tgrep \"\\\\.\\\\./submodule\" output &&\n+\t! grep submodule2 output &&\n \ttest -d \"$(\n \t\tcd super-clone/submodule &&\n \t\tgit config remote.origin.url\n@@ -177,7 +181,7 @@ test_expect_success '\"git submodule sync\" should update submodule URLs - subdire\n \t)\n '\n \n-test_expect_success '\"git submodule sync --recursive\" should update all submodule URLs - subdirectory' '\n+test_expect_success '\"git submodule sync --recursive\" should update all specified submodule URLs - subdirectory' '\n \t(\n \t\tcd super-clone &&\n \t\t(\n@@ -186,9 +190,10 @@ test_expect_success '\"git submodule sync --recursive\" should update all submodul\n \t\t) &&\n \t\tmkdir -p sub &&\n \t\tcd sub &&\n-\t\tgit submodule sync --recursive >../../output\n+\t\tgit submodule sync --recursive ../submodule >../../output\n \t) &&\n \tgrep \"\\\\.\\\\./submodule/sub-submodule\" output &&\n+\t! grep submodule2 output &&\n \ttest -d \"$(\n \t\tcd super-clone/submodule &&\n \t\tgit config remote.origin.url\n-- \n2.7.2.368.g934fe14\n"},{"id":"279596","messageId":"CACsJy8Bsy1-14RhKES7AWWS8RSKbvPtu46cgGnmkuSXrUsq84g@mail.gmail.com","threadId":"41540","inReplyTo":"1456519912-10641-1-git-send-email-sbeller@google.com","subject":"Re: [PATCH 0/2] recursive submodules: paths are hard","fromName":"Duy Nguyen","fromEmail":"pclouds@gmail.com","sentAt":"2016-02-27T01:58:43Z","receivedAt":"2016-02-27T01:58:43Z","isPatch":true,"sender":{"key":"pclouds@gmail.com","avatar":"https://avatars.githubusercontent.com/u/720?v=4"},"body":"On Sat, Feb 27, 2016 at 3:51 AM, Stefan Beller <sbeller@google.com> wrote:\n> I think we may need to enable Git to pass in 'negative' pathes for the prefix,\n> i.e.\n>   Although operating on this repository, your reference for displaying paths\n>   should be '../untracked' for the example above, when the submodule is in the\n>   root directory of the superproject.\n>\n> This seems currently not possible with the standard way to pass down the prefix.\n\nThe problem is, prefix is meant for inside worktree area only. Many\ncode paths are not ready for prefix \"../\". Worse, in some cases when\nyou launch outside worktree, prefix is empty and I think some builtin\ncommands rely on that. However, I think you can introduce \"prefix v2\"\nthat is used in parallel with \"prefix v1\".\n-- \nDuy\n"}]}