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

[Glen Choo <chooglen@google.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.

> -	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 \
> -		-- \
> -		"$@"
> -}

[...]

> -
> -# 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.

>  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