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
