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@xxxxxxxxx/ 2. https://lore.kernel.org/git/RFC-cover-0.2-00000000000-20211117T113134Z-avarab@xxxxxxxxx/ 3. https://lore.kernel.org/git/211124.86a6hue2wk.gmgdl@xxxxxxxxxxxxxxxxxxx/ 4. https://lore.kernel.org/git/RFC-patch-2.2-b49d4c8db7d-20211117T113134Z-avarab@xxxxxxxxx/
