Re: [PATCH v3 1/3] submodule: use "fetch" logic instead of custom remote discovery

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

Ævar Arnfjörð Bjarmason  <avarab@gmail.com> writes:

> Replace a use of the get_default_remote() function with an invocation
> of "git fetch"
>
> The "fetch" command already has logic to discover the remote for the
> current branch. However, before it learned to accept a custom
> refspec *and* use its idea of the default remote, it wasn't possible
> to get rid of some equivalent of the "get_default_remote" invocation
> here.
>
> As it turns out the recently added "--stdin" option to fetch[1] gives
> us a way to do that. Let's use it instead.

Ah, that was what your earlier question was about.

The hits from "git grep fetch_in_submodule" says that $1 is the path
to the submodule, $2 is the $depth and $3 (when exists) is the
object name of what we are fetching:

git-submodule.sh:578:	fetch_in_submodule "$sm_path" $depth ||
git-submodule.sh:601:	fetch_in_submodule "$sm_path" $depth ||
git-submodule.sh:607:	fetch_in_submodule "$sm_path" $depth "$sha1" ||

> While I'm at it simplify the "fetch_in_submodule" function. It wasn't
> necessary to pass "$@" to "fetch" since we'd only ever provide one
> SHA-1 as an argument in the previous "*" codepath (in addition to
> "--depth=N"). Rewrite the function to more narrowly reflect its
> use-case.

>  	cd "$1" &&
> -	case "$2" in
> -	'')
> -		git fetch ;;
> -	*)
> -		shift
> -		git fetch $(get_default_remote) "$@" ;;
> -	esac

So when $2 (i.e. --depth=N) is empty, we just did the default "git
fetch".  Otherwise, "git fetch <remote> --depth=N [<sha1>]" was run.

> +	if test $# -eq 3
> +	then
> +		echo "$3" | git fetch --stdin "$2"

This is not quite equivalent in that an empty depth would have
resulted in the default "git fetch" in the original code, with or
without "$3", totally ignoring "$sha1".  Is this a bugfix snuck in
"while we are at it", or is this an unintended regression?

> +	elif test "$2" -ne ""
> +	then
> +		git fetch "$2"
> +	else
> +		git fetch
> +	fi

Even if it is an unintended regression, fixing it should be trivial;
we just need to flip the order of conditions, like so:

	if test -z "$2"
	then
		git fetch
	elif test $# = 3
	then
		echo "$3" | git fetch --stdin "$2"
	else
		git fetch "$2"
	fi