git/list[1] front-page[2] threads[3] people[4] search[5] about
 

Re: [PATCH 05/10] git-submodule.sh: dispatch "update" to helper

From
Glen Choo <chooglen@google.com>
Date
Oct 20, 2022, 21:50 UTC
Message-ID
<kl6lv8oexiyy.fsf@chooglen-macbookpro.roam.corp.google.com>
In-Reply-To
<patch-05.10-7d9c13eb637-20221017T115544Z-avarab@gmail.com>
Ævar Arnfjörð Bjarmason <avarab@gmail.com> writes:
> As noted in a preceding commit the only behavior change here should be
> the desirable change of better "-h" output, and that this
> implementation understands the "--verbose" synonym for "-v". Let's
> update the documentation to reflect the new "--verbose" synonym.
Hm, I didn't see this change in the patch.
Show 24 quoted lines
> -	git ${wt_prefix:+-C "$wt_prefix"} submodule--helper update \
> -		${quiet:+--quiet} \
> -		${force:+--force} \
> -		${progress:+"--progress"} \
> -		${remote:+--remote} \
> -		${recursive:+--recursive} \
> -		${init:+--init} \
> -		${nofetch:+--no-fetch} \
> -		${wt_prefix:+--prefix "$wt_prefix"} \
> -		${rebase:+--rebase} \
> -		${merge:+--merge} \
> -		${checkout:+--checkout} \
> -		${reference:+"$reference"} \
> -		${dissociate:+"--dissociate"} \
> -		${depth:+"$depth"} \
> -		${require_init:+--require-init} \
> -		${dissociate:+"--dissociate"} \
> -		$single_branch \
> -		$recommend_shallow \
> -		$jobs \
> -		$filter \
> -		-- \
> -		"$@"
> -}
[...]
Show 6 quoted lines
> -
> -# This loop parses the command line arguments to find the
> -# subcommand name to dispatch.  Parsing of the subcommand specific
> -# options are primarily done by the subcommand implementations.
> -# Subcommand specific options such as --branch and --cached are
> -# parsed here as well, for backward compatibility.
This comment still seems relevant as of this patch.
Show 12 quoted lines
>  while test $# != 0 && test -z "$command"
>  do
> @@ -233,7 +80,8 @@ absorbgitdirs)
>  	git submodule--helper "$command" --prefix "$wt_prefix" "$@"
>  	;;
>  update)
> -	cmd_update "$@"
> +	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" \

I haven't read ahead to see whether this was fixed, but it looks like the reason we couldn't combine "update" into this arms is that "update" consumes "wt_prefix" twice.

There's no good reason to have "--prefix" at all actually. "git submodule update" used to use that instead of -C, and we could have removed it once we passed -C in 29a5e9e1ff (submodule--helper update-clone: learn --init, 2022-03-04). That simplification got lost in the big shell -> C conversion, but we could do it quite easily right now, e.g.

  diff --git a/builtin/submodule--helper.c b/builtin/submodule--helper.c
  index d11e100301..a4f59e91c5 100644
  --- a/builtin/submodule--helper.c
  +++ b/builtin/submodule--helper.c
  @@ -2636,9 +2636,6 @@ static int module_update(int argc, const char **argv, const char *prefix)
        N_("traverse submodules recursively")),
      OPT_BOOL('N', "no-fetch", &opt.nofetch,
        N_("don't fetch new objects from the remote site")),
  -		OPT_STRING(0, "prefix", &opt.prefix,
  -			   N_("path"),
  -			   N_("path into the working tree")),
      OPT_SET_INT(0, "checkout", &opt.update_default,
        N_("use the 'checkout' update strategy (default)"),
        SM_UPDATE_CHECKOUT),
  @@ -2694,6 +2691,7 @@ static int module_update(int argc, const char **argv, const char *prefix)
    }
    opt.filter_options = &filter_options;
  +	opt.prefix = prefix;
    if (opt.update_default)
      opt.update_strategy.type = opt.update_default;
  diff --git a/git-submodule.sh b/git-submodule.sh
  index ac2f95c128..2787aaa60c 100755
  --- a/git-submodule.sh
  +++ b/git-submodule.sh
  @@ -79,11 +79,8 @@ case "$command" in
  absorbgitdirs)
    git submodule--helper "$command" --prefix "$wt_prefix" "$@"
    ;;
  -update)
  -	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)
  +
  +add | foreach | init | deinit | set-branch | set-url | status | summary | sync | update)
    git ${wt_prefix:+-C "$wt_prefix"} submodule--helper "$command" \
      ${quiet:+--quiet} ${cached:+--cached} "$@"
    ;;
> -- 
> 2.38.0.1091.gf9d18265e59
Previous: Ævar Arnfjörð BjarmasonNext: Ævar Arnfjörð Bjarmason
Message 11 of 18 in “submodule: make it a built-in, remove git-submodule.sh”
  1. 00/10 submodule: make it a built-in, remove git-submodule.shÆvar Arnfjörð Bjarmason, Oct 17, 2022
  2. 01/10 git-submodule.sh: create a "case" dispatch statementÆvar Arnfjörð Bjarmason, Oct 17, 2022
  3. 02/10 git-submodule.sh: dispatch "sync" to helperÆvar Arnfjörð Bjarmason, Oct 17, 2022
  4. Glen ChooOct 20, 2022
  5. 04/10 git-submodule.sh: dispatch "foreach" to helperÆvar Arnfjörð Bjarmason, Oct 17, 2022
  6. Glen ChooOct 20, 2022
  7. 03/10 git-submodule.sh: dispatch directly to helperÆvar Arnfjörð Bjarmason, Oct 17, 2022
  8. 06/10 git-submodule.sh: don't support top-level "--cached"Ævar Arnfjörð Bjarmason, Oct 17, 2022
  9. Glen ChooOct 20, 2022
  10. 05/10 git-submodule.sh: dispatch "update" to helperÆvar Arnfjörð Bjarmason, Oct 17, 2022
  11. Glen ChooOct 20, 2022
  12. 08/10 submodule: support "--" with no other argumentsÆvar Arnfjörð Bjarmason, Oct 17, 2022
  13. 09/10 submodule: support sub-command-less "--recursive" optionÆvar Arnfjörð Bjarmason, Oct 17, 2022
  14. Glen ChooOct 20, 2022
  15. 07/10 submodule: make it a built-in, remove git-submodule.shÆvar Arnfjörð Bjarmason, Oct 17, 2022
  16. Glen ChooOct 20, 2022
  17. 10/10 submodule: don't use a subprocess to invoke "submodule--helper"Ævar Arnfjörð Bjarmason, Oct 17, 2022
  18. Glen ChooOct 20, 2022

Read the whole thread, see it on lore, or plain text.

$ cat FOOTERMessages come from the public archive at lore.kernel.org/git, fetched every hour. The front page is chosen and written each morning by an AI editor and can be wrong; the threads themselves are the record. About and API. For agents: an MCP server at https://gitlist.dev/mcp, and any thread, story or person page as Markdown by adding .md to its URL (or sending Accept: text/markdown). Details in /llms.txt.