threads / discuss / 31841

git submodule sync --recursive

Subject: git submodule sync --recursive

## tl;dr

12 messages between Oct 16, 2012 and Oct 26, 2012.

replies: 11people: 2as markdown or json

Phil Hord· Oct 16, 2012, 23:20 UTC · lore

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

Jens Lehmann· Oct 17, 2012, 18:26 UTC · re: Phil Hord · lore

Re: git submodule sync --recursive

Am 17.10.2012 01:20, schrieb Phil Hord:

Show 8 quoted lines

> 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.

Phil Hord· Oct 23, 2012, 23:15 UTC · re: Jens Lehmann · lore

[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

Phil Hord· Oct 23, 2012, 23:15 UTC · re: Phil Hord · lore

[PATCH 1/2] Teach --recursive to submodule sync

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 <hordp@cisco.com>
---
 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

Jens Lehmann· Oct 25, 2012, 22:23 UTC · re: Phil Hord · lore

Re: [PATCH 1/2] Teach --recursive to submodule sync

Am 24.10.2012 01:15, schrieb Phil Hord:

Show 11 quoted lines

> 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 <hordp@cisco.com>

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
>

Phil Hord· Oct 26, 2012, 17:19 UTC · re: Jens Lehmann · lore

Re: [PATCH 1/2] Teach --recursive to submodule sync

Jens Lehmann wrote:

Show 85 quoted lines

> 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 <hordp@cisco.com>
> 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.

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.

Show 6 quoted lines

>> +				)
>> +			fi
>>  			)
>>  			fi
>>  		fi
>>

Phil Hord· Oct 26, 2012, 17:55 UTC · re: Phil Hord · lore

Re: [PATCH 1/2] Teach --recursive to submodule sync

On Fri, Oct 26, 2012 at 1:19 PM, Phil Hord <hordp@cisco.com> 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

Jens Lehmann· Oct 26, 2012, 18:50 UTC · re: Phil Hord · lore

Re: [PATCH 1/2] Teach --recursive to submodule sync

Am 26.10.2012 19:55, schrieb Phil Hord:

Show 7 quoted lines

> On Fri, Oct 26, 2012 at 1:19 PM, Phil Hord <hordp@cisco.com> 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?

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 ;-)

Phil Hord· Oct 26, 2012, 17:31 UTC · re: Jens Lehmann · lore

[PATCHv2 0/2] Reroll submodule sync --recursive

This one fixes "$orig_flags" problems noticed by Jens. Copy-and-paste is gift from Satan.

Phil

Phil Hord· Oct 26, 2012, 17:31 UTC · re: Phil Hord · lore

[PATCHv2 1/2] Teach --recursive to submodule sync

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 <hordp@cisco.com>
---
 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

Phil Hord· Oct 26, 2012, 17:31 UTC · re: Phil Hord · lore

[PATCHv2 2/2] Add tests for submodule sync --recursive

Signed-off-by: Phil Hord <hordp@cisco.com>
---
 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

Phil Hord· Oct 23, 2012, 23:15 UTC · re: Phil Hord · lore

[PATCH 2/2] Add tests for submodule sync --recursive

Signed-off-by: Phil Hord <hordp@cisco.com>
---
 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