All of lore.kernel.org
 help / color / mirror / Atom feed
From: Glen Choo <chooglen@google.com>
To: git@vger.kernel.org
Cc: Glen Choo <chooglen@google.com>
Subject: [RFC PATCH 0/2] branch: implement in-process --recurse-submodules
Date: Tue, 21 Sep 2021 16:25:27 -0700	[thread overview]
Message-ID: <20210921232529.81811-1-chooglen@google.com> (raw)

These patches are an attempt to implement git branch
--recurse-submodules in-process. This is part of the general submodule
UX improvements discussed in [1]. Specifically, this is the use case
discussed in the section titled "Create Mode (git switch -c / git
checkout -b)", but without checking out the newly created branch.

Doing this in-process allows "git switch -c" and "git checkout -b" to
reuse create_branch(), instead of relying on a child process to create
the branch. create_branch() nominally takes a struct repository *
parameter, but there is some hidden reliance on the_repository and
global state.

Broadly speaking, I'd like to know how we should approach
"--recurse-submodules", particularly when we are implementing [1]. I'm
sending these patches as an RFC because I think this is somewhat
indicative of submodule issues.

In this patchset, branching works slightly differently between the
superproject and submodule (skip ahead for specifics). There are two
very obvious alternatives that can address this:

* A: only implement --recurse-submodules behavior after we are able to
  eliminate any kind of dependence on the_repository/global state that
  shouldn't be shared.
* B: implement --recurse-submodules as child processes, which won't be
  bothered by global state.

My cursory thoughts on the matter are that A seems like a good direction
for code health, but it seems difficult to do in practice. B seems
hacky, but it might be a good stopgap while we work on A.

Obviously these aren't the only options and this isn't the end of the
discussion, so I'd appreciate any thoughts on the matter :)

Changes:
* refactor the refs.h functions to accept struct repository * where
  reasonable.
* add two new functions to refs.h that accept struct
  repository *, repo_ref_transaction_commit() and
  repo_ref_transaction_prepare().
* change ref_transaction_commit() and ref_transaction_prepare() to be
  thin wrappers of their repo_ counterparts.
* use repo_ref_transaction_commit() in create_branch()
* add a create_branches() function to builtin/branch.c that recursively
  creates branches in submodules

What doesn't work (marked with NEEDSWORK):
* branch tracking is disabled for submodules because remotes.c only
  holds state for a single repository
* similarly, bare repository checking does not work as expected because
  environment.c only holds state for a single repository

What is questionable (specific to these patches, not
--recurse-submodules in general):
* files-backend.c only uses the_repository to validate the oid being
  used and it is the only ref backend that does so. Instead of passing
  struct repository * through more places in refs.h, we might be able to
  eliminate this check altogether. This is discussed briefly in [2].
* create_branches() implements its own submodule iterating, which is
  similar to that of ls-files, but wholly different from
  submodule--helper.c.

[1] https://lore.kernel.org/git/YHofmWcIAidkvJiD@google.com/
[2] https://lore.kernel.org/git/20210916172432.1073546-1-jonathantanmy@google.com/

Glen Choo (2):
  refs: pass struct repository *r through to write_ref_to_lockfile()
  branch: add --recurse-submodules option for branch creation

 branch.c                  | 26 +++++++++------
 branch.h                  |  4 +--
 builtin/branch.c          | 69 ++++++++++++++++++++++++++++++++++++---
 builtin/checkout.c        |  4 +--
 refs.c                    | 21 ++++++------
 refs.h                    | 18 +++++++---
 refs/debug.c              | 14 ++++----
 refs/files-backend.c      | 30 ++++++++---------
 refs/packed-backend.c     |  7 ++--
 refs/refs-internal.h      |  7 ++--
 t/helper/test-ref-store.c |  2 +-
 t/t3200-branch.sh         | 58 ++++++++++++++++++++++++++++++++
 12 files changed, 199 insertions(+), 61 deletions(-)

-- 
2.33.0.464.g1972c5931b-goog


             reply	other threads:[~2021-09-21 23:25 UTC|newest]

Thread overview: 7+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-09-21 23:25 Glen Choo [this message]
2021-09-21 23:25 ` [RFC PATCH 1/2] refs: pass struct repository *r through to write_ref_to_lockfile() Glen Choo
2021-09-21 23:25 ` [RFC PATCH 2/2] branch: add --recurse-submodules option for branch creation Glen Choo
2021-09-22 11:10   ` Ævar Arnfjörð Bjarmason
2021-09-22 16:55     ` Glen Choo
2021-09-22 12:28   ` Philippe Blain
2021-09-22 17:24     ` 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

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

  git send-email \
    --in-reply-to=20210921232529.81811-1-chooglen@google.com \
    --to=chooglen@google.com \
    --cc=git@vger.kernel.org \
    /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.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.