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

["Ævar Arnfjörð Bjarmason" <avarab@gmail.com>]

On Thu, Feb 03 2022, Emily Shaffer wrote:

> Since v6:

Thanks for the re-roll!

> I've dropped the fifth commit to use this new config for `git rev-parse
> --show-superproject-working-tree`. I think it did more harm than good -
> that tool uses an odd way to determine whether the superproject is
> actually the superproject, anyways.
>
> I poked a little bit at trying to find some benchmark to demonstrate
> that "submodule.superprojectGitDir" is actually faster - but I didn't
> end up able to write one without writing a ton of new code to traverse
> the filesystem.

I'm assuming that's tested against some variant of the submodule-in-C[1]
conversion. I.e. at least when I tested it [2][3] it seemed easy to come
up with (probably overly artificial) benchmarks where it would matter
for the shell-out in this series.

The question performance-wise was rather whether we'd just be
introducing the config mechanism as a transitory performance workaround,
and the need for it would evaporate once that C migration happened (re
original CL quoted in [3]).

> To be honest, I'm not all that interested in performance
> - I want the config added for correctness, instead.

And I'm honestly still at the point of not even being against this whole
thing, although it probably sounds like that. I'm really not.

I just genuinely don't get where this is headed. I.e. for the last
iteration I did a demo patch on top that showed that there was no case
added by the series where the on-the-fly discovery wasn't equivalent to
the set-in-config value[4].

That change showed that after this series in a state where the config
*is* redundant to on the fly discovery (or maybe not, and we're just
missing test coverage).

But since you're citing correctness do you have some repo->sub
relationship in mind that would be ambiguous in a way where the
configuration would resolve the ambiguity?

I can imagine how such a thing might work, e.g. if we gave submodules
some git-worktree-like method of being completely detached from the
parent. I.e. being able to place a /usr/me/repo.git whose submodule
entry for a "test" dir lives in /opt/tests or something. So when you "cd
/opt/tests" you wouldn't be able to detect you're within a submodule.

(I'm assuming the case where the submodule has its own "in-tree" .git,
is that even supported anymore...?)

But I can't think of one where such an ambiguity would arise within our
current featureset.

What I really can't see is how if the need for such "config [...] for
correctness" would arise how that doesn't also invalidate the
assumptions you're making in 3/4 and 4/4.

I.e. surely if we need the config for correctness it's also true that we
can't after-the-fact add the config on the fly to (such) existing
submodules without user intervention. Or maybe the ambiguity would only
arise from the POV of the submodule, but not for commands executed
within the parent?

1. https://lore.kernel.org/git/cover-v5-0.9-00000000000-20220128T125206Z-avarab@gmail.com/
2. https://lore.kernel.org/git/RFC-cover-0.2-00000000000-20211117T113134Z-avarab@gmail.com/
3. https://lore.kernel.org/git/211124.86a6hue2wk.gmgdl@evledraar.gmail.com/
4. https://lore.kernel.org/git/RFC-patch-2.2-b49d4c8db7d-20211117T113134Z-avarab@gmail.com/