Re: [PATCH 06/27] checkout-index: ensure full index
flat view
From: Elijah Newren <hidden>
Date: 2021-03-17 17:51:27
On Tue, Mar 16, 2021 at 2:17 PM Derrick Stolee via GitGitGadget [off-list ref] wrote:
quoted hunk ↗ jump to hunk
From: Derrick Stolee <redacted> Before we iterate over all cache entries, ensure that the index is not sparse. This loop in checkout_all() might be safe to iterate over a sparse index, but let's put this protection here until it can be carefully tested. Signed-off-by: Derrick Stolee <redacted> --- builtin/checkout-index.c | 1 + 1 file changed, 1 insertion(+)diff --git a/builtin/checkout-index.c b/builtin/checkout-index.c index 023e49e271c2..ba31a036f193 100644 --- a/builtin/checkout-index.c +++ b/builtin/checkout-index.c@@ -119,6 +119,7 @@ static void checkout_all(const char *prefix, int prefix_length) int i, errs = 0; struct cache_entry *last_ce = NULL; + ensure_full_index(&the_index); for (i = 0; i < active_nr ; i++) { struct cache_entry *ce = active_cache[i]; if (ce_stage(ce) != checkout_stage
With the caveat in the commit message, this change looks okay, but checkout-index may be buggy regardless of the presence of ensure_full_index(). If ensure_full_index() really is needed here because it needs to operate on all SKIP_WORKTREE paths and not just leading directories, that's because it's writing all those SKIP_WORKTREE entries to the working tree. When it writes them to the working tree, is it clearing the SKIP_WORKTREE bit? If not, we're in a bit of a pickle... Might be nice to add a /* TODO: audit if this is needed; if it is, we may have other bugs... */ or something like that. But then again, perhaps you're considering all uses of ensure_full_index() to be need-to-be-reaudited codepaths? If so, and we determine we really do need one and want to keep it indefinitely, will we mark those with a comment about why it's considered correct? I just want a way to know what still needs to be audited and what doesn't without doing a lot of history spelunking...