Re: [PATCH v5 7/8] reset: make --mixed sparse-aware

From: Victoria Dye <redacted>

Remove the `ensure_full_index` guard on `read_from_tree` and update `git
reset --mixed` to ensure it can use sparse directory index entries wherever
possible. Sparse directory entries are reset use `diff_tree_oid`, which
requires `change` and `add_remove` functions to process the internal
contents of the sparse directory. The `recursive` diff option handles cases
in which `reset --mixed` must diff/merge files that are nested multiple
levels deep in a sparse directory.

The use of pathspecs with `git reset --mixed` introduces scenarios in which
internal contents of sparse directories may be matched by the pathspec. In
order to reset *all* files in the repo that may match the pathspec, the
following conditions on the pathspec require index expansion before
performing the reset:

* "magic" pathspecs
* wildcard pathspecs that do not match only in-cone files or entire sparse
  directories
* literal pathspecs matching something outside the sparse checkout
  definition

Helped-by: Elijah Newren [off-list ref]
Signed-off-by: Victoria Dye <redacted>
---
 builtin/reset.c                          | 78 +++++++++++++++++++++++-
 t/t1092-sparse-checkout-compatibility.sh | 17 ++++++
 2 files changed, 93 insertions(+), 2 deletions(-)

diff --git a/builtin/reset.c b/builtin/reset.c
index 0ac0de7dc97..60517e7e1d6 100644
--- a/builtin/reset.c
+++ b/builtin/reset.c

@@ -148,7 +148,9 @@ static void update_index_from_diff(struct diff_queue_struct *q,
                 * If the file 1) corresponds to an existing index entry with
                 * skip-worktree set, or 2) does not exist in the index but is
                 * outside the sparse checkout definition, add a skip-worktree bit
-                * to the new index entry.
+                * to the new index entry. Note that a sparse index will be expanded
+                * if this entry is outside the sparse cone - this is necessary
+                * to properly construct the reset sparse directory.
                 */
                pos = cache_name_pos(one->path, strlen(one->path));
                if ((pos >= 0 && ce_skip_worktree(active_cache[pos])) ||

@@ -166,6 +168,73 @@ static void update_index_from_diff(struct diff_queue_struct *q,
        }
 }

+static int pathspec_needs_expanded_index(const struct pathspec *pathspec)
+{
+       unsigned int i, pos;
+       int res = 0;
+       char *skip_worktree_seen = NULL;
+
+       /*
+        * When using a magic pathspec, assume for the sake of simplicity that
+        * the index needs to be expanded to match all matchable files.
+        */
+       if (pathspec->magic)
+               return 1;
+
+       for (i = 0; i < pathspec->nr; i++) {
+               struct pathspec_item item = pathspec->items[i];
+
+               /*
+                * If the pathspec item has a wildcard, the index should be expanded
+                * if the pathspec has the possibility of matching a subset of entries inside
+                * of a sparse directory (but not the entire directory).
+                *
+                * If the pathspec item is a literal path, the index only needs to be expanded
+                * if a) the pathspec isn't in the sparse checkout cone (to make sure we don't
+                * expand for in-cone files) and b) it doesn't match any sparse directories
+                * (since we can reset whole sparse directories without expanding them).
+                */
+               if (item.nowildcard_len < item.len) {
+                       for (pos = 0; pos < active_nr; pos++) {
+                               struct cache_entry *ce = active_cache[pos];
+
+                               if (!S_ISSPARSEDIR(ce->ce_mode))
+                                       continue;
+
+                               /*
+                                * If the pre-wildcard length is longer than the sparse
+                                * directory name and the sparse directory is the first
+                                * component of the pathspec, need to expand the index.
+                                */
+                               if (item.nowildcard_len > ce_namelen(ce) &&
+                                   !strncmp(item.original, ce->name, ce_namelen(ce))) {
+                                       res = 1;
+                                       break;
+                               }
+
+                               /*
+                                * If the pre-wildcard length is shorter than the sparse
+                                * directory and the pathspec does not match the whole
+                                * directory, need to expand the index.
+                                */
+                               if (!strncmp(item.original, ce->name, item.nowildcard_len) &&
+                                   wildmatch(item.original, ce->name, 0)) {
+                                       res = 1;
+                                       break;
+                               }
+                       }
+               } else if (!path_in_cone_mode_sparse_checkout(item.original, &the_index) &&
+                          !matches_skip_worktree(pathspec, i, &skip_worktree_seen))
+                       res = 1;
+
+               if (res > 0)
+                       break;
+       }
+
+       free(skip_worktree_seen);
+       return res;
+}
+
 static int read_from_tree(const struct pathspec *pathspec,
                          struct object_id *tree_oid,
                          int intent_to_add)

@@ -178,9 +247,14 @@ static int read_from_tree(const struct pathspec *pathspec,
        opt.format_callback = update_index_from_diff;
        opt.format_callback_data = &intent_to_add;
        opt.flags.override_submodule_config = 1;
+       opt.flags.recursive = 1;
        opt.repo = the_repository;
+       opt.change = diff_change;
+       opt.add_remove = diff_addremove;
+
+       if (pathspec->nr && the_index.sparse_index && pathspec_needs_expanded_index(pathspec))
+               ensure_full_index(&the_index);

-       ensure_full_index(&the_index);
        if (do_diff_cache(tree_oid, &opt))
                return 1;
        diffcore_std(&opt);

diff --git a/t/t1092-sparse-checkout-compatibility.sh b/t/t1092-sparse-checkout-compatibility.sh
index 5664ff8f039..44d5e11c762 100755
--- a/t/t1092-sparse-checkout-compatibility.sh
+++ b/t/t1092-sparse-checkout-compatibility.sh

@@ -781,11 +781,28 @@ test_expect_success 'sparse-index is not expanded' '
                ensure_not_expanded reset --hard $ref || return 1
        done &&

+       ensure_not_expanded reset --mixed base &&
        ensure_not_expanded reset --hard update-deep &&
        ensure_not_expanded reset --keep base &&
        ensure_not_expanded reset --merge update-deep &&
        ensure_not_expanded reset --hard &&

+       ensure_not_expanded reset base -- deep/a &&
+       ensure_not_expanded reset base -- nonexistent-file &&
+       ensure_not_expanded reset deepest -- deep &&
+
+       # Although folder1 is outside the sparse definition, it exists as a
+       # directory entry in the index, so the pathspec will not force the
+       # index to be expanded.
+       ensure_not_expanded reset deepest -- folder1 &&
+       ensure_not_expanded reset deepest -- folder1/ &&
+
+       # Wildcard identifies only in-cone files, no index expansion
+       ensure_not_expanded reset deepest -- deep/\* &&
+
+       # Wildcard identifies only full sparse directories, no index expansion
+       ensure_not_expanded reset deepest -- folder\* &&
+