Stefan Beller wrote:
> Subject: submodule.sh update --remote: default to oid instead of master
Yay!
Nit: it wasn't clear to me at first what default this subject line was
referring to. Perhaps:
submodule update --remote: skip GITLINK update when no branch is set
[...]
> --- a/Documentation/gitmodules.txt
> +++ b/Documentation/gitmodules.txt
> @@ -50,11 +50,12 @@ submodule.<name>.update::
>
> submodule.<name>.branch::
[...]
> + If the option is not specified, do not update to any branch but
> + the object id of the remote.
Likewise: how about something like
If not set, the default is for `git submodule update --remote`
to update the submodule to the superproject's recorded SHA-1.
[...]
> --- a/git-submodule.sh
> +++ b/git-submodule.sh
> @@ -568,16 +568,19 @@ cmd_update()
> if test -n "$remote"
> then
> branch=$(git submodule--helper remote-branch "$sm_path")
> - if test -z "$nofetch"
> + if test -n "$branch"
> then
> - # Fetch remote before determining tracking $sha1
> - fetch_in_submodule "$sm_path" $depth ||
> - die "$(eval_gettext "Unable to fetch in
> submodule path '\$sm_path'")"
> + if test -z "$nofetch"
> + then
> + # Fetch remote before determining
> tracking $sha1
> + fetch_in_submodule "$sm_path" $depth ||
Makes sense. If $sha1 isn't available in the submodule, it will fetch
again later.
[...]
> --- a/t/t7406-submodule-update.sh
> +++ b/t/t7406-submodule-update.sh
> @@ -260,6 +260,28 @@ test_expect_success 'submodule update --remote should
> fetch upstream changes wit
> )
> '
>
> +test_expect_success 'submodule update --remote should not fetch upstream
> when no branch is set' '
> + (
> + cd super &&
> + test_might_fail git config --unset -f .gitmodules
> submodule."submodule".branch &&
Not about this patch: the quoting here is strange.
> + git add .gitmodules &&
> + git commit --allow-empty -m "submodules: pin in superproject
> branch"
> + ) &&
I wonder if we can do simpler by using -C + some helpers: something like
git config --unset -f super/.gitmodules ... &&
test_commit -C submodule ... &&
git -C super submodule update ... &&
test_cmp_rev ...
Unfortunately test_cmp_rev doesn't accept a -C argument.
Broader comment: do you think people will be surprised by this new
behavior? Is there anything special we'd need to do to call it out
(e.g., print a warning or put something in release notes)?
Thanks,
Jonathan
