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

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

[Johan Herland]

Hi,

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:

  Cloning into 'qtbase'...
  remote: Counting objects: 267400, done.
  remote: Compressing objects: 100% (61070/61070), done.
  remote: Total 267400 (delta 210431), reused 258876 (delta 202642)
  Receiving objects: 100% (267400/267400), 136.23 MiB | 6.73 MiB/s, done.
  Resolving deltas: 100% (210431/210431), done.
  Checking connectivity... done.
  error: pathspec 'origin/master' did not match any file(s) known to git.
  Unable to setup cloned submodule 'qtbase'

Bisection points to 23d25e48f5ead73c9ce233986f90791abec9f1e8 (W.
Trevor King: submodule: explicit local branch creation in
module_clone). Looking at the patch, it seems to introduce an implicit
assumption on the submodule origin having a "master" branch. Is this
an intended change in behaviour?

...Johan

-- 
Johan Herland, <johan@herland.net>
www.herland.net

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

[W. Trevor King]

[-- Attachment #1: Type: text/plain, Size: 892 bytes --]

On Thu, Mar 27, 2014 at 03:21:49PM +0100, Johan Herland wrote:
> 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:

The docs say [1]:

  A remote branch name for tracking updates in the upstream submodule.
  If the option is not specified, it defaults to 'master'.

which is what we do now.  Working around that to default to the
upstream submodule's HEAD is possible (you can just use --branch
HEAD), but I think it's easier to just explicitly specify your
preferred branch.

Cheers,
Trevor

[1]: submodule.<name>.branch in gitmodules(5)
     http://git-scm.com/docs/gitmodules.html

-- 
This email may be signed or encrypted with GnuPG (http://www.gnupg.org).
For more information, see http://en.wikipedia.org/wiki/Pretty_Good_Privacy

[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

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

[W. Trevor King]

[-- Attachment #1: Type: text/plain, Size: 955 bytes --]

On Thu, Mar 27, 2014 at 08:52:08AM -0700, W. Trevor King wrote:
> Working around that to default to the upstream submodule's HEAD is
> possible (you can just use --branch HEAD)

Actually, this is probably not a good idea.  The initial submodule
addition works:

  $ git submodule add -b HEAD /tmp/submod.git submod
  Cloning into 'submod'...
  done.

But subsequent log calls (from the superproject) do not:

  $ git log
  fatal: bad default revision 'HEAD'
  $ echo $?
  128

and status calls (from the superproject) also have trouble:

  $ git status
  warning: refname 'HEAD' is ambiguous
  warning: refname 'HEAD' is ambiguous.
  On branch master
  …

So it's better to just specify your preferred upstream branch directly
(e.g. --branch next).

Cheers,
Trevor

-- 
This email may be signed or encrypted with GnuPG (http://www.gnupg.org).
For more information, see http://en.wikipedia.org/wiki/Pretty_Good_Privacy

[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

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

[Jens Lehmann]

Am 27.03.2014 16:52, schrieb W. Trevor King:
> On Thu, Mar 27, 2014 at 03:21:49PM +0100, Johan Herland wrote:
>> 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:
> 
> The docs say [1]:
> 
>   A remote branch name for tracking updates in the upstream submodule.
>   If the option is not specified, it defaults to 'master'.

But the "branch" setting isn't configured for Qt, the .gitmodules
file contains only this:

[submodule "qtbase"]
	path = qtbase
	url = ../qtbase.git
...

> which is what we do now.  Working around that to default to the
> upstream submodule's HEAD is possible (you can just use --branch
> HEAD), but I think it's easier to just explicitly specify your
> preferred branch.

That is *not* easier, as Johan did not have to do that before.

I think your patch 23d25e48f5ead73c9ce233986f90791abec9f1e8 does
not do what the commit message promised:

    With this change, folks cloning submodules for the first time via:

      $ git submodule update ...

    will get a local branch instead of a detached HEAD, unless they are
    using the default checkout-mode updates.

And Qt uses the "default checkout-mode updates" and doesn't have
"branch" configured either. So we are facing a serious regression
here.

> Cheers,
> Trevor
> 
> [1]: submodule.<name>.branch in gitmodules(5)
>      http://git-scm.com/docs/gitmodules.html
> 

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

[Junio C Hamano]

Jens Lehmann <Jens.Lehmann@web.de> writes:

> Am 27.03.2014 16:52, schrieb W. Trevor King:
>> On Thu, Mar 27, 2014 at 03:21:49PM +0100, Johan Herland wrote:
>>> 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:
>> 
>> The docs say [1]:
>> 
>>   A remote branch name for tracking updates in the upstream submodule.
>>   If the option is not specified, it defaults to 'master'.
>
> But the "branch" setting isn't configured for Qt, the .gitmodules
> file contains only this:
>
> [submodule "qtbase"]
> 	path = qtbase
> 	url = ../qtbase.git
> ...
>
>> which is what we do now.  Working around that to default to the
>> upstream submodule's HEAD is possible (you can just use --branch
>> HEAD), but I think it's easier to just explicitly specify your
>> preferred branch.
>
> That is *not* easier, as Johan did not have to do that before.
>
> I think your patch 23d25e48f5ead73c9ce233986f90791abec9f1e8 does
> not do what the commit message promised:
>
>     With this change, folks cloning submodules for the first time via:
>
>       $ git submodule update ...
>
>     will get a local branch instead of a detached HEAD, unless they are
>     using the default checkout-mode updates.
>
> And Qt uses the "default checkout-mode updates" and doesn't have
> "branch" configured either. So we are facing a serious regression
> here.

There are two potential issues (and a half) then:

 - When cloning with the "default checkout-mode updates", the new
   feature to avoid detaching the HEAD should not kick in at all.

 - For a repository that does not have that "branch" thing
   configured, the doc says that it will default to 'master'.

   I do not think this was brought up during the review, but is it a
   sensible default if the project does not even have that branch?

   What are viable alternatives?

   - use 'master' and fail just the way Johan saw?

   - use any random branch that happens to be at the same commit as
     what is being checked out?

   - use the branch "clone" for the submodule repository saw the
     upstream was pointing at with its HEAD?

   - something else?

 - Johan's set-up was apparently not covered in the addition to t/
   in 23d25e48 (submodule: explicit local branch creation in
   module_clone, 2014-01-26)---otherwise we would have caught this
   regression.  Are there other conditions that are not covered?

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

[Jens Lehmann]

Am 27.03.2014 19:30, schrieb Junio C Hamano:
> Jens Lehmann <Jens.Lehmann@web.de> writes:
> 
>> Am 27.03.2014 16:52, schrieb W. Trevor King:
>>> On Thu, Mar 27, 2014 at 03:21:49PM +0100, Johan Herland wrote:
>>>> 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:
>>>
>>> The docs say [1]:
>>>
>>>   A remote branch name for tracking updates in the upstream submodule.
>>>   If the option is not specified, it defaults to 'master'.
>>
>> But the "branch" setting isn't configured for Qt, the .gitmodules
>> file contains only this:
>>
>> [submodule "qtbase"]
>> 	path = qtbase
>> 	url = ../qtbase.git
>> ...
>>
>>> which is what we do now.  Working around that to default to the
>>> upstream submodule's HEAD is possible (you can just use --branch
>>> HEAD), but I think it's easier to just explicitly specify your
>>> preferred branch.
>>
>> That is *not* easier, as Johan did not have to do that before.
>>
>> I think your patch 23d25e48f5ead73c9ce233986f90791abec9f1e8 does
>> not do what the commit message promised:
>>
>>     With this change, folks cloning submodules for the first time via:
>>
>>       $ git submodule update ...
>>
>>     will get a local branch instead of a detached HEAD, unless they are
>>     using the default checkout-mode updates.
>>
>> And Qt uses the "default checkout-mode updates" and doesn't have
>> "branch" configured either. So we are facing a serious regression
>> here.
> 
> There are two potential issues (and a half) then:
> 
>  - When cloning with the "default checkout-mode updates", the new
>    feature to avoid detaching the HEAD should not kick in at all.

Yep.

>  - For a repository that does not have that "branch" thing
>    configured, the doc says that it will default to 'master'.
> 
>    I do not think this was brought up during the review, but is it a
>    sensible default if the project does not even have that branch?
> 
>    What are viable alternatives?
> 
>    - use 'master' and fail just the way Johan saw?
> 
>    - use any random branch that happens to be at the same commit as
>      what is being checked out?
> 
>    - use the branch "clone" for the submodule repository saw the
>      upstream was pointing at with its HEAD?
> 
>    - something else?

Good question. Me thinks that when a superproject doesn't have
'branch' configured and does set 'update' to something other than
'checkout' for a submodule it should better make sure 'master'
is a valid branch in there. Everything else sounds like a
misconfiguration on the superproject's part that warrants an
error. But I may be wrong here as I only use 'checkout' together
with a detached HEADs myself. Comments welcome.

>  - Johan's set-up was apparently not covered in the addition to t/
>    in 23d25e48 (submodule: explicit local branch creation in
>    module_clone, 2014-01-26)---otherwise we would have caught this
>    regression.  Are there other conditions that are not covered?

I suspect so. This is one of the reasons I started the submodule
testing framework I posted an RFC for a few days ago, as an attempt
to start a systematic approach to submodule testing. This is not
the first time a breakage was not caught by the tests, so we need
to do better here. (Note to self: test for the detached HEAD for
the checkout case in the framework too)

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

[Johan Herland]

On Thu, Mar 27, 2014 at 11:55 PM, Jens Lehmann <Jens.Lehmann@web.de> wrote:
> Am 27.03.2014 19:30, schrieb Junio C Hamano:
>>  - For a repository that does not have that "branch" thing
>>    configured, the doc says that it will default to 'master'.
>>
>>    I do not think this was brought up during the review, but is it a
>>    sensible default if the project does not even have that branch?
>>
>>    What are viable alternatives?
>>
>>    - use 'master' and fail just the way Johan saw?
>>
>>    - use any random branch that happens to be at the same commit as
>>      what is being checked out?
>>
>>    - use the branch "clone" for the submodule repository saw the
>>      upstream was pointing at with its HEAD?
>>
>>    - something else?
>
> Good question. Me thinks that when a superproject doesn't have
> 'branch' configured and does set 'update' to something other than
> 'checkout' for a submodule it should better make sure 'master'
> is a valid branch in there. Everything else sounds like a
> misconfiguration on the superproject's part that warrants an
> error. But I may be wrong here as I only use 'checkout' together
> with a detached HEADs myself. Comments welcome.

I believe unset 'branch' and 'update' != 'checkout' is somewhat
analogous to unset branch.<name>.merge while pulling. I.e. "you have
told me to merge/rebase, but you have not told me against which
branch, therefore error out".

...Johan

-- 
Johan Herland, <johan@herland.net>
www.herland.net

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

[W. Trevor King]

[-- Attachment #1: Type: text/plain, Size: 1175 bytes --]

On Thu, Mar 27, 2014 at 11:55:21PM +0100, Jens Lehmann wrote:
> Me thinks that when a superproject doesn't have 'branch' configured
> and does set 'update' to something other than 'checkout' for a
> submodule it should better make sure 'master' is a valid branch in
> there. Everything else sounds like a misconfiguration on the
> superproject's part that warrants an error.

submodule.<name>.branch should only matter for --remote updates (and
the initial clone, which is a special case of remote update).  So
having an alternative update mode and no submodule.<name>.branch *is*
a valid configuration.  It says:

* I want to integrate local submodule commits with superproject
  gitlink changes using the submodule.<name>.update strategy.
* I never use --remote updates, so I haven't bothered to setup
  submodule.<name>.branch.

I can imagine folks using a workflow like that.  And I think erroring
out if they *do* try a --remote update shouldn't be too surprising for
them.

Cheers,
Trevor

-- 
This email may be signed or encrypted with GnuPG (http://www.gnupg.org).
For more information, see http://en.wikipedia.org/wiki/Pretty_Good_Privacy

[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

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

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

>   Cloning into 'qtbase'...
>   remote: Counting objects: 267400, done.
>   remote: Compressing objects: 100% (61070/61070), done.
>   remote: Total 267400 (delta 210431), reused 258876 (delta 202642)
>   Receiving objects: 100% (267400/267400), 136.23 MiB | 6.73 MiB/s, done.
>   Resolving deltas: 100% (210431/210431), done.
>   Checking connectivity... done.
>   error: pathspec 'origin/master' did not match any file(s) known to git.
>   Unable to setup cloned submodule 'qtbase'
>
> Bisection points to 23d25e48f5ead73c9ce233986f90791abec9f1e8 (W.
> Trevor King: submodule: explicit local branch creation in
> module_clone). Looking at the patch, it seems to introduce an implicit
> assumption on the submodule origin having a "master" branch. Is this
> an intended change in behaviour?

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.

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

[Jens Lehmann]

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.

>>   Cloning into 'qtbase'...
>>   remote: Counting objects: 267400, done.
>>   remote: Compressing objects: 100% (61070/61070), done.
>>   remote: Total 267400 (delta 210431), reused 258876 (delta 202642)
>>   Receiving objects: 100% (267400/267400), 136.23 MiB | 6.73 MiB/s, done.
>>   Resolving deltas: 100% (210431/210431), done.
>>   Checking connectivity... done.
>>   error: pathspec 'origin/master' did not match any file(s) known to git.
>>   Unable to setup cloned submodule 'qtbase'
>>
>> Bisection points to 23d25e48f5ead73c9ce233986f90791abec9f1e8 (W.
>> Trevor King: submodule: explicit local branch creation in
>> module_clone). Looking at the patch, it seems to introduce an implicit
>> assumption on the submodule origin having a "master" branch. Is this
>> an intended change in behaviour?
> 
> 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.

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

[W. Trevor King]

[-- Attachment #1: Type: text/plain, Size: 3162 bytes --]

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

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?  For folks using non-checkout updates, should the preferred
local branch name still be master, or should it match upstream's HEAD?
If upstream's HEAD changes, should we update the local branch name to
stay in sync?  If we don't rename the local branch, do we keep
integrating remote changes from upstream's original branch or keep
integrating HEAD?

I think this would all be simpler if we just made the
superproject-branch-to-submodule-local-branch binding explicit and
pushed this submodule-to-upstream-subproject binding down into the
submodule itself [1].  Then the usual single-project commands would
handle the tricky remote-tracking cases (with explicit
branch.<name>.merge entries, etc.), and a dumb syncing mechanism would
pull those clever choices back up into the superproject for
distribution.

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

Cheers,
Trevor

[1]: http://thread.gmane.org/gmane.comp.version-control.git/239955/focus=240336

-- 
This email may be signed or encrypted with GnuPG (http://www.gnupg.org).
For more information, see http://en.wikipedia.org/wiki/Pretty_Good_Privacy

[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

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

[Junio C Hamano]

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

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

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

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

[Heiko Voigt]

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

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

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

Cheers Heiko

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

[Jens Lehmann]

Am 27.03.2014 21:27, schrieb Heiko Voigt:
> 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.
>>>
>>> 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?

Definitely. But not only for the initial clone, that should hold
true for all subsequent updates too.

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

I believe that depends on the 'update' setting. If it is either not
set or set to 'checkout', it should simply detach when 'branch' is
not set. So it is "(2) do the same and detach" in that case. Otherwise
I agree with Heiko.

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

I'd really like to see more feedback on this from people who actually
use the 'merge' and 'rebase' update modes with or without 'branch' set.

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

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

[Johan Herland]

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

>> > 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@herland.net>
www.herland.net

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

[W. Trevor King]

[-- Attachment #1: Type: text/plain, Size: 4086 bytes --]

On Fri, Mar 28, 2014 at 12:21:23AM +0100, Johan Herland wrote:
> On Thu, Mar 27, 2014 at 9:27 PM, Heiko Voigt wrote:
> > On Thu, Mar 27, 2014 at 12:39:03PM -0700, Junio C Hamano wrote:
> >> 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.
> >>
> >> …
> >> 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.

Branch-attachment is mostly a function of submodule.<name>.update, not
a function of submodule.<name>.branch.  We could certainly interpret a
missing submodule.<name>.branch as:

* Use the subproject's HEAD for the initial clone (clear start_point
  in cmd_update if submodule."$name".branch is not set).
* Don't change the branch name on subsequent local updates (what we
  already do).
* Do $something if the user tries a --remote update.

I just don't know what that $something should be.

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

Agreed, checkout-mode should *always* detach the submodule's HEAD.

>  - In rebase/merge-mode, if submodule.<name>.branch is not set, we
> should _always_ abort with an error.

Why?  Can't we mimic clone and use the remote's HEAD (for --remote
updates)?  That seems more intuitive to me.  For local updates, we're
just integrating the gitlinked commit with the submodule's HEAD, and
you don't need submodule.<name>.branch for that at all.

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

Sounds good to me, and should match the current functionality.

> In other words, submodule.<name>.branch is _necessary_ in
> rebase/merge mode, but _optional_ in checkout-mode (its absence
> indicating that we should detach).

The main trigger for “we should detach” is the update mode
(checkout-mode detaches, all others integrate with the submodule's
HEAD (without changing submodule branches).  You only need
submodule.<name>.branch for determining which *remote* commit you're
trying to integrate (or clone from).  HEAD, master, and “die
screaming” all sound like reasonable defaults in that case.  Deciding
between them is a policy/UI decision, not a technical decision.

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

Except for the initial clone (where it's easy to fix),
submodule.<name>.branch *is* ignored in non --remote updates.

Cheers,
Trevor

-- 
This email may be signed or encrypted with GnuPG (http://www.gnupg.org).
For more information, see http://en.wikipedia.org/wiki/Pretty_Good_Privacy

[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

[RFC] submodule: change submodule.<name>.branch default from master to HEAD

[W. Trevor King]

gitmodule(5) mentioned 'master' as the default remote branch, but
folks using checkout-style updates are unlikely to care which upstream
branch their commit comes from (they only care that the clone fetches
that commit).  If they haven't set submodule.<name>.branch, it makes
more sense to mirror 'git clone' and use the subproject's HEAD than to
default to 'master' (which may not even exist).

After the initial clone, subsequent updates may be local or remote.
Local updates (integrating gitlink changes) have no need to fetch a
specific remote branch, and get along just fine without
submodule.<name>.branch.  Remote updates do need a remote branch, but
HEAD works as well here as it did for the initial clone.

Reported-by: Johan Herland <johan@herland.net>
Signed-off-by: W. Trevor King <wking@tremily.us>
---
This still needs tests, but it gets through the following fine:

  rm -rf superproject subproject &&
  mkdir subproject &&
  (cd subproject &&
   git init &&
   echo 'Subproject' > README &&
   git add README &&
   git commit -m 'Subproject commit' &&
   git branch -m master next
  ) &&
  mkdir superproject &&
  (cd superproject &&
   git init &&
   git submodule add ../subproject submod &&
   git commit -am 'Add submod'
  )
  (cd subproject &&
   echo 'work work work' >> README &&
   git commit -am 'Subproject commit 2'
  ) &&
  (cd superproject &&
   git submodule update --remote &&
   git commit -am 'Add submod'
  )

The main drawback to this approach is that we're changing a default,
but I agree with John's:

On Fri, Mar 28, 2014 at 12:21:23AM +0100, Johan Herland wrote:
> 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.

so the default-change may not be particularly intrusive.

Cheers,
Trevor

 Documentation/git-submodule.txt |  2 +-
 Documentation/gitmodules.txt    |  5 +++--
 git-submodule.sh                | 11 ++++++++---
 3 files changed, 12 insertions(+), 6 deletions(-)

diff --git a/Documentation/git-submodule.txt b/Documentation/git-submodule.txt
index 46c1eeb..c485a17 100644
--- a/Documentation/git-submodule.txt
+++ b/Documentation/git-submodule.txt
@@ -284,7 +284,7 @@ OPTIONS
 	the superproject's recorded SHA-1 to update the submodule, use the
 	status of the submodule's remote-tracking branch.  The remote used
 	is branch's remote (`branch.<name>.remote`), defaulting to `origin`.
-	The remote branch used defaults to `master`, but the branch name may
+	The remote branch used defaults to `HEAD`, but the branch name may
 	be overridden by setting the `submodule.<name>.branch` option in
 	either `.gitmodules` or `.git/config` (with `.git/config` taking
 	precedence).
diff --git a/Documentation/gitmodules.txt b/Documentation/gitmodules.txt
index f539e3f..1aecce9 100644
--- a/Documentation/gitmodules.txt
+++ b/Documentation/gitmodules.txt
@@ -53,8 +53,9 @@ submodule.<name>.update::
 
 submodule.<name>.branch::
 	A remote branch name for tracking updates in the upstream submodule.
-	If the option is not specified, it defaults to 'master'.  See the
-	`--remote` documentation in linkgit:git-submodule[1] for details.
+	If the option is not specified, it defaults to the subproject's
+	HEAD.  See the `--remote` documentation in linkgit:git-submodule[1]
+	for details.
 +
 This branch name is also used for the local branch created by
 non-checkout cloning updates.  See the `update` documentation in
diff --git a/git-submodule.sh b/git-submodule.sh
index 6135cfa..5f08e6c 100755
--- a/git-submodule.sh
+++ b/git-submodule.sh
@@ -819,8 +819,8 @@ cmd_update()
 		name=$(module_name "$sm_path") || exit
 		url=$(git config submodule."$name".url)
 		config_branch=$(get_submodule_config "$name" branch)
-		branch="${config_branch:-master}"
-		local_branch="$branch"
+		branch="${config_branch:-HEAD}"
+		local_branch="$config_branch"
 		if ! test -z "$update"
 		then
 			update_module=$update
@@ -860,7 +860,12 @@ Maybe you want to use 'update --init'?")"
 
 		if ! test -d "$sm_path"/.git -o -f "$sm_path"/.git
 		then
-			start_point="origin/${branch}"
+			if test -n "$config_branch"
+			then
+				start_point="origin/$branch"
+			else
+				start_point=""
+			fi
 			module_clone "$sm_path" "$name" "$url" "$reference" "$depth" "$start_point" "$local_branch" || exit
 			cloned_modules="$cloned_modules;$name"
 			subsha1=
-- 
1.9.1.352.gd393d14.dirty

Re: [RFC] submodule: change submodule.<name>.branch default from master to HEAD

[Eric Sunshine]

On Thu, Mar 27, 2014 at 11:36 PM, W. Trevor King <wking@tremily.us> wrote:
> gitmodule(5) mentioned 'master' as the default remote branch, but
> folks using checkout-style updates are unlikely to care which upstream
> branch their commit comes from (they only care that the clone fetches
> that commit).  If they haven't set submodule.<name>.branch, it makes
> more sense to mirror 'git clone' and use the subproject's HEAD than to
> default to 'master' (which may not even exist).
>
> After the initial clone, subsequent updates may be local or remote.
> Local updates (integrating gitlink changes) have no need to fetch a
> specific remote branch, and get along just fine without
> submodule.<name>.branch.  Remote updates do need a remote branch, but
> HEAD works as well here as it did for the initial clone.
>
> Reported-by: Johan Herland <johan@herland.net>
> Signed-off-by: W. Trevor King <wking@tremily.us>
> ---
> diff --git a/Documentation/git-submodule.txt b/Documentation/git-submodule.txt
> index 46c1eeb..c485a17 100644
> --- a/Documentation/git-submodule.txt
> +++ b/Documentation/git-submodule.txt
> @@ -284,7 +284,7 @@ OPTIONS
>         the superproject's recorded SHA-1 to update the submodule, use the
>         status of the submodule's remote-tracking branch.  The remote used
>         is branch's remote (`branch.<name>.remote`), defaulting to `origin`.
> -       The remote branch used defaults to `master`, but the branch name may
> +       The remote branch used defaults to `HEAD`, but the branch name may
>         be overridden by setting the `submodule.<name>.branch` option in
>         either `.gitmodules` or `.git/config` (with `.git/config` taking
>         precedence).
> diff --git a/Documentation/gitmodules.txt b/Documentation/gitmodules.txt
> index f539e3f..1aecce9 100644
> --- a/Documentation/gitmodules.txt
> +++ b/Documentation/gitmodules.txt
> @@ -53,8 +53,9 @@ submodule.<name>.update::
>
>  submodule.<name>.branch::
>         A remote branch name for tracking updates in the upstream submodule.
> -       If the option is not specified, it defaults to 'master'.  See the
> -       `--remote` documentation in linkgit:git-submodule[1] for details.
> +       If the option is not specified, it defaults to the subproject's

Did you mean s/subproject/submodule/ ?

> +       HEAD.  See the `--remote` documentation in linkgit:git-submodule[1]
> +       for details.
>  +
>  This branch name is also used for the local branch created by
>  non-checkout cloning updates.  See the `update` documentation in
> diff --git a/git-submodule.sh b/git-submodule.sh
> index 6135cfa..5f08e6c 100755
> --- a/git-submodule.sh
> +++ b/git-submodule.sh
> @@ -819,8 +819,8 @@ cmd_update()
>                 name=$(module_name "$sm_path") || exit
>                 url=$(git config submodule."$name".url)
>                 config_branch=$(get_submodule_config "$name" branch)
> -               branch="${config_branch:-master}"
> -               local_branch="$branch"
> +               branch="${config_branch:-HEAD}"
> +               local_branch="$config_branch"
>                 if ! test -z "$update"
>                 then
>                         update_module=$update
> @@ -860,7 +860,12 @@ Maybe you want to use 'update --init'?")"
>
>                 if ! test -d "$sm_path"/.git -o -f "$sm_path"/.git
>                 then
> -                       start_point="origin/${branch}"
> +                       if test -n "$config_branch"
> +                       then
> +                               start_point="origin/$branch"
> +                       else
> +                               start_point=""
> +                       fi
>                         module_clone "$sm_path" "$name" "$url" "$reference" "$depth" "$start_point" "$local_branch" || exit
>                         cloned_modules="$cloned_modules;$name"
>                         subsha1=
> --
> 1.9.1.352.gd393d14.dirty

Re: [RFC] submodule: change submodule.<name>.branch default from master to HEAD

[W. Trevor King]

[-- Attachment #1: Type: text/plain, Size: 1035 bytes --]

On Thu, Mar 27, 2014 at 11:43:47PM -0400, Eric Sunshine wrote:
> On Thu, Mar 27, 2014 at 11:36 PM, W. Trevor King <wking@tremily.us> wrote:
> >  submodule.<name>.branch::
> >         A remote branch name for tracking updates in the upstream submodule.
> > -       If the option is not specified, it defaults to 'master'.  See the
> > -       `--remote` documentation in linkgit:git-submodule[1] for details.
> > +       If the option is not specified, it defaults to the subproject's
> 
> Did you mean s/subproject/submodule/ ?
> 
> > +       HEAD.  See the `--remote` documentation in linkgit:git-submodule[1]
> > +       for details.

No the remote branch is in the upstream subproject.  I suppose I meant
“the submodule's remote-tracking branch following the upstream
subproject's HEAD which we just fetched so it's fairly current” ;).

Cheers,
Trevor

-- 
This email may be signed or encrypted with GnuPG (http://www.gnupg.org).
For more information, see http://en.wikipedia.org/wiki/Pretty_Good_Privacy

[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

Re: [RFC] submodule: change submodule.<name>.branch default from master to HEAD

[W. Trevor King]

[-- Attachment #1: Type: text/plain, Size: 1239 bytes --]

On Thu, Mar 27, 2014 at 08:52:55PM -0700, W. Trevor King wrote:
> On Thu, Mar 27, 2014 at 11:43:47PM -0400, Eric Sunshine wrote:
> > On Thu, Mar 27, 2014 at 11:36 PM, W. Trevor King <wking@tremily.us> wrote:
> > >  submodule.<name>.branch::
> > >         A remote branch name for tracking updates in the upstream submodule.
> > > -       If the option is not specified, it defaults to 'master'.  See the
> > > -       `--remote` documentation in linkgit:git-submodule[1] for details.
> > > +       If the option is not specified, it defaults to the subproject's
> > 
> > Did you mean s/subproject/submodule/ ?
> > 
> > > +       HEAD.  See the `--remote` documentation in linkgit:git-submodule[1]
> > > +       for details.
> 
> No the remote branch is in the upstream subproject.  I suppose I meant
> “the submodule's remote-tracking branch following the upstream
> subproject's HEAD which we just fetched so it's fairly current” ;).

Hmm, maybe we should change the existing “upstream submodule” to
“upstream subproject” for consistency?

Trevor

-- 
This email may be signed or encrypted with GnuPG (http://www.gnupg.org).
For more information, see http://en.wikipedia.org/wiki/Pretty_Good_Privacy

[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

Re: [RFC] submodule: change submodule.<name>.branch default from master to HEAD

[Jens Lehmann]

Am 28.03.2014 04:58, schrieb W. Trevor King:
> On Thu, Mar 27, 2014 at 08:52:55PM -0700, W. Trevor King wrote:
>> On Thu, Mar 27, 2014 at 11:43:47PM -0400, Eric Sunshine wrote:
>>> On Thu, Mar 27, 2014 at 11:36 PM, W. Trevor King <wking@tremily.us> wrote:
>>>>  submodule.<name>.branch::
>>>>         A remote branch name for tracking updates in the upstream submodule.
>>>> -       If the option is not specified, it defaults to 'master'.  See the
>>>> -       `--remote` documentation in linkgit:git-submodule[1] for details.
>>>> +       If the option is not specified, it defaults to the subproject's
>>>
>>> Did you mean s/subproject/submodule/ ?
>>>
>>>> +       HEAD.  See the `--remote` documentation in linkgit:git-submodule[1]
>>>> +       for details.
>>
>> No the remote branch is in the upstream subproject.  I suppose I meant
>> “the submodule's remote-tracking branch following the upstream
>> subproject's HEAD which we just fetched so it's fairly current” ;).
> 
> Hmm, maybe we should change the existing “upstream submodule” to
> “upstream subproject” for consistency?

For me it's still an "upstream submodule" ...

Re: [RFC] submodule: change submodule.<name>.branch default from master to HEAD

[W. Trevor King]

[-- Attachment #1: Type: text/plain, Size: 1471 bytes --]

On Fri, Mar 28, 2014 at 05:57:50PM +0100, Jens Lehmann wrote:
> Am 28.03.2014 04:58, schrieb W. Trevor King:
> > On Thu, Mar 27, 2014 at 08:52:55PM -0700, W. Trevor King wrote:
> >> No the remote branch is in the upstream subproject.  I suppose I meant
> >> “the submodule's remote-tracking branch following the upstream
> >> subproject's HEAD which we just fetched so it's fairly current” ;).
> > 
> > Hmm, maybe we should change the existing “upstream submodule” to
> > “upstream subproject” for consistency?
> 
> For me it's still an "upstream submodule" ...

We have a few existing “[upstream] subproject” references though.  I
prefer subproject, because the submodule's upstream repository is
likely a bare repo and not a submodule itself.  It's also possible
(likely?) that the upstream repository is a stand-alone project, and
not designed to always be a submodule.  However, “upstream submodule”
and “submodule's upstream” are both clear enough, and if they're the
consensus phrasing, I'd rather standardize on them than jump back and
forth between phrasings in the docs.  I can write up a patch that
shifts us to consistently use one form, once we decide what that
should be (although I'm happy to let someone else write the patch too
;).

Cheers,
Trevor

-- 
This email may be signed or encrypted with GnuPG (http://www.gnupg.org).
For more information, see http://en.wikipedia.org/wiki/Pretty_Good_Privacy

[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

Re: [RFC] submodule: change submodule.<name>.branch default from master to HEAD

[Jens Lehmann]

Am 28.03.2014 18:10, schrieb W. Trevor King:
> On Fri, Mar 28, 2014 at 05:57:50PM +0100, Jens Lehmann wrote:
>> Am 28.03.2014 04:58, schrieb W. Trevor King:
>>> On Thu, Mar 27, 2014 at 08:52:55PM -0700, W. Trevor King wrote:
>>>> No the remote branch is in the upstream subproject.  I suppose I meant
>>>> “the submodule's remote-tracking branch following the upstream
>>>> subproject's HEAD which we just fetched so it's fairly current” ;).
>>>
>>> Hmm, maybe we should change the existing “upstream submodule” to
>>> “upstream subproject” for consistency?
>>
>> For me it's still an "upstream submodule" ...
> 
> We have a few existing “[upstream] subproject” references though.  I
> prefer subproject, because the submodule's upstream repository is
> likely a bare repo and not a submodule itself.  It's also possible
> (likely?) that the upstream repository is a stand-alone project, and
> not designed to always be a submodule.  However, “upstream submodule”
> and “submodule's upstream” are both clear enough, and if they're the
> consensus phrasing, I'd rather standardize on them than jump back and
> forth between phrasings in the docs.  I can write up a patch that
> shifts us to consistently use one form, once we decide what that
> should be (although I'm happy to let someone else write the patch too
> ;).

Apart from the RelNotes there are only seven places in the
Documentation directory where the term "subproject" is used:

- Two in git-submodule.txt (which are those you recently added in
  the series that introduced the regression and would be gone if
  we revert that)

- One in git-write-tree.txt (but as I understand it the --prefix
  option can be used to record tree objects for other tools too,
  so the more generic term subproject looks OK to me there)

- Four occurrences in user-manual.txt describing the diff format
  for submodules (which I assume will always stay "[+-]Subproject"
  for backwards compatibility reasons)

If we do not revert your series I'll be happy to write up a patch
replacing the two usages of subproject in git-submodule.txt ;-)

Re: [RFC] submodule: change submodule.<name>.branch default from master to HEAD

[Junio C Hamano]

Jens Lehmann <Jens.Lehmann@web.de> writes:

> Am 28.03.2014 04:58, schrieb W. Trevor King:
>> On Thu, Mar 27, 2014 at 08:52:55PM -0700, W. Trevor King wrote:
>>> On Thu, Mar 27, 2014 at 11:43:47PM -0400, Eric Sunshine wrote:
>>>> On Thu, Mar 27, 2014 at 11:36 PM, W. Trevor King <wking@tremily.us> wrote:
>>>>>  submodule.<name>.branch::
>>>>>         A remote branch name for tracking updates in the upstream submodule.
>>>>> -       If the option is not specified, it defaults to 'master'.  See the
>>>>> -       `--remote` documentation in linkgit:git-submodule[1] for details.
>>>>> +       If the option is not specified, it defaults to the subproject's
>>>>
>>>> Did you mean s/subproject/submodule/ ?
>>>>
>>>>> +       HEAD.  See the `--remote` documentation in linkgit:git-submodule[1]
>>>>> +       for details.
>>>
>>> No the remote branch is in the upstream subproject.  I suppose I meant
>>> “the submodule's remote-tracking branch following the upstream
>>> subproject's HEAD which we just fetched so it's fairly current” ;).
>> 
>> Hmm, maybe we should change the existing “upstream submodule” to
>> “upstream subproject” for consistency?
>
> For me it's still an "upstream submodule" ...

I inherited the habit of saying "submodule" vs "superproject" from
Linus (I think) during the very early days, and there is no such
thing as "subproject" or "supermodule" in my vocabulary.

Just one documentation-reader's opinion, not an edict from the
maintainer.

Re: [RFC] submodule: change submodule.<name>.branch default from master to HEAD

[Jens Lehmann]

Am 28.03.2014 04:36, schrieb W. Trevor King:
> gitmodule(5) mentioned 'master' as the default remote branch, but
> folks using checkout-style updates are unlikely to care which upstream
> branch their commit comes from (they only care that the clone fetches
> that commit).  If they haven't set submodule.<name>.branch, it makes
> more sense to mirror 'git clone' and use the subproject's HEAD than to
> default to 'master' (which may not even exist).
> 
> After the initial clone, subsequent updates may be local or remote.
> Local updates (integrating gitlink changes) have no need to fetch a
> specific remote branch, and get along just fine without
> submodule.<name>.branch.  Remote updates do need a remote branch, but
> HEAD works as well here as it did for the initial clone.
> 
> Reported-by: Johan Herland <johan@herland.net>
> Signed-off-by: W. Trevor King <wking@tremily.us>
> ---
> This still needs tests, but it gets through the following fine:
> 
>   rm -rf superproject subproject &&
>   mkdir subproject &&
>   (cd subproject &&
>    git init &&
>    echo 'Subproject' > README &&
>    git add README &&
>    git commit -m 'Subproject commit' &&
>    git branch -m master next
>   ) &&
>   mkdir superproject &&
>   (cd superproject &&
>    git init &&
>    git submodule add ../subproject submod &&
>    git commit -am 'Add submod'
>   )
>   (cd subproject &&
>    echo 'work work work' >> README &&
>    git commit -am 'Subproject commit 2'
>   ) &&
>   (cd superproject &&
>    git submodule update --remote &&
>    git commit -am 'Add submod'
>   )
> 
> The main drawback to this approach is that we're changing a default,
> but I agree with John's:
> 
> On Fri, Mar 28, 2014 at 12:21:23AM +0100, Johan Herland wrote:
>> 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.
> 
> so the default-change may not be particularly intrusive.

I'd prefer a solution that doesn't change any defaults for the
checkout use case (again). Maybe it is a better route to revert
this series, then add tests describing the current behavior for
checkout submodules as a next step before adding the branch mode
for rebase and merge users again?

> Cheers,
> Trevor
> 
>  Documentation/git-submodule.txt |  2 +-
>  Documentation/gitmodules.txt    |  5 +++--
>  git-submodule.sh                | 11 ++++++++---
>  3 files changed, 12 insertions(+), 6 deletions(-)
> 
> diff --git a/Documentation/git-submodule.txt b/Documentation/git-submodule.txt
> index 46c1eeb..c485a17 100644
> --- a/Documentation/git-submodule.txt
> +++ b/Documentation/git-submodule.txt
> @@ -284,7 +284,7 @@ OPTIONS
>  	the superproject's recorded SHA-1 to update the submodule, use the
>  	status of the submodule's remote-tracking branch.  The remote used
>  	is branch's remote (`branch.<name>.remote`), defaulting to `origin`.
> -	The remote branch used defaults to `master`, but the branch name may
> +	The remote branch used defaults to `HEAD`, but the branch name may
>  	be overridden by setting the `submodule.<name>.branch` option in
>  	either `.gitmodules` or `.git/config` (with `.git/config` taking
>  	precedence).
> diff --git a/Documentation/gitmodules.txt b/Documentation/gitmodules.txt
> index f539e3f..1aecce9 100644
> --- a/Documentation/gitmodules.txt
> +++ b/Documentation/gitmodules.txt
> @@ -53,8 +53,9 @@ submodule.<name>.update::
>  
>  submodule.<name>.branch::
>  	A remote branch name for tracking updates in the upstream submodule.
> -	If the option is not specified, it defaults to 'master'.  See the
> -	`--remote` documentation in linkgit:git-submodule[1] for details.
> +	If the option is not specified, it defaults to the subproject's
> +	HEAD.  See the `--remote` documentation in linkgit:git-submodule[1]
> +	for details.
>  +
>  This branch name is also used for the local branch created by
>  non-checkout cloning updates.  See the `update` documentation in
> diff --git a/git-submodule.sh b/git-submodule.sh
> index 6135cfa..5f08e6c 100755
> --- a/git-submodule.sh
> +++ b/git-submodule.sh
> @@ -819,8 +819,8 @@ cmd_update()
>  		name=$(module_name "$sm_path") || exit
>  		url=$(git config submodule."$name".url)
>  		config_branch=$(get_submodule_config "$name" branch)
> -		branch="${config_branch:-master}"
> -		local_branch="$branch"
> +		branch="${config_branch:-HEAD}"
> +		local_branch="$config_branch"
>  		if ! test -z "$update"
>  		then
>  			update_module=$update
> @@ -860,7 +860,12 @@ Maybe you want to use 'update --init'?")"
>  
>  		if ! test -d "$sm_path"/.git -o -f "$sm_path"/.git
>  		then
> -			start_point="origin/${branch}"
> +			if test -n "$config_branch"
> +			then
> +				start_point="origin/$branch"
> +			else
> +				start_point=""
> +			fi
>  			module_clone "$sm_path" "$name" "$url" "$reference" "$depth" "$start_point" "$local_branch" || exit
>  			cloned_modules="$cloned_modules;$name"
>  			subsha1=
> 

Re: [RFC] submodule: change submodule.<name>.branch default from master to HEAD

[W. Trevor King]

[-- Attachment #1: Type: text/plain, Size: 2305 bytes --]

On Mon, Mar 31, 2014 at 09:35:07PM +0200, Jens Lehmann wrote:
> Am 28.03.2014 04:36, schrieb W. Trevor King:
> > The main drawback to this approach is that we're changing a default,
> > but I agree with John's:
> > 
> > On Fri, Mar 28, 2014 at 12:21:23AM +0100, Johan Herland wrote:
> >> 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.
> > 
> > so the default-change may not be particularly intrusive.
> 
> I'd prefer a solution that doesn't change any defaults for the
> checkout use case (again). Maybe it is a better route to revert
> this series, then add tests describing the current behavior for
> checkout submodules as a next step before adding the branch mode
> for rebase and merge users again?

The change in defaults should only adversely effect users who:

* don't configure submodule.<name>.branch,
* use --remote updates,
* expect them to pull the master branch of the submodule's upstream, and
* have an upstream whose HEAD doesn't point at master.

Since 23d25e4 (submodule: explicit local branch creation in
module_clone, 2014-01-26), we adversely effects users (regardless of
update strategy) who:

* don't configure submodule.<name>.branch, and
* update-clone from a submodule upstream that doesn't have a master branch.

But with this patch we'll fix that.  Before 23d25e4, we adversly
affected users who:

* used non-checkout update strategies, and
* wanted an attached HEAD after update-clones.

All of these were easy to work around, with either:

  $ git config submodule.$name.branch $branch

or:

  $ cd $submod
  $ git checkout $branch

I'm fine reverting 23d25e4 if we want to kick it around some more, but
I have trouble imagining a future UI that works for both:

* users that want --remote to pull master even though upstream's HEAD
  points elsewhere, and
* users that want --remote to pull HEAD because upstream doesn't have
  a master branch,

if neither of those users are willing to configure an explicit
submodule.<name>.branch.

Cheers,
Trevor

-- 
This email may be signed or encrypted with GnuPG (http://www.gnupg.org).
For more information, see http://en.wikipedia.org/wiki/Pretty_Good_Privacy

[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

Re: [RFC] submodule: change submodule.<name>.branch default from master to HEAD

[Junio C Hamano]

Jens Lehmann <Jens.Lehmann@web.de> writes:

> I'd prefer a solution that doesn't change any defaults for the
> checkout use case (again). Maybe it is a better route to revert
> this series, then add tests describing the current behavior for
> checkout submodules as a next step before adding the branch mode
> for rebase and merge users again?

Sounds like a good idea to revert the entire series, as that would
give us longer to think the whole thing through.  It would certainly
avoid unexpected fallout in 2.0 where we already have other changes,
all of which are known/designed to be backward incompatible with a
set of carefully planned transition plans.  This submodule change
may have meant well, but does not seem to sit well together with
these other changes.

Perhaps in 2.1 or 2.2.

submodule.<path>.branch vs. submodule.<name>.branch (was: Possible regression in master? (submodules without a "master" branch).

[W. Trevor King]

[-- Attachment #1: Type: text/plain, Size: 1321 bytes --]

I'm breaking this off into a sub-thread, so it doesn't distract from
the main issue.

On Thu, Mar 27, 2014 at 12:39:03PM -0700, Junio C Hamano wrote:
> 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?

Good catch.

The transition from submodule.<path>.* to submodule.<name>.* happened
in 73b0898d (Teach "git submodule add" the --name option, 2012-09-30),
which landed in v1.8.1-rc0 on 2012-12-03.  The first
submodule.<path>.branch reference landed a short time later in
b9289227 (submodule add: If --branch is given, record it in
.gitmodules, 2012-12-19), and I was probably just not aware of
73b0898d.  The second submodule.<path>.branch reference landed in
23d25e48 (submodule: explicit local branch creation in module_clone,
2014-01-26), and is just a copy paste error.  Both should be updated
to submodule.<name>.branch.

Cheers,
Trevor

-- 
This email may be signed or encrypted with GnuPG (http://www.gnupg.org).
For more information, see http://en.wikipedia.org/wiki/Pretty_Good_Privacy

[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

Re: submodule.<path>.branch vs. submodule.<name>.branch

[Junio C Hamano]

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

>>   [side note] Isn't that a typo of submodule.<name>.branch?
>
> Good catch.
>
> The transition from submodule.<path>.* to submodule.<name>.* happened
> in 73b0898d (Teach "git submodule add" the --name option, 2012-09-30),
> which landed in v1.8.1-rc0 on 2012-12-03.  

Thanks for digging.

Strictly speaking, I think this was not even a transition (rather,
there was no way to give a submodule a name that is different from
its path).  In any version of git whose git-submodule.sh has
module_name helper function, the path and the name were conceptually
two different things, and we should have been using the name, not
path, throughout.

> ...  Both should be updated
> to submodule.<name>.branch.

I agree.  Thanks.