Re: [PATCH 1/2] Teach --recursive to submodule sync
Am 26.10.2012 19:55, schrieb Phil Hord: > On Fri, Oct 26, 2012 at 1:19 PM, Phil Hord 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 ;-) -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 1/2] Teach --recursive to submodule sync
On Fri, Oct 26, 2012 at 1:19 PM, Phil Hord 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 -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 1/2] Teach --recursive to submodule sync
Jens Lehmann wrote:
> 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
> 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
>> ] [--] > or: $dashless [--quiet] update [--init] [-N|--no-fetch] [-f|--force]
>> [--rebase] [--reference ] [--merge] [--recursive] [--]
>> [...]
>> or: $dashless [--quiet] summary [--cached|--files] [--summary-limit ]
>> [commit] [--] [...]
>> or: $dashless [--quiet] foreach [--recursive]
>> - or: $dashless [--quiet] sync [--] [...]"
>> + or: $dashless [--quiet] sync [--recursive] [--] [...]"
>> 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.
>> +)
>> +fi
>> )
>> fi
>> fi
>>
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 1/2] Teach --recursive to submodule sync
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
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
> ] [--] or: $dashless [--quiet] update [--init] [-N|--no-fetch] [-f|--force]
> [--rebase] [--reference ] [--merge] [--recursive] [--] [...]
> or: $dashless [--quiet] summary [--cached|--files] [--summary-limit ]
> [commit] [--] [...]
> or: $dashless [--quiet] foreach [--recursive]
> - or: $dashless [--quiet] sync [--] [...]"
> + or: $dashless [--quiet] sync [--recursive] [--] [...]"
> 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
>
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[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 --- 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 ] [--] ] [--merge] [--recursive] [--] [...] or: $dashless [--quiet] summary [--cached|--files] [--summary-limit ] [commit] [--] [...] or: $dashless [--quiet] foreach [--recursive] - or: $dashless [--quiet] sync [--] [...]" + or: $dashless [--quiet] sync [--recursive] [--] [...]" 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 -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
