Re: [RFC] Branches with --recurse-submodules

[Glen Choo <chooglen@google.com>]

Thanks so much Philippe, your responses are very thoughtful.

Philippe Blain <levraiphilippeblain@gmail.com> writes:

>> * `git branch --recurse-submodules topic` should create the branch
>>    `topic` in each of the repositories.
>
> I guess for some workflow this would be the good, but for others you might
> not need to create submodule branches for each new superproject branch you
> create.  I think I pointed that out before; I don't necessarily think that
> creating branches in all submodules should *not* be the default behaviour,
> but I think that it should be configurable. I mean that if I have 'submodule.recurse'
> set to true, I would not like 'git branch topic' to create a 'topic' branch
> in each submodule. So I wish I'll be able to add 'branch.recurseSubmodules = false'
> to my config (or something similar) to have more granularity in behaviour.

Yes, as we discussed earlier, this behavior may not be desirable for
different workflows. I've come to suspect that the branching behavior
that I proposed should be the default, but I'm ambivalent on being able
to opt out of the branching.

In favor of letting users opt out: I'd imagine that behavior might be
disruptive to users who make frequent changes on the submodule and may
not appreciate having two sets of branch names (one from the
superproject and one from the submodule's remotes). I'm not clear on
whether or not this is disruptive primarily because it is a breaking
change, or if this just an objectively bad fit for what these users
want.

In favor of not letting users opt out: exposing fewer switches to users
makes it easier for them to get a good user experience. Instead of
giving users the ability to build-your-own UX, maintaining a small
configuration surface makes configuration easy and puts the onus back on
Git (or me, really :P) to make sure that the UX really works well for
all users, instead of opting out and saying "oh the user has
branches.recurseSubmodules=false, so I'll choose not to support them".
I think this stance is good from a product excellence perspective, but
it's an obvious risk.

A way forward might be:

* mitigate the breaking changes by flagging this with
  feature.experimental
* test this behavior with real users (aka internal) and iterate from
  there

Does that make sense? I'd like to make sure I'm not missing something
very big here.

> Also, I assume the new behaviour would carry over to 'git checkout -b' and
> 'git switch -c' ?
>> * `git switch --recurse-submodules topic` should checkout the branch
>>    `topic` in each of the repositories
>
> Nit: I guess you also include 'git checkout --r topic' here also ?

Yes and yes (I believe --r refers to --recurse-submodules?).

>> * If the branch is in an unexpected state, we either:
>> ** Fallback to the version that the user would expect (if it is safe to
>>      do so).
>
> What would be 'the version the user would expect' here ?

The issues is that defaulting to 'the version the user would expect' is
a fairly uncontroversial opinion, but it leaves a lot of room for
interpretation. I suspect that there won't be a single set of rules that
can apply in every single command and situation; we would never make
progress if we tried to start with a top down approach.

Instead, this RFC prescribes one consistent set of 'expected versions'
under a subset of operations {branch,switch,checkout}...

>> === Switching _to_ a branch `topic`, i.e. `git {switch,checkout} topic`
>> 
>> Switch to `topic` in the superproject. Then in each submodule, switch to:
>> 
>> * `topic`, if it exists
>> * Otherwise, the commit id in the superproject gitlink (and warn the
>>    user that HEAD is detached)
>> 
>> If the submodule `topic` points to a different commit from the
>> superproject gitlink, this will leave the superproject with a dirty
>> worktree with respect to the gitlinks. This allows a user to recover
>> work if they had previously switched _away from_ "topic".
>
> OK, so you seem to answer my interrogation above about "what is the version
> the user would expect ?" with "the commit at the tip of 'topic' in the submodule,
> if that branch exists.".

which you have noted here :)

>> === Switching _away from_ a branch `topic`, i.e. `git {switch,checkout} other-branch`
>> 
>> Checkout `other-branch` if each submodule’s worktree is clean (except for
>> gitlinks), and has one of the following checked out:
>> 
>> * `topic`
>> * the commit id in the superproject gitlink at the tip of 'topic'
>
> Is that what you meant ? (that would indeed make sense).

Yes, thanks for the wording suggestion.

>> If a dirty worktree is unacceptable, we may need an option that is
>> guaranteed to check out the superproject’s `topic`.
>
> Yes, I would think that should be configurable, maybe something like
> '--recurse-submodules=branch' vs '--recurse-submodules=detached' (which
> is the actual behaviour). Just thinking out loud here.

Yes, your wording is also similar to what I was thinking of. I'm holding
back from this because, as stated earlier, I'm worried about having a
build-your-own UX situation and bifurcating (or worse) our development
efforts.

>> Check each submodule at the superproject’s `start-point` (not the
>> submodule’s `start-point`) for the following:
>> 
>> * The submodule is initialized (in .git/modules)
>
> The submodule should also be active, no ? Maybe it was cloned before,
> so exists in .git/modules, but was then set as inactive (submodule.<name>.active=false)...

Ah yes, good catch.

>> * For uninitialized submodules, prompt them to initialize it via git
>>    checkout start-point && git submodule update (we are working to
>>    eliminate manual initialization in the long run, so this will become
>>    obsolete eventually).
>
> I think if the submodule are not initialized, they should be left alone, without
> prompting the user. Projects that use non-optional submodules already instruct
> their users to clone with --recurse-submodules or run 'git submodule
> update --init --recursive' after the clone, so I'm not sure that sort
> of nagging would be necessary...

To make sure we're on the same page, I'll give a motivating example.
Let's consider a superproject with remote-tracking branches
`origin/main` and `origin/topic`.

`origin/main` has submodules `sub1` and `sub2`.
`origin/topic` has submodules `sub1` and `sub3`.

Imagine a user has branched off `origin/main` and has initialized the
submodules using git submodule update --init. At this point, `sub1` and
`sub2` are initialized. `sub3` is not initialized (because it's not in
`origin/main`).

What happens when the user now wants to work off `origin/topic` with
`git branch --recurse-submodules topic origin/topic`? `sub3` isn't
initialized, so the branch can't be created.

So to get back to your main point, before we eliminate this problem,
what do we do? Do we abort and naggily warn the user? Do we try our best
and just ignore `sub3`?

Your suggestion might be superior to mine:

* for users who don't care about `sub3`, they are not blocked from
  creating branches
* for users who do care, they would checkout `topic`, realize
  that `sub3` isn't initialized and then perform the initialization and
  (at some point) realize that `sub3` doesn't have the `topic` branch
  and so they fix this manually.

I'll consider this more deeply, thanks.