Re: [PATCH v3 03/10] submodule sync: use submodule--helper is-active
- From
Junio C Hamano <gitster@pobox.com>
- Date
- Mar 14, 2017, 17:53 UTC
- Message-ID
- <xmqq8to7kamv.fsf@gitster.mtv.corp.google.com>
- In-Reply-To
- <20170313214341.172676-4-bmwill@google.com>
Brandon Williams <bmwill@google.com> writes:
Show 18 quoted lines
> Signed-off-by: Brandon Williams <bmwill@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 }