Re: [PATCH v3 8/8] clone, submodule update: create and check out branches

[Jonathan Tan <jonathantanmy@google.com>]

Glen Choo <chooglen@google.com> writes:
> > 1. Checkout the branch, ignoring OID (as in this patch).
> > 2. Checkout the branch, erroring out if the OID is wrong.
> > 3. 1 + creating the branch if it does not exist.
> > 4. 2 + creating the branch if it does not exist.
> > 5. Always forcibly create the branch at the gitlink's OID and then checking
> >    it out.

[snip]

> > A. Create only the branch that is checked out in the superproject (as in this
> >    patch).
> > B. Create all branches that are present in the superproject.
> > C. Go back on our previous decision, switching to 3.

[snip]

> But it doesn't make sense to mix both A _and_ C, since C would already
> give us the same result as A, so it probably makes sense to go straight
> to C in this series (i.e. only for the initial clone, not subsequent
> checkouts). I'll do that in v3.
> 
> I prefer C in the long run, since both A and B require that the list of
> submodule branches never get out of sync with the superproject, which is
> hard to enforce, e.g.:
 
I discussed this with Glen in-office and Glen pointed out that A is actually
not necessarily redundant with respect to C, since a "git submodule add" may
clone a submodule, but it would not run "git submodule update" (so 3 + C would
mean that no branch is created in the submodule, which is not what we want). So
we still need A.

As for 1 vs. 3, we will still need 3 in the future for the reasons I described
in my previous e-mail, but I think that that can be done incrementally. My
concern is to avoid doing something in a patch set that we will later need to
undo; I think that we are indeed avoiding it here (we're doing A but we will
still need it in the future, so there is no undoing of A needed).

So overall, after this discussion, this patch set looks good to me, except for
the minor points that I have commented on in my previous emails.