{"thread":{"id":"41866","subject":"[PATCHv3 0/6] Fix path bugs in submodule commands executed from sub dir","startedAt":"2016-03-29T23:02:32Z","lastAt":"2016-03-30T01:00:11Z","messageCount":12,"participants":["Stefan Beller","Junio C Hamano"],"isPatch":false,"patchVersion":null,"patchTotal":null},"messages":[{"id":"282136","messageId":"1459292558-5840-1-git-send-email-sbeller@google.com","threadId":"41866","inReplyTo":null,"subject":"[PATCHv3 0/6] Fix path bugs in submodule commands executed from sub dir","fromName":"Stefan Beller","fromEmail":"sbeller@google.com","sentAt":"2016-03-29T23:02:32Z","receivedAt":"2016-03-29T23:02:32Z","isPatch":false,"sender":{"key":"stefanbeller@gmail.com","avatar":"https://avatars.githubusercontent.com/u/455868?v=4"},"body":"v3:\nResending without `test_pause` in the last test.\n\nv2:\nThe first 3 commits are\n* Test and bugfix in one commit each\n* better explained than before\n\nThe expansion of an expected test result moved to the back of the series.\n\nThere are two new commits\n* one being another bugfix of the display path for `submodule update`\n* another test for `submodule update` as I suspect it may break further on\n  refactoring and we currently have no tests for it.\n\nThanks,\nStefan\n\nStefan Beller (6):\n  submodule foreach: correct path display in recursive submodules\n  submodule update --init: correct path handling in recursive submodules\n  submodule status: correct path handling in recursive submodules\n  t7407: make expectation as clear as possible\n  submodule update: align reporting path for custom command execution\n  submodule update: test recursive path reporting from subdirectory\n\n git-submodule.sh             | 10 +++---\n t/t7406-submodule-update.sh  | 83 ++++++++++++++++++++++++++++++++++++++++++--\n t/t7407-submodule-foreach.sh | 49 ++++++++++++++++++++++++--\n 3 files changed, 133 insertions(+), 9 deletions(-)\n\n-- \n2.8.0.4.g5639dee.dirty\n"},{"id":"282138","messageId":"1459292558-5840-2-git-send-email-sbeller@google.com","threadId":"41866","inReplyTo":"1459292558-5840-1-git-send-email-sbeller@google.com","subject":"[PATCH 1/6] submodule foreach: correct path display in recursive submodules","fromName":"Stefan Beller","fromEmail":"sbeller@google.com","sentAt":"2016-03-29T23:02:33Z","receivedAt":"2016-03-29T23:02:33Z","isPatch":true,"sender":{"key":"stefanbeller@gmail.com","avatar":"https://avatars.githubusercontent.com/u/455868?v=4"},"body":"The new test replicates the previous test with the difference of executing\nfrom a sub directory. By executing from a sub directory all we would\nexpect all displayed paths to be prefixed by '../'.\n\nPrior to this patch the test would report\n    Entering 'nested1/nested2/../nested3'\ninstead of the expected\n    Entering '../nested1/nested2/nested3'\n\nbecause the prefix is put unconditionally in front and after that a\ncomputed display path with is affected by `wt_prefix`. This is wrong as\nany relative path computation would need to be at the front. By emptying\nthe `wt_prefix` in recursive submodules and adding the information of any\nrelative path into the `prefix` this is fixed.\n\nSigned-off-by: Stefan Beller <sbeller@google.com>\n---\n git-submodule.sh             |  3 ++-\n t/t7407-submodule-foreach.sh | 20 ++++++++++++++++++++\n 2 files changed, 22 insertions(+), 1 deletion(-)\n\ndiff --git a/git-submodule.sh b/git-submodule.sh\nindex 43c68de..2838069 100755\n--- a/git-submodule.sh\n+++ b/git-submodule.sh\n@@ -417,10 +417,11 @@ cmd_foreach()\n \t\t\tsay \"$(eval_gettext \"Entering '\\$prefix\\$displaypath'\")\"\n \t\t\tname=$(git submodule--helper name \"$sm_path\")\n \t\t\t(\n-\t\t\t\tprefix=\"$prefix$sm_path/\"\n+\t\t\t\tprefix=\"$(relative_path $prefix$sm_path)/\"\n \t\t\t\tclear_local_git_env\n \t\t\t\tcd \"$sm_path\" &&\n \t\t\t\tsm_path=$(relative_path \"$sm_path\") &&\n+\t\t\t\twt_prefix=\n \t\t\t\t# we make $path available to scripts ...\n \t\t\t\tpath=$sm_path &&\n \t\t\t\tif test $# -eq 1\ndiff --git a/t/t7407-submodule-foreach.sh b/t/t7407-submodule-foreach.sh\nindex 7ca10b8..776b349 100755\n--- a/t/t7407-submodule-foreach.sh\n+++ b/t/t7407-submodule-foreach.sh\n@@ -178,6 +178,26 @@ test_expect_success 'test messages from \"foreach --recursive\"' '\n '\n \n cat > expect <<EOF\n+Entering '../nested1'\n+Entering '../nested1/nested2'\n+Entering '../nested1/nested2/nested3'\n+Entering '../nested1/nested2/nested3/submodule'\n+Entering '../sub1'\n+Entering '../sub2'\n+Entering '../sub3'\n+EOF\n+\n+test_expect_success 'test messages from \"foreach --recursive\" from subdirectory' '\n+\t(\n+\t\tcd clone2 &&\n+\t\tmkdir untracked &&\n+\t\tcd untracked &&\n+\t\tgit submodule foreach --recursive >../../actual\n+\t) &&\n+\ttest_i18ncmp expect actual\n+'\n+\n+cat > expect <<EOF\n nested1-nested1\n nested2-nested2\n nested3-nested3\n-- \n2.8.0.6.g3d0b0ba\n"},{"id":"282137","messageId":"1459292558-5840-3-git-send-email-sbeller@google.com","threadId":"41866","inReplyTo":"1459292558-5840-1-git-send-email-sbeller@google.com","subject":"[PATCH 2/6] submodule update --init: correct path handling in recursive submodules","fromName":"Stefan Beller","fromEmail":"sbeller@google.com","sentAt":"2016-03-29T23:02:34Z","receivedAt":"2016-03-29T23:02:34Z","isPatch":true,"sender":{"key":"stefanbeller@gmail.com","avatar":"https://avatars.githubusercontent.com/u/455868?v=4"},"body":"The new test demonstrates a failure in the code prior to this patch.\nInstead of getting the expected\n    Submodule 'submodule' (${pwd}/submodule) registered for path '../super/submodule'\nthe `super` directory is omitted and you get\n    Submodule 'submodule' (${pwd}/submodule) registered for path '../submodule'\ninstead.\n\nThat happens because the prefix is ignored in `git submodule add`, probably\nbecause that function itself cannot recurse; it may however called by\nrecursive instances of `git submodule update`, so we need to respect the\n`prefix`.\n\nSigned-off-by: Stefan Beller <sbeller@google.com>\n---\n git-submodule.sh            |  2 +-\n t/t7406-submodule-update.sh | 33 +++++++++++++++++++++++++++++++++\n 2 files changed, 34 insertions(+), 1 deletion(-)\n\ndiff --git a/git-submodule.sh b/git-submodule.sh\nindex 2838069..fdb5fbd 100755\n--- a/git-submodule.sh\n+++ b/git-submodule.sh\n@@ -474,7 +474,7 @@ cmd_init()\n \t\tdie_if_unmatched \"$mode\"\n \t\tname=$(git submodule--helper name \"$sm_path\") || exit\n \n-\t\tdisplaypath=$(relative_path \"$sm_path\")\n+\t\tdisplaypath=$(relative_path \"$prefix$sm_path\")\n \n \t\t# Copy url setting when it is not set yet\n \t\tif test -z \"$(git config \"submodule.$name.url\")\"\ndiff --git a/t/t7406-submodule-update.sh b/t/t7406-submodule-update.sh\nindex 68ea31d..9a4ba41 100755\n--- a/t/t7406-submodule-update.sh\n+++ b/t/t7406-submodule-update.sh\n@@ -63,6 +63,10 @@ test_expect_success 'setup a submodule tree' '\n \t git submodule add ../none none &&\n \t test_tick &&\n \t git commit -m \"none\"\n+\t) &&\n+\tgit clone . recursivesuper &&\n+\t( cd recursivesuper\n+\t git submodule add ../super super\n \t)\n '\n \n@@ -95,6 +99,35 @@ test_expect_success 'submodule update from subdirectory' '\n \t)\n '\n \n+supersha1=$(cd super && git rev-parse HEAD)\n+mergingsha1=$(cd super/merging && git rev-parse HEAD)\n+nonesha1=$(cd super/none && git rev-parse HEAD)\n+rebasingsha1=$(cd super/rebasing && git rev-parse HEAD)\n+submodulesha1=$(cd super/submodule && git rev-parse HEAD)\n+pwd=$(pwd)\n+\n+cat <<EOF >expect\n+Submodule path '../super': checked out '$supersha1'\n+Submodule 'merging' ($pwd/merging) registered for path '../super/merging'\n+Submodule 'none' ($pwd/none) registered for path '../super/none'\n+Submodule 'rebasing' ($pwd/rebasing) registered for path '../super/rebasing'\n+Submodule 'submodule' ($pwd/submodule) registered for path '../super/submodule'\n+Submodule path '../super/merging': checked out '$mergingsha1'\n+Submodule path '../super/none': checked out '$nonesha1'\n+Submodule path '../super/rebasing': checked out '$rebasingsha1'\n+Submodule path '../super/submodule': checked out '$submodulesha1'\n+EOF\n+\n+test_expect_success 'submodule update --init --recursive from subdirectory' '\n+\tgit -C recursivesuper/super reset --hard HEAD^ &&\n+\t(cd recursivesuper &&\n+\t mkdir tmp &&\n+\t cd tmp &&\n+\t git submodule update --init --recursive ../super >../../actual\n+\t) &&\n+\ttest_cmp expect actual\n+'\n+\n apos=\"'\";\n test_expect_success 'submodule update does not fetch already present commits' '\n \t(cd submodule &&\n-- \n2.8.0.6.g3d0b0ba\n"},{"id":"282142","messageId":"1459292558-5840-4-git-send-email-sbeller@google.com","threadId":"41866","inReplyTo":"1459292558-5840-1-git-send-email-sbeller@google.com","subject":"[PATCH 3/6] submodule status: correct path handling in recursive submodules","fromName":"Stefan Beller","fromEmail":"sbeller@google.com","sentAt":"2016-03-29T23:02:35Z","receivedAt":"2016-03-29T23:02:35Z","isPatch":true,"sender":{"key":"stefanbeller@gmail.com","avatar":"https://avatars.githubusercontent.com/u/455868?v=4"},"body":"The new test which is a replica of the previous test except\nthat it executes from a sub directory. Prior to this patch\nthe test failed by having too many '../' prefixed:\n\n  --- expect\t2016-03-29 19:02:33.087336115 +0000\n  +++ actual\t2016-03-29 19:02:33.359343311 +0000\n  @@ -1,7 +1,7 @@\n    b23f134787d96fae589a6b76da41f4db112fc8db ../nested1 (heads/master)\n  -+25d56d1ddfb35c3e91ff7d8f12331c2e53147dcc ../nested1/nested2 (file2)\n  - 5ec83512b76a0b8170b899f8e643913c3e9b72d9 ../nested1/nested2/nested3 (heads/master)\n  - 509f622a4f36a3e472affcf28fa959174f3dd5b5 ../nested1/nested2/nested3/submodule (heads/master)\n  ++25d56d1ddfb35c3e91ff7d8f12331c2e53147dcc ../../nested1/nested2 (file2)\n  + 5ec83512b76a0b8170b899f8e643913c3e9b72d9 ../../../nested1/nested2/nested3 (heads/master)\n  + 509f622a4f36a3e472affcf28fa959174f3dd5b5 ../../../../nested1/nested2/nested3/submodule (heads/master)\n    0c90624ab7f1aaa301d3bb79f60dcfed1ec4897f ../sub1 (0c90624)\n    0c90624ab7f1aaa301d3bb79f60dcfed1ec4897f ../sub2 (0c90624)\n    509f622a4f36a3e472affcf28fa959174f3dd5b5 ../sub3 (heads/master)\n\nThe path code in question:\n  displaypath=$(relative_path \"$prefix$sm_path\")\n  prefix=$displaypath\n  if recursive:\n    eval cmd_status\n\nThat way we change `prefix` each iteration to contain another\n'../', because of the the relative_path computation is done\non an already computed relative path.\n\nWe must call relative_path exactly once with `wt_prefix` non empty.\nFurther calls in recursive instances to to calculate the displaypath\nalready incorporate the correct prefix from before. Fix the issue by\nclearing `wt_prefix` in recursive calls.\n\nSigned-off-by: Stefan Beller <sbeller@google.com>\n---\n git-submodule.sh             |  1 +\n t/t7407-submodule-foreach.sh | 21 +++++++++++++++++++++\n 2 files changed, 22 insertions(+)\n\ndiff --git a/git-submodule.sh b/git-submodule.sh\nindex fdb5fbd..11ed32a 100755\n--- a/git-submodule.sh\n+++ b/git-submodule.sh\n@@ -1160,6 +1160,7 @@ cmd_status()\n \t\t\t(\n \t\t\t\tprefix=\"$displaypath/\"\n \t\t\t\tclear_local_git_env\n+\t\t\t\twt_prefix=\n \t\t\t\tcd \"$sm_path\" &&\n \t\t\t\teval cmd_status\n \t\t\t) ||\ndiff --git a/t/t7407-submodule-foreach.sh b/t/t7407-submodule-foreach.sh\nindex 776b349..4b35e12 100755\n--- a/t/t7407-submodule-foreach.sh\n+++ b/t/t7407-submodule-foreach.sh\n@@ -277,6 +277,27 @@ test_expect_success 'ensure \"status --cached --recursive\" preserves the --cached\n \ttest_cmp expect actual\n '\n \n+nested2sha1=$(git -C clone3/nested1/nested2 rev-parse HEAD)\n+\n+cat > expect <<EOF\n+ $nested1sha1 ../nested1 (heads/master)\n++$nested2sha1 ../nested1/nested2 (file2)\n+ $nested3sha1 ../nested1/nested2/nested3 (heads/master)\n+ $submodulesha1 ../nested1/nested2/nested3/submodule (heads/master)\n+ $sub1sha1 ../sub1 ($sub1sha1_short)\n+ $sub2sha1 ../sub2 ($sub2sha1_short)\n+ $sub3sha1 ../sub3 (heads/master)\n+EOF\n+\n+test_expect_success 'test \"status --recursive\" from sub directory' '\n+\t(\n+\t\tcd clone3 &&\n+\t\tmkdir tmp && cd tmp &&\n+\t\tgit submodule status --recursive > ../../actual\n+\t) &&\n+\ttest_cmp expect actual\n+'\n+\n test_expect_success 'use \"git clone --recursive\" to checkout all submodules' '\n \tgit clone --recursive super clone4 &&\n \t(\n-- \n2.8.0.6.g3d0b0ba\n"},{"id":"282140","messageId":"1459292558-5840-5-git-send-email-sbeller@google.com","threadId":"41866","inReplyTo":"1459292558-5840-1-git-send-email-sbeller@google.com","subject":"[PATCH 4/6] submodule update: align reporting path for custom command execution","fromName":"Stefan Beller","fromEmail":"sbeller@google.com","sentAt":"2016-03-29T23:02:36Z","receivedAt":"2016-03-29T23:02:36Z","isPatch":true,"sender":{"key":"stefanbeller@gmail.com","avatar":"https://avatars.githubusercontent.com/u/455868?v=4"},"body":"In the predefined actions (merge, rebase, none, checkout), we use\nthe display path, which is relative to the current working directory.\nAlso use the display path when running a custom command.\n\nSigned-off-by: Stefan Beller <sbeller@google.com>\n---\n git-submodule.sh            |  4 ++--\n t/t7406-submodule-update.sh | 29 ++++++++++++++++++++++++++---\n 2 files changed, 28 insertions(+), 5 deletions(-)\n\ndiff --git a/git-submodule.sh b/git-submodule.sh\nindex 11ed32a..be2a2b5 100755\n--- a/git-submodule.sh\n+++ b/git-submodule.sh\n@@ -803,8 +803,8 @@ Maybe you want to use 'update --init'?\")\"\n \t\t\t\t;;\n \t\t\t!*)\n \t\t\t\tcommand=\"${update_module#!}\"\n-\t\t\t\tdie_msg=\"$(eval_gettext \"Execution of '\\$command \\$sha1' failed in submodule path '\\$prefix\\$sm_path'\")\"\n-\t\t\t\tsay_msg=\"$(eval_gettext \"Submodule path '\\$prefix\\$sm_path': '\\$command \\$sha1'\")\"\n+\t\t\t\tdie_msg=\"$(eval_gettext \"Execution of '\\$command \\$sha1' failed in submodule path '\\$displaypath'\")\"\n+\t\t\t\tsay_msg=\"$(eval_gettext \"Submodule path '\\$displaypath': '\\$command \\$sha1'\")\"\n \t\t\t\tmust_die_on_failure=yes\n \t\t\t\t;;\n \t\t\t*)\ndiff --git a/t/t7406-submodule-update.sh b/t/t7406-submodule-update.sh\nindex 9a4ba41..f062065 100755\n--- a/t/t7406-submodule-update.sh\n+++ b/t/t7406-submodule-update.sh\n@@ -344,16 +344,39 @@ test_expect_success 'submodule update - command in .git/config' '\n \t)\n '\n \n+cat << EOF >expect\n+Execution of 'false $submodulesha1' failed in submodule path 'submodule'\n+EOF\n+\n test_expect_success 'submodule update - command in .git/config catches failure' '\n \t(cd super &&\n \t git config submodule.submodule.update \"!false\"\n \t) &&\n \t(cd super/submodule &&\n-\t  git reset --hard HEAD^\n+\t  git reset --hard $submodulesha1^\n \t) &&\n \t(cd super &&\n-\t test_must_fail git submodule update submodule\n-\t)\n+\t test_must_fail git submodule update submodule 2>../actual\n+\t) &&\n+\ttest_cmp actual expect\n+'\n+\n+cat << EOF >expect\n+Execution of 'false $submodulesha1' failed in submodule path '../submodule'\n+EOF\n+\n+test_expect_success 'submodule update - command in .git/config catches failure -- subdirectory' '\n+\t(cd super &&\n+\t git config submodule.submodule.update \"!false\"\n+\t) &&\n+\t(cd super/submodule &&\n+\t  git reset --hard $submodulesha1^\n+\t) &&\n+\t(cd super &&\n+\t mkdir tmp && cd tmp &&\n+\t test_must_fail git submodule update ../submodule 2>../../actual\n+\t) &&\n+\ttest_cmp actual expect\n '\n \n test_expect_success 'submodule init does not copy command into .git/config' '\n-- \n2.8.0.6.g3d0b0ba\n"},{"id":"282141","messageId":"1459292558-5840-6-git-send-email-sbeller@google.com","threadId":"41866","inReplyTo":"1459292558-5840-1-git-send-email-sbeller@google.com","subject":"[PATCH 5/6] submodule update: test recursive path reporting from subdirectory","fromName":"Stefan Beller","fromEmail":"sbeller@google.com","sentAt":"2016-03-29T23:02:37Z","receivedAt":"2016-03-29T23:02:37Z","isPatch":true,"sender":{"key":"stefanbeller@gmail.com","avatar":"https://avatars.githubusercontent.com/u/455868?v=4"},"body":"This patch is just a test and fixes no bug as there is currently no bug\nin the path handling of `submodule update`.\n\nIn `submodule update` we make a call to `submodule--helper list --prefix\n\"$wt_prefix\"` which looks a bit brittle and likely to introduce a bug\nfor the path handling. It is not a bug as the prefix is ignored inside\nthe submodule helper for now. If this test breaks eventually, we want\nto make sure the `wt_prefix` is passed correctly into recursive submodules.\nHint: In recursive submodules we expect `wt_prefix` to be empty.\n\nSigned-off-by: Stefan Beller <sbeller@google.com>\n---\n t/t7406-submodule-update.sh | 20 ++++++++++++++++++++\n 1 file changed, 20 insertions(+)\n\ndiff --git a/t/t7406-submodule-update.sh b/t/t7406-submodule-update.sh\nindex f062065..6dcf2d4 100755\n--- a/t/t7406-submodule-update.sh\n+++ b/t/t7406-submodule-update.sh\n@@ -379,6 +379,26 @@ test_expect_success 'submodule update - command in .git/config catches failure -\n \ttest_cmp actual expect\n '\n \n+cat << EOF >expect\n+Execution of 'false $submodulesha1' failed in submodule path '../super/submodule'\n+Failed to recurse into submodule path '../super'\n+EOF\n+\n+test_expect_success 'recursive submodule update - command in .git/config catches failure -- subdirectory' '\n+\t(cd recursivesuper &&\n+\t git submodule update --remote super &&\n+\t git add super &&\n+\t git commit -m \"update to latest to have more than one commit in submodules\"\n+\t) &&\n+\tgit -C recursivesuper/super config submodule.submodule.update \"!false\" &&\n+\tgit -C recursivesuper/super/submodule reset --hard $submodulesha1^ &&\n+\t(cd recursivesuper &&\n+\t mkdir -p tmp && cd tmp &&\n+\t test_must_fail git submodule update --recursive ../super 2>../../actual\n+\t) &&\n+\ttest_cmp actual expect\n+'\n+\n test_expect_success 'submodule init does not copy command into .git/config' '\n \t(cd super &&\n \t H=$(git ls-files -s submodule | cut -d\" \" -f2) &&\n-- \n2.8.0.6.g3d0b0ba\n"},{"id":"282139","messageId":"1459292558-5840-7-git-send-email-sbeller@google.com","threadId":"41866","inReplyTo":"1459292558-5840-1-git-send-email-sbeller@google.com","subject":"[PATCH 6/6] t7407: make expectation as clear as possible","fromName":"Stefan Beller","fromEmail":"sbeller@google.com","sentAt":"2016-03-29T23:02:38Z","receivedAt":"2016-03-29T23:02:38Z","isPatch":true,"sender":{"key":"stefanbeller@gmail.com","avatar":"https://avatars.githubusercontent.com/u/455868?v=4"},"body":"Not everyone (including me) grasps the sed expression in a split second as\nthey would grasp the 4 lines printed as is.\n\nSigned-off-by: Stefan Beller <sbeller@google.com>\n---\n t/t7407-submodule-foreach.sh | 8 ++++++--\n 1 file changed, 6 insertions(+), 2 deletions(-)\n\ndiff --git a/t/t7407-submodule-foreach.sh b/t/t7407-submodule-foreach.sh\nindex 4b35e12..6ba5daf 100755\n--- a/t/t7407-submodule-foreach.sh\n+++ b/t/t7407-submodule-foreach.sh\n@@ -262,8 +262,12 @@ test_expect_success 'test \"status --recursive\"' '\n \ttest_cmp expect actual\n '\n \n-sed -e \"/nested2 /s/.*/+$nested2sha1 nested1\\/nested2 (file2~1)/;/sub[1-3]/d\" < expect > expect2\n-mv -f expect2 expect\n+cat > expect <<EOF\n+ $nested1sha1 nested1 (heads/master)\n++$nested2sha1 nested1/nested2 (file2~1)\n+ $nested3sha1 nested1/nested2/nested3 (heads/master)\n+ $submodulesha1 nested1/nested2/nested3/submodule (heads/master)\n+EOF\n \n test_expect_success 'ensure \"status --cached --recursive\" preserves the --cached flag' '\n \t(\n-- \n2.8.0.6.g3d0b0ba\n"},{"id":"282149","messageId":"xmqqshz8vo8k.fsf@gitster.mtv.corp.google.com","threadId":"41866","inReplyTo":"1459292558-5840-2-git-send-email-sbeller@google.com","subject":"Re: [PATCH 1/6] submodule foreach: correct path display in recursive submodules","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2016-03-29T23:49:31Z","receivedAt":"2016-03-29T23:49:31Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Stefan Beller <sbeller@google.com> writes:\n\n> The new test replicates the previous test with the difference of executing\n> from a sub directory. By executing from a sub directory all we would\n> expect all displayed paths to be prefixed by '../'.\n\n-ECANTPARSE on the last sentence.  Remove the first \"all\" perhaps?\n\nAs this patch is no longer about a new test but is about a fix of a\nproblem, for which a new test serves as a typical example, it sounds\nsomewhat funny to start the log message with description of the test.\n\n> Prior to this patch the test would report\n>     Entering 'nested1/nested2/../nested3'\n> instead of the expected\n>     Entering '../nested1/nested2/nested3'\n>\n> because the prefix is put unconditionally in front and after that a\n> computed display path with is affected by `wt_prefix`.\n\n-ECANTPARSE even though I can guess what you want to say.\n\n> diff --git a/git-submodule.sh b/git-submodule.sh\n> index 43c68de..2838069 100755\n> --- a/git-submodule.sh\n> +++ b/git-submodule.sh\n> @@ -417,10 +417,11 @@ cmd_foreach()\n>  \t\t\tsay \"$(eval_gettext \"Entering '\\$prefix\\$displaypath'\")\"\n>  \t\t\tname=$(git submodule--helper name \"$sm_path\")\n>  \t\t\t(\n> -\t\t\t\tprefix=\"$prefix$sm_path/\"\n> +\t\t\t\tprefix=\"$(relative_path $prefix$sm_path)/\"\n\nWhat happens when prefix or sm_path has $IFS whitespace?  Imitate the\ncorrect quoting you do three lines below, i.e.\n\n\tprefix=$(relative_path \"$prefix$sm_path\")/\n\n>  \t\t\t\tclear_local_git_env\n>  \t\t\t\tcd \"$sm_path\" &&\n>  \t\t\t\tsm_path=$(relative_path \"$sm_path\") &&\n> +\t\t\t\twt_prefix=\n>  \t\t\t\t# we make $path available to scripts ...\n>  \t\t\t\tpath=$sm_path &&\n>  \t\t\t\tif test $# -eq 1\n"},{"id":"282150","messageId":"xmqqoa9wvo0w.fsf@gitster.mtv.corp.google.com","threadId":"41866","inReplyTo":"1459292558-5840-3-git-send-email-sbeller@google.com","subject":"Re: [PATCH 2/6] submodule update --init: correct path handling in recursive submodules","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2016-03-29T23:54:07Z","receivedAt":"2016-03-29T23:54:07Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Stefan Beller <sbeller@google.com> writes:\n\n> The new test demonstrates a failure in the code prior to this patch.\n> Instead of getting the expected\n>     Submodule 'submodule' (${pwd}/submodule) registered for path '../super/submodule'\n> the `super` directory is omitted and you get\n>     Submodule 'submodule' (${pwd}/submodule) registered for path '../submodule'\n> instead.\n\nSame \"is this about test?\" comment applies here.\n\n> That happens because the prefix is ignored in `git submodule add`, probably\n> because that function itself cannot recurse;\n\n\"probably\"???\n\n> it may however called by\n\nProbably \"be\" needs to be somewhere on this line.\n\n> recursive instances of `git submodule update`, so we need to respect the\n> `prefix`.\n>\n> Signed-off-by: Stefan Beller <sbeller@google.com>\n> ---\n>  git-submodule.sh            |  2 +-\n>  t/t7406-submodule-update.sh | 33 +++++++++++++++++++++++++++++++++\n>  2 files changed, 34 insertions(+), 1 deletion(-)\n>\n> diff --git a/git-submodule.sh b/git-submodule.sh\n> index 2838069..fdb5fbd 100755\n> --- a/git-submodule.sh\n> +++ b/git-submodule.sh\n> @@ -474,7 +474,7 @@ cmd_init()\n>  \t\tdie_if_unmatched \"$mode\"\n>  \t\tname=$(git submodule--helper name \"$sm_path\") || exit\n>  \n> -\t\tdisplaypath=$(relative_path \"$sm_path\")\n> +\t\tdisplaypath=$(relative_path \"$prefix$sm_path\")\n>  \n>  \t\t# Copy url setting when it is not set yet\n>  \t\tif test -z \"$(git config \"submodule.$name.url\")\"\n> diff --git a/t/t7406-submodule-update.sh b/t/t7406-submodule-update.sh\n> index 68ea31d..9a4ba41 100755\n> --- a/t/t7406-submodule-update.sh\n> +++ b/t/t7406-submodule-update.sh\n> @@ -63,6 +63,10 @@ test_expect_success 'setup a submodule tree' '\n>  \t git submodule add ../none none &&\n>  \t test_tick &&\n>  \t git commit -m \"none\"\n> +\t) &&\n> +\tgit clone . recursivesuper &&\n> +\t( cd recursivesuper\n> +\t git submodule add ../super super\n>  \t)\n>  '\n>  \n> @@ -95,6 +99,35 @@ test_expect_success 'submodule update from subdirectory' '\n>  \t)\n>  '\n>  \n> +supersha1=$(cd super && git rev-parse HEAD)\n\nPerhaps \"git -C super rev-parse HEAD\"?\n\n> +test_expect_success 'submodule update --init --recursive from subdirectory' '\n> +\tgit -C recursivesuper/super reset --hard HEAD^ &&\n> +\t(cd recursivesuper &&\n> +\t mkdir tmp &&\n> +\t cd tmp &&\n> +\t git submodule update --init --recursive ../super >../../actual\n> +\t) &&\n> +\ttest_cmp expect actual\n> +'\n> +\n>  apos=\"'\";\n>  test_expect_success 'submodule update does not fetch already present commits' '\n>  \t(cd submodule &&\n"},{"id":"282151","messageId":"xmqqk2kkvny3.fsf@gitster.mtv.corp.google.com","threadId":"41866","inReplyTo":"1459292558-5840-4-git-send-email-sbeller@google.com","subject":"Re: [PATCH 3/6] submodule status: correct path handling in recursive submodules","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2016-03-29T23:55:48Z","receivedAt":"2016-03-29T23:55:48Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Stefan Beller <sbeller@google.com> writes:\n\n> The new test which is a replica of the previous test except\n> that it executes from a sub directory. Prior to this patch\n> the test failed by having too many '../' prefixed:\n>\n>   --- expect\t2016-03-29 19:02:33.087336115 +0000\n>   +++ actual\t2016-03-29 19:02:33.359343311 +0000\n>   @@ -1,7 +1,7 @@\n>     b23f134787d96fae589a6b76da41f4db112fc8db ../nested1 (heads/master)\n>   -+25d56d1ddfb35c3e91ff7d8f12331c2e53147dcc ../nested1/nested2 (file2)\n>   - 5ec83512b76a0b8170b899f8e643913c3e9b72d9 ../nested1/nested2/nested3 (heads/master)\n>   - 509f622a4f36a3e472affcf28fa959174f3dd5b5 ../nested1/nested2/nested3/submodule (heads/master)\n>   ++25d56d1ddfb35c3e91ff7d8f12331c2e53147dcc ../../nested1/nested2 (file2)\n>   + 5ec83512b76a0b8170b899f8e643913c3e9b72d9 ../../../nested1/nested2/nested3 (heads/master)\n>   + 509f622a4f36a3e472affcf28fa959174f3dd5b5 ../../../../nested1/nested2/nested3/submodule (heads/master)\n>     0c90624ab7f1aaa301d3bb79f60dcfed1ec4897f ../sub1 (0c90624)\n>     0c90624ab7f1aaa301d3bb79f60dcfed1ec4897f ../sub2 (0c90624)\n>     509f622a4f36a3e472affcf28fa959174f3dd5b5 ../sub3 (heads/master)\n>\n> The path code in question:\n>   displaypath=$(relative_path \"$prefix$sm_path\")\n>   prefix=$displaypath\n>   if recursive:\n>     eval cmd_status\n>\n> That way we change `prefix` each iteration to contain another\n> '../', because of the the relative_path computation is done\n> on an already computed relative path.\n>\n> We must call relative_path exactly once with `wt_prefix` non empty.\n> Further calls in recursive instances to to calculate the displaypath\n> already incorporate the correct prefix from before. Fix the issue by\n> clearing `wt_prefix` in recursive calls.\n\nOK, nicely analyzed and explained.\n\n> Signed-off-by: Stefan Beller <sbeller@google.com>\n> ---\n>  git-submodule.sh             |  1 +\n>  t/t7407-submodule-foreach.sh | 21 +++++++++++++++++++++\n>  2 files changed, 22 insertions(+)\n>\n> diff --git a/git-submodule.sh b/git-submodule.sh\n> index fdb5fbd..11ed32a 100755\n> --- a/git-submodule.sh\n> +++ b/git-submodule.sh\n> @@ -1160,6 +1160,7 @@ cmd_status()\n>  \t\t\t(\n>  \t\t\t\tprefix=\"$displaypath/\"\n>  \t\t\t\tclear_local_git_env\n> +\t\t\t\twt_prefix=\n>  \t\t\t\tcd \"$sm_path\" &&\n>  \t\t\t\teval cmd_status\n>  \t\t\t) ||\n\nMakes sense.\n\nThanks.\n\n\n> diff --git a/t/t7407-submodule-foreach.sh b/t/t7407-submodule-foreach.sh\n> index 776b349..4b35e12 100755\n> --- a/t/t7407-submodule-foreach.sh\n> +++ b/t/t7407-submodule-foreach.sh\n> @@ -277,6 +277,27 @@ test_expect_success 'ensure \"status --cached --recursive\" preserves the --cached\n>  \ttest_cmp expect actual\n>  '\n>  \n> +nested2sha1=$(git -C clone3/nested1/nested2 rev-parse HEAD)\n> +\n> +cat > expect <<EOF\n> + $nested1sha1 ../nested1 (heads/master)\n> ++$nested2sha1 ../nested1/nested2 (file2)\n> + $nested3sha1 ../nested1/nested2/nested3 (heads/master)\n> + $submodulesha1 ../nested1/nested2/nested3/submodule (heads/master)\n> + $sub1sha1 ../sub1 ($sub1sha1_short)\n> + $sub2sha1 ../sub2 ($sub2sha1_short)\n> + $sub3sha1 ../sub3 (heads/master)\n> +EOF\n> +\n> +test_expect_success 'test \"status --recursive\" from sub directory' '\n> +\t(\n> +\t\tcd clone3 &&\n> +\t\tmkdir tmp && cd tmp &&\n> +\t\tgit submodule status --recursive > ../../actual\n> +\t) &&\n> +\ttest_cmp expect actual\n> +'\n> +\n>  test_expect_success 'use \"git clone --recursive\" to checkout all submodules' '\n>  \tgit clone --recursive super clone4 &&\n>  \t(\n"},{"id":"282152","messageId":"xmqqfuv8vnvk.fsf@gitster.mtv.corp.google.com","threadId":"41866","inReplyTo":"1459292558-5840-5-git-send-email-sbeller@google.com","subject":"Re: [PATCH 4/6] submodule update: align reporting path for custom command execution","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2016-03-29T23:57:19Z","receivedAt":"2016-03-29T23:57:19Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Stefan Beller <sbeller@google.com> writes:\n\n> In the predefined actions (merge, rebase, none, checkout), we use\n> the display path, which is relative to the current working directory.\n> Also use the display path when running a custom command.\n\nVery sensible.\n\n> Signed-off-by: Stefan Beller <sbeller@google.com>\n> ---\n>  git-submodule.sh            |  4 ++--\n>  t/t7406-submodule-update.sh | 29 ++++++++++++++++++++++++++---\n>  2 files changed, 28 insertions(+), 5 deletions(-)\n>\n> diff --git a/git-submodule.sh b/git-submodule.sh\n> index 11ed32a..be2a2b5 100755\n> --- a/git-submodule.sh\n> +++ b/git-submodule.sh\n> @@ -803,8 +803,8 @@ Maybe you want to use 'update --init'?\")\"\n>  \t\t\t\t;;\n>  \t\t\t!*)\n>  \t\t\t\tcommand=\"${update_module#!}\"\n> -\t\t\t\tdie_msg=\"$(eval_gettext \"Execution of '\\$command \\$sha1' failed in submodule path '\\$prefix\\$sm_path'\")\"\n> -\t\t\t\tsay_msg=\"$(eval_gettext \"Submodule path '\\$prefix\\$sm_path': '\\$command \\$sha1'\")\"\n> +\t\t\t\tdie_msg=\"$(eval_gettext \"Execution of '\\$command \\$sha1' failed in submodule path '\\$displaypath'\")\"\n> +\t\t\t\tsay_msg=\"$(eval_gettext \"Submodule path '\\$displaypath': '\\$command \\$sha1'\")\"\n>  \t\t\t\tmust_die_on_failure=yes\n>  \t\t\t\t;;\n>  \t\t\t*)\n> diff --git a/t/t7406-submodule-update.sh b/t/t7406-submodule-update.sh\n> index 9a4ba41..f062065 100755\n> --- a/t/t7406-submodule-update.sh\n> +++ b/t/t7406-submodule-update.sh\n> @@ -344,16 +344,39 @@ test_expect_success 'submodule update - command in .git/config' '\n>  \t)\n>  '\n>  \n> +cat << EOF >expect\n> +Execution of 'false $submodulesha1' failed in submodule path 'submodule'\n> +EOF\n> +\n>  test_expect_success 'submodule update - command in .git/config catches failure' '\n>  \t(cd super &&\n>  \t git config submodule.submodule.update \"!false\"\n>  \t) &&\n>  \t(cd super/submodule &&\n> -\t  git reset --hard HEAD^\n> +\t  git reset --hard $submodulesha1^\n>  \t) &&\n>  \t(cd super &&\n> -\t test_must_fail git submodule update submodule\n> -\t)\n> +\t test_must_fail git submodule update submodule 2>../actual\n> +\t) &&\n> +\ttest_cmp actual expect\n> +'\n> +\n> +cat << EOF >expect\n> +Execution of 'false $submodulesha1' failed in submodule path '../submodule'\n> +EOF\n> +\n> +test_expect_success 'submodule update - command in .git/config catches failure -- subdirectory' '\n> +\t(cd super &&\n> +\t git config submodule.submodule.update \"!false\"\n> +\t) &&\n> +\t(cd super/submodule &&\n> +\t  git reset --hard $submodulesha1^\n> +\t) &&\n> +\t(cd super &&\n> +\t mkdir tmp && cd tmp &&\n> +\t test_must_fail git submodule update ../submodule 2>../../actual\n> +\t) &&\n> +\ttest_cmp actual expect\n>  '\n>  \n>  test_expect_success 'submodule init does not copy command into .git/config' '\n"},{"id":"282165","messageId":"CAGZ79ka3ZL8Avs4gt5ODtQQh7o2TFQAj2RQEJz3SdYV7_Z1Oqw@mail.gmail.com","threadId":"41866","inReplyTo":"xmqqoa9wvo0w.fsf@gitster.mtv.corp.google.com","subject":"Re: [PATCH 2/6] submodule update --init: correct path handling in recursive submodules","fromName":"Stefan Beller","fromEmail":"sbeller@google.com","sentAt":"2016-03-30T01:00:11Z","receivedAt":"2016-03-30T01:00:11Z","isPatch":true,"sender":{"key":"stefanbeller@gmail.com","avatar":"https://avatars.githubusercontent.com/u/455868?v=4"},"body":"On Tue, Mar 29, 2016 at 4:54 PM, Junio C Hamano <gitster@pobox.com> wrote:\n> Stefan Beller <sbeller@google.com> writes:\n>\n>> The new test demonstrates a failure in the code prior to this patch.\n>> Instead of getting the expected\n>>     Submodule 'submodule' (${pwd}/submodule) registered for path '../super/submodule'\n>> the `super` directory is omitted and you get\n>>     Submodule 'submodule' (${pwd}/submodule) registered for path '../submodule'\n>> instead.\n>\n> Same \"is this about test?\" comment applies here.\n>\n>> That happens because the prefix is ignored in `git submodule add`, probably\n>> because that function itself cannot recurse;\n>\n> \"probably\"???\n\nI am speculating there. I have no idea why it originally was written that way\nbut I would assume this is the most likeliest explanation.\nI'll reword the whole commit message.\n\n>\n>>\n>> +supersha1=$(cd super && git rev-parse HEAD)\n>\n> Perhaps \"git -C super rev-parse HEAD\"?\n\ndone\n"}]}