git@vger.kernel.org mailing list mirror (one of many)
 help / color / mirror / code / Atom feed
From: Glen Choo <chooglen@google.com>
To: Jonathan Tan <jonathantanmy@google.com>,
	Glen Choo via GitGitGadget <gitgitgadget@gmail.com>
Cc: Jonathan Tan <jonathantanmy@google.com>,
	git@vger.kernel.org,
	Philippe Blain <levraiphilippeblain@gmail.com>
Subject: Re: [PATCH v3 8/8] clone, submodule update: create and check out branches
Date: Tue, 22 Nov 2022 10:44:25 -0800	[thread overview]
Message-ID: <kl6lmt8iesmu.fsf@chooglen-macbookpro.roam.corp.google.com> (raw)
In-Reply-To: <20221115181524.164472-1-jonathantanmy@google.com>

Thanks for the thoughtful response, Jonathan :)

Jonathan Tan <jonathantanmy@google.com> writes:

> "Glen Choo via GitGitGadget" <gitgitgadget@gmail.com> writes:
>> +# Test the behavior of an already-cloned submodule.
>> +# NEEDSWORK When updating with branches, we always use the branch instead of the
>> +# gitlink's OID. This results in some imperfect behavior:
>> +#
>> +# - If the gitlink's OID disagrees with the branch OID, updating with branches
>> +#   may result in a dirty worktree
>> +# - If the branch does not exist, the update fails.
>> +#
>> +# We will reevaluate when "git checkout --recurse-submodules" supports branches
>> +# For now, just test for this imperfect behavior.
>
> I think the rationale for this behavior is as follows:
>
> We want a world in which submodules have branches and Git commands use them
> wherever possible. There are a few options for "git submodule update" when the
> superproject has a branch checked out:
>
> 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.
>
> At this point in the discussion, for a low-level command like "git submodule
> update", doing as little as possible makes sense to me, which is 1.
>
> But since we do not automatically create the branch if it does not exist, this
> means that we have to do it when we clone the submodule. Our options are:
>
> 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.
>
> My instinct is that we want to maintain, as much as possible, the invariant
> that for each branch in the superproject, if the branch tip has a gitlink
> pointing to a submodule, that submodule has a branch of the same name. And I
> think that this invariant can only be maintained by "git submodule update" if
> we use B or C.

I think C is good to have in this series, though for slightly different
reasons.

I agree that the invariant should be preserved when we check out
branches both in the initial clone and in subsequent checkouts. However,
I don't think that we necessarily need to have all superproject branches
after the initial clone. Even if the submodule only has a single
superproject branch, that's enough to have an ephemeral clone for
writing small changes. We could defer the "all superproject branches"
problem til after we worry about subsequent checkouts (i.e. "git
checkout" with branches).

We can handle "initial clone" and "subsequent checkout" as smaller, more
digestible series as long as the work for "initial clone" doesn't get in
the way of "subsequent checkout". My plan (as of v2) was:

- For the intial clone, create only the checked out superproject branch
  at clone time and check it out (aka A)
- For subsequent checkouts, check out the superproject branch, creating
  it if it does not exist (aka C)

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.:

- The user could create a branch in the superproject without recursing
  in to submodules.
- The user could delete the branch in the submodule.
- (Worst yet) The process that creates branches in the submodule _after_
  creating the branch in the superproject could exit unexpectedly (e.g.
  SIGINT). There is no atomic way to create branches in both repos.

We could create a command that would repair broken branch states ("git
submodule repair"?), but C can self-repair, which avoids this problem
entirely.

  reply	other threads:[~2022-11-22 18:44 UTC|newest]

Thread overview: 56+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-08-29 20:54 [PATCH 0/6] clone, submodule update: check out submodule branches Glen Choo via GitGitGadget
2022-08-29 20:54 ` [PATCH 1/6] clone: teach --detach option Glen Choo via GitGitGadget
2022-08-30  4:02   ` Philippe Blain
2022-08-29 20:54 ` [PATCH 2/6] repo-settings: add submodule_propagate_branches Glen Choo via GitGitGadget
2022-08-30  4:02   ` Philippe Blain
2022-08-29 20:54 ` [PATCH 3/6] t5617: drop references to remote-tracking branches Glen Choo via GitGitGadget
2022-08-30  4:03   ` Philippe Blain
2022-08-29 20:54 ` [PATCH 4/6] submodule: return target of submodule symref Glen Choo via GitGitGadget
2022-09-01 20:01   ` Jonathan Tan
2022-09-01 20:46     ` Glen Choo
2022-08-29 20:54 ` [PATCH 5/6] submodule--helper: refactor up-to-date criterion Glen Choo via GitGitGadget
2022-08-29 20:54 ` [PATCH 6/6] clone, submodule update: check out branches Glen Choo via GitGitGadget
2022-08-30  4:03   ` Philippe Blain
2022-08-30 22:54     ` Glen Choo
2022-09-01 20:00   ` Jonathan Tan
2022-10-20 20:20 ` [PATCH v2 0/7] clone, submodule update: check out submodule branches Glen Choo via GitGitGadget
2022-10-20 20:20   ` [PATCH v2 1/7] clone: teach --detach option Glen Choo via GitGitGadget
2022-10-20 20:20   ` [PATCH v2 2/7] repo-settings: add submodule_propagate_branches Glen Choo via GitGitGadget
2022-10-25 18:03     ` Jonathan Tan
2022-10-20 20:20   ` [PATCH v2 3/7] submodule--helper clone: create named branch Glen Choo via GitGitGadget
2022-10-25 18:00     ` Jonathan Tan
2022-10-20 20:20   ` [PATCH v2 4/7] t5617: drop references to remote-tracking branches Glen Choo via GitGitGadget
2022-10-20 20:20   ` [PATCH v2 5/7] submodule: return target of submodule symref Glen Choo via GitGitGadget
2022-10-20 20:20   ` [PATCH v2 6/7] submodule update: refactor update targets Glen Choo via GitGitGadget
2022-10-20 20:20   ` [PATCH v2 7/7] clone, submodule update: create and check out branches Glen Choo via GitGitGadget
2022-10-25 17:56     ` Jonathan Tan
2022-10-25 21:49       ` Glen Choo
2022-10-20 22:40   ` [PATCH v2 0/7] clone, submodule update: check out submodule branches Junio C Hamano
2022-10-20 23:53     ` Glen Choo
2022-10-21  0:01       ` Junio C Hamano
2022-10-28 20:14   ` [PATCH v3 0/8] " Glen Choo via GitGitGadget
2022-10-28 20:14     ` [PATCH v3 1/8] clone: teach --detach option Glen Choo via GitGitGadget
2022-10-28 21:40       ` Junio C Hamano
2022-10-28 21:54         ` Junio C Hamano
2022-10-28 22:55           ` Glen Choo
2022-10-30 18:14             ` Taylor Blau
2022-10-31 17:07               ` Glen Choo
2022-11-08 13:32       ` Philippe Blain
2022-10-28 20:14     ` [PATCH v3 2/8] repo-settings: add submodule_propagate_branches Glen Choo via GitGitGadget
2022-10-28 20:14     ` [PATCH v3 3/8] submodule--helper clone: create named branch Glen Choo via GitGitGadget
2022-10-28 20:14     ` [PATCH v3 4/8] t5617: drop references to remote-tracking branches Glen Choo via GitGitGadget
2022-10-28 20:14     ` [PATCH v3 5/8] submodule: return target of submodule symref Glen Choo via GitGitGadget
2022-10-28 21:49       ` Junio C Hamano
2022-10-28 23:11         ` Glen Choo
2022-10-28 20:14     ` [PATCH v3 6/8] submodule update: refactor update targets Glen Choo via GitGitGadget
2022-10-28 20:14     ` [PATCH v3 7/8] submodule--helper: remove update_data.suboid Glen Choo via GitGitGadget
2022-11-14 23:45       ` Jonathan Tan
2022-10-28 20:14     ` [PATCH v3 8/8] clone, submodule update: create and check out branches Glen Choo via GitGitGadget
2022-11-08 13:53       ` Philippe Blain
2022-11-15 18:15       ` Jonathan Tan
2022-11-22 18:44         ` Glen Choo [this message]
2022-11-23  1:33           ` Jonathan Tan
2022-11-23  4:00             ` Junio C Hamano
2022-10-30 18:19     ` [PATCH v3 0/8] clone, submodule update: check out submodule branches Taylor Blau
2022-11-08 14:23     ` Philippe Blain
2022-11-08 20:43       ` Glen Choo

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

  List information: http://vger.kernel.org/majordomo-info.html

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=kl6lmt8iesmu.fsf@chooglen-macbookpro.roam.corp.google.com \
    --to=chooglen@google.com \
    --cc=git@vger.kernel.org \
    --cc=gitgitgadget@gmail.com \
    --cc=jonathantanmy@google.com \
    --cc=levraiphilippeblain@gmail.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
Code repositories for project(s) associated with this public inbox

	https://80x24.org/mirrors/git.git

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).