archive mirror
 help / color / mirror / Atom feed
From: "Elijah Newren via GitGitGadget" <>
	Elijah Newren <>,
	Elijah Newren <>
Subject: [PATCH] unpack-trees: do not set SKIP_WORKTREE on submodules
Date: Wed, 17 Jun 2020 01:21:24 +0000	[thread overview]
Message-ID: <> (raw)

From: Elijah Newren <>

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

    $ git ls-files -t | grep sha1collisiondetection
    S sha1collisiondetection
    $ ls -al sha1collisiondetection/ | wc -l

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 <>
    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.
    > 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.
    > 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?
    >> 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?

Fetch-It-Via: git fetch pr-git-809/newren/sparse-submodule-no-skip-worktree-v1

 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))
-		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;
 			ce->ce_flags &= ~skip_wt_flag;

base-commit: eebb51ba8cab97c0b3f3f18eaab7796803b8494b

             reply	other threads:[~2020-06-17  1:21 UTC|newest]

Thread overview: 8+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-06-17  1:21 Elijah Newren via GitGitGadget [this message]
2020-06-17 17:58 ` Matheus Tavares Bernardino
2020-06-18  0:24   ` Elijah Newren
2020-06-18 14:34     ` Matheus Tavares Bernardino
2020-06-18 19:19       ` Elijah Newren
2020-06-18 20:29         ` Matheus Tavares Bernardino
2020-06-18  1:51 How soon
2020-10-06  0:06 Luv MeZza

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \ \ \ \ \ \ \
    --subject='Re: [PATCH] unpack-trees: do not set SKIP_WORKTREE on submodules' \

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).