Re: Possible regression in master? (submodules without a "master" branch)

From: Junio C Hamano <gitster@pobox.com>
Date: Mar 27, 2014, 19:39 UTC
Message-ID: <xmqqwqffctyg.fsf@gitster.dls.corp.google.com>
In-Reply-To: <20140327185405.GS4008@odin.tremily.us>

"W. Trevor King" <wking@tremily.us> writes:

Show 25 quoted lines

> 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.
>
> 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?

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?

Show 10 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?

> 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.

Show 9 quoted lines

>> > If an existing set-up that was working in a sensible way is broken
>> > by a change that assumes something that should not be assumed,
>> > then that is a serious regression, I would have to say.
>> 
>> Yes, especially as it promised to not change this use case.
>
> Sorry.  A side effect of relying too much on our existing
> documentation and not enough on testing actual use cases.  I can work
> up some non-master submodule tests to go with the fix.

I was wondering if we need to revert the merge with that branch out of 'master', or submodule folks can work on a set of fixes to apply on top.

Will wait to see how it goes.  Thanks.