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
signature.asc
Description: OpenPGP digital signature
