Re: [PATCH] submodule update: run custom update script for initial populating as well

[Junio C Hamano <gitster@pobox.com>]

Stefan Beller <sbeller@google.com> writes:

> In 1b4735d9f3 (submodule: no [--merge|--rebase] when newly cloned,
> 2011-02-17), all actions were defaulted to checkout for populating
> a submodule initially, because merging or rebasing makes no sense
> in that situation.
>
> Other commands however do make sense, such as the custom command
> that was added later (6cb5728c43, submodule update: allow custom
> command to update submodule working tree, 2013-07-03).

Makes sense.

> I am unsure about the "none" command, as I can see an initial
> checkout there as a useful thing. On the other hand going strictly
> by our own documentation, we should do nothing in case of "none"
> as well, because the user asked for it.

I think "none" is "I'll decide which revision of the submodule
should be there---do not decide it for me".  If the user is
explicitly saying with "git submodule init" to have "some" version,
and if the user did not have any (because the user didn't show
interest in any checkout of the submodule before), then I think it
probably makes more sense to checkout the version bound to the
superproject, than leaving the directory empty.

> Reported-by: Han-Wen Nienhuys <hanwen@google.com>
> Signed-off-by: Stefan Beller <sbeller@google.com>
> ---
>  git-submodule.sh            |  7 ++++++-
>  t/t7406-submodule-update.sh | 15 +++++++++++++++
>  2 files changed, 21 insertions(+), 1 deletion(-)
>
> diff --git a/git-submodule.sh b/git-submodule.sh
> index 554bd1c494..aeb721ab7e 100755
> --- a/git-submodule.sh
> +++ b/git-submodule.sh
> @@ -606,7 +606,12 @@ cmd_update()
>  		if test $just_cloned -eq 1
>  		then
>  			subsha1=
> -			update_module=checkout
> +			if test "$update_module" = "merge" ||
> +			   test "$update_module" = "rebase" ||
> +			   test "$update_module" = "none"
> +			then
> +				update_module=checkout
> +			fi

... which seems to be what you did.  Do we need a documentation
update, or does this just make the behaviour of this corner case
consistent with what is already documented?

Thanks.