On 10/13/2021 2:51 PM, Emily Shaffer wrote:
On Thu, Aug 19, 2021 at 09:09:46PM -0400, Derrick Stolee wrote:
quoted
but I do have some concerns that
we have not covered all the cases that could lead to the relative path
needing an update, such as a 'git checkout' across commits in the super-
project which moves a submodule.
Leading more to my confusion is that I thought 'git submodule update'
was the way to move a submodule, but that does not appear to be the case.
I used 'git mv' to move a submodule and that correctly updated the
.gitmodules file, but did not run any 'git submodule' subcommands (based
on GIT_TRACE2_PERF=1).
During a live review a few weeks ago it was pointed out, I forget by
who, that this whole series would make a lot more sense if it was
treated as the path from the submodule's gitdir to the superproject's
gitdir. I think this would also fix your 'git mv' example here, as the
submodule gitdir would not change.
I think that's a great way to deliver similar functionality without
the issues I was thinking about.
quoted
You mention in an earlier cover letter that this config does not need to
exist, but it seems to me that if it exists it needs to be correct for us
to depend on it for future features. I'm not convinced that we can rely
on it as-written, but you might be able to convince me by pointing out
why these submodule movements are safe.
Yeah, that is a good point, and I wonder how to be defensive about
this... Maybe it makes sense to BUG() if we don't find the
superproject's gitdir from this config? Maybe it makes sense to warn
(asking users to 'git submodule update') and erase the incorrect config
line?
I think we can complain with something like a die() if the config points
to data that doesn't make sense (not a .git directory), but a BUG() is
too heavy-handed because it can just be a user modifying their config
file incorrectly.
I'm happy to shut down a process because a user messed with config
incorrectly. Since your proposed change allows operations like 'git mv'
to move submodules without needing to change this config, I'm much
happier with the design. It becomes impossible for the config to get
out of sync unless the user does something outside of a Git command.
Thanks,
-Stolee