Hi, Stolee On Tue, Apr 13, 2021 at 11:02 AM Derrick Stolee via GitGitGadget <gitgitgadget@xxxxxxxxx> wrote: > > This is the first "payoff" series in the sparse-index work. It makes 'git > status' and 'git add' very fast when a sparse-index is enabled on a > repository with cone-mode sparse-checkout (and a small populated set). > > This is based on ds/sparse-index-protections AND mt/add-rm-sparse-checkout. I just noticed that our ds/sparse-index-protections and mt/add-rm-sparse-checkout had a small semantic conflict. It didn't appear before, but it does now with this new series. ds/sparse-index-protections added `ensure_full_index()` guards before the loops that traverse over all cache entries. At the same time, mt/add-rm-sparse-checkout added yet another one of these loops, at `pathspec.c::find_pathspecs_matching_skip_worktree()`. Although the new place didn't get the `ensure_full_index()` guard, all of its callers (in `add` and `rm`) did call `ensure_full_index()` before calling it, so it was fine. However, patches 7 and 8 remove some of these protections in `add`s code. And, as a result, if "dir" is a sparse directory entry, `git add [--refresh] dir/file` no longer emits the warning added at mt/add-rm-sparse-checkout. Adding `ensure_full_index()` at `find_pathspecs_matching_skip_worktree()` fixes the problem. We have to consider the performance implications, but they _might_ be acceptable as we only call this function when a pathspec given to `add` or `rm` does not match any non-ignored file inside the sparse checkout. Additionally, the tests I added at t3705 won't catch this problem, even when running with GIT_TEST_SPARSE_INDEX=true :( That's because they don't set core.sparseCheckout and core.sparseCheckoutCone, they only set individual index entries with the SKIP_WORKTREE bit. And therefore, the index is always written fully. Perhaps, should I reroll my series using cone mode for these tests? (And a semi-related question: do you plan on adding GIT_TEST_SPARSE_INDEX=true to one of the CI jobs? )
