Hi Peff,
On Tue, 16 Jun 2020, Jeff King wrote:
> On Mon, Jun 15, 2020 at 12:50:15PM +0000, Johannes Schindelin via GitGitGadget wrote:
>
> > From: Johannes Schindelin <johannes.schindelin@xxxxxx>
> >
> > To allow for overriding the default branch name, we have introduced a
> > config setting. With this patch, the `git submodule` command learns
> > about this, too.
>
> This was the other reading case (besides guess_remote_head()) that I'm
> most concerned with causing regressions in a world where some repos are
> on "master" and some are on "main".
>
> This value ends up as the output of "submodule--helper remote-branch".
>
> I was initially worried that we used this branch name for the fallback
> when the server doesn't allow us to fetch the sha1 directly, but it
> doesn't look like it. That's good, because handling fallbacks there
> would be tricky.
>
> Instead, we seem to use this only after fetching all of the refs for a
> submodule:
>
> $ git grep -h -B2 -A11 remote-branch git-submodule.sh
> if test -n "$remote"
> then
> branch=$(git submodule--helper remote-branch "$sm_path")
> if test -z "$nofetch"
> then
> # Fetch remote before determining tracking $sha1
> fetch_in_submodule "$sm_path" $depth ||
> die "$(eval_gettext "Unable to fetch in submodule path '\$sm_path'")"
> fi
> remote_name=$(sanitize_submodule_env; cd "$sm_path" && get_default_remote)
> sha1=$(sanitize_submodule_env; cd "$sm_path" &&
> git rev-parse --verify "${remote_name}/${branch}") ||
> die "$(eval_gettext "Unable to find current \${remote_name}/\${branch} revision in submodule path '\$sm_path'")"
> fi
>
> and then we just use that branch name to resolve a sha1. So this will
> break cases where you've set init.mainBranch, the submodule repo is
> still on "master", and you haven't configured a branch in .gitmodules.
>
> It seems like, independent of any change in the default branch names, we
> ought to be using $remote_name/HEAD for this case anyway. I suspect that
> would be a behavior improvement by itself, as it means more cases could
> avoid having to specify the branch name in .gitmodules manually.
> Probably nobody noticed so far because "HEAD" is almost always "master"
> in the current world. It technically breaks the case that you truly did
> want to use "master" in the submodule, but they set HEAD to something
> else, and you couldn't be bothered to put it into your .gitmodules file.
> That seems rather unlikely to me.
>
> And then everything would Just Work without having to worry about the
> local mainbranch value at all.
This is the route that I am taking.
Please note that t7519 contains a few test cases that rely on the current
confusing behavior where `git submodule update --remote` fetches the
remote `master` even if that is not the remote repository's current
branch!
I did adjust t7519 to stop verifying this confusing behavior, and to
verify the saner behavior instead.
This is of course a bit worrisome, as there might actually be users out
there relying on the confusing behavior.
However, I think it is okay to fix this:
- The `git submodule update --remote` command does not strike me as
awfully common. In fact, I had never heard of it before I worked on this
here patch.
- Current Git's behavior when running this command is outright confusing,
unless the remote repository's current branch _is_ `master` (in which
case the proposed behavior matches the old behavior).
- It is actually easily fixed by setting `submodule.<name>.branch` to
`master` _iff_ users want to reinstate the old behavior.
> Alternatively, submodule--helper could pass back the empty string for
> "no, we don't have a configured branch name" and this shell code could
> actually try a sequence of reasonable guesses: init.mainbranch, then
> "master" (and between the two, "main" if that later becomes the
> default).
Quite honestly: I'd rather not.
Thank you,
Dscho
