On Wed, Nov 17, 2021 at 03:28:46PM -0800, Jonathan Tan wrote: > > Emily Shaffer <emilyshaffer@xxxxxxxxxx> 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@xxxxxxxxxx/ > > > 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@xxxxxxxxxx/
