git/list[1] front-page[2] threads[3] people[4] search[5] about
 

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
Previous: Jens LehmannNext: W. Trevor King
Message 15 of 29 in “Possible regression in master? (submodules without a "master" branch)”
  1. Johan HerlandMar 27, 2014
  2. W. Trevor KingMar 27, 2014
  3. W. Trevor KingMar 27, 2014
  4. Jens LehmannMar 27, 2014
  5. Junio C HamanoMar 27, 2014
  6. Jens LehmannMar 27, 2014
  7. Johan HerlandMar 27, 2014
  8. W. Trevor KingMar 28, 2014
  9. Junio C HamanoMar 27, 2014
  10. Jens LehmannMar 27, 2014
  11. W. Trevor KingMar 27, 2014
  12. Junio C HamanoMar 27, 2014
  13. Heiko VoigtMar 27, 2014
  14. Jens LehmannMar 27, 2014
  15. Johan HerlandMar 27, 2014
  16. W. Trevor KingMar 28, 2014
  17. [RFC] submodule: change submodule.<name>.branch default from master to HEADW. Trevor King, Mar 28, 2014
  18. Eric SunshineMar 28, 2014
  19. W. Trevor KingMar 28, 2014
  20. W. Trevor KingMar 28, 2014
  21. Jens LehmannMar 28, 2014
  22. W. Trevor KingMar 28, 2014
  23. Jens LehmannMar 31, 2014
  24. Junio C HamanoMar 28, 2014
  25. Jens LehmannMar 31, 2014
  26. W. Trevor KingMar 31, 2014
  27. Junio C HamanoMar 31, 2014
  28. submodule.<path>.branch vs. submodule.<name>.branch (was: Possible regression in master? (submodules without a "master" branch).W. Trevor King, Mar 27, 2014
  29. Junio C HamanoMar 27, 2014

Read the whole thread, see it on lore, or plain text.

$ cat FOOTERMessages come from the public archive at lore.kernel.org/git, fetched every hour. The front page is chosen and written each morning by an AI editor and can be wrong; the threads themselves are the record. About and API. For agents: an MCP server at https://gitlist.dev/mcp, and any thread, story or person page as Markdown by adding .md to its URL (or sending Accept: text/markdown). Details in /llms.txt.