git@vger.kernel.org mailing list mirror (one of many)
 help / color / mirror / code / Atom feed
From: Emily Shaffer <emilyshaffer@google.com>
To: Derrick Stolee <stolee@gmail.com>
Cc: git@vger.kernel.org, "Albert Cui" <albertcui@google.com>,
	"Phillip Wood" <phillip.wood123@gmail.com>,
	"Johannes Schindelin" <Johannes.Schindelin@gmx.de>,
	"Ævar Arnfjörð Bjarmason" <avarab@gmail.com>,
	"Junio C Hamano" <gitster@pobox.com>,
	"Matheus Tavares Bernardino" <matheus.bernardino@usp.br>,
	"Jonathan Nieder" <jrnieder@gmail.com>,
	"Jacob Keller" <jacob.keller@gmail.com>,
	"Atharva Raykar" <raykar.ath@gmail.com>,
	"Jonathan Tan" <jonathantanmy@google.com>
Subject: Re: [PATCH v5 0/4] cache parent project's gitdir in submodules
Date: Mon, 8 Nov 2021 15:11:36 -0800	[thread overview]
Message-ID: <YYmuqEQUaB1a8Gs1@google.com> (raw)
In-Reply-To: <920c3133-b6ee-b82c-0876-f06713e9337b@gmail.com>

On Sun, Nov 07, 2021 at 08:24:43PM -0500, Derrick Stolee wrote:
> 
> On 11/4/2021 7:49 PM, Emily Shaffer wrote:
> 
> > The only real change here is a slight semantics change to map from
> > <submodule gitdir> to <superproject common git dir>. In every case
> > *except* for when the superproject has a worktree, this changes nothing.
> > For the case when the superproject has a worktree, this means that now
> > submodules will refer to the general superproject common dir (e.g. no
> > worktree-specific refs or configs or whatnot).
> > 
> > I *think* that because a submodule should exist in the context of the
> > common dir, not the worktree gitdir, that is ok. However, it does mean
> > it would be difficult to do something like sharing a config specific to
> > the worktree (the initial goal of this series).
> > 
> > $ROOT/.git
> > $ROOT/.git/config.superproject <- shared by $ROOT/.git/modules/sub
> > $ROOT/.git/modules/sub <- points to $ROOT/.git
> > $ROOT/.git/worktrees/wt
> > $ROOT/.git/worktrees/wt/config.superproject <- contains a certain config-based pre-commit hook
> > 
> > If the submodule only knows about the common dir, that is tough, because
> > the submodule would basically have to guess which worktree it's in from
> > its own path. There would be no way for '$WT/sub' to inherit
> > '$ROOT/.git/worktrees/wt/config.superproject'.
> > 
> > That said... right now, we don't support submodules in worktrees very
> > well at all. A submodule in a worktree will get a brand new gitdir in
> > $ROOT/.git/worktrees/modules/ (and that brand new gitdir would point to
> > the super's common dir). So I think we can punt on this entire question
> > until we teach submodules and worktrees to play more gracefully together
> > (it's on my long list...),
> 
> (I omit a portion that will be discussed later.)
> 
> > Or, to summarize the long ramble above: "this is still kind of weird
> > with worktrees, but let's fix it later when we fix worktrees more
> > thoroughly".
> 
> I'm concerned about punting here, because making a messy situation worse
> is unlikely to have a clean way out. Could we set up a design that works
> with superproject worktrees?
> 
> You mentioned that submodules cannot have worktrees. At least, you said
> that 'absorbgitdirs' does not allow them. Could those subprojects still
> exist and be registered as submodules without using that command?
> 
> What I'm trying to hint at is that if the submodules can't have
> worktrees, then maybe we could make their 'config.worktree' files be
> relative to the superproject worktrees. Then, these submodules could
> point to the commondir in their base config and _also_ to the worktree
> gitdir in their config.worktree.
> 
> The issue that is immediately obvious here is that my definition is
> circular: we need to know the superproject worktree in order to discover
> the config.worktree which contains the information about the superproject
> worktree.
> 
> > and at that time we can probably introduce a
> > pointer from $ROOT/.git/modules/sub/worktrees/wt/ to
> > $ROOT/.git/worktrees/wt/....
> 
> Your idea here appears to assume that if the superproject has worktrees,
> then the submodule is divided into worktrees in an exact correspondence.
> This would allow the submodule's config.worktree to point to the
> superproject's worktree (or possibly it could be inferred from the
> submodule's worktree relative to the submodule's commondir).
> 
> This seems like an interesting way forward, but requires changing how
> 'git absorbgitdirs' works, along with changes to 'git worktree' or other
> submodule commands when the submodule first appears during a 'git checkout'
> in a worktree. I imagine there are a lot of "gotchas" here. It is worth
> spending some time imagining how to create this setup and/or enforce it
> as submodules are added in the lifecycle of a repository, if only to
> validate the config design presented by this series.

Yeah, I think we may be overthinking it, especially with the concerns
about common dir vs. gitdir. More specifically - I think we accidentally
did the right thing in the previous iteration by using the gitdir :)

I think we can probably put it pretty simply:
submodule.superprojectGitDir should point from the most local gitdir of
the submodule to the most local gitdir of the superproject.

Luckily there are not so many permutations to worry about here.

Super doesn't have worktrees, sub doesn't have worktrees:
.git/
  modules/
    sub/
      config <- contains a pointer to ".git/"
  config

Super doesn't have worktrees, sub does have worktrees (and as you
suggest above, right now this would have to be created carefully and
manually, but later we probably want this to Just Work):
.git/
  modules/
    sub/
      config
      config.worktree <- contains a pointer to ".git/"
      worktrees/
        sub-wt/
	config <- contains a pointer to ".git/"
  config

Super has worktrees, sub doesn't have worktrees:
Actually, I think in the future this might not be possible, if we want
to make `git worktree add --recurse-submodules` work gracefully (and I
do want that). But in the interim, in practice it looks like this:
.git/
  modules/
    sub/
      config <- contains a pointer to ".git/"
  worktrees/
    super-wt/
      modules/
        sub/
	  config <- contains a pointer to ".git/worktrees/super-wt"
      config
  config
This case is pretty weird anyway, because in order for a submodule to be
present in multiple worktrees of the superproject, the submodule itself
needs to either have multiple worktrees or multiple repos. But on the
flip side, today it's impossible for a single submodule gitdir to need
to know about more superproject worktrees than the one it was
initted/whatever into.

Both super and sub have worktrees:
And this won't exist until we have graceful support of `git worktree add
--recurse-submodules` or with some manual effort, now.
.git/
  modules/
    sub/
      worktrees/
        sub-wt/
	  config <- contains a pointer to ".git/worktrees/super-wt/"
      config
      config.worktree <- contains a pointer to ".git/"
  worktrees/
    super-wt/
      config
  config
  config.worktree

I think this will give us access to both the worktree gitdir *and* the
common gitdir:

  ~/git/.git/worktrees/git-second [GIT_DIR!]$ git rev-parse --git-common-dir
  /usr/local/google/home/emilyshaffer/git/.git

So that means from any submodule, we can determine:
 - submodule's gitdir (from the .git link in the submodule wt)
 - submodule's common dir (from existing commands)
 - gitdir of superproject which submodule inhabits (from the config in
   the submodule's gitdir, or the submodule's config.worktree)
 - common dir of superproject (from existing commands + prior config)

The upshot to me, then, means that we should be 1) making sure to get
the path to the gitdir, not the common dir, of the superproject; and 2)
using helpers to write to the worktree config, not to the local config,
of the submodule. In other words, we want to avoid the following:

.git/
  modules/
    sub/
      worktrees/
        wt/
	  config
      config <- "submodule.superprojectGitDir = ../../../.." as written by the worktree

Will take a look at the rest of the comments too, but this sounds like a
reasonable approach to me.

 - Emily

> 
> Thanks,
> -Stolee

  reply	other threads:[~2021-11-08 23:11 UTC|newest]

Thread overview: 31+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-10-14 20:34 [PATCH v4 0/4] cache parent project's gitdir in submodules Emily Shaffer
2021-10-14 20:34 ` [PATCH v4 1/4] t7400-submodule-basic: modernize inspect() helper Emily Shaffer
2021-10-14 20:34 ` [PATCH v4 2/4] introduce submodule.superprojectGitDir record Emily Shaffer
2021-10-14 20:34 ` [PATCH v4 3/4] submodule: record superproject gitdir during absorbgitdirs Emily Shaffer
2021-10-18 23:18   ` Jonathan Tan
2021-10-25 16:14     ` Derrick Stolee
2021-11-04 23:22       ` Emily Shaffer
2021-11-08  1:07         ` Derrick Stolee
2021-11-04 22:09     ` Emily Shaffer
2021-10-14 20:34 ` [PATCH v4 4/4] submodule: record superproject gitdir during 'update' Emily Shaffer
2021-10-25 16:17   ` Derrick Stolee
2021-10-25 16:19 ` [PATCH v4 0/4] cache parent project's gitdir in submodules Derrick Stolee
2021-11-04 23:49 ` [PATCH v5 " Emily Shaffer
2021-11-04 23:49   ` [PATCH v5 1/4] t7400-submodule-basic: modernize inspect() helper Emily Shaffer
2021-11-04 23:49   ` [PATCH v5 2/4] introduce submodule.superprojectGitDir record Emily Shaffer
2021-11-05  7:50     ` Junio C Hamano
2021-11-08 23:16       ` Emily Shaffer
2021-11-04 23:49   ` [PATCH v5 3/4] submodule: record superproject gitdir during absorbgitdirs Emily Shaffer
2021-11-04 23:49   ` [PATCH v5 4/4] submodule: record superproject gitdir during 'update' Emily Shaffer
2021-11-05  4:49     ` Junio C Hamano
2021-11-05  8:43       ` Ævar Arnfjörð Bjarmason
2021-11-08 23:21         ` Emily Shaffer
2021-11-09  0:42           ` Ævar Arnfjörð Bjarmason
2021-11-09 20:36             ` Emily Shaffer
2021-11-09 21:46               ` Emily Shaffer
2021-11-05  8:51     ` Ævar Arnfjörð Bjarmason
2021-11-08 23:22       ` Emily Shaffer
2021-11-09  1:12         ` Ævar Arnfjörð Bjarmason
2021-11-08  1:24   ` [PATCH v5 0/4] cache parent project's gitdir in submodules Derrick Stolee
2021-11-08 23:11     ` Emily Shaffer [this message]
2021-11-09 15:58       ` Derrick Stolee

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=YYmuqEQUaB1a8Gs1@google.com \
    --to=emilyshaffer@google.com \
    --cc=Johannes.Schindelin@gmx.de \
    --cc=albertcui@google.com \
    --cc=avarab@gmail.com \
    --cc=git@vger.kernel.org \
    --cc=gitster@pobox.com \
    --cc=jacob.keller@gmail.com \
    --cc=jonathantanmy@google.com \
    --cc=jrnieder@gmail.com \
    --cc=matheus.bernardino@usp.br \
    --cc=phillip.wood123@gmail.com \
    --cc=raykar.ath@gmail.com \
    --cc=stolee@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).