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

["W. Trevor King" <wking@tremily.us>]

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