{"thread":{"id":"31946","subject":"[PATCH] submodule status: properly pass options with --recursive","startedAt":"2012-10-25T22:20:29Z","lastAt":"2012-10-29T07:27:19Z","messageCount":12,"participants":["Jens Lehmann","Jeff King","Phil Hord"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"201922","messageId":"5089BB2D.90400@web.de","threadId":"31946","inReplyTo":null,"subject":"[PATCH] submodule status: properly pass options with --recursive","fromName":"Jens Lehmann","fromEmail":"jens.lehmann@web.de","sentAt":"2012-10-25T22:20:29Z","receivedAt":"2012-10-25T22:20:29Z","isPatch":true,"sender":{"key":"jens.lehmann@web.de","avatar":"https://avatars.githubusercontent.com/u/135220?v=4"},"body":"When renaming orig_args to orig_flags in 98dbe63d (submodule: only\npreserve flags across recursive status/update invocations) the call site\nof the recursive cmd_status was forgotten. At that place orig_args is\nstill passed into the recursion, which is always empty now. This clears\nall options when recursing, as that variable is never set.\n\nFix that by renaming orig_args to orig_flags there too and add a test to\ncatch that bug.\n\nSigned-off-by: Jens Lehmann <Jens.Lehmann@web.de>\n---\n\nI noticed that when reviewing Phil's \"Teach --recursive to submodule\nsync\" patch.\n\n git-submodule.sh             | 2 +-\n t/t7407-submodule-foreach.sh | 8 ++++++++\n 2 files changed, 9 insertions(+), 1 deletion(-)\n\ndiff --git a/git-submodule.sh b/git-submodule.sh\nindex ab6b110..c089d48 100755\n--- a/git-submodule.sh\n+++ b/git-submodule.sh\n@@ -990,7 +990,7 @@ cmd_status()\n \t\t\t\tprefix=\"$displaypath/\"\n \t\t\t\tclear_local_git_env\n \t\t\t\tcd \"$sm_path\" &&\n-\t\t\t\teval cmd_status \"$orig_args\"\n+\t\t\t\teval cmd_status \"$orig_flags\"\n \t\t\t) ||\n \t\t\tdie \"$(eval_gettext \"Failed to recurse into submodule path '\\$sm_path'\")\"\n \t\tfi\ndiff --git a/t/t7407-submodule-foreach.sh b/t/t7407-submodule-foreach.sh\nindex 9b69fe2..eca36b5 100755\n--- a/t/t7407-submodule-foreach.sh\n+++ b/t/t7407-submodule-foreach.sh\n@@ -245,6 +245,14 @@ test_expect_success 'ensure \"status --cached --recursive\" preserves the --cached\n \ttest_cmp expect actual\n '\n\n+test_expect_success 'ensure \"status --quiet --recursive\" preserves the --quiet flag' '\n+\t(\n+\t\tcd clone3 &&\n+\t\tgit submodule status --quiet --recursive -- nested1 > ../actual\n+\t) &&\n+\t! test -s actual\n+'\n+\n test_expect_success 'use \"git clone --recursive\" to checkout all submodules' '\n \tgit clone --recursive super clone4 &&\n \t(\n-- \n1.8.0.dirty\n"},{"id":"201940","messageId":"20121026131507.GA2747@sigill.intra.peff.net","threadId":"31946","inReplyTo":"5089BB2D.90400@web.de","subject":"Re: [PATCH] submodule status: properly pass options with --recursive","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2012-10-26T13:15:07Z","receivedAt":"2012-10-26T13:15:07Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Fri, Oct 26, 2012 at 12:20:29AM +0200, Jens Lehmann wrote:\n\n> When renaming orig_args to orig_flags in 98dbe63d (submodule: only\n> preserve flags across recursive status/update invocations) the call site\n> of the recursive cmd_status was forgotten. At that place orig_args is\n> still passed into the recursion, which is always empty now. This clears\n> all options when recursing, as that variable is never set.\n> \n> Fix that by renaming orig_args to orig_flags there too and add a test to\n> catch that bug.\n\nThanks. I back-ported your patch on top of 98dbe63d so it can go to\n'maint'. I'm curious, though: why didn't the test right before (which\nchecks recursion for --cached) catch this?\n\n-Peff\n"},{"id":"201979","messageId":"CABURp0op2+QUvusUmAFUxT8s8c02bB9V3=ag9gTTSiiN4t96OA@mail.gmail.com","threadId":"31946","inReplyTo":"20121026131507.GA2747@sigill.intra.peff.net","subject":"Re: [PATCH] submodule status: properly pass options with --recursive","fromName":"Phil Hord","fromEmail":"phil.hord@gmail.com","sentAt":"2012-10-26T19:07:11Z","receivedAt":"2012-10-26T19:07:11Z","isPatch":true,"sender":{"key":"phil.hord@gmail.com","avatar":"https://avatars.githubusercontent.com/u/123908?v=4"},"body":"On Fri, Oct 26, 2012 at 9:15 AM, Jeff King <peff@peff.net> wrote:\n> On Fri, Oct 26, 2012 at 12:20:29AM +0200, Jens Lehmann wrote:\n>\n>> When renaming orig_args to orig_flags in 98dbe63d (submodule: only\n>> preserve flags across recursive status/update invocations) the call site\n>> of the recursive cmd_status was forgotten. At that place orig_args is\n>> still passed into the recursion, which is always empty now. This clears\n>> all options when recursing, as that variable is never set.\n>>\n>> Fix that by renaming orig_args to orig_flags there too and add a test to\n>> catch that bug.\n>\n> Thanks. I back-ported your patch on top of 98dbe63d so it can go to\n> 'maint'. I'm curious, though: why didn't the test right before (which\n> checks recursion for --cached) catch this?\n\nI was wondering the same thing about why 'git submodule sync\n--recursive --quiet' succeeded, so I checked on it.  The answer is\nthat \"--quiet\" sets GIT_QUIET=1, which is then inherited by the\nrecursive call. Indeed, Jens' new test passes even without the\naccompanying fix.  :-\\\n\nThe 'status --recursive --cached' test passes for two reasons.  The\nfirst is that the test is checking for a cached change in a level-1\nsubmodule, but it is the level-2+ submodules which do not get the\nflags passed down correctly in \"$@\".  The 2nd reason is that the\n$cached variable is _also_ inherited by the recursive call to\ncmd_status, specifically because it is a call to cmd_status() and not\nto '$SHELL git submodule status', where the latter would have cleared\nthe flags at startup, but the former does not.\n\nI'm a bit wary about \"fixing\" the flags for the recursion machinery.\nI'm starting to think $orig_flags is moot in almost all cases.  It\nlooks like clearing all the flags on each iteration would break 'git\nsubmodule foreach --recursive --quiet' because it does not use\n$orig_flags at all.  Who knows what other surprises lurk there.\n\nHere's a fix for the test, though, even though it still does not fail\nin the absence of the $orig_flags patch.\n\n-- >8 --\n\ndiff --git i/t/t7407-submodule-foreach.sh w/t/t7407-submodule-foreach.sh\nindex 9b69fe2..8442b32 100755\n--- i/t/t7407-submodule-foreach.sh\n+++ w/t/t7407-submodule-foreach.sh\n@@ -226,14 +226,14 @@ test_expect_success 'test \"status --recursive\"' '\n        test_cmp expect actual\n '\n\n-sed -e \"/nested1 /s/.*/+$nested1sha1 nested1 (file2~1)/;/sub[1-3]/d\"\n< expect > expect2\n+sed -e \"/nested2 /s/.*/+$nested2sha1 nested1\\/nested2\n(file2~1)/;/sub[1-3]/d\" < expect > expect2\n mv -f expect2 expect\n\n test_expect_success 'ensure \"status --cached --recursive\" preserves\nthe --cached flag' '\n        (\n                cd clone3 &&\n                (\n-                       cd nested1 &&\n+                       cd nested1/nested2 &&\n                        test_commit file2\n                ) &&\n                git submodule status --cached --recursive -- nested1 > ../actual\n@@ -245,6 +245,14 @@ test_expect_success 'ensure \"status --cached\n--recursive\" preserves the --cached\n        test_cmp expect actual\n '\n test_expect_success 'use \"git clone --recursive\" to checkout all submodules' '\n        git clone --recursive super clone4 &&\n        (\n\nPhil\n"},{"id":"201982","messageId":"1351278834-28867-1-git-send-email-hordp@cisco.com","threadId":"31946","inReplyTo":"CABURp0op2+QUvusUmAFUxT8s8c02bB9V3=ag9gTTSiiN4t96OA@mail.gmail.com","subject":"[PATCH] t7407: Fix recursive submodule test","fromName":"Phil Hord","fromEmail":"hordp@cisco.com","sentAt":"2012-10-26T19:13:54Z","receivedAt":"2012-10-26T19:13:54Z","isPatch":true,"sender":{"key":"phil.hord@gmail.com","avatar":"https://avatars.githubusercontent.com/u/123908?v=4"},"body":"A test in t7404-submodule-foreach purports to test that\nthe --cached flag is properly noticed by --recursive calls\nto the foreach command as it descends into nested\nsubmodules.  However, the test really does not perform this\ntest since the change it looks for is in a top-level\nsubmodule handled by the first invocation of the command.\nTo properly test for the flag being passed to recursive\ninvocations, the change must be buried deeper in the\nhierarchy.\n\nMove the change one level deeper so it properly verifies\nthe recursive machinery of the 'git submodule status'\ncommand.\n\nSigned-off-by: Phil Hord <hordp@cisco.com>\n---\n t/t7407-submodule-foreach.sh | 4 ++--\n 1 file changed, 2 insertions(+), 2 deletions(-)\n\ndiff --git a/t/t7407-submodule-foreach.sh b/t/t7407-submodule-foreach.sh\nindex 9b69fe2..107b4b7 100755\n--- a/t/t7407-submodule-foreach.sh\n+++ b/t/t7407-submodule-foreach.sh\n@@ -226,14 +226,14 @@ test_expect_success 'test \"status --recursive\"' '\n \ttest_cmp expect actual\n '\n \n-sed -e \"/nested1 /s/.*/+$nested1sha1 nested1 (file2~1)/;/sub[1-3]/d\" < expect > expect2\n+sed -e \"/nested2 /s/.*/+$nested2sha1 nested1\\/nested2 (file2~1)/;/sub[1-3]/d\" < expect > expect2\n mv -f expect2 expect\n \n test_expect_success 'ensure \"status --cached --recursive\" preserves the --cached flag' '\n \t(\n \t\tcd clone3 &&\n \t\t(\n-\t\t\tcd nested1 &&\n+\t\t\tcd nested1/nested2 &&\n \t\t\ttest_commit file2\n \t\t) &&\n \t\tgit submodule status --cached --recursive -- nested1 > ../actual\n-- \n1.8.0.2.gd008466.dirty\n"},{"id":"201983","messageId":"508AE3E9.6000304@web.de","threadId":"31946","inReplyTo":"CABURp0op2+QUvusUmAFUxT8s8c02bB9V3=ag9gTTSiiN4t96OA@mail.gmail.com","subject":"Re: [PATCH] submodule status: properly pass options with --recursive","fromName":"Jens Lehmann","fromEmail":"jens.lehmann@web.de","sentAt":"2012-10-26T19:26:33Z","receivedAt":"2012-10-26T19:26:33Z","isPatch":true,"sender":{"key":"jens.lehmann@web.de","avatar":"https://avatars.githubusercontent.com/u/135220?v=4"},"body":"Am 26.10.2012 21:07, schrieb Phil Hord:\n> On Fri, Oct 26, 2012 at 9:15 AM, Jeff King <peff@peff.net> wrote:\n>> On Fri, Oct 26, 2012 at 12:20:29AM +0200, Jens Lehmann wrote:\n>>\n>>> When renaming orig_args to orig_flags in 98dbe63d (submodule: only\n>>> preserve flags across recursive status/update invocations) the call site\n>>> of the recursive cmd_status was forgotten. At that place orig_args is\n>>> still passed into the recursion, which is always empty now. This clears\n>>> all options when recursing, as that variable is never set.\n>>>\n>>> Fix that by renaming orig_args to orig_flags there too and add a test to\n>>> catch that bug.\n>>\n>> Thanks. I back-ported your patch on top of 98dbe63d so it can go to\n>> 'maint'. I'm curious, though: why didn't the test right before (which\n>> checks recursion for --cached) catch this?\n> \n> I was wondering the same thing about why 'git submodule sync\n> --recursive --quiet' succeeded, so I checked on it.  The answer is\n> that \"--quiet\" sets GIT_QUIET=1, which is then inherited by the\n> recursive call. Indeed, Jens' new test passes even without the\n> accompanying fix.  :-\\\n\nDang, you're right! At least that explains why nobody noticed that\nso far ... (and that's what you get for skipping the \"does the test\nfail without your fix?\" part because the fix is so obvious :-( )\n\n> The 'status --recursive --cached' test passes for two reasons.  The\n> first is that the test is checking for a cached change in a level-1\n> submodule, but it is the level-2+ submodules which do not get the\n> flags passed down correctly in \"$@\".  The 2nd reason is that the\n> $cached variable is _also_ inherited by the recursive call to\n> cmd_status, specifically because it is a call to cmd_status() and not\n> to '$SHELL git submodule status', where the latter would have cleared\n> the flags at startup, but the former does not.\n> \n> I'm a bit wary about \"fixing\" the flags for the recursion machinery.\n> I'm starting to think $orig_flags is moot in almost all cases.  It\n> looks like clearing all the flags on each iteration would break 'git\n> submodule foreach --recursive --quiet' because it does not use\n> $orig_flags at all.  Who knows what other surprises lurk there.\n\nI agree, it looks like ripping out the orig_flags would be the way\nto go.\n"},{"id":"201984","messageId":"508AE4AB.4070209@web.de","threadId":"31946","inReplyTo":"1351278834-28867-1-git-send-email-hordp@cisco.com","subject":"Re: [PATCH] t7407: Fix recursive submodule test","fromName":"Jens Lehmann","fromEmail":"jens.lehmann@web.de","sentAt":"2012-10-26T19:29:47Z","receivedAt":"2012-10-26T19:29:47Z","isPatch":true,"sender":{"key":"jens.lehmann@web.de","avatar":"https://avatars.githubusercontent.com/u/135220?v=4"},"body":"Am 26.10.2012 21:13, schrieb Phil Hord:\n> A test in t7404-submodule-foreach purports to test that\n> the --cached flag is properly noticed by --recursive calls\n> to the foreach command as it descends into nested\n> submodules.  However, the test really does not perform this\n> test since the change it looks for is in a top-level\n> submodule handled by the first invocation of the command.\n> To properly test for the flag being passed to recursive\n> invocations, the change must be buried deeper in the\n> hierarchy.\n> \n> Move the change one level deeper so it properly verifies\n> the recursive machinery of the 'git submodule status'\n> command.\n\nMe thinks we should definitely do this.\n\n> Signed-off-by: Phil Hord <hordp@cisco.com>\n> ---\n>  t/t7407-submodule-foreach.sh | 4 ++--\n>  1 file changed, 2 insertions(+), 2 deletions(-)\n> \n> diff --git a/t/t7407-submodule-foreach.sh b/t/t7407-submodule-foreach.sh\n> index 9b69fe2..107b4b7 100755\n> --- a/t/t7407-submodule-foreach.sh\n> +++ b/t/t7407-submodule-foreach.sh\n> @@ -226,14 +226,14 @@ test_expect_success 'test \"status --recursive\"' '\n>  \ttest_cmp expect actual\n>  '\n>  \n> -sed -e \"/nested1 /s/.*/+$nested1sha1 nested1 (file2~1)/;/sub[1-3]/d\" < expect > expect2\n> +sed -e \"/nested2 /s/.*/+$nested2sha1 nested1\\/nested2 (file2~1)/;/sub[1-3]/d\" < expect > expect2\n>  mv -f expect2 expect\n>  \n>  test_expect_success 'ensure \"status --cached --recursive\" preserves the --cached flag' '\n>  \t(\n>  \t\tcd clone3 &&\n>  \t\t(\n> -\t\t\tcd nested1 &&\n> +\t\t\tcd nested1/nested2 &&\n>  \t\t\ttest_commit file2\n>  \t\t) &&\n>  \t\tgit submodule status --cached --recursive -- nested1 > ../actual\n> \n"},{"id":"201985","messageId":"1351280683-8402-1-git-send-email-hordp@cisco.com","threadId":"31946","inReplyTo":"508AE3E9.6000304@web.de","subject":"[PATCHv3 0/2] Teach --recursive to submodule sync","fromName":"Phil Hord","fromEmail":"hordp@cisco.com","sentAt":"2012-10-26T19:44:41Z","receivedAt":"2012-10-26T19:44:41Z","isPatch":false,"sender":{"key":"phil.hord@gmail.com","avatar":"https://avatars.githubusercontent.com/u/123908?v=4"},"body":"[PATCHv3 1/2] Teach --recursive to submodule sync\n\nNow with less noise and no redundant flags passed to the recursive call.\n\n\n[PATCHv3 2/2] Add tests for submodule sync --recursive\n\nThe test remains unchanged.\n"},{"id":"201986","messageId":"1351280683-8402-2-git-send-email-hordp@cisco.com","threadId":"31946","inReplyTo":"1351280683-8402-1-git-send-email-hordp@cisco.com","subject":"[PATCHv3 1/2] Teach --recursive to submodule sync","fromName":"Phil Hord","fromEmail":"hordp@cisco.com","sentAt":"2012-10-26T19:44:42Z","receivedAt":"2012-10-26T19:44:42Z","isPatch":false,"sender":{"key":"phil.hord@gmail.com","avatar":"https://avatars.githubusercontent.com/u/123908?v=4"},"body":"The submodule sync command was somehow left out when\n--recursive was added to the other submodule commands.\n\nTeach sync to handle the --recursive switch by recursing\nwhen we're in a submodule we are sync'ing.\n\nChange the report during sync to show submodule-path\ninstead of submodule-name to be consistent with the other\nsubmodule commands and to help recursed paths make sense.\n\nSigned-off-by: Phil Hord <hordp@cisco.com>\n---\n git-submodule.sh | 14 ++++++++++++--\n 1 file changed, 12 insertions(+), 2 deletions(-)\n\ndiff --git a/git-submodule.sh b/git-submodule.sh\nindex ab6b110..0ca3af2 100755\n--- a/git-submodule.sh\n+++ b/git-submodule.sh\n@@ -11,7 +11,7 @@ USAGE=\"[--quiet] add [-b branch] [-f|--force] [--reference <repository>] [--] <r\n    or: $dashless [--quiet] update [--init] [-N|--no-fetch] [-f|--force] [--rebase] [--reference <repository>] [--merge] [--recursive] [--] [<path>...]\n    or: $dashless [--quiet] summary [--cached|--files] [--summary-limit <n>] [commit] [--] [<path>...]\n    or: $dashless [--quiet] foreach [--recursive] <command>\n-   or: $dashless [--quiet] sync [--] [<path>...]\"\n+   or: $dashless [--quiet] sync [--recursive] [--] [<path>...]\"\n OPTIONS_SPEC=\n . git-sh-setup\n . git-sh-i18n\n@@ -1010,6 +1010,10 @@ cmd_sync()\n \t\t\tGIT_QUIET=1\n \t\t\tshift\n \t\t\t;;\n+\t\t--recursive)\n+\t\t\trecursive=1\n+\t\t\tshift\n+\t\t\t;;\n \t\t--)\n \t\t\tshift\n \t\t\tbreak\n@@ -1051,7 +1055,7 @@ cmd_sync()\n \n \t\tif git config \"submodule.$name.url\" >/dev/null 2>/dev/null\n \t\tthen\n-\t\t\tsay \"$(eval_gettext \"Synchronizing submodule url for '\\$name'\")\"\n+\t\t\tsay \"$(eval_gettext \"Synchronizing submodule url for '\\$prefix\\$sm_path'\")\"\n \t\t\tgit config submodule.\"$name\".url \"$super_config_url\"\n \n \t\t\tif test -e \"$sm_path\"/.git\n@@ -1061,6 +1065,12 @@ cmd_sync()\n \t\t\t\tcd \"$sm_path\"\n \t\t\t\tremote=$(get_default_remote)\n \t\t\t\tgit config remote.\"$remote\".url \"$sub_origin_url\"\n+\n+\t\t\t\tif test -n \"$recursive\"\n+\t\t\t\tthen\n+\t\t\t\t\tprefix=\"$prefix$sm_path/\"\n+\t\t\t\t\teval cmd_sync\n+\t\t\t\tfi\n \t\t\t)\n \t\t\tfi\n \t\tfi\n-- \n1.8.0.3.g9dae067\n"},{"id":"201987","messageId":"1351280683-8402-3-git-send-email-hordp@cisco.com","threadId":"31946","inReplyTo":"1351280683-8402-1-git-send-email-hordp@cisco.com","subject":"[PATCHv3 2/2] Add tests for submodule sync --recursive","fromName":"Phil Hord","fromEmail":"hordp@cisco.com","sentAt":"2012-10-26T19:44:43Z","receivedAt":"2012-10-26T19:44:43Z","isPatch":false,"sender":{"key":"phil.hord@gmail.com","avatar":"https://avatars.githubusercontent.com/u/123908?v=4"},"body":"Signed-off-by: Phil Hord <hordp@cisco.com>\n---\n t/t7403-submodule-sync.sh | 55 +++++++++++++++++++++++++++++++++++++++++++++--\n 1 file changed, 53 insertions(+), 2 deletions(-)\n\ndiff --git a/t/t7403-submodule-sync.sh b/t/t7403-submodule-sync.sh\nindex 524d5c1..94e26c4 100755\n--- a/t/t7403-submodule-sync.sh\n+++ b/t/t7403-submodule-sync.sh\n@@ -17,18 +17,25 @@ test_expect_success setup '\n \tgit commit -m upstream &&\n \tgit clone . super &&\n \tgit clone super submodule &&\n+\t(cd submodule &&\n+\t git submodule add ../submodule sub-submodule &&\n+\t test_tick &&\n+\t git commit -m \"sub-submodule\"\n+\t) &&\n \t(cd super &&\n \t git submodule add ../submodule submodule &&\n \t test_tick &&\n \t git commit -m \"submodule\"\n \t) &&\n \tgit clone super super-clone &&\n-\t(cd super-clone && git submodule update --init) &&\n+\t(cd super-clone && git submodule update --init --recursive) &&\n \tgit clone super empty-clone &&\n \t(cd empty-clone && git submodule init) &&\n \tgit clone super top-only-clone &&\n \tgit clone super relative-clone &&\n-\t(cd relative-clone && git submodule update --init)\n+\t(cd relative-clone && git submodule update --init --recursive) &&\n+\tgit clone super recursive-clone &&\n+\t(cd recursive-clone && git submodule update --init --recursive)\n '\n \n test_expect_success 'change submodule' '\n@@ -46,6 +53,11 @@ test_expect_success 'change submodule url' '\n \t git pull\n \t) &&\n \tmv submodule moved-submodule &&\n+\t(cd moved-submodule &&\n+\t git config -f .gitmodules submodule.sub-submodule.url ../moved-submodule &&\n+\t test_tick &&\n+\t git commit -a -m moved-sub-submodule\n+\t) &&\n \t(cd super &&\n \t git config -f .gitmodules submodule.submodule.url ../moved-submodule &&\n \t test_tick &&\n@@ -61,6 +73,9 @@ test_expect_success '\"git submodule sync\" should update submodule URLs' '\n \ttest -d \"$(cd super-clone/submodule &&\n \t git config remote.origin.url\n \t)\" &&\n+\ttest ! -d \"$(cd super-clone/submodule/sub-submodule &&\n+\t git config remote.origin.url\n+\t)\" &&\n \t(cd super-clone/submodule &&\n \t git checkout master &&\n \t git pull\n@@ -70,6 +85,25 @@ test_expect_success '\"git submodule sync\" should update submodule URLs' '\n \t)\n '\n \n+test_expect_success '\"git submodule sync --recursive\" should update all submodule URLs' '\n+\t(cd super-clone &&\n+\t (cd submodule &&\n+\t  git pull --no-recurse-submodules\n+\t ) &&\n+\t git submodule sync --recursive\n+\t) &&\n+\ttest -d \"$(cd super-clone/submodule &&\n+\t git config remote.origin.url\n+\t)\" &&\n+\ttest -d \"$(cd super-clone/submodule/sub-submodule &&\n+\t git config remote.origin.url\n+\t)\" &&\n+\t(cd super-clone/submodule/sub-submodule &&\n+\t git checkout master &&\n+\t git pull\n+\t)\n+'\n+\n test_expect_success '\"git submodule sync\" should update known submodule URLs' '\n \t(cd empty-clone &&\n \t git pull &&\n@@ -107,6 +141,23 @@ test_expect_success '\"git submodule sync\" handles origin URL of the form foo/bar\n \t #actual foo/submodule\n \t test \"$(git config remote.origin.url)\" = \"../foo/submodule\"\n \t)\n+\t(cd submodule/sub-submodule &&\n+\t test \"$(git config remote.origin.url)\" != \"../../foo/submodule\"\n+\t)\n+\t)\n+'\n+\n+test_expect_success '\"git submodule sync --recursive\" propagates changes in origin' '\n+\t(cd recursive-clone &&\n+\t git remote set-url origin foo/bar &&\n+\t git submodule sync --recursive &&\n+\t(cd submodule &&\n+\t #actual foo/submodule\n+\t test \"$(git config remote.origin.url)\" = \"../foo/submodule\"\n+\t)\n+\t(cd submodule/sub-submodule &&\n+\t test \"$(git config remote.origin.url)\" = \"../../foo/submodule\"\n+\t)\n \t)\n '\n \n-- \n1.8.0.3.g9dae067\n"},{"id":"202064","messageId":"508D9D76.5090501@web.de","threadId":"31946","inReplyTo":"1351280683-8402-1-git-send-email-hordp@cisco.com","subject":"Re: [PATCHv3 0/2] Teach --recursive to submodule sync","fromName":"Jens Lehmann","fromEmail":"jens.lehmann@web.de","sentAt":"2012-10-28T21:02:46Z","receivedAt":"2012-10-28T21:02:46Z","isPatch":false,"sender":{"key":"jens.lehmann@web.de","avatar":"https://avatars.githubusercontent.com/u/135220?v=4"},"body":"Am 26.10.2012 21:44, schrieb Phil Hord:\n> [PATCHv3 1/2] Teach --recursive to submodule sync\n> \n> Now with less noise and no redundant flags passed to the recursive call.\n> \n> \n> [PATCHv3 2/2] Add tests for submodule sync --recursive\n> \n> The test remains unchanged.\n\nBoth are looking good.\n\nAcked-By: Jens Lehmann <Jens.Lehmann@web.de>\n"},{"id":"202070","messageId":"508DA58C.2040504@web.de","threadId":"31946","inReplyTo":"508AE4AB.4070209@web.de","subject":"[PATCH] submodule status: remove unused orig_* variables","fromName":"Jens Lehmann","fromEmail":"jens.lehmann@web.de","sentAt":"2012-10-28T21:37:16Z","receivedAt":"2012-10-28T21:37:16Z","isPatch":true,"sender":{"key":"jens.lehmann@web.de","avatar":"https://avatars.githubusercontent.com/u/135220?v=4"},"body":"When renaming orig_args to orig_flags in 98dbe63d (submodule: only\npreserve flags across recursive status/update invocations) the call site\nof the recursive cmd_status was forgotten. At that place orig_args is\nstill passed into the recursion, which is always empty since then. This\ndid not break anything because the orig_flags logic is not needed at all\nwhen a function from the submodule script is called with eval, as that\ninherits all the variables set by the option parsing done in the first\nlevel of the recursion.\n\nNow that we know that orig_flags and orig_args aren't needed at all,\nlet's just remove them from cmd_status().\n\nThanks-to: Phil Hord <hordp@cisco.com>\nSigned-off-by: Jens Lehmann <Jens.Lehmann@web.de>\n---\n\nAm 26.10.2012 21:29, schrieb Jens Lehmann:\n> Am 26.10.2012 21:13, schrieb Phil Hord:\n>> A test in t7404-submodule-foreach purports to test that\n>> the --cached flag is properly noticed by --recursive calls\n>> to the foreach command as it descends into nested\n>> submodules.  However, the test really does not perform this\n>> test since the change it looks for is in a top-level\n>> submodule handled by the first invocation of the command.\n>> To properly test for the flag being passed to recursive\n>> invocations, the change must be buried deeper in the\n>> hierarchy.\n>>\n>> Move the change one level deeper so it properly verifies\n>> the recursive machinery of the 'git submodule status'\n>> command.\n> \n> Me thinks we should definitely do this.\n\nAnd I also think this patch should go on top of Phil's patch after what\nwe learned so far.\n\nI'm currently checking if we can also safely remove orig_flags from\ncmd_update(). At least the test suite runs fine without it ...\n\n\n git-submodule.sh | 4 +---\n 1 file changed, 1 insertion(+), 3 deletions(-)\n\ndiff --git a/git-submodule.sh b/git-submodule.sh\nindex ab6b110..c287464 100755\n--- a/git-submodule.sh\n+++ b/git-submodule.sh\n@@ -926,7 +926,6 @@ cmd_summary() {\n cmd_status()\n {\n \t# parse $args after \"submodule ... status\".\n-\torig_flags=\n \twhile test $# -ne 0\n \tdo\n \t\tcase \"$1\" in\n@@ -950,7 +949,6 @@ cmd_status()\n \t\t\tbreak\n \t\t\t;;\n \t\tesac\n-\t\torig_flags=\"$orig_flags $(git rev-parse --sq-quote \"$1\")\"\n \t\tshift\n \tdone\n\n@@ -990,7 +988,7 @@ cmd_status()\n \t\t\t\tprefix=\"$displaypath/\"\n \t\t\t\tclear_local_git_env\n \t\t\t\tcd \"$sm_path\" &&\n-\t\t\t\teval cmd_status \"$orig_args\"\n+\t\t\t\teval cmd_status\n \t\t\t) ||\n \t\t\tdie \"$(eval_gettext \"Failed to recurse into submodule path '\\$sm_path'\")\"\n \t\tfi\n-- \n1.8.0.42.g2ea983b\n"},{"id":"202099","messageId":"20121029072719.GE5102@sigill.intra.peff.net","threadId":"31946","inReplyTo":"508DA58C.2040504@web.de","subject":"Re: [PATCH] submodule status: remove unused orig_* variables","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2012-10-29T07:27:19Z","receivedAt":"2012-10-29T07:27:19Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Sun, Oct 28, 2012 at 10:37:16PM +0100, Jens Lehmann wrote:\n\n> When renaming orig_args to orig_flags in 98dbe63d (submodule: only\n> preserve flags across recursive status/update invocations) the call site\n> of the recursive cmd_status was forgotten. At that place orig_args is\n> still passed into the recursion, which is always empty since then. This\n> did not break anything because the orig_flags logic is not needed at all\n> when a function from the submodule script is called with eval, as that\n> inherits all the variables set by the option parsing done in the first\n> level of the recursion.\n> \n> Now that we know that orig_flags and orig_args aren't needed at all,\n> let's just remove them from cmd_status().\n> \n> Thanks-to: Phil Hord <hordp@cisco.com>\n> Signed-off-by: Jens Lehmann <Jens.Lehmann@web.de>\n\nI'll replace your earlier fix (which just fixes the s/args/flags/ and\nadds a test) with Phil's patch, and this on top.\n\nThanks.\n\n-Peff\n"}]}