From: Elijah Newren <newren@xxxxxxxxx>
As noted in commit e7d7c73249 ("git-sparse-checkout: clarify
interactions with submodules", 2020-06-10), sparse-checkout cannot
remove submodules even if they don't match the sparsity patterns,
because doing so would risk data loss -- unpushed, uncommitted, or
untracked changes could all be lost. That commit also updated the
documentation to point out that submodule initialization state was a
parallel, orthogonal reason that entries in the index might not be
present in the working tree.
However, sparsity and submodule initialization weren't actually fully
orthogonal yet. The SKIP_WORKTREE handling in unpack_trees would
attempt to set the SKIP_WORKTREE bit on submodules when the submodule
did not match the sparsity patterns. This resulted in innocuous but
potentially alarming warning messages:
warning: unable to rmdir 'sha1collisiondetection': Directory not empty
It could also make things confusing since the entry would be marked as
SKIP_WORKTREE in the index but actually still be present in the working
tree:
$ git ls-files -t | grep sha1collisiondetection
S sha1collisiondetection
$ ls -al sha1collisiondetection/ | wc -l
13
Submodules have always been their own form of "partial checkout"
behavior, with their own "present or not" state determined by running
"git submodule [init|deinit|update]". Enforce that separation by having
the SKIP_WORKTREE logic not touch submodules and allow submodules to
continue using their own initialization state for determining if the
submodule is present.
Signed-off-by: Elijah Newren <newren@xxxxxxxxx>
---
unpack-trees: do not set SKIP_WORKTREE on submodules
Interactions between submodules and sparsity patterns have come up a few
times, with a certain edge case coming up multiple times recently:
should a submodule have the SKIP_WORKTREE bit set if the submodule path
does not match the sparsity patterns?[1][2][3].
Here I try to resolve the question, with the answer of 'no'.
This patch depends on en/sparse-with-submodule-doc lightly -- the commit
message in this patch references the commit from that other series. It
could possibly be considered an addition to that other topic, but
"sparse-with-submodule-doc" implies the other topic is only a
documentation change, whereas this patch involves a code change.
[1]
https://lore.kernel.org/git/pull.805.git.git.1591831009762.gitgitgadget@xxxxxxxxx/
> Since submodules may have unpushed changes or untracked files,
> removing them could result in data loss. Thus, changing sparse
> inclusion/exclusion rules will not cause an already checked out
> submodule to be removed from the working copy. Said another way, just
> as checkout will not cause submodules to be automatically removed or
> initialized even when switching between branches that remove or add
> submodules, using sparse-checkout to reduce or expand the scope of
> "interesting" files will not cause submodules to be automatically
> deinitialized or initialized either.
[2]
https://lore.kernel.org/git/CABPp-BE+BL3Nq=Co=-kNB_wr=6gqX8zcGwa0ega_pGBpk6xYsg@xxxxxxxxxxxxxx/
> If sparsity patterns would exclude a submodule that is initialized,
> sparse-checkout clearly can't remove the submodule. However, should it
> set the SKIP_WORKTREE bit for that submodule if it's not going to
> remove it?
[3]
https://lore.kernel.org/git/CABPp-BFJG7uFAZNidDPK2o7eHv-eYBsmcdnVxkOnKcZo7WzmFQ@xxxxxxxxxxxxxx/
>> Or if you don't deinitialize a submodule that is excluded by the
>> sparsity patterns (thus remaining in the working copy, anyway).
>
> This case requires more thought. If a submodule doesn't match the
> sparsity patterns, we already said elsewhere that sparse-checkout
> should not remove the submodule (since doing so would risk data loss).
> But do we set the SKIP_WORKTREE bit for it? Generally, sparse-checkout
> avoids removing files with modifications, and if it doesn't remove
> them it also doesn't set the SKIP_WORKTREE bit. For consistency,
> should sparse-checkout not set SKIP_WORKTREE for initialized
> submodules?
Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-git-809%2Fnewren%2Fsparse-submodule-no-skip-worktree-v1
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-git-809/newren/sparse-submodule-no-skip-worktree-v1
Pull-Request: https://github.com/git/git/pull/809
unpack-trees.c | 5 +++--
1 file changed, 3 insertions(+), 2 deletions(-)
diff --git a/unpack-trees.c b/unpack-trees.c
index 4be5fc30754..9922522b29d 100644
--- a/unpack-trees.c
+++ b/unpack-trees.c
@@ -1524,7 +1524,7 @@ static void mark_new_skip_worktree(struct pattern_list *pl,
int i;
/*
- * 1. Pretend the narrowest worktree: only unmerged entries
+ * 1. Pretend the narrowest worktree: only unmerged files and symlinks
* are checked out
*/
for (i = 0; i < istate->cache_nr; i++) {
@@ -1533,7 +1533,8 @@ static void mark_new_skip_worktree(struct pattern_list *pl,
if (select_flag && !(ce->ce_flags & select_flag))
continue;
- if (!ce_stage(ce) && !(ce->ce_flags & CE_CONFLICTED))
+ if (!ce_stage(ce) && !(ce->ce_flags & CE_CONFLICTED) &&
+ !S_ISGITLINK(ce->ce_mode))
ce->ce_flags |= skip_wt_flag;
else
ce->ce_flags &= ~skip_wt_flag;
base-commit: eebb51ba8cab97c0b3f3f18eaab7796803b8494b
--
gitgitgadget
