| Message ID | 20210819200953.2105230-5-emilyshaffer@google.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | cache parent project's gitdir in submodules | expand |
On 8/19/2021 4:09 PM, Emily Shaffer wrote: > + # Cache a pointer to the superproject's gitdir. This may have > + # changed, so rewrite it unconditionally. Writes it to worktree > + # if applicable, otherwise to local. > + relative_gitdir="$(git rev-parse --path-format=relative \ > + --prefix "${sm_path}" \ > + --git-dir)" > + > + git -C "$sm_path" config --worktree \> + submodule.superprojectgitdir "$relative_gitdir" Ok, I see now why you care about the worktree config. The scenario you are covering is something like moving a submodule within a worktree and not wanting to change the relative path of other copies of that submodule within other worktrees, yes? For commands such as 'init' and 'add', we don't have the possibility of colliding with other worktrees because the submodule is being created for the first time, so the relative path should be safe to place in the non-worktree config. I do struggle with the fact that these are inconsistent across the two commits. It makes me think that there should only be one way to do it, and either the NEEDSWORK needs to be fixed now, or this line shouldn't include --worktree. Much of this can depend on the reason the worktree config exists for a submodule. I expect you have more context than me, so could you help me understand? Moving to a different concern I am now realizing with this config: What happens if a submodule moves, and then a user runs 'git checkout' in the superproject to move it back? How do we make this config value update across those movements? Is there a possibility of integrating with unpack_trees() to notice that a submodule has moved and hence we should update this config value? Thanks, -Stoolee
On Thu, Aug 19, 2021 at 08:59:55PM -0400, Derrick Stolee wrote: > > On 8/19/2021 4:09 PM, Emily Shaffer wrote: > > + # Cache a pointer to the superproject's gitdir. This may have > > + # changed, so rewrite it unconditionally. Writes it to worktree > > + # if applicable, otherwise to local. > > + relative_gitdir="$(git rev-parse --path-format=relative \ > > + --prefix "${sm_path}" \ > > + --git-dir)" > > + > > + git -C "$sm_path" config --worktree \> + submodule.superprojectgitdir "$relative_gitdir" > > Ok, I see now why you care about the worktree config. The scenario you > are covering is something like moving a submodule within a worktree and > not wanting to change the relative path of other copies of that submodule > within other worktrees, yes? > > For commands such as 'init' and 'add', we don't have the possibility of > colliding with other worktrees because the submodule is being created > for the first time, so the relative path should be safe to place in the > non-worktree config. > > I do struggle with the fact that these are inconsistent across the > two commits. It makes me think that there should only be one way to > do it, and either the NEEDSWORK needs to be fixed now, or this line > shouldn't include --worktree. Much of this can depend on the reason > the worktree config exists for a submodule. I expect you have more > context than me, so could you help me understand? > > Moving to a different concern I am now realizing with this config: > What happens if a submodule moves, and then a user runs 'git checkout' > in the superproject to move it back? How do we make this config value > update across those movements? Is there a possibility of integrating > with unpack_trees() to notice that a submodule has moved and hence we > should update this config value? I think that switching from "sub worktree to super gitdir" to "sub gitdir to super gitdir" fixes this neatly - the gitdir-to-gitdir path will be identical regardless of worktree, so we can set it in the main submodule config (lose the '--worktree' arg). This also will fix the case you describe, moving around the submodule in the superproject's tree from checkout to checkout, as the gitdir will not move. Thanks a bunch for the review and sorry it took me so long to reply. v4 incoming. - Emily
diff --git a/git-submodule.sh b/git-submodule.sh index 4678378424..f98dcc16ae 100755 --- a/git-submodule.sh +++ b/git-submodule.sh @@ -648,6 +648,16 @@ cmd_update() fi fi + # Cache a pointer to the superproject's gitdir. This may have + # changed, so rewrite it unconditionally. Writes it to worktree + # if applicable, otherwise to local. + relative_gitdir="$(git rev-parse --path-format=relative \ + --prefix "${sm_path}" \ + --git-dir)" + + git -C "$sm_path" config --worktree \ + submodule.superprojectgitdir "$relative_gitdir" + if test -n "$recursive" then ( diff --git a/t/t7406-submodule-update.sh b/t/t7406-submodule-update.sh index f4f61fe554..c39821ba8e 100755 --- a/t/t7406-submodule-update.sh +++ b/t/t7406-submodule-update.sh @@ -1061,4 +1061,14 @@ test_expect_success 'submodule update --quiet passes quietness to fetch with a s ) ' +test_expect_success 'submodule update adds superproject gitdir to older repos' ' + (cd super && + git -C submodule config --unset submodule.superprojectGitdir && + git submodule update && + echo "../.git" >expect && + git -C submodule config submodule.superprojectGitdir >actual && + test_cmp expect actual + ) +' + test_done
A recorded hint path to the superproject's gitdir might be added during 'git submodule add', but in some cases - like submodules which were created before 'git submodule add' learned to record that info - it might be useful to update the hint. Let's do it during 'git submodule update', when we already have a handle to the superproject while calling operations on the submodules. Signed-off-by: Emily Shaffer <emilyshaffer@google.com> --- git-submodule.sh | 10 ++++++++++ t/t7406-submodule-update.sh | 10 ++++++++++ 2 files changed, 20 insertions(+)