[PATCH 00/10] submodule: make it a built-in, remove git-submodule.sh

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On "master" any special behavior in git-submodule.sh itself has almost
entirely gone away, it's a trivial shimmy layer that dispatches to the
built-in "submodule--helper" command.

This series takes the logical next step and removes "git-submodule.sh"
entirely, replacing it wtih a "builtin/submodule.c". We leave
"builtin/submodule--helper.c" in-place and invoke its
cmd_submodule__helper() from the much smaller
"builtin/submodule.c". We should consolidate them, but let's leave
that cleanup for later.

An earlier version of this was submitted as an RFC in [1], since then
we've had various "prep" series's land to prepare for this final step,
namely:

  - 361cbe6d6d2 (Merge branch 'ab/submodule-cleanup', 2022-07-14)
  - 5647d743e39 (Merge branch 'ab/submodule-helper-prep' into ab/submodule-helper-leakfix, 2022-09-02)
  - b563638d2cf (Merge branch 'ab/submodule-helper-leakfix', 2022-09-14)

The first two made it much easier to take these last steps, as the
"submodule--helper" interface was already mostly mirroring that of
"submodule" itself. The "ab/submodule-helper-leakfix" series then made
it easier to test that our object ownership was still sane.

There's a recent report at [2] that "submodule--helper list" going
away in 5647d743e39 only left the much slower "foreach 'echo $name'"
alternative. As 07/10 and 10/10 here note rewriting this in C makes
that 'echo' version only about 0.1x slower, instead of >6x slower.

For CI & a branch that can be cloned for this series see:
https://github.com/avar/git/tree/avar/submodule-sh-dispatch-to-helper-directly-2

1. https://lore.kernel.org/git/RFC-cover-00.20-00000000000-20220610T011725Z-avarab@xxxxxxxxx/
2. https://lore.kernel.org/git/87czatrpyb.fsf@xxxxxxxxxx/

Ævar Arnfjörð Bjarmason (10):
  git-submodule.sh: create a "case" dispatch statement
  git-submodule.sh: dispatch "sync" to helper
  git-submodule.sh: dispatch directly to helper
  git-submodule.sh: dispatch "foreach" to helper
  git-submodule.sh: dispatch "update" to helper
  git-submodule.sh: don't support top-level "--cached"
  submodule: make it a built-in, remove git-submodule.sh
  submodule: support "--" with no other arguments
  submodule: support sub-command-less "--recursive" option
  submodule: don't use a subprocess to invoke "submodule--helper"

 Documentation/git-submodule.txt |   2 +-
 Makefile                        |   2 +-
 builtin.h                       |   1 +
 builtin/submodule--helper.c     |   3 +-
 builtin/submodule.c             | 178 ++++++++++
 git-submodule.sh                | 611 --------------------------------
 git.c                           |   1 +
 t/t7400-submodule-basic.sh      |  27 +-
 8 files changed, 200 insertions(+), 625 deletions(-)
 create mode 100644 builtin/submodule.c
 delete mode 100755 git-submodule.sh

Range-diff:
 1:  0e9f13822ef <  -:  ----------- git-submodule.sh: remove unused sanitize_submodule_env()
 7:  9f5cfbb864a !  1:  2379d5dc0e0 git-submodule.sh: create a "case" dispatch statement
    @@ Commit message
     
      ## git-submodule.sh ##
     @@ git-submodule.sh: cmd_sync()
    - 	git ${wt_prefix:+-C "$wt_prefix"} submodule--helper sync ${GIT_QUIET:+--quiet} ${recursive:+--recursive} -- "$@"
    + 	git ${wt_prefix:+-C "$wt_prefix"} submodule--helper sync ${quiet:+--quiet} ${recursive:+--recursive} -- "$@"
      }
      
     -cmd_absorbgitdirs()
 9:  bd0e4a4f8b8 !  2:  46bf600820b git-submodule.sh: dispatch "sync" to helper
    @@ Commit message
      ## git-submodule.sh ##
     @@ git-submodule.sh: cmd_status()
      
    - 	git ${wt_prefix:+-C "$wt_prefix"} submodule--helper status ${GIT_QUIET:+--quiet} ${cached:+--cached} ${recursive:+--recursive} -- "$@"
    + 	git ${wt_prefix:+-C "$wt_prefix"} submodule--helper status ${quiet:+--quiet} ${cached:+--cached} ${recursive:+--recursive} -- "$@"
      }
     -#
     -# Sync remote urls for submodules
    @@ git-submodule.sh: cmd_status()
     -	do
     -		case "$1" in
     -		-q|--quiet)
    --			GIT_QUIET=1
    +-			quiet=1
     -			shift
     -			;;
     -		--recursive)
    @@ git-submodule.sh: cmd_status()
     -		esac
     -	done
     -
    --	git ${wt_prefix:+-C "$wt_prefix"} submodule--helper sync ${GIT_QUIET:+--quiet} ${recursive:+--recursive} -- "$@"
    +-	git ${wt_prefix:+-C "$wt_prefix"} submodule--helper sync ${quiet:+--quiet} ${recursive:+--recursive} -- "$@"
     -}
      
      # This loop parses the command line arguments to find the
    @@ git-submodule.sh: case "$command" in
      	;;
     +sync)
     +	git ${wt_prefix:+-C "$wt_prefix"} submodule--helper "$command" \
    -+		${GIT_QUIET:+--quiet} "$@"
    ++		${quiet:+--quiet} "$@"
     +	;;
      *)
      	"cmd_$(echo $command | sed -e s/-/_/g)" "$@"
10:  498a1fd275b !  3:  97cb470c96a git-submodule.sh: dispatch directly to helper
    @@ Commit message
         Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@xxxxxxxxx>
     
      ## git-submodule.sh ##
    -@@ git-submodule.sh: GIT_PROTOCOL_FROM_USER=0
    - export GIT_PROTOCOL_FROM_USER
    +@@ git-submodule.sh: export GIT_PROTOCOL_FROM_USER
      
      command=
    + quiet=
     -branch=
      force=
      reference=
    @@ git-submodule.sh: GIT_PROTOCOL_FROM_USER=0
     -files=
      remote=
      nofetch=
    - update=
    + rebase=
    + merge=
    + checkout=
     -custom_name=
      depth=
      progress=
    @@ git-submodule.sh: jobs=
     -			force=$1
     -			;;
     -		-q|--quiet)
    --			GIT_QUIET=1
    +-			quiet=1
     -			;;
     -		--progress)
     -			progress=1
    @@ git-submodule.sh: jobs=
     -		usage
     -	fi
     -
    --	git ${wt_prefix:+-C "$wt_prefix"} submodule--helper add ${GIT_QUIET:+--quiet} ${force:+--force} ${progress:+"--progress"} ${branch:+--branch "$branch"} ${reference_path:+--reference "$reference_path"} ${dissociate:+--dissociate} ${custom_name:+--name "$custom_name"} ${depth:+"$depth"} -- "$@"
    +-	git ${wt_prefix:+-C "$wt_prefix"} submodule--helper add ${quiet:+--quiet} ${force:+--force} ${progress:+"--progress"} ${branch:+--branch "$branch"} ${reference_path:+--reference "$reference_path"} ${dissociate:+--dissociate} ${custom_name:+--name "$custom_name"} ${depth:+"$depth"} -- "$@"
     -}
      
     -#
    @@ git-submodule.sh: jobs=
      # submodule
      #
     @@ git-submodule.sh: cmd_foreach()
    - 	git ${wt_prefix:+-C "$wt_prefix"} submodule--helper foreach ${GIT_QUIET:+--quiet} ${recursive:+--recursive} -- "$@"
    + 	git ${wt_prefix:+-C "$wt_prefix"} submodule--helper foreach ${quiet:+--quiet} ${recursive:+--recursive} -- "$@"
      }
      
     -#
    @@ git-submodule.sh: cmd_foreach()
     -	do
     -		case "$1" in
     -		-q|--quiet)
    --			GIT_QUIET=1
    +-			quiet=1
     -			;;
     -		--)
     -			shift
    @@ git-submodule.sh: cmd_foreach()
     -		shift
     -	done
     -
    --	git ${wt_prefix:+-C "$wt_prefix"} submodule--helper init ${GIT_QUIET:+--quiet} -- "$@"
    +-	git ${wt_prefix:+-C "$wt_prefix"} submodule--helper init ${quiet:+--quiet} -- "$@"
     -}
     -
     -#
    @@ git-submodule.sh: cmd_foreach()
     -			force=$1
     -			;;
     -		-q|--quiet)
    --			GIT_QUIET=1
    +-			quiet=1
     -			;;
     -		--all)
     -			deinit_all=t
    @@ git-submodule.sh: cmd_foreach()
     -		shift
     -	done
     -
    --	git ${wt_prefix:+-C "$wt_prefix"} submodule--helper deinit ${GIT_QUIET:+--quiet} ${force:+--force} ${deinit_all:+--all} -- "$@"
    +-	git ${wt_prefix:+-C "$wt_prefix"} submodule--helper deinit ${quiet:+--quiet} ${force:+--force} ${deinit_all:+--all} -- "$@"
     -}
     -
      #
    @@ git-submodule.sh: cmd_update()
     -		shift
     -	done
     -
    --	git ${wt_prefix:+-C "$wt_prefix"} submodule--helper set-branch ${GIT_QUIET:+--quiet} ${branch:+--branch "$branch"} ${default:+--default} -- "$@"
    +-	git ${wt_prefix:+-C "$wt_prefix"} submodule--helper set-branch ${quiet:+--quiet} ${branch:+--branch "$branch"} ${default:+--default} -- "$@"
     -}
     -
     -#
    @@ git-submodule.sh: cmd_update()
     -	do
     -		case "$1" in
     -		-q|--quiet)
    --			GIT_QUIET=1
    +-			quiet=1
     -			;;
     -		--)
     -			shift
    @@ git-submodule.sh: cmd_update()
     -		shift
     -	done
     -
    --	git ${wt_prefix:+-C "$wt_prefix"} submodule--helper set-url ${GIT_QUIET:+--quiet} -- "$@"
    +-	git ${wt_prefix:+-C "$wt_prefix"} submodule--helper set-url ${quiet:+--quiet} -- "$@"
     -}
     -
     -#
    @@ git-submodule.sh: cmd_update()
     -	do
     -		case "$1" in
     -		-q|--quiet)
    --			GIT_QUIET=1
    +-			quiet=1
     -			;;
     -		--cached)
     -			cached=1
    @@ git-submodule.sh: cmd_update()
     -		shift
     -	done
     -
    --	git ${wt_prefix:+-C "$wt_prefix"} submodule--helper status ${GIT_QUIET:+--quiet} ${cached:+--cached} ${recursive:+--recursive} -- "$@"
    +-	git ${wt_prefix:+-C "$wt_prefix"} submodule--helper status ${quiet:+--quiet} ${cached:+--cached} ${recursive:+--recursive} -- "$@"
     -}
     -
      # This loop parses the command line arguments to find the
    @@ git-submodule.sh: case "$command" in
      	;;
     -sync)
     -	git ${wt_prefix:+-C "$wt_prefix"} submodule--helper "$command" \
    --		${GIT_QUIET:+--quiet} "$@"
    +-		${quiet:+--quiet} "$@"
     +foreach | update)
     +	"cmd_$command" "$@"
      	;;
    @@ git-submodule.sh: case "$command" in
     -	"cmd_$(echo $command | sed -e s/-/_/g)" "$@"
     +add | init | deinit | set-branch | set-url | status | summary | sync)
     +	git ${wt_prefix:+-C "$wt_prefix"} submodule--helper "$command" \
    -+		${GIT_QUIET:+--quiet} ${cached:+--cached} "$@"
    ++		${quiet:+--quiet} ${cached:+--cached} "$@"
      	;;
      esac
11:  625320e13b9 !  4:  db6a09ee42a git-submodule.sh: dispatch "foreach" to helper
    @@ Commit message
     
      ## builtin/submodule--helper.c ##
     @@ builtin/submodule--helper.c: 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);
    @@ builtin/submodule--helper.c: static int module_foreach(int argc, const char **ar
     +			     PARSE_OPT_STOP_AT_NON_OPTION);
      
      	if (module_list_compute(0, NULL, prefix, &pathspec, &list) < 0)
    - 		return 1;
    + 		goto cleanup;
     
      ## git-submodule.sh ##
     @@ git-submodule.sh: jobs=
    @@ git-submodule.sh: jobs=
     -	do
     -		case "$1" in
     -		-q|--quiet)
    --			GIT_QUIET=1
    +-			quiet=1
     -			;;
     -		--recursive)
     -			recursive=1
    @@ git-submodule.sh: jobs=
     -		shift
     -	done
     -
    --	git ${wt_prefix:+-C "$wt_prefix"} submodule--helper foreach ${GIT_QUIET:+--quiet} ${recursive:+--recursive} -- "$@"
    +-	git ${wt_prefix:+-C "$wt_prefix"} submodule--helper foreach ${quiet:+--quiet} ${recursive:+--recursive} -- "$@"
     -}
     -
      #
    @@ git-submodule.sh: case "$command" in
     -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" \
    - 		${GIT_QUIET:+--quiet} ${cached:+--cached} "$@"
    + 		${quiet:+--quiet} ${cached:+--cached} "$@"
      	;;
16:  08abadda7c3 !  5:  7d9c13eb637 git-submodule.sh: dispatch "update" to helper
    @@ Commit message
     
         Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@xxxxxxxxx>
     
    - ## Documentation/git-submodule.txt ##
    -@@ Documentation/git-submodule.txt: SYNOPSIS
    - 'git submodule' [--quiet] status [--cached] [--recursive] [--] [<path>...]
    - 'git submodule' [--quiet] init [--] [<path>...]
    - 'git submodule' [--quiet] deinit [-f|--force] (--all|[--] <path>...)
    --'git submodule' [--quiet] update [-v] [<options>] [--] [<path>...]
    -+'git submodule' [--quiet] update [-v | --verbose] [<options>] [--] [<path>...]
    - 'git submodule' [--quiet] set-branch [<options>] [--] <path>
    - 'git submodule' [--quiet] set-url [--] <path> <newurl>
    - 'git submodule' [--quiet] summary [<options>] [--] [<path>...]
    -@@ Documentation/git-submodule.txt: If you really want to remove a submodule from the repository and commit
    - that use linkgit:git-rm[1] instead. See linkgit:gitsubmodules[7] for removal
    - options.
    - 
    --update [-v] [--init] [--remote] [-N|--no-fetch] [--[no-]recommend-shallow] [-f|--force] [--checkout|--rebase|--merge] [--reference <repository>] [--depth <depth>] [--recursive] [--jobs <n>] [--[no-]single-branch] [--filter <filter spec>] [--] [<path>...]::
    -+update [-v | --verbose] [--init] [--remote] [-N|--no-fetch] [--[no-]recommend-shallow] [-f|--force] [--checkout|--rebase|--merge] [--reference <repository>] [--depth <depth>] [--recursive] [--jobs <n>] [--[no-]single-branch] [--filter <filter spec>] [--] [<path>...]::
    - +
    - --
    - Update the registered submodules to match what the superproject
    -@@ Documentation/git-submodule.txt: OPTIONS
    - 	Only print error messages.
    - 
    - -v::
    -+--verbose::
    - 	Don't be quiet. This option is only valid for the update command.
    - 
    - --progress::
    -
      ## git-submodule.sh ##
    -@@ git-submodule.sh: GIT_PROTOCOL_FROM_USER=0
    - export GIT_PROTOCOL_FROM_USER
    +@@ git-submodule.sh: export GIT_PROTOCOL_FROM_USER
      
      command=
    + quiet=
     -force=
     -reference=
      cached=
    @@ git-submodule.sh: GIT_PROTOCOL_FROM_USER=0
     -	do
     -		case "$1" in
     -		-q|--quiet)
    --			GIT_QUIET=1
    --			;;
    --		-v)
    --			unset GIT_QUIET
    +-			quiet=1
     -			;;
     -		--progress)
     -			progress=1
    @@ git-submodule.sh: GIT_PROTOCOL_FROM_USER=0
     -	done
     -
     -	git ${wt_prefix:+-C "$wt_prefix"} submodule--helper update \
    --		${GIT_QUIET:+--quiet} \
    +-		${quiet:+--quiet} \
     -		${force:+--force} \
     -		${progress:+"--progress"} \
     -		${remote:+--remote} \
    @@ git-submodule.sh: absorbgitdirs)
      update)
     -	cmd_update "$@"
     +	git ${wt_prefix:+-C "$wt_prefix"} submodule--helper "$command" \
    -+		${GIT_QUIET:+--quiet} ${wt_prefix:+--prefix "$wt_prefix"} "$@"
    ++		${quiet:+--quiet} ${wt_prefix:+--prefix "$wt_prefix"} "$@"
      	;;
      add | foreach | init | deinit | set-branch | set-url | status | summary | sync)
      	git ${wt_prefix:+-C "$wt_prefix"} submodule--helper "$command" \
 4:  f27723aa0a2 !  6:  25fadf3ffc1 git-submodule.sh: normalize parsing of "--branch"
    @@ Metadata
     Author: Ævar Arnfjörð Bjarmason <avarab@xxxxxxxxx>
     
      ## Commit message ##
    -    git-submodule.sh: normalize parsing of "--branch"
    +    git-submodule.sh: don't support top-level "--cached"
     
    -    In 5c08dbbdf1a (git-submodule: fix subcommand parser, 2008-01-15) the
    -    "--branch" option was supported as an option to "git submodule"
    -    itself, i.e. "git submodule --branch" as a side-effect of its
    -    implementation.
    +    Since the preceding commit all sub-commands of "git submodule" have
    +    been dispatched to "git submodule--helper" directly, we therefore
    +    don't need to emit "usage()" if we see "--cached" without the
    +    sub-command being "status" or "summary", we can trust that
    +    parse_options() will spot that and barf on it.
     
    -    Then in b57e8119e6e (submodule: teach set-branch subcommand,
    -    2019-02-08) when the "set-branch" subcommand was added the assertion
    -    that we shouldn't have "--branch" anywhere except as an argument to
    -    "add" and "set-branch" was copy/pasted from the adjacent check for
    -    "--cache" added (or rather modified) in 496eeeb19b9 (git-submodule.sh:
    -    avoid "test <cond> -a/-o <cond>", 2014-06-10).
    +    This does change one obscure aspect of undocumented behavior, for
    +    "status" and "summary" we supported these undocumented forms:
     
    -    But there's been a logic error in that check, this looked like it
    -    should be supporting:
    +        git submodule --cached (status | summary)
     
    -        git submodule --branch <branch> (add | set-branch) [<options>]
    +    As noted in a preceding commit to git-submodule.sh which removed the
    +    "--branch" special-case, this comes down to emergent behavior seen in
    +    5c08dbbdf1a (git-submodule: fix subcommand parser,
    +    2008-01-15). I.e. we wanted to support was for subcommand-less invocations like:
     
    -    But due to "||" in the condition (as opposed to "&&" for "--cache") if
    -    we have "--branch" here already we'll emit usage, even for "add" and
    -    "set-branch".
    +        git submodule --cached
     
    -    Since nobody's complained about "--branch <branch>" not being
    -    supported as argument to "git submodule" itself, i.e. we want to
    -    support:
    +    To be synonymous with invocations that explicitly named the "status"
    +    sub-command:
     
    -        git submodule (add | set-branch) --branch <branch>  [<options>]
    +        git submodule status --cached
     
    -    But not the first form noted above. Let's just remove this code, we've
    -    never documented "--branch" as a top-level option (unlike "--quiet"),
    -    so this looks like it was an accident of the implementation, which we
    -    broke v2.22.0, so we also know it must not have been important to
    -    anyone.
    +    But we did not intend to mix the two, and allow "--cached" to be an
    +    option to the top-level "submodule" command when the "status" or
    +    "summary" sub-commands were explicitly provided.
    +
    +    Let's remove this undocumented edge case, which makes a subsequent
    +    removal of git-submodule.sh easier to reason about. The test case
    +    added here is duplicated from the existing for-loop, except for the
    +    different and desired handling of "git submodule --cached status".
     
         Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@xxxxxxxxx>
     
      ## git-submodule.sh ##
     @@ git-submodule.sh: do
    - 	-q|--quiet)
    - 		GIT_QUIET=1
    + 		quiet=1
      		;;
    --	-b|--branch)
    --		case "$2" in
    --		'')
    --			usage
    --			;;
    --		esac
    --		branch="$2"; shift
    --		;;
      	--cached)
    - 		cached="$1"
    +-		cached=1
    ++		if test -z "$command"
    ++		then
    ++			cached=1 &&
    ++			shift &&
    ++			break
    ++		else
    ++			usage
    ++		fi
      		;;
    + 	--)
    + 		break
     @@ git-submodule.sh: then
          fi
      fi
      
    --# "-b branch" is accepted only by "add" and "set-branch"
    --if test -n "$branch" && (test "$command" != add || test "$command" != set-branch)
    +-# "--cached" is accepted only by "status" and "summary"
    +-if test -n "$cached" && test "$command" != status && test "$command" != summary
     -then
     -	usage
     -fi
     -
    - # "--cached" is accepted only by "status" and "summary"
    - if test -n "$cached" && test "$command" != status && test "$command" != summary
    - then
    + case "$command" in
    + absorbgitdirs)
    + 	git submodule--helper "$command" --prefix "$wt_prefix" "$@"
    +
    + ## t/t7400-submodule-basic.sh ##
    +@@ t/t7400-submodule-basic.sh: test_expect_success 'submodule usage: status --' '
    + 	test_expect_code 1 git submodule --end-of-options
    + '
    + 
    +-for opt in '--quiet' '--cached'
    ++for opt in '--quiet'
    + do
    + 	test_expect_success "submodule usage: status $opt" '
    + 		git submodule $opt &&
    +@@ t/t7400-submodule-basic.sh: do
    + 	'
    + done
    + 
    ++for opt in '--cached'
    ++do
    ++	test_expect_success "submodule usage: status $opt" '
    ++		git submodule $opt &&
    ++		git submodule status $opt &&
    ++		test_expect_code 1 git submodule $opt status >out 2>err &&
    ++		grep "^usage: git submodule" err &&
    ++		test_must_be_empty out
    ++	'
    ++done
    ++
    + test_expect_success 'submodule deinit works on empty repository' '
    + 	git submodule deinit --all
    + '
    +@@ t/t7400-submodule-basic.sh: test_expect_success 'status should be "modified" after submodule commit' '
    + '
    + 
    + test_expect_success 'the --cached sha1 should be rev1' '
    +-	git submodule --cached status >list &&
    ++	git submodule status --cached >list &&
    + 	grep "^+$rev1" list
    + '
    + 
19:  1423950de08 !  7:  2c77ed38d90 submodule: make it a built-in, remove git-submodule.sh
    @@ Metadata
      ## Commit message ##
         submodule: make it a built-in, remove git-submodule.sh
     
    -    Replace the now-trivial git-submodule.sh script with a built-in
    -    builtin/submodule.c. For now this new command is only a dumb
    +    Replace the "git-submodule.sh" script with a built-in
    +    "builtin/submodule.c. For" now this new command is only a dumb
         dispatcher that uses run-command.c to invoke "git submodule--helper",
         just as "git-submodule.sh" used to do.
     
    -    This is obviously not ideal, and we should follow-up and merge the
    -    builtin/submodule--helper.c code into builtin/submodule.c, but doing it
    -    this way makes it easy to review that this new C implementation isn't
    -    doing anything more clever than the old shellscript implementation.
    +    This is obviously not ideal, and we should eventually follow-up and
    +    merge the "builtin/submodule--helper.c" code into
    +    "builtin/submodule.c". Doing it this way makes it easy to review that
    +    this new C implementation isn't doing anything more clever than the
    +    old shellscript implementation.
     
    -    The "define BUILTIN_" macros will help with that, i.e. the usage
    -    information we emit can be merged with what
    -    builtin/submodule--helper.c is now emitting. See
    -    8757b35d443 (commit-graph: define common usage with a macro,
    -    2021-08-23) and 1e91d3faf6c (reflog: move "usage" variables and use
    -    macros, 2022-03-17) for prior art using this pattern.
    +    This is a large win for performance, we're now more than 4x as fast as
    +    before in terms of the fixed cost of invoking any "git submodule"
    +    command[1]:
    +
    +            $ git hyperfine -L rev HEAD~1,HEAD -s 'make CFLAGS=-O3' './git --exec-path=$PWD submodule foreach "echo \$name"'
    +            Benchmark 1: ./git --exec-path=$PWD submodule foreach "echo \$name"' in 'HEAD~1
    +              Time (mean ± σ):      42.2 ms ±   0.4 ms    [User: 34.9 ms, System: 9.1 ms]
    +              Range (min … max):    41.3 ms …  43.2 ms    70 runs
    +
    +            Benchmark 2: ./git --exec-path=$PWD submodule foreach "echo \$name"' in 'HEAD
    +              Time (mean ± σ):       9.7 ms ±   0.1 ms    [User: 7.6 ms, System: 2.2 ms]
    +              Range (min … max):     9.5 ms …  10.3 ms    282 runs
    +
    +            Summary
    +              './git --exec-path=$PWD submodule foreach "echo \$name"' in 'HEAD' ran
    +                4.33 ± 0.07 times faster than './git --exec-path=$PWD submodule foreach "echo \$name"' in 'HEAD~1'
    +
    +    We're taking pains here to faithfully reproduce existing
    +    "git-submodule.sh" behavior, even when that behavior is stupid. Some
    +    of it we'll fix in subsequent commits, but let's first faithfully
    +    reproduce the behavior.
    +
    +    One exception is the change in the behavior of the exit code
    +    stand-alone "-h" and "--" yield, see the altered tests. Returning 129
    +    instead of 0 and 1 for "-h" and "--" respectively is a concession to
    +    basic sanity.
    +
    +    The pattern of using "define BUILTIN_" macros here isn't needed for
    +    now, but as we'll move code out of "builtin/submodule--helper.c" we'll
    +    want to re-use these strings. See 8757b35d443 (commit-graph: define
    +    common usage with a macro, 2021-08-23) and 1e91d3faf6c (reflog: move
    +    "usage" variables and use macros, 2022-03-17) for prior art using this
    +    pattern.
     
         The "(argc < 2 || !strcmp(argv[1], "-h"))" path at the top of
         cmd_submodule__helper() could now be a "(argc < 2)" if not for
         t0012-help.sh (which invokes all built-ins manually with "-h"). Let's
         leave it for now, eventually we'll consolidate the two.
     
    +    1. Using the "git hyperfine" wrapper for "hyperfine":
    +       https://lore.kernel.org/git/211201.86r1aw9gbd.gmgdl@xxxxxxxxxxxxxxxxxxx/
    +
         Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@xxxxxxxxx>
     
      ## Makefile ##
    @@ builtin/submodule.c (new)
     +};
     +
     +static void setup_helper_args(int argc, const char **argv, const char *prefix,
    -+			      int quiet, int cached, struct strvec *args)
    ++			      int quiet, int cached, struct strvec *args,
    ++			      const struct option *options)
     +{
     +	const char *cmd;
     +	int do_quiet_cache = 1;
    @@ builtin/submodule.c (new)
     +		return;
     +	}
     +
    ++	/* Did we get --cached with a command? */
    ++	if (cached)
    ++		usage_msg_optf(_("'%s' option is only supported with explicit 'status'"),
    ++			       git_submodule_usage, options, "--cached");
    ++
    ++
     +	/* Either a valid command, or submodule--helper will barf! */
     +	cmd = argv[0];
     +	strvec_push(args, cmd);
     +	argv++;
     +	argc--;
     +
    ++	/*
    ++	  * This is stupid, but don't support "[--]" to
    ++	 * subcommand-less "git-submodule" for now.
    ++	 */
    ++	if (!strcmp(cmd, "--") || !strcmp(cmd, "--end-of-options"))
    ++		usage_msg_optf(_("need explicit sub-command name to delimit with '%s'"),
    ++			       git_submodule_usage, options, cmd);
    ++
     +	/* Options that need to go before user-supplied options */
     +	if (!strcmp(cmd, "absorbgitdirs"))
     +		do_quiet_cache = 0;
    @@ builtin/submodule.c (new)
     +			 N_("print the OID of submodules")),
     +		OPT_END()
     +	};
    -+	int ret;
     +
     +	argc = parse_options(argc, argv, prefix, options, git_submodule_usage,
    -+			     PARSE_OPT_STOP_AT_NON_OPTION);
    ++			     PARSE_OPT_STOP_AT_NON_OPTION | PARSE_OPT_KEEP_DASHDASH);
     +
     +	/*
     +	 * Tell the rest of git that any URLs we get don't come
     +	 * directly from the user, so it can apply policy as appropriate.
     +	 */
    -+	strvec_push(&cp.env_array, "GIT_PROTOCOL_FROM_USER=0");
    ++	strvec_push(&cp.env, "GIT_PROTOCOL_FROM_USER=0");
     +	setup_helper_args(argc, argv, prefix, opt_quiet, opt_cached,
    -+			  &cp.args);
    ++			  &cp.args, options);
     +
     +	cp.git_cmd = 1;
     +	cp.no_stdin = 0; /* for git submodule foreach */
     +	cp.dir = startup_info->original_cwd;
    -+	ret = run_command(&cp);
     +
    -+	return ret;
    ++	return run_command(&cp);
     +}
     
      ## git-submodule.sh (deleted) ##
    @@ git-submodule.sh (deleted)
     -   or: $dashless [--quiet] status [--cached] [--recursive] [--] [<path>...]
     -   or: $dashless [--quiet] init [--] [<path>...]
     -   or: $dashless [--quiet] deinit [-f|--force] (--all| [--] <path>...)
    --   or: $dashless [--quiet] update [-v] [--init [--filter=<filter-spec>]] [--remote] [-N|--no-fetch] [-f|--force] [--checkout|--merge|--rebase] [--[no-]recommend-shallow] [--reference <repository>] [--recursive] [--[no-]single-branch] [--] [<path>...]
    +-   or: $dashless [--quiet] update [--init [--filter=<filter-spec>]] [--remote] [-N|--no-fetch] [-f|--force] [--checkout|--merge|--rebase] [--[no-]recommend-shallow] [--reference <repository>] [--recursive] [--[no-]single-branch] [--] [<path>...]
     -   or: $dashless [--quiet] set-branch (--default|--branch <branch>) [--] <path>
     -   or: $dashless [--quiet] set-url [--] <path> <newurl>
     -   or: $dashless [--quiet] summary [--cached|--files] [--summary-limit <n>] [commit] [--] [<path>...]
    @@ git-submodule.sh (deleted)
     -GIT_PROTOCOL_FROM_USER=0
     -export GIT_PROTOCOL_FROM_USER
     -
    +-command=
     -quiet=
     -cached=
     -
    --while test $# != 0
    +-while test $# != 0 && test -z "$command"
     -do
     -	case "$1" in
    +-	add | foreach | init | deinit | update | set-branch | set-url | status | summary | sync | absorbgitdirs)
    +-		command=$1
    +-		;;
     -	-q|--quiet)
    --		quiet=1 &&
    --		shift
    +-		quiet=1
     -		;;
     -	--cached)
    --		cached=1 &&
    --		shift
    +-		if test -z "$command"
    +-		then
    +-			cached=1 &&
    +-			shift &&
    +-			break
    +-		else
    +-			usage
    +-		fi
    +-		;;
    +-	--)
    +-		break
    +-		;;
    +-	-*)
    +-		usage
     -		;;
     -	*)
     -		break
     -		;;
     -	esac
    +-	shift
     -done
     -
     -# No command word defaults to "status"
    --command=
    --if test $# = 0
    +-if test -z "$command"
     -then
    +-    if test $# = 0
    +-    then
     -	command=status
    --else
    --	case "$1" in
    --	add | foreach | init | deinit | update | set-branch | set-url | status | summary | sync | absorbgitdirs)
    --		command=$1 &&
    --		shift
    --		;;
    --	*)
    --		usage
    --	esac
    +-    else
    +-	usage
    +-    fi
     -fi
     -
     -case "$command" in
    @@ git-submodule.sh (deleted)
     -	git ${wt_prefix:+-C "$wt_prefix"} submodule--helper "$command" \
     -		${quiet:+--quiet} ${wt_prefix:+--prefix "$wt_prefix"} "$@"
     -	;;
    --*)
    +-add | foreach | init | deinit | set-branch | set-url | status | summary | sync)
     -	git ${wt_prefix:+-C "$wt_prefix"} submodule--helper "$command" \
     -		${quiet:+--quiet} ${cached:+--cached} "$@"
     -	;;
    @@ git.c: static struct cmd_struct commands[] = {
      	{ "submodule--helper", cmd_submodule__helper, RUN_SETUP | SUPPORT_SUPER_PREFIX | NO_PARSEOPT },
      	{ "switch", cmd_switch, RUN_SETUP | NEED_WORK_TREE },
      	{ "symbolic-ref", cmd_symbolic_ref, RUN_SETUP },
    +
    + ## t/t7400-submodule-basic.sh ##
    +@@ t/t7400-submodule-basic.sh: export GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME
    + . ./test-lib.sh
    + 
    + test_expect_success 'submodule usage: -h' '
    +-	git submodule -h >out 2>err &&
    ++	test_expect_code 129 git submodule -h >out 2>err &&
    + 	grep "^usage: git submodule" out &&
    + 	test_must_be_empty err
    + '
    + 
    + test_expect_success 'submodule usage: --recursive' '
    +-	test_expect_code 1 git submodule --recursive >out 2>err &&
    +-	grep "^usage: git submodule" err &&
    +-	test_must_be_empty out
    ++	test_expect_code 129 git submodule --recursive
    + '
    + 
    + test_expect_success 'submodule usage: status --' '
    +-	test_expect_code 1 git submodule -- &&
    +-	test_expect_code 1 git submodule --end-of-options
    ++	test_expect_code 129 git submodule -- &&
    ++	test_expect_code 129 git submodule --end-of-options
    + '
    + 
    + for opt in '--quiet'
    +@@ t/t7400-submodule-basic.sh: do
    + 	test_expect_success "submodule usage: status $opt" '
    + 		git submodule $opt &&
    + 		git submodule status $opt &&
    +-		test_expect_code 1 git submodule $opt status >out 2>err &&
    ++		test_expect_code 129 git submodule $opt status >out 2>err &&
    + 		grep "^usage: git submodule" err &&
    + 		test_must_be_empty out
    + 	'
 3:  6c774505ac5 !  8:  8dbb81e2fa5 git-submodule.sh: remove unused --super-prefix logic
    @@ Metadata
     Author: Ævar Arnfjörð Bjarmason <avarab@xxxxxxxxx>
     
      ## Commit message ##
    -    git-submodule.sh: remove unused --super-prefix logic
    +    submodule: support "--" with no other arguments
     
    -    The "$prefix" variable has not been set since b3c5f5cb048 (submodule:
    -    move core cmd_update() logic to C, 2022-03-15), so we'd never pass the
    -    --super-prefix option to "git submodule--helper", and can therefore
    -    remove the handling of it from builtin/submodule--helper.c as well.
    +    Address an edge case that "git-submodule.sh" has had all along, but
    +    which became painfully obvious in the *.sh to *.c migration in the
    +    preceding commit.
    +
    +    We didn't support the "--" delimiter in the argument-less
    +    invocation. Let's not bend over backwards to behave unusually in this
    +    scenario, simply accepting "--" is harmless.
     
         Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@xxxxxxxxx>
     
    - ## builtin/submodule--helper.c ##
    -@@ builtin/submodule--helper.c: static int module_add(int argc, const char **argv, const char *prefix)
    - 	return 0;
    - }
    + ## Documentation/git-submodule.txt ##
    +@@ Documentation/git-submodule.txt: git-submodule - Initialize, update or inspect submodules
    + SYNOPSIS
    + --------
    + [verse]
    +-'git submodule' [--quiet] [--cached]
    ++'git submodule' [--quiet] [--cached] [--]
    + 'git submodule' [--quiet] add [<options>] [--] <repository> [<path>]
    + 'git submodule' [--quiet] status [--cached] [--recursive] [--] [<path>...]
    + 'git submodule' [--quiet] init [--] [<path>...]
    +
    + ## builtin/submodule.c ##
    +@@
    + #include "strvec.h"
    + 
    + #define BUILTIN_SUBMODULE_USAGE \
    +-	"git submodule [--quiet] [--cached]"
    ++	"git submodule [--quiet] [--cached] [--]"
      
    --#define SUPPORT_SUPER_PREFIX (1<<0)
    + #define BUILTIN_SUBMODULE_ADD_USAGE \
    + 	N_("git submodule [--quiet] add [-b <branch>] [-f | --force] [--name <name>]\n" \
    +@@ builtin/submodule.c: static void setup_helper_args(int argc, const char **argv, const char *prefix,
    + 	argv++;
    + 	argc--;
    + 
    +-	/*
    +-	  * This is stupid, but don't support "[--]" to
    +-	 * subcommand-less "git-submodule" for now.
    +-	 */
    +-	if (!strcmp(cmd, "--") || !strcmp(cmd, "--end-of-options"))
    +-		usage_msg_optf(_("need explicit sub-command name to delimit with '%s'"),
    +-			       git_submodule_usage, options, cmd);
     -
    - struct cmd_struct {
    - 	const char *cmd;
    - 	int (*fn)(int, const char **, const char *);
    --	unsigned option;
    - };
    + 	/* Options that need to go before user-supplied options */
    + 	if (!strcmp(cmd, "absorbgitdirs"))
    + 		do_quiet_cache = 0;
    +@@ builtin/submodule.c: int cmd_submodule(int argc, const char **argv, const char *prefix)
    + 	};
      
    - static struct cmd_struct commands[] = {
    --	{"list", module_list, 0},
    --	{"name", module_name, 0},
    --	{"clone", module_clone, 0},
    --	{"add", module_add, SUPPORT_SUPER_PREFIX},
    --	{"update", module_update, 0},
    --	{"resolve-relative-url-test", resolve_relative_url_test, 0},
    --	{"foreach", module_foreach, SUPPORT_SUPER_PREFIX},
    --	{"init", module_init, SUPPORT_SUPER_PREFIX},
    --	{"status", module_status, SUPPORT_SUPER_PREFIX},
    --	{"sync", module_sync, SUPPORT_SUPER_PREFIX},
    --	{"deinit", module_deinit, 0},
    --	{"summary", module_summary, SUPPORT_SUPER_PREFIX},
    --	{"push-check", push_check, 0},
    --	{"absorb-git-dirs", absorb_git_dirs, SUPPORT_SUPER_PREFIX},
    --	{"is-active", is_active, 0},
    --	{"check-name", check_name, 0},
    --	{"config", module_config, 0},
    --	{"set-url", module_set_url, 0},
    --	{"set-branch", module_set_branch, 0},
    --	{"create-branch", module_create_branch, 0},
    -+	{"list", module_list},
    -+	{"name", module_name},
    -+	{"clone", module_clone},
    -+	{"add", module_add},
    -+	{"update", module_update},
    -+	{"resolve-relative-url-test", resolve_relative_url_test},
    -+	{"foreach", module_foreach},
    -+	{"init", module_init},
    -+	{"status", module_status},
    -+	{"sync", module_sync},
    -+	{"deinit", module_deinit},
    -+	{"summary", module_summary},
    -+	{"push-check", push_check},
    -+	{"absorb-git-dirs", absorb_git_dirs},
    -+	{"is-active", is_active},
    -+	{"check-name", check_name},
    -+	{"config", module_config},
    -+	{"set-url", module_set_url},
    -+	{"set-branch", module_set_branch},
    -+	{"create-branch", module_create_branch},
    - };
    + 	argc = parse_options(argc, argv, prefix, options, git_submodule_usage,
    +-			     PARSE_OPT_STOP_AT_NON_OPTION | PARSE_OPT_KEEP_DASHDASH);
    ++			     PARSE_OPT_STOP_AT_NON_OPTION);
      
    - int cmd_submodule__helper(int argc, const char **argv, const char *prefix)
    -@@ builtin/submodule--helper.c: int cmd_submodule__helper(int argc, const char **argv, const char *prefix)
    - 	if (argc < 2 || !strcmp(argv[1], "-h"))
    - 		usage("git submodule--helper <command>");
    + 	/*
    + 	 * Tell the rest of git that any URLs we get don't come
    +
    + ## t/t7400-submodule-basic.sh ##
    +@@ t/t7400-submodule-basic.sh: test_expect_success 'submodule usage: --recursive' '
    + '
      
    --	for (i = 0; i < ARRAY_SIZE(commands); i++) {
    --		if (!strcmp(argv[1], commands[i].cmd)) {
    --			if (get_super_prefix() &&
    --			    !(commands[i].option & SUPPORT_SUPER_PREFIX))
    --				die(_("%s doesn't support --super-prefix"),
    --				    commands[i].cmd);
    -+	for (i = 0; i < ARRAY_SIZE(commands); i++)
    -+		if (!strcmp(argv[1], commands[i].cmd))
    - 			return commands[i].fn(argc - 1, argv + 1, prefix);
    --		}
    --	}
    + test_expect_success 'submodule usage: status --' '
    +-	test_expect_code 129 git submodule -- &&
    +-	test_expect_code 129 git submodule --end-of-options
    ++	git submodule -- &&
    ++	git submodule --end-of-options
    + '
      
    - 	die(_("'%s' is not a valid submodule--helper "
    - 	      "subcommand"), argv[1]);
    + for opt in '--quiet'
20:  b2aaad5c008 !  9:  7f232c5e503 submodule: add a subprocess-less submodule.useBuiltin setting
    @@ Metadata
     Author: Ævar Arnfjörð Bjarmason <avarab@xxxxxxxxx>
     
      ## Commit message ##
    -    submodule: add a subprocess-less submodule.useBuiltin setting
    +    submodule: support sub-command-less "--recursive" option
     
    -    Add an experimental setting to avoid the subprocess invocation of "git
    -    submodule--helper", instead we'll call cmd_submodule__helper()
    -    directly.
    +    The inability to specify "--recursive" when we're not providing a
    +    sub-command name appears to have been an omission in 15fc56a8536 (git
    +    submodule foreach: Add --recursive to recurse into nested submodules,
    +    2009-08-19). Let's support it along with the other "status" options.
     
         Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@xxxxxxxxx>
     
    - ## Documentation/config/submodule.txt ##
    -@@ Documentation/config/submodule.txt: setting.
    - * `branch` is supported only if `submodule.propagateBranches` is
    - enabled
    - 
    -+submodule.useBuiltin::
    -+	Set to `true` to use a faster but possibly less stable subprocess-less
    -+	implementation of linkgit:git-submodule[1]. Is `false` by default.
    -+
    - submodule.propagateBranches::
    - 	[EXPERIMENTAL] A boolean that enables branching support when
    - 	using `--recurse-submodules` or `submodule.recurse=true`.
    + ## Documentation/git-submodule.txt ##
    +@@ Documentation/git-submodule.txt: git-submodule - Initialize, update or inspect submodules
    + SYNOPSIS
    + --------
    + [verse]
    +-'git submodule' [--quiet] [--cached] [--]
    ++'git submodule' [--quiet] [--cached] [--recursive] [--]
    + 'git submodule' [--quiet] add [<options>] [--] <repository> [<path>]
    + 'git submodule' [--quiet] status [--cached] [--recursive] [--] [<path>...]
    + 'git submodule' [--quiet] init [--] [<path>...]
     
      ## builtin/submodule.c ##
    -@@
    - #include "parse-options.h"
    - #include "run-command.h"
    - #include "strvec.h"
    -+#include "config.h"
    +@@ builtin/submodule.c: static const char * const git_submodule_usage[] = {
    + };
      
    - #define BUILTIN_SUBMODULE_USAGE \
    - 	"git submodule [--quiet] [--cached]"
    + static void setup_helper_args(int argc, const char **argv, const char *prefix,
    +-			      int quiet, int cached, struct strvec *args,
    ++			      int quiet, int cached, int recursive,
    ++			      struct strvec *args,
    + 			      const struct option *options)
    + {
    + 	const char *cmd;
     @@ builtin/submodule.c: static void setup_helper_args(int argc, const char **argv, const char *prefix,
    - 	strvec_pushv(args, argv);
    - }
    + 		return;
    + 	}
      
    -+static int get_use_builtin(void)
    -+{
    -+	int v;
    -+
    -+	if (git_env_bool("GIT_TEST_SUBMODULE_USE_BUILTIN", 0))
    -+		v = 1;
    -+	else if (!git_config_get_bool("submodule.usebuiltin", &v))
    -+		;
    -+	else if (!git_config_get_bool("feature.experimental", &v))
    -+	      ;
    -+
    -+	return v;
    -+}
    +-	/* Did we get --cached with a command? */
    ++	/* Did we get a forbidden top-level option with a command? */
    + 	if (cached)
    + 		usage_msg_optf(_("'%s' option is only supported with explicit 'status'"),
    + 			       git_submodule_usage, options, "--cached");
    ++	if (recursive)
    ++		usage_msg_optf(_("'%s' option is only supported with explicit 'status'"),
    ++			       git_submodule_usage, options, "--recursive");
    + 
    + 
    + 	/* Either a valid command, or submodule--helper will barf! */
    +@@ builtin/submodule.c: static void setup_helper_args(int argc, const char **argv, const char *prefix,
    + 	argc--;
    + 
    + 	/* Options that need to go before user-supplied options */
    ++	if (!strcmp(cmd, "status") && recursive)
    ++		strvec_push(args, "--recursive");
     +
    - int cmd_submodule(int argc, const char **argv, const char *prefix)
    + 	if (!strcmp(cmd, "absorbgitdirs"))
    + 		do_quiet_cache = 0;
    + 	else if (!strcmp(cmd, "update"))
    +@@ builtin/submodule.c: int cmd_submodule(int argc, const char **argv, const char *prefix)
      {
      	int opt_quiet = 0;
      	int opt_cached = 0;
    ++	int opt_recursive = 0;
      	struct child_process cp = CHILD_PROCESS_INIT;
    -+	struct strvec args = STRVEC_INIT;
      	struct option options[] = {
      		OPT__QUIET(&opt_quiet, N_("be quiet")),
      		OPT_BOOL(0, "cached", &opt_cached,
      			 N_("print the OID of submodules")),
    ++		OPT_BOOL(0, "recursive", &opt_recursive,
    ++			 N_("recurse into nested submodules")),
      		OPT_END()
      	};
    -+	const int use_builtin = get_use_builtin();
    - 	int ret;
      
    - 	argc = parse_options(argc, argv, prefix, options, git_submodule_usage,
     @@ builtin/submodule.c: int cmd_submodule(int argc, const char **argv, const char *prefix)
    - 	 * Tell the rest of git that any URLs we get don't come
    - 	 * directly from the user, so it can apply policy as appropriate.
      	 */
    --	strvec_push(&cp.env_array, "GIT_PROTOCOL_FROM_USER=0");
    -+	if (use_builtin)
    -+		xsetenv("GIT_PROTOCOL_FROM_USER", "0", 1);
    -+	else
    -+		strvec_push(&cp.env_array, "GIT_PROTOCOL_FROM_USER=0");
    -+
    + 	strvec_push(&cp.env, "GIT_PROTOCOL_FROM_USER=0");
      	setup_helper_args(argc, argv, prefix, opt_quiet, opt_cached,
    --			  &cp.args);
    -+			  use_builtin ? &args : &cp.args);
    -+
    -+	if (use_builtin) {
    -+		ret = cmd_submodule__helper(args.nr, args.v, prefix);
    -+		goto cleanup;
    -+	}
    +-			  &cp.args, options);
    ++			  opt_recursive, &cp.args, options);
      
      	cp.git_cmd = 1;
      	cp.no_stdin = 0; /* for git submodule foreach */
    - 	cp.dir = startup_info->original_cwd;
    - 	ret = run_command(&cp);
    - 
    -+cleanup:
    -+	if (!use_builtin)
    -+		/* TODO: Double free? */
    -+		strvec_clear(&args);
    -+
    - 	return ret;
    - }
    -
    - ## ci/run-build-and-tests.sh ##
    -@@ ci/run-build-and-tests.sh: linux-TEST-vars)
    - 	export GIT_TEST_MULTI_PACK_INDEX=1
    - 	export GIT_TEST_MULTI_PACK_INDEX_WRITE_BITMAP=1
    - 	export GIT_TEST_ADD_I_USE_BUILTIN=0
    -+	export GIT_TEST_SUBMODULE_USE_BUILTIN=1
    - 	export GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME=master
    - 	export GIT_TEST_WRITE_REV_INDEX=1
    - 	export GIT_TEST_CHECKOUT_WORKERS=2
     
    - ## t/README ##
    -@@ t/README: GIT_TEST_ADD_I_USE_BUILTIN=<boolean>, when false, disables the
    - built-in version of git add -i. See 'add.interactive.useBuiltin' in
    - git-config(1).
    + ## t/t7400-submodule-basic.sh ##
    +@@ t/t7400-submodule-basic.sh: test_expect_success 'submodule usage: -h' '
    + 	test_must_be_empty err
    + '
      
    -+GIT_TEST_SUBMODULE_USE_BUILTIN=<boolean>, when true, enables the
    -+built-in subprocess-less invocation of "git submodule--helper".
    -+See 'submodule.useBuiltin' in git-config(1).
    -+
    - GIT_TEST_INDEX_THREADS=<n> enables exercising the multi-threaded loading
    - of the index for the whole test suite by bypassing the default number of
    - cache entries and thread minimums. Setting this to 1 will make the
    +-test_expect_success 'submodule usage: --recursive' '
    +-	test_expect_code 129 git submodule --recursive
    +-'
    +-
    + test_expect_success 'submodule usage: status --' '
    + 	git submodule -- &&
    + 	git submodule --end-of-options
    +@@ t/t7400-submodule-basic.sh: do
    + 	'
    + done
    + 
    +-for opt in '--cached'
    ++for opt in '--cached' '--recursive'
    + do
    + 	test_expect_success "submodule usage: status $opt" '
    + 		git submodule $opt &&
 2:  8fcd832e58f ! 10:  81f138e460c git-submodule.sh: remove unused $prefix variable
    @@ Metadata
     Author: Ævar Arnfjörð Bjarmason <avarab@xxxxxxxxx>
     
      ## Commit message ##
    -    git-submodule.sh: remove unused $prefix variable
    +    submodule: don't use a subprocess to invoke "submodule--helper"
     
    -    Remove the $prefix variable which is never set, and never resulted in
    -    an option being passed to our "git submodule--helper". The
    -    --recursive-prefix option to "git submodule--helper" is still used,
    -    but only when it invokes itself.
    +    In a preceding commit we created "builtin/submodule.c" and faithfully
    +    tried to reproduce every aspect of "git-submodule.sh", including its
    +    invocation of "git submodule--helper" as a sub-process.
    +
    +    Let's do away with the sub-process and invoke
    +    "cmd_submodule__helper()" directly. Eventually we'll want to do away
    +    with "builtin/submodule--helper.c" altogether, but let's not do that
    +    for now to avoid conflicts with other in-flight topics. Even without
    +    those conflicts the resulting diff would be large. We can leave that
    +    for a later cleanup.
    +
    +    This speeds up invocations of all "git submodule" commands, E.g. a
    +    trivial "foreach" command on git.git is around 1.50 times
    +    faster[1]. For more expensive commands this'll make less of a
    +    difference, as the fixed cost of invoking the sub-process will be
    +    amortized away.
    +
    +            $ git hyperfine -L rev HEAD~1,HEAD -s 'make CFLAGS=-O3' './git submodule foreach "echo \$name"'
    +            Benchmark 1: ./git submodule foreach "echo \$name"' in 'HEAD~1
    +              Time (mean ± σ):       9.7 ms ±   0.1 ms    [User: 7.6 ms, System: 2.1 ms]
    +              Range (min … max):     9.4 ms …  10.2 ms    285 runs
    +
    +            Benchmark 2: ./git submodule foreach "echo \$name"' in 'HEAD
    +              Time (mean ± σ):       6.6 ms ±   0.1 ms    [User: 5.1 ms, System: 1.5 ms]
    +              Range (min … max):     6.2 ms …   7.2 ms    414 runs
    +
    +            Summary
    +              './git submodule foreach "echo \$name"' in 'HEAD' ran
    +                1.48 ± 0.04 times faster than './git submodule foreach "echo \$name"' in 'HEAD~1'
    +
    +    It's also worth noting that some users were using e.g. "git
    +    submodule--helper list" directly for performance reasons[2]. With
    +    31955475d1c (submodule--helper: remove unused "list" helper,
    +    2022-09-01) released with v2.38.0 the "list" command was no longer
    +    provided. Users who had to switch to "git submodule--helper foreach"
    +    were given a command that (on my system) is around 6.5x slower.
    +
    +    Now the "foreach" is around 0.10x slower (due to the slight shell
    +    overhead), with 31955475d1c reverted on top of this:
    +
    +            $ hyperfine './git submodule--helper list' './git submodule foreach --quiet "echo \$name"' --warmup 10
    +            Benchmark 1: ./git submodule--helper list
    +              Time (mean ± σ):       6.4 ms ±   0.1 ms    [User: 5.0 ms, System: 1.5 ms]
    +              Range (min … max):     6.2 ms …   7.2 ms    427 runs
    +
    +            Benchmark 2: ./git submodule foreach --quiet "echo \$name"
    +              Time (mean ± σ):       7.0 ms ±   0.1 ms    [User: 4.8 ms, System: 2.3 ms]
    +              Range (min … max):     6.8 ms …   7.4 ms    390 runs
    +
    +            Summary
    +              './git submodule--helper list' ran
    +                1.10 ± 0.03 times faster than './git submodule foreach --quiet "echo \$name"'
    +
    +    I think it would make sense to implement a "--format" option for "git
    +    submodule foreach" to help anyone who cares about that remaining
    +    performance (and to improve the API, e.g. by supporting "-z"), but as
    +    far as performance goes this makes the runtime acceptable again.
    +
    +    The pattern in "cmd_submodule_builtin()" of saving "struct strvec"
    +    arguments to a "struct string_list" and free()-ing them after the
    +    "argv" has been modified by "cmd_submodule__helper()" is new, without
    +    it we'd get various already-passing tests failing under SANITIZE=leak.
    +
    +    1. Using the "git hyperfine" wrapper for "hyperfine":
    +       https://lore.kernel.org/git/211201.86r1aw9gbd.gmgdl@xxxxxxxxxxxxxxxxxxx/
    +    2. https://lore.kernel.org/git/87czatrpyb.fsf@xxxxxxxxxx/
     
         Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@xxxxxxxxx>
     
    - ## git-submodule.sh ##
    -@@ git-submodule.sh: files=
    - remote=
    - nofetch=
    - update=
    --prefix=
    - custom_name=
    - depth=
    - progress=
    -@@ git-submodule.sh: cmd_add()
    - 		usage
    - 	fi
    - 
    --	git ${wt_prefix:+-C "$wt_prefix"} ${prefix:+--super-prefix "$prefix"} submodule--helper add ${GIT_QUIET:+--quiet} ${force:+--force} ${progress:+"--progress"} ${branch:+--branch "$branch"} ${reference_path:+--reference "$reference_path"} ${dissociate:+--dissociate} ${custom_name:+--name "$custom_name"} ${depth:+"$depth"} -- "$@"
    -+	git ${wt_prefix:+-C "$wt_prefix"} submodule--helper add ${GIT_QUIET:+--quiet} ${force:+--force} ${progress:+"--progress"} ${branch:+--branch "$branch"} ${reference_path:+--reference "$reference_path"} ${dissociate:+--dissociate} ${custom_name:+--name "$custom_name"} ${depth:+"$depth"} -- "$@"
    + ## builtin/submodule.c ##
    +@@ builtin/submodule.c: static void setup_helper_args(int argc, const char **argv, const char *prefix,
    + 	strvec_pushv(args, argv);
      }
      
    - #
    -@@ git-submodule.sh: cmd_init()
    - 		shift
    - 	done
    ++static int cmd_submodule_builtin(struct strvec *args, const char *prefix)
    ++{
    ++	size_t i;
    ++	struct string_list to_free = STRING_LIST_INIT_DUP;
    ++	int ret;
    ++
    ++	/*
    ++	 * The cmd_submodule__helper() will treat the argv as
    ++	 * its own and modify it, so e.g. for "git submodule
    ++	 * add" the "add" argument will be removed, and we'll
    ++	 * thus leak from the strvec_push()'s in
    ++	 * setup_helper_args().
    ++	 *
    ++	 * So in lieu of some generic "snapshot for a free"
    ++	 * API for "struct strvec" squirrel away the pointers
    ++	 * to free with string_list_clear() later.
    ++	 */
    ++	for (i = 0; i < args->nr; i++)
    ++		string_list_append_nodup(&to_free, (char *)args->v[i]);
    ++
    ++	ret = cmd_submodule__helper(args->nr, args->v, prefix);
    ++
    ++	string_list_clear(&to_free, 0);
    ++	free(strvec_detach(args));
    ++
    ++	return ret;
    ++}
    ++
    + int cmd_submodule(int argc, const char **argv, const char *prefix)
    + {
    + 	int opt_quiet = 0;
    + 	int opt_cached = 0;
    + 	int opt_recursive = 0;
    +-	struct child_process cp = CHILD_PROCESS_INIT;
    ++	struct strvec args = STRVEC_INIT;
    + 	struct option options[] = {
    + 		OPT__QUIET(&opt_quiet, N_("be quiet")),
    + 		OPT_BOOL(0, "cached", &opt_cached,
    +@@ builtin/submodule.c: int cmd_submodule(int argc, const char **argv, const char *prefix)
    + 	 * Tell the rest of git that any URLs we get don't come
    + 	 * directly from the user, so it can apply policy as appropriate.
    + 	 */
    +-	strvec_push(&cp.env, "GIT_PROTOCOL_FROM_USER=0");
    +-	setup_helper_args(argc, argv, prefix, opt_quiet, opt_cached,
    +-			  opt_recursive, &cp.args, options);
    ++	xsetenv("GIT_PROTOCOL_FROM_USER", "0", 1);
      
    --	git ${wt_prefix:+-C "$wt_prefix"} ${prefix:+--super-prefix "$prefix"} submodule--helper init ${GIT_QUIET:+--quiet} -- "$@"
    -+	git ${wt_prefix:+-C "$wt_prefix"} submodule--helper init ${GIT_QUIET:+--quiet} -- "$@"
    - }
    +-	cp.git_cmd = 1;
    +-	cp.no_stdin = 0; /* for git submodule foreach */
    +-	cp.dir = startup_info->original_cwd;
    ++	setup_helper_args(argc, argv, prefix, opt_quiet, opt_cached,
    ++			  opt_recursive, &args, options);
      
    - #
    -@@ git-submodule.sh: cmd_update()
    - 		${init:+--init} \
    - 		${nofetch:+--no-fetch} \
    - 		${wt_prefix:+--prefix "$wt_prefix"} \
    --		${prefix:+--recursive-prefix "$prefix"} \
    - 		${update:+--update "$update"} \
    - 		${reference:+"$reference"} \
    - 		${dissociate:+"--dissociate"} \
    +-	return run_command(&cp);
    ++	return cmd_submodule_builtin(&args, prefix);
    + }
 5:  124c062e3a1 <  -:  ----------- git-submodule.sh: normalize parsing of --cached
 6:  b1ca1183885 <  -:  ----------- submodule--helper: rename "absorb-git-dirs" to "absorbgitdirs"
 8:  0c1a5063653 <  -:  ----------- submodule--helper: pretend to be "git submodule" in "-h" output
12:  57b9df29ea6 <  -:  ----------- submodule--helper: have --require-init imply --init
13:  20db979a094 <  -:  ----------- submodule--helper: understand --checkout, --merge and --rebase synonyms
14:  1cb40a5f42e <  -:  ----------- git-submodule doc: document the -v" option to "update"
15:  0c388eed1d1 <  -:  ----------- submodule--helper: understand -v option for "update"
17:  59a72296967 <  -:  ----------- git-submodule.sh: use "$quiet", not "$GIT_QUIET"
18:  c5796878f0b <  -:  ----------- git-submodule.sh: simplify parsing loop
-- 
2.38.0.1091.gf9d18265e59




[Index of Archives]     [Linux Kernel Development]     [Gcc Help]     [IETF Annouce]     [DCCP]     [Netdev]     [Networking]     [Security]     [V4L]     [Bugtraq]     [Yosemite]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux SCSI]     [Fedora Users]

  Powered by Linux