(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@xxxxxxxxxx> wrote:
> On Thu, Mar 27, 2014 at 12:39:03PM -0700, Junio C Hamano wrote:
>> "W. Trevor King" <wking@xxxxxxxxxx> 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@xxxxxxxxxxx> 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.
>> > 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).
>> > 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.
> 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.
>> > 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@xxxxxxxxxxx>
www.herland.net
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html