Re: Re: Possible regression in master? (submodules without a "master" branch)
- From
Johan Herland <johan@herland.net>
- Date
- Mar 27, 2014, 23:21 UTC
- Message-ID
- <CALKQrgdAhe0AKk51=__fDWX9QqFH=WVCO=gU+mtO6sb250==0A@mail.gmail.com>
- In-Reply-To
- <20140327202702.GA3984@sandbox-ub>
(Thanks to all of you for picking this up and more or less resolving it while I was away from email for a few hours...)
On Thu, Mar 27, 2014 at 9:27 PM, Heiko Voigt <hvoigt@hvoigt.net> wrote:
Show 22 quoted lines
> On Thu, Mar 27, 2014 at 12:39:03PM -0700, Junio C Hamano wrote: >> "W. Trevor King" <wking@tremily.us> writes: >> > On Thu, Mar 27, 2014 at 06:31:27PM +0100, Jens Lehmann wrote: >> >> Am 27.03.2014 18:16, schrieb Junio C Hamano: >> >> > Johan Herland <johan@herland.net> writes: >> >> >> I just found a failure to checkout a project with submodules where >> >> >> there is no explicit submodule branch configuration, and the >> >> >> submodules happen to not have a "master" branch: >> >> >> >> >> >> git clone git://gitorious.org/qt/qt5.git qt5 >> >> >> cd qt5 >> >> >> git submodule init qtbase >> >> >> git submodule update >> >> >> >> >> >> In current master, the last command fails with the following output: >> >> > >> >> > ... and with a bug-free system, what does it do instead? Just clone >> >> > 'qtbase' and make a detached-head checkout at the commit recorded in >> >> > the superproject's tree, or something else? >> >> >> >> After reverting 23d25e48f5ead73 on current master it clones 'qtbase' >> >> nicely with a detached HEAD.
...which is exactly the behaviour I (and the Qt project - I assume) expected.
Show 24 quoted lines
>> > Fixing this for initial update clone is pretty easy, we just need to >> > unset start_point before calling module_clone if >> > submodule.<name>.branch is not set. >> >> There is this bit for "update" in git-submodule.txt: >> >> For updates that clone missing submodules, checkout-mode updates >> will create submodules with detached HEADs; all other modes will >> create submodules with a local branch named after >> submodule.<path>.branch. >> >> [side note] Isn't that a typo of submodule.<name>.branch? > > Yep, thats is a typo. Trevor will you fix that as well? Or how should be > do that? Since its just such a small change. > >> So the proposed change is to make the part before semicolon true? >> If we are not newly cloning (because we already have it), if the >> submodule.<name>.branch is not set *OR* refers to a branch that does >> not even exist, shouldn't we either (1) abort as an error, or (2) do >> the same and detach? > > I would expect "(1) abort as an error" since the user is not getting what > he would expect.
FWIW, here is the behaviour I would expect from "git submodule update":
- In checkout-mode, if submodule.<name>.branch is not set, we should _always_ detach. Whether or not the submodule is already cloned does not matter.
- In rebase/merge-mode, if submodule.<name>.branch is not set, we should _always_ abort with an error.
- If submodule.<name>.branch is set - but the branch it refers to does not exist - we should _always_ abort with an error. The current checkout/rebase/merge-mode does not matter.
In other words, submodule.<name>.branch is _necessary_ in rebase/merge mode, but _optional_ in checkout-mode (its absence indicating that we should detach).
Show 20 quoted lines
>> > However, that's just going to
>> > push remote branch ambiguity problems back to the --remote update
>> > functionality. What should happen when submodule.<name>.branch is not
>> > set and you run a --remote update, which has used:
>> >
>> > git rev-parse "${remote_name}/${branch}"
>> >
>> > since the submodule.<name>.branch setting was introduced in 06b1abb
>> > (submodule update: add --remote for submodule's upstream changes,
>> > 2012-12-19)?
>>
>> Isn't --remote about following one specific branch the user who
>> issues that command has in mind? If you as the end user did not
>> give any indication which branch you meant, e.g. by leaving the
>> submodule.<name>.branch empty, shouldn't that be diagnosed as an
>> error?
>
> Well to simplify things there was this fallback to origin/master
> (similar to the master branch we create on init) since that is a branch
> which many projects have.I think the analogy to "the master branch we create on init" is false. A better analogy is running "git pull" or "git pull -rebase" in a branch where branch.<name>.merge has not yet been set. And this currently fails with "Please specify which branch you want to merge with." So I would be inclined to agree with Junio here: We should error out.
Show 5 quoted lines
> E.g. for the users that share one central > server and just directly commit, push and pull to/from master. They > would have an easy way to start working in a submodule, by simply saying > --remote and then committing to master. At least that is what I > imagine.
If there are compelling arguments for providing a default fallback (and I'm not sure the above argument is enough), I say we should rather follow clone's lead, and use the submodule's upstream's HEAD, instead of blindly assuming "origin/master" to be present. I expect in most cases where "origin/master" happens to be the Right Answer, using the submodule's upstream's HEAD will yield the same result.
Show 12 quoted lines
>> > gitmodules(5) is pretty clear that 'submodule.<name>.branch' defaults >> > to master (and not upstream's HEAD), do we want to adjust this at the >> > same time? >> >> That may be likely. If the value set to a configuration variable >> causes an established behaviour of a program change a lot, silently >> defaulting that variable to something many people are expected to >> have (e.g. 'master') would likely to cause a usability regression. > > IMO this branch configuration should completely ignored in the default, > non --remote, usage. Since we simply checkout a specific SHA1 in this > case, that should be possible.
Yes. Checkout-mode with no submodule.<name>.branch configured should always detach.
...Johan
-- Johan Herland, <johan@herland.net> www.herland.net