diff mbox series

[04/10] git-submodule.sh: dispatch "foreach" to helper

Message ID patch-04.10-db6a09ee42a-20221017T115544Z-avarab@gmail.com (mailing list archive)
State New, archived
Headers show
Series submodule: make it a built-in, remove git-submodule.sh | expand

Commit Message

Ævar Arnfjörð Bjarmason Oct. 17, 2022, 12:09 p.m. UTC
Dispatch the "git submodule foreach" command directly to "git
submodule--helper foreach". This case requires the addition of the
PARSE_OPT_STOP_AT_NON_OPTION flag, since the shellscript was
unconditionally adding "--" to the "git submodule--helper"
command-line.

Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
---
 builtin/submodule--helper.c |  3 ++-
 git-submodule.sh            | 37 +++----------------------------------
 2 files changed, 5 insertions(+), 35 deletions(-)

Comments

Glen Choo Oct. 20, 2022, 9:14 p.m. UTC | #1
Ævar Arnfjörð Bjarmason <avarab@gmail.com> writes:

> Dispatch the "git submodule foreach" command directly to "git
> submodule--helper foreach". This case requires the addition of the
> PARSE_OPT_STOP_AT_NON_OPTION flag, since the shellscript was
> unconditionally adding "--" to the "git submodule--helper"
> command-line.

The commands in the previous patch also unconditionally add "--", so
this wasn't clear to me on first read.

It might be clearer to include some extra context, which is that "git
submodule foreach" is different because everything after "foreach"
should be treated like a single comand to run even if they are passed as
separate strings, e.g. from t/t7407-submodule-foreach.sh.21, these two
should be equivalent:

  git submodule foreach "echo be --quiet"

vs

  git submodule foreach echo be --quiet

So PARSE_OPT_STOP_AT_NON_OPTION is necessary in order to stop us from
intepreting option-like args as args to "git submodule foreach" instead
of as part of the command to run. Without PARSE_OPT_STOP_AT_NON_OPTION,

  git submodule foreach echo be --quiet

becomes more like

  git submodule foreach --quiet "echo be"

There could have been a subtle change in behavior, since "git submodule
foreach -- --foo" will be parsed as if "--foo" is the command, but
PARSE_OPT_STOP_AT_NON_OPTION will happily accept "--foo" as an option.
But we could never get into this situation anyway since we used to emit
usage on the first option-like arg...

> -# Execute an arbitrary command sequence in each checked out
> -# submodule
> -#
> -# $@ = command to execute
> -#
> -cmd_foreach()
> -{
> -	# parse $args after "submodule ... foreach".
> -	while test $# -ne 0
> -	do
> -		case "$1" in
> -		-q|--quiet)
> -			quiet=1
> -			;;
> -		--recursive)
> -			recursive=1
> -			;;
> -		-*)
> -			usage
> -			;;

i.e. here. 

So this patch looks safe.
diff mbox series

Patch

diff --git a/builtin/submodule--helper.c b/builtin/submodule--helper.c
index 0b4acb442b2..d11e1003019 100644
--- a/builtin/submodule--helper.c
+++ b/builtin/submodule--helper.c
@@ -403,7 +403,8 @@  static int module_foreach(int argc, const char **argv, const char *prefix)
 	int ret = 1;
 
 	argc = parse_options(argc, argv, prefix, module_foreach_options,
-			     git_submodule_helper_usage, 0);
+			     git_submodule_helper_usage,
+			     PARSE_OPT_STOP_AT_NON_OPTION);
 
 	if (module_list_compute(0, NULL, prefix, &pathspec, &list) < 0)
 		goto cleanup;
diff --git a/git-submodule.sh b/git-submodule.sh
index 2bdff5119c1..7874e33beea 100755
--- a/git-submodule.sh
+++ b/git-submodule.sh
@@ -50,37 +50,6 @@  jobs=
 recommend_shallow=
 filter=
 
-
-# Execute an arbitrary command sequence in each checked out
-# submodule
-#
-# $@ = command to execute
-#
-cmd_foreach()
-{
-	# parse $args after "submodule ... foreach".
-	while test $# -ne 0
-	do
-		case "$1" in
-		-q|--quiet)
-			quiet=1
-			;;
-		--recursive)
-			recursive=1
-			;;
-		-*)
-			usage
-			;;
-		*)
-			break
-			;;
-		esac
-		shift
-	done
-
-	git ${wt_prefix:+-C "$wt_prefix"} submodule--helper foreach ${quiet:+--quiet} ${recursive:+--recursive} -- "$@"
-}
-
 #
 # Update each submodule path to correct revision, using clone and checkout as needed
 #
@@ -263,10 +232,10 @@  case "$command" in
 absorbgitdirs)
 	git submodule--helper "$command" --prefix "$wt_prefix" "$@"
 	;;
-foreach | update)
-	"cmd_$command" "$@"
+update)
+	cmd_update "$@"
 	;;
-add | init | deinit | set-branch | set-url | status | summary | sync)
+add | foreach | init | deinit | set-branch | set-url | status | summary | sync)
 	git ${wt_prefix:+-C "$wt_prefix"} submodule--helper "$command" \
 		${quiet:+--quiet} ${cached:+--cached} "$@"
 	;;