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

On Wed, Nov 17, 2021 at 03:28:46PM -0800, Jonathan Tan wrote:

Emily Shaffer [off-list ref] writes:

quoted

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/ (local)

quoted

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.

quoted

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/ (local)