git@vger.kernel.org mailing list mirror (one of many)
 help / color / mirror / code / Atom feed
From: Emily Shaffer <emilyshaffer@google.com>
To: Junio C Hamano <gitster@pobox.com>
Cc: git@vger.kernel.org
Subject: Re: [PATCH v5 2/4] introduce submodule.superprojectGitDir record
Date: Mon, 8 Nov 2021 15:16:07 -0800	[thread overview]
Message-ID: <YYmvt+wFQ97QLaO8@google.com> (raw)
In-Reply-To: <xmqqo86zxcqp.fsf@gitster.g>

On Fri, Nov 05, 2021 at 12:50:22AM -0700, Junio C Hamano wrote:
> 
> Emily Shaffer <emilyshaffer@google.com> writes:
> 
> > By using a relative path instead of an absolute path, we can move the
> > superproject directory around on the filesystem without breaking the
> > submodule's cache. And by using the path from gitdir to gitdir, we can
> > move the submodule within the superproject's tree structure without
> > breaking the submodule's cache, too.
> 
> All of the above explains what the design choice is, and what
> benefit the chosen design brings us.  But this last one ...
> 
> > Finally, by pointing at "get_git_common_dir()" instead of
> > "get_git_dir()", we ensure the link will refer to the parent repo,
> > not to a specific worktree.
> 
> ... only says that we choose to point at the common one, not a
> specific worktree (i.e. what behaviour was chosen by the design),
> but it is left unclear what benefit it is trying to bring us.
> 
> Thinking aloud, imagine that there are two worktrees for the
> superproject.  For simplicity, let's further imagine that these
> worktrees have a clean check-out of the same commit, hence, these
> two worktrees have the same commit from the same submodule checked
> out at the same location relative to the project root.
> 
> The subdirectory in each of these two superproject worktrees that
> checks out the submodule has .git file (as we "absorb" the gitdir in
> the modern submodule layout) pointing at somewhere.  It probably is
> OK if they point at the same place, but it might be more natural if
> these two submodule checkouts are two worktrees for a single
> submodule repository (this part I am not very clear, and that is why
> I labeled the discussion "thinking aloud").
> 
> It seems to me that both design choices would have equally valid
> arguments for them.  If both of these submodule worktrees point at
> the "common" dir of the superproject, because the "common" one is
> part of the primary worktree, which is harder to lose than secondary
> worktrees, of the superproject, it is presumably harder to go stale
> when "worktree rm" is done at superproject, which may be a plus.
> But then from the "cached" pointer, each of these submodule
> worktrees cannot tell which worktree of the superproject they are
> checked out as a part of.  Of course we can go to the root level of
> the submodule worktree and do the usual repository discovery to
> learn where the root level of the superproject worktree it belongs
> to, but it somehow feels that it defeats half the point of having
> this "cache" information.
> 
> If we instead point at the git-dir, from there, it is just one level
> of indirection to find out where the common dir of the superproject
> is.
> 
> > If the new config is present, we can do some optional value-added
> > behavior, like letting "git status" print additional info about the
> > submodule's status in relation to its superproject, or like letting the
> > superproject and submodule share an additional config file separate from
> > either one's local config.
> 
> And one value-add that I would think of off the top of my head is to
> show "we have commit X checked out, which is 4 commits on top of
> what the superproject's index records for this submodule" when "git
> status" is run in the submodule's worktree.  I do not know that is
> an operation to specifically optimize for, but by choosing to
> "cache" the common dir, not the one specific to the worktree of the
> superporject our submodule checkout is a part of, the chosen design
> seems to make it harder to implement?

Yeah, that is compelling. Sorry not to reply in depth, but I am
convinced.

 - Emily

  reply	other threads:[~2021-11-08 23:16 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 [this message]
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
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=YYmvt+wFQ97QLaO8@google.com \
    --to=emilyshaffer@google.com \
    --cc=git@vger.kernel.org \
    --cc=gitster@pobox.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).