Re: [PATCH 06/13] add: skip paths that are outside sparse-checkout cone

On 9/8/2021 3:46 PM, Derrick Stolee wrote:

On 8/27/2021 5:13 PM, Matheus Tavares wrote:

quoted

On Tue, Aug 24, 2021 at 6:54 PM Derrick Stolee via GitGitGadget [off-list ref] wrote:

quoted

Subject: [PATCH 06/13] add: skip paths that are outside sparse-checkout cone

Perhaps this could be "skip _tracked_ paths that ..." (to help
differentiate the end goal of this patch from the previous one).

quoted

diff --git a/pathspec.c b/pathspec.c
index 44306fdaca2..0e6e60fdc5a 100644
--- a/pathspec.c
+++ b/pathspec.c

@@ -39,7 +39,8 @@ void add_pathspec_matches_against_index(const struct pathspec *pathspec,
                return;
        for (i = 0; i < istate->cache_nr; i++) {
                const struct cache_entry *ce = istate->cache[i];
-               if (sw_action == PS_IGNORE_SKIP_WORKTREE && ce_skip_worktree(ce))
+               if (sw_action == PS_IGNORE_SKIP_WORKTREE &&
+                   (ce_skip_worktree(ce) || !path_in_sparse_checkout(ce->name, istate)))

Hmm, even though we skip the sparse paths here, cmd_add() will call
add_files_to_cache() at the end and still update these paths in the
index. I think there are two ways to fix this. We could either change
run_diff_files() to skip these paths (but I don't know how other callers
of this functions want to handle this, so maybe this needs to hide
behind an option flag):

You are absolutely right to point this out. I had missed this interaction.
But, this is also already broken. The patch below adds a check to show that
'git add' does not add the sparse_entry, but it does (even when applied
before any patch in this series). That is: all the modified tests fail
after this change. I'll work to fix this issue before the next version of
this series.

Of course, the reason for the failures is because the 'sparse_entry' is
staged as part of setup_sparse_entry. Not a bug.

This makes things more difficult to test, so I'll look around for an
alternative way to test that 'git add' is behaving correctly.

Thanks,
-Stolee