Re: Re: Possible regression in master? (submodules without a "master" branch)
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..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..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..update, not a function of submodule..branch. We could certainly interpret a missing submodule..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..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..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..branch for that at all. > - If submodule..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..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..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..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..branch configured should > always detach. Except for the initial clone (where it's easy to fix), submodule..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 signature.asc Description: OpenPGP digital signature
Re: Possible regression in master? (submodules without a "master" branch)
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..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..branch *is* a valid configuration. It says: * I want to integrate local submodule commits with superproject gitlink changes using the submodule..update strategy. * I never use --remote updates, so I haven't bothered to setup submodule..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 signature.asc Description: OpenPGP digital signature
Re: Possible regression in master? (submodules without a "master" branch)
On Thu, Mar 27, 2014 at 11:55 PM, Jens Lehmann 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..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, www.herland.net -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Re: Possible regression in master? (submodules without a "master" branch)
(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 wrote:
> On Thu, Mar 27, 2014 at 12:39:03PM -0700, Junio C Hamano wrote:
>> "W. Trevor King" 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 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..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..branch.
>>
>> [side note] Isn't that a typo of submodule..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..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..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..branch is not set, we
should _always_ abort with an error.
- If submodule..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..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..branch is not
>> > set and you run a --remote update, which has used:
>> >
>> > git rev-parse "${remote_name}/${branch}"
>> >
>> > since the submodule..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..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..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..branch' defaults
>> > to master (and not upstream's HEAD), do we want to adjust this at the
>>
Re: Possible regression in master? (submodules without a "master" branch)
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" 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 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..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..branch.
>>
>> [side note] Isn't that a typo of submodule..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..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..branch is not
>>> set and you run a --remote update, which has used:
>>>
>>> git rev-parse "${remote_name}/${branch}"
>>>
>>> since the submodule..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..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..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.
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Possible regression in master? (submodules without a "master" branch)
Am 27.03.2014 19:30, schrieb Junio C Hamano: > Jens Lehmann 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) -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Re: Possible regression in master? (submodules without a "master" branch)
On Thu, Mar 27, 2014 at 12:39:03PM -0700, Junio C Hamano wrote:
> "W. Trevor King" 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 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..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..branch.
>
> [side note] Isn't that a typo of submodule..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..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..branch is not
> > set and you run a --remote update, which has used:
> >
> > git rev-parse "${remote_name}/${branch}"
> >
> > since the submodule..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..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..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
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Possible regression in master? (submodules without a "master" branch)
"W. Trevor King" 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 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..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..branch.
[side note] Isn't that a typo of submodule..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..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..branch is not
> set and you run a --remote update, which has used:
>
> git rev-parse "${remote_name}/${branch}"
>
> since the submodule..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..branch empty, shouldn't that be diagnosed as an
error?
> gitmodules(5) is pretty clear that 'submodule..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.
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [git] Re: Possible regression in master? (submodules without a "master" branch)
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 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..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..branch is not
set and you run a --remote update, which has used:
git rev-parse "${remote_name}/${branch}"
since the submodule..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..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..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
signature.asc
Description: OpenPGP digital signature
Re: Possible regression in master? (submodules without a "master" branch)
Jens Lehmann 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? -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Possible regression in master? (submodules without a "master" branch)
Am 27.03.2014 18:16, schrieb Junio C Hamano: > Johan Herland 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. -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Possible regression in master? (submodules without a "master" branch)
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..branch in gitmodules(5) > http://git-scm.com/docs/gitmodules.html > -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Possible regression in master? (submodules without a "master" branch)
Johan Herland 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. -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Possible regression in master? (submodules without a "master" branch)
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 signature.asc Description: OpenPGP digital signature
Re: Possible regression in master? (submodules without a "master" branch)
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..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 signature.asc Description: OpenPGP digital signature
