Re: [PATCH v6 0/5] teach submodules to know they're submodules

[Emily Shaffer <emilyshaffer@google.com>]

On Wed, Nov 17, 2021 at 03:28:46PM -0800, Jonathan Tan wrote:
> 
> Emily Shaffer <emilyshaffer@google.com> writes:
> > For the original cover letter, see
> > https://lore.kernel.org/git/20210611225428.1208973-1-emilyshaffer%40google.com.
> 
> Also for reference, v4 and v5 (as a reply to v4) can be found here:
> https://lore.kernel.org/git/20211014203416.2802639-1-emilyshaffer@google.com/
> 
> > Since v5:
> > 
> > A couple things. Firstly, a semantics change *back* to the semantics of
> > v3 - we map from gitdir to gitdir, *not* from common dir to common dir,
> > so that theoretically a submodule with multiple worktrees in multiple
> > superproject worktrees will be able to figure out which worktree of the
> > superproject it's in. (Realistically, that's not really possible right
> > now, but I'd like to change that soon.)
> 
> Makes sense. Also, thanks for the tests covering what happens when this
> is run from worktrees.
> 
> > Secondly, a rewording of comments and commit messages to indicate that
> > this isn't a cache of some expensive operation, but rather intended to
> > be the source of truth for all submodules. I also added a fifth commit
> > rewriting `git rev-parse --show-superproject-working-tree` to
> > demonstrate what that means in practice - but from a practical
> > standpoint, I'm a little worried about that fifth patch. More details in
> > the patch 5 description.
> 
> OK - this is not the "this variable being missing is OK" idea that I had
> [1], but we want to be able to depend on it to some extent. (And it is
> not a cache either - we are not planning to perform an operation to
> obtain the superproject gitdir if the cache is missing, but we are just
> going to assume that there is no superproject.)
> 
> To that end, the 5th patch is misleading - it is behaving exactly like a
> cache. I think it's better to drop it.

Yeah, I think you are right.

> 
> What would make sense to me (and seems to be in the spirit of this patch
> set) is to describe this as something that Git commands can rely on to
> determine if the current repo is a submodule, for performance reasons.
> So maybe Git commands/parameters that directly reference the submodule
> concept like "--show-superproject-working-tree" will work hard to find
> the superproject (by searching the filesystem), but those that do not
> (e.g. "git status") can make assumptions.

Oh interesting, I like the idea of that distinction. Thanks for the
suggestion.

I wonder, though, how best to delineate. I'd almost rather say like I
suggested to Ævar
(https://lore.kernel.org/git/YZ1KLNwsxx7IR1%2B5%40google.com), that we
can treat "--show-superproject-working-tree" as a "legacy" option people
are already using, and treat anything added from now on as "if it
doesn't think it is a submodule, you should 'git submodule update' in
the superproject". That relies on us being able to reliably keep the
value of submodule.superprojectGitDir correct, but I think the
gitdir->gitdir linking helps with that (as opposed to earlier
iterations).
> 
> Making this variable a source of truth wouldn't work, I think, because
> the source of truth is whether this repo appears in a .gitmodules file
> (and that hasn't changed).

Interestingly, '--show-superproject-working-tree' doesn't check the
.gitmodules file at all. It checks whether the project found in the
filesystem above it knows about an object named path/to/superproject/
which is a gitlink - that is, it asks the index, not .gitmodules, as far
as I understand it. So if the only existing alternative to
submodule.superprojectGitDir isn't treating .gitmodules as source of
truth, then what does that mean?

> 
> To this end, I'll comment on the changes I'd like to see on the
> individual patches too.
> 
> [1] https://lore.kernel.org/git/20210727174650.2462099-1-jonathantanmy@google.com/