12 messages, 3 authors, 2016-06-15 · open the first message on its own page
From: Phil Hord <hidden>
Date: 2016-06-15 22:55:03
I noticed that this is not supported:
git submodule sync --recursive
I do not see any discussion in the relevant commits about why it
cannot or should not be supported. Is it just an itch no one has
scratched?
Phil
From: Jens Lehmann <hidden>
Date: 2016-06-15 22:55:03
Am 17.10.2012 01:20, schrieb Phil Hord:
I noticed that this is not supported:
git submodule sync --recursive
I do not see any discussion in the relevant commits about why it
cannot or should not be supported. Is it just an itch no one has
scratched?I can't remember any discussions about that either, but can't think of a reason why we shouldn't support that.
From: Phil Hord <hidden>
Date: 2016-06-15 22:55:06
The submodule sync command was somehow left out when --recursive was added to the other submodule commands. Teach sync to handle the --recursive switch by recursing when we're in a submodule we are sync'ing. Change the report during sync to show submodule-path instead of submodule-name to be consistent with the other submodule commands and to help recursed paths make sense. Signed-off-by: Phil Hord <redacted> --- git-submodule.sh | 18 +++++++++++++++--- 1 file changed, 15 insertions(+), 3 deletions(-)
diff --git a/git-submodule.sh b/git-submodule.sh
index ab6b110..6dd2338 100755
--- a/git-submodule.sh
+++ b/git-submodule.sh@@ -11,7 +11,7 @@ USAGE="[--quiet] add [-b branch] [-f|--force] [--reference <repository>] [--] <r or: $dashless [--quiet] update [--init] [-N|--no-fetch] [-f|--force] [--rebase] [--reference <repository>] [--merge] [--recursive] [--] [<path>...] or: $dashless [--quiet] summary [--cached|--files] [--summary-limit <n>] [commit] [--] [<path>...] or: $dashless [--quiet] foreach [--recursive] <command> - or: $dashless [--quiet] sync [--] [<path>...]" + or: $dashless [--quiet] sync [--recursive] [--] [<path>...]" OPTIONS_SPEC= . git-sh-setup . git-sh-i18n
@@ -1008,7 +1008,9 @@ cmd_sync() case "$1" in -q|--quiet) GIT_QUIET=1 - shift + ;; + --recursive) + recursive=1 ;; --) shift
@@ -1021,6 +1023,8 @@ cmd_sync() break ;; esac + orig_flags="$orig_flags $(git rev-parse --sq-quote "$1")" + shift done cd_to_toplevel module_list "$@" |
@@ -1051,7 +1055,7 @@ cmd_sync() if git config "submodule.$name.url" >/dev/null 2>/dev/null then - say "$(eval_gettext "Synchronizing submodule url for '\$name'")" + say "$(eval_gettext "Synchronizing submodule url for '\$prefix\$sm_path'")" git config submodule."$name".url "$super_config_url" if test -e "$sm_path"/.git
@@ -1061,6 +1065,14 @@ cmd_sync() cd "$sm_path" remote=$(get_default_remote) git config remote."$remote".url "$sub_origin_url" + + if test -n "$recursive" + then + ( + prefix="$prefix$sm_path/" + eval cmd_sync "$orig_args" + ) + fi ) fi fi
--
1.8.0.2.gcde19fc.dirty
From: Phil Hord <hidden>
Date: 2016-06-15 22:55:06
Signed-off-by: Phil Hord <redacted> --- t/t7403-submodule-sync.sh | 55 +++++++++++++++++++++++++++++++++++++++++++++-- 1 file changed, 53 insertions(+), 2 deletions(-)
diff --git a/t/t7403-submodule-sync.sh b/t/t7403-submodule-sync.sh
index 524d5c1..94e26c4 100755
--- a/t/t7403-submodule-sync.sh
+++ b/t/t7403-submodule-sync.sh@@ -17,18 +17,25 @@ test_expect_success setup ' git commit -m upstream && git clone . super && git clone super submodule && + (cd submodule && + git submodule add ../submodule sub-submodule && + test_tick && + git commit -m "sub-submodule" + ) && (cd super && git submodule add ../submodule submodule && test_tick && git commit -m "submodule" ) && git clone super super-clone && - (cd super-clone && git submodule update --init) && + (cd super-clone && git submodule update --init --recursive) && git clone super empty-clone && (cd empty-clone && git submodule init) && git clone super top-only-clone && git clone super relative-clone && - (cd relative-clone && git submodule update --init) + (cd relative-clone && git submodule update --init --recursive) && + git clone super recursive-clone && + (cd recursive-clone && git submodule update --init --recursive) ' test_expect_success 'change submodule' '
@@ -46,6 +53,11 @@ test_expect_success 'change submodule url' ' git pull ) && mv submodule moved-submodule && + (cd moved-submodule && + git config -f .gitmodules submodule.sub-submodule.url ../moved-submodule && + test_tick && + git commit -a -m moved-sub-submodule + ) && (cd super && git config -f .gitmodules submodule.submodule.url ../moved-submodule && test_tick &&
@@ -61,6 +73,9 @@ test_expect_success '"git submodule sync" should update submodule URLs' ' test -d "$(cd super-clone/submodule && git config remote.origin.url )" && + test ! -d "$(cd super-clone/submodule/sub-submodule && + git config remote.origin.url + )" && (cd super-clone/submodule && git checkout master && git pull
@@ -70,6 +85,25 @@ test_expect_success '"git submodule sync" should update submodule URLs' ' ) ' +test_expect_success '"git submodule sync --recursive" should update all submodule URLs' ' + (cd super-clone && + (cd submodule && + git pull --no-recurse-submodules + ) && + git submodule sync --recursive + ) && + test -d "$(cd super-clone/submodule && + git config remote.origin.url + )" && + test -d "$(cd super-clone/submodule/sub-submodule && + git config remote.origin.url + )" && + (cd super-clone/submodule/sub-submodule && + git checkout master && + git pull + ) +' + test_expect_success '"git submodule sync" should update known submodule URLs' ' (cd empty-clone && git pull &&
@@ -107,6 +141,23 @@ test_expect_success '"git submodule sync" handles origin URL of the form foo/bar #actual foo/submodule test "$(git config remote.origin.url)" = "../foo/submodule" ) + (cd submodule/sub-submodule && + test "$(git config remote.origin.url)" != "../../foo/submodule" + ) + ) +' + +test_expect_success '"git submodule sync --recursive" propagates changes in origin' ' + (cd recursive-clone && + git remote set-url origin foo/bar && + git submodule sync --recursive && + (cd submodule && + #actual foo/submodule + test "$(git config remote.origin.url)" = "../foo/submodule" + ) + (cd submodule/sub-submodule && + test "$(git config remote.origin.url)" = "../../foo/submodule" + ) ) '
--
1.8.0.2.gcde19fc.dirty
From: Phil Hord <hidden>
Date: 2016-06-15 22:55:06
[PATCH 1/2] Teach --recursive to submodule sync [PATCH 2/2] Add tests for submodule sync --recursive This series implements and tests git submodule sync --recursive
From: Jens Lehmann <hidden>
Date: 2016-06-15 22:55:07
Am 24.10.2012 01:15, schrieb Phil Hord:
The submodule sync command was somehow left out when --recursive was added to the other submodule commands. Teach sync to handle the --recursive switch by recursing when we're in a submodule we are sync'ing. Change the report during sync to show submodule-path instead of submodule-name to be consistent with the other submodule commands and to help recursed paths make sense. Signed-off-by: Phil Hord <redacted>
This makes perfect sense to me. Two things though: First it would be nice to initialize orig_flags like all the other call sites do:
@@ -1003,6 +1003,7 @@ cmd_status() # cmd_sync() { + orig_flags= while test $# -ne 0 do case "$1" in
--- git-submodule.sh | 18 +++++++++++++++--- 1 file changed, 15 insertions(+), 3 deletions(-)diff --git a/git-submodule.sh b/git-submodule.sh index ab6b110..6dd2338 100755 --- a/git-submodule.sh +++ b/git-submodule.sh@@ -11,7 +11,7 @@ USAGE="[--quiet] add [-b branch] [-f|--force] [--reference <repository>] [--] <r or: $dashless [--quiet] update [--init] [-N|--no-fetch] [-f|--force] [--rebase] [--reference <repository>] [--merge] [--recursive] [--] [<path>...] or: $dashless [--quiet] summary [--cached|--files] [--summary-limit <n>] [commit] [--] [<path>...] or: $dashless [--quiet] foreach [--recursive] <command> - or: $dashless [--quiet] sync [--] [<path>...]" + or: $dashless [--quiet] sync [--recursive] [--] [<path>...]" OPTIONS_SPEC= . git-sh-setup . git-sh-i18n@@ -1008,7 +1008,9 @@ cmd_sync() case "$1" in -q|--quiet) GIT_QUIET=1 - shift + ;; + --recursive) + recursive=1 ;; --) shift@@ -1021,6 +1023,8 @@ cmd_sync() break ;; esac + orig_flags="$orig_flags $(git rev-parse --sq-quote "$1")" + shift done cd_to_toplevel module_list "$@" |@@ -1051,7 +1055,7 @@ cmd_sync() if git config "submodule.$name.url" >/dev/null 2>/dev/null then - say "$(eval_gettext "Synchronizing submodule url for '\$name'")" + say "$(eval_gettext "Synchronizing submodule url for '\$prefix\$sm_path'")" git config submodule."$name".url "$super_config_url" if test -e "$sm_path"/.git@@ -1061,6 +1065,14 @@ cmd_sync() cd "$sm_path" remote=$(get_default_remote) git config remote."$remote".url "$sub_origin_url" + + if test -n "$recursive" + then + ( + prefix="$prefix$sm_path/" + eval cmd_sync "$orig_args"
This should read 'eval cmd_sync "$orig_flags"'. I think you copied that from cmd_status(), where this is also incorrect, I just sent a patch to correct that one.
+ ) + fi ) fi fi
From: Phil Hord <hidden>
Date: 2016-06-15 22:55:07
Jens Lehmann wrote:
Am 24.10.2012 01:15, schrieb Phil Hord:quoted
The submodule sync command was somehow left out when --recursive was added to the other submodule commands. Teach sync to handle the --recursive switch by recursing when we're in a submodule we are sync'ing. Change the report during sync to show submodule-path instead of submodule-name to be consistent with the other submodule commands and to help recursed paths make sense. Signed-off-by: Phil Hord <redacted>This makes perfect sense to me. Two things though: First it would be nice to initialize orig_flags like all the other call sites do:@@ -1003,6 +1003,7 @@ cmd_status() # cmd_sync() { + orig_flags= while test $# -ne 0 do case "$1" inquoted
--- git-submodule.sh | 18 +++++++++++++++--- 1 file changed, 15 insertions(+), 3 deletions(-)diff --git a/git-submodule.sh b/git-submodule.sh index ab6b110..6dd2338 100755 --- a/git-submodule.sh +++ b/git-submodule.sh@@ -11,7 +11,7 @@ USAGE="[--quiet] add [-b branch] [-f|--force] [--reference <repository>] [--] <r or: $dashless [--quiet] update [--init] [-N|--no-fetch] [-f|--force] [--rebase] [--reference <repository>] [--merge] [--recursive] [--] [<path>...] or: $dashless [--quiet] summary [--cached|--files] [--summary-limit <n>] [commit] [--] [<path>...] or: $dashless [--quiet] foreach [--recursive] <command> - or: $dashless [--quiet] sync [--] [<path>...]" + or: $dashless [--quiet] sync [--recursive] [--] [<path>...]" OPTIONS_SPEC= . git-sh-setup . git-sh-i18n@@ -1008,7 +1008,9 @@ cmd_sync() case "$1" in -q|--quiet) GIT_QUIET=1 - shift + ;; + --recursive) + recursive=1 ;; --) shift@@ -1021,6 +1023,8 @@ cmd_sync() break ;; esac + orig_flags="$orig_flags $(git rev-parse --sq-quote "$1")" + shift done cd_to_toplevel module_list "$@" |@@ -1051,7 +1055,7 @@ cmd_sync() if git config "submodule.$name.url" >/dev/null 2>/dev/null then - say "$(eval_gettext "Synchronizing submodule url for '\$name'")" + say "$(eval_gettext "Synchronizing submodule url for '\$prefix\$sm_path'")" git config submodule."$name".url "$super_config_url" if test -e "$sm_path"/.git@@ -1061,6 +1065,14 @@ cmd_sync() cd "$sm_path" remote=$(get_default_remote) git config remote."$remote".url "$sub_origin_url" + + if test -n "$recursive" + then + ( + prefix="$prefix$sm_path/" + eval cmd_sync "$orig_args"This should read 'eval cmd_sync "$orig_flags"'. I think you copied that from cmd_status(), where this is also incorrect, I just sent a patch to correct that one.
Yes, thanks for catching that. I think I should add a test for that except I notice that sync doesn't take any other flags useful for passing. v2 is on the way.
quoted
+ ) + fi ) fi fi
From: Phil Hord <hidden>
Date: 2016-06-15 22:55:07
This one fixes "$orig_flags" problems noticed by Jens. Copy-and-paste is gift from Satan. Phil
From: Phil Hord <hidden>
Date: 2016-06-15 22:55:07
The submodule sync command was somehow left out when --recursive was added to the other submodule commands. Teach sync to handle the --recursive switch by recursing when we're in a submodule we are sync'ing. Change the report during sync to show submodule-path instead of submodule-name to be consistent with the other submodule commands and to help recursed paths make sense. Signed-off-by: Phil Hord <redacted> --- git-submodule.sh | 19 ++++++++++++++++--- 1 file changed, 16 insertions(+), 3 deletions(-)
diff --git a/git-submodule.sh b/git-submodule.sh
index ab6b110..f40da72 100755
--- a/git-submodule.sh
+++ b/git-submodule.sh@@ -11,7 +11,7 @@ USAGE="[--quiet] add [-b branch] [-f|--force] [--reference <repository>] [--] <r or: $dashless [--quiet] update [--init] [-N|--no-fetch] [-f|--force] [--rebase] [--reference <repository>] [--merge] [--recursive] [--] [<path>...] or: $dashless [--quiet] summary [--cached|--files] [--summary-limit <n>] [commit] [--] [<path>...] or: $dashless [--quiet] foreach [--recursive] <command> - or: $dashless [--quiet] sync [--] [<path>...]" + or: $dashless [--quiet] sync [--recursive] [--] [<path>...]" OPTIONS_SPEC= . git-sh-setup . git-sh-i18n
@@ -1003,12 +1003,15 @@ cmd_status() # cmd_sync() { + orig_flags= while test $# -ne 0 do case "$1" in -q|--quiet) GIT_QUIET=1 - shift + ;; + --recursive) + recursive=1 ;; --) shift
@@ -1021,6 +1024,8 @@ cmd_sync() break ;; esac + orig_flags="$orig_flags $(git rev-parse --sq-quote "$1")" + shift done cd_to_toplevel module_list "$@" |
@@ -1051,7 +1056,7 @@ cmd_sync() if git config "submodule.$name.url" >/dev/null 2>/dev/null then - say "$(eval_gettext "Synchronizing submodule url for '\$name'")" + say "$(eval_gettext "Synchronizing submodule url for '\$prefix\$sm_path'")" git config submodule."$name".url "$super_config_url" if test -e "$sm_path"/.git
@@ -1061,6 +1066,14 @@ cmd_sync() cd "$sm_path" remote=$(get_default_remote) git config remote."$remote".url "$sub_origin_url" + + if test -n "$recursive" + then + ( + prefix="$prefix$sm_path/" + eval cmd_sync "$orig_flags" + ) + fi ) fi fi
--
1.8.0.2.g35128e8
From: Phil Hord <hidden>
Date: 2016-06-15 22:55:07
Signed-off-by: Phil Hord <redacted> --- t/t7403-submodule-sync.sh | 55 +++++++++++++++++++++++++++++++++++++++++++++-- 1 file changed, 53 insertions(+), 2 deletions(-)
diff --git a/t/t7403-submodule-sync.sh b/t/t7403-submodule-sync.sh
index 524d5c1..94e26c4 100755
--- a/t/t7403-submodule-sync.sh
+++ b/t/t7403-submodule-sync.sh@@ -17,18 +17,25 @@ test_expect_success setup ' git commit -m upstream && git clone . super && git clone super submodule && + (cd submodule && + git submodule add ../submodule sub-submodule && + test_tick && + git commit -m "sub-submodule" + ) && (cd super && git submodule add ../submodule submodule && test_tick && git commit -m "submodule" ) && git clone super super-clone && - (cd super-clone && git submodule update --init) && + (cd super-clone && git submodule update --init --recursive) && git clone super empty-clone && (cd empty-clone && git submodule init) && git clone super top-only-clone && git clone super relative-clone && - (cd relative-clone && git submodule update --init) + (cd relative-clone && git submodule update --init --recursive) && + git clone super recursive-clone && + (cd recursive-clone && git submodule update --init --recursive) ' test_expect_success 'change submodule' '
@@ -46,6 +53,11 @@ test_expect_success 'change submodule url' ' git pull ) && mv submodule moved-submodule && + (cd moved-submodule && + git config -f .gitmodules submodule.sub-submodule.url ../moved-submodule && + test_tick && + git commit -a -m moved-sub-submodule + ) && (cd super && git config -f .gitmodules submodule.submodule.url ../moved-submodule && test_tick &&
@@ -61,6 +73,9 @@ test_expect_success '"git submodule sync" should update submodule URLs' ' test -d "$(cd super-clone/submodule && git config remote.origin.url )" && + test ! -d "$(cd super-clone/submodule/sub-submodule && + git config remote.origin.url + )" && (cd super-clone/submodule && git checkout master && git pull
@@ -70,6 +85,25 @@ test_expect_success '"git submodule sync" should update submodule URLs' ' ) ' +test_expect_success '"git submodule sync --recursive" should update all submodule URLs' ' + (cd super-clone && + (cd submodule && + git pull --no-recurse-submodules + ) && + git submodule sync --recursive + ) && + test -d "$(cd super-clone/submodule && + git config remote.origin.url + )" && + test -d "$(cd super-clone/submodule/sub-submodule && + git config remote.origin.url + )" && + (cd super-clone/submodule/sub-submodule && + git checkout master && + git pull + ) +' + test_expect_success '"git submodule sync" should update known submodule URLs' ' (cd empty-clone && git pull &&
@@ -107,6 +141,23 @@ test_expect_success '"git submodule sync" handles origin URL of the form foo/bar #actual foo/submodule test "$(git config remote.origin.url)" = "../foo/submodule" ) + (cd submodule/sub-submodule && + test "$(git config remote.origin.url)" != "../../foo/submodule" + ) + ) +' + +test_expect_success '"git submodule sync --recursive" propagates changes in origin' ' + (cd recursive-clone && + git remote set-url origin foo/bar && + git submodule sync --recursive && + (cd submodule && + #actual foo/submodule + test "$(git config remote.origin.url)" = "../foo/submodule" + ) + (cd submodule/sub-submodule && + test "$(git config remote.origin.url)" = "../../foo/submodule" + ) ) '
--
1.8.0.2.g35128e8
From: Phil Hord <hidden>
Date: 2016-06-15 22:55:07
On Fri, Oct 26, 2012 at 1:19 PM, Phil Hord [off-list ref] wrote:
Yes, thanks for catching that. I think I should add a test for that except I notice that sync doesn't take any other flags useful for passing.
Which, of course, suggests that I should not add this flag-propagating-machinery to submodule-sync at all. yes? Phil
From: Jens Lehmann <hidden>
Date: 2016-06-15 22:55:07
Am 26.10.2012 19:55, schrieb Phil Hord:
On Fri, Oct 26, 2012 at 1:19 PM, Phil Hord [off-list ref] wrote:quoted
Yes, thanks for catching that. I think I should add a test for that except I notice that sync doesn't take any other flags useful for passing.Which, of course, suggests that I should not add this flag-propagating-machinery to submodule-sync at all. yes?
Nope, the new --recursive option has to be passed on! To catch that bug in your test you'd need another submodule inside your sub-submodule. The first level submodule is initialized by sync anyways, the sub-submodule is initialized by the --recursive logic you added but the sub-sub-submodule would not have been synced because the option was dropped. I really can't blame you for not adding that third level of submodules ;-)