Re: [PATCH 06/13] add: skip paths that are outside sparse-checkout cone
From: Matheus Tavares <hidden>
Date: 2021-08-27 21:13:20
Subsystem:
the rest · Maintainer:
Linus Torvalds
On Tue, Aug 24, 2021 at 6:54 PM Derrick Stolee via GitGitGadget [off-list ref] wrote:
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 hunk ↗ jump to hunk
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):
diff --git a/diff-lib.c b/diff-lib.c
index f9eadc4fc1..4245d7ead5 100644
--- a/diff-lib.c
+++ b/diff-lib.c@@ -198,7 +198,8 @@ int run_diff_files(struct rev_info *revs, unsigned int option) continue; } - if (ce_uptodate(ce) || ce_skip_worktree(ce)) + if (ce_uptodate(ce) || ce_skip_worktree(ce) || + !path_in_sparse_checkout(ce->name, istate)) continue; /*
Or we could change the callback in add itself:
diff --git a/builtin/add.c b/builtin/add.c
index f675bdeae4..3d7762aac2 100644
--- a/builtin/add.c
+++ b/builtin/add.c@@ -94,6 +94,10 @@ static void update_callback(struct diff_queue_struct *q, for (i = 0; i < q->nr; i++) { struct diff_filepair *p = q->queue[i]; const char *path = p->one->path; + + if (!path_in_sparse_checkout(path, &the_index)) + continue; + switch (fix_unmerged_status(p, data)) { default: die(_("unexpected diff status %c"), p->status);
I believe we also need to update a few other places to use the `(ce_skip_worktree(ce) || !path_in_sparse_checkout())` logic in order to avoid updating tracked sparse paths: chmod_pathspec() for add's --chmod option, renormalize_tracked_files() for --renormalize, and read-cache.c:refresh_index() for --refresh.
continue;
ce_path_match(istate, ce, pathspec, seen);
}Hmm, don't we also want to update find_pathspecs_matching_skip_worktree() in this patch to use path_in_sparse_checkout()? I see you did that in patch 8, but I think this should be together with this current patch as, without it, we stop adding tracked sparse paths but we print no error/advice message about it.