Re: [PATCH v3 03/10] submodule sync: use submodule--helper is-active

[Junio C Hamano]

Brandon Williams <bmw...@google.com> writes:

> Signed-off-by: Brandon Williams <bmw...@google.com>
> ---
>  git-submodule.sh | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/git-submodule.sh b/git-submodule.sh
> index ab233712d..e2d08595f 100755
> --- a/git-submodule.sh
> +++ b/git-submodule.sh
> @@ -1111,7 +1111,7 @@ cmd_sync()
>                       ;;
>               esac
>  
> -             if git config "submodule.$name.url" >/dev/null 2>/dev/null
> +             if git submodule--helper is-active "$sm_path"
>               then
>                       displaypath=$(git submodule--helper relative-path 
> "$prefix$sm_path" "$wt_prefix")
>                       say "$(eval_gettext "Synchronizing submodule url for 
> '\$displaypath'")"

This is not a problem this patch introduces, but the loop this hunk
is in seems a bit inefficient.  It maps the sm_path to its name and
then asks .gitmodules the URL the upstream suggests to clone it from,
munges it with a large case statement and discards all of that if
the module is not active.

Adding this patch on top would be a way to remove the inefficiency
and one level of nesting while at it, I think, but I may have missed
something, so please double check, and if you agree that this is a
good way to go, please do so as a preparatory clean-up.

Thanks.

 git-submodule.sh | 19 +++++++++++--------
 1 file changed, 11 insertions(+), 8 deletions(-)

diff --git a/git-submodule.sh b/git-submodule.sh
index e2d08595f0..dcdd36fa64 100755
--- a/git-submodule.sh
+++ b/git-submodule.sh
@@ -1089,6 +1089,12 @@ cmd_sync()
        while read mode sha1 stage sm_path
        do
                die_if_unmatched "$mode" "$sha1"
+
+               if ! git submodule--helper is-active "$sm_path"
+               then
+                       continue
+               fi
+
                name=$(git submodule--helper name "$sm_path")
                url=$(git config -f .gitmodules --get submodule."$name".url)
 
@@ -1111,14 +1117,12 @@ cmd_sync()
                        ;;
                esac
 
-               if git submodule--helper is-active "$sm_path"
-               then
-                       displaypath=$(git submodule--helper relative-path 
"$prefix$sm_path" "$wt_prefix")
-                       say "$(eval_gettext "Synchronizing submodule url for 
'\$displaypath'")"
-                       git config submodule."$name".url "$super_config_url"
+               displaypath=$(git submodule--helper relative-path 
"$prefix$sm_path" "$wt_prefix")
+               say "$(eval_gettext "Synchronizing submodule url for 
'\$displaypath'")"
+               git config submodule."$name".url "$super_config_url"
 
-                       if test -e "$sm_path"/.git
-                       then
+               if test -e "$sm_path"/.git
+               then
                        (
                                sanitize_submodule_env
                                cd "$sm_path"
@@ -1131,7 +1135,6 @@ cmd_sync()
                                        eval cmd_sync
                                fi
                        )
-                       fi
                fi
        done
 }