Sparse Index

From: Derrick Stolee <[email protected]>

We will mark an in-memory index_state as having sparse directory entries with the sparse_index bit. These currently cannot exist, but we will add a mechanism for collapsing a full index to a sparse one in a later change. That will happen at write time, so we must first allow parsing the format before writing it.

Commands or methods that require a full index in order to operate can call ensure_full_index() to expand that index in-memory. This requires parsing trees using that index's repository.

Sparse directory entries have a specific 'ce_mode' value. The macro S_ISSPARSEDIR(ce) can check if a cache_entry 'ce' has this type. This ce_mode is not possible with the existing index formats, so we don't also verify all properties of a sparse-directory entry, which are:

1. ce->ce_mode == 01000755

2. ce->flags & CE_SKIP_WORKTREE is true

3. ce->name[ce->namelen - 1] == '/' (ends in dir separator)

4. ce->oid references a tree object.

These are all semi-enforced in ensure_full_index() to some extent. Any deviation will cause a warning at minimum or a failure in the worst case.

Signed-off-by Derrick Stolee <[email protected]>

---
 cache.h        | 11 +++++-
 read-cache.c   |  9 +++++
 sparse-index.c | 94 +++++++++++++++++++++++++++++++++++++++++++++++++-
 sparse-index.h |  1 +
 4 files changed, 113 insertions(+), 2 deletions(-)

cache.h +0 -0

index f9c7a603841..884046ca5b8 100644

cache.h +7 -1

+
 /* Forward structure decls */
 struct pathspec;
 struct child_process;

@@ -249,6 +253,8 @@ static inline unsigned int create_ce_mode(unsigned int mode)
 {
        if (S_ISLNK(mode))
                return S_IFLNK;
+       if (mode == SPARSE_DIR_MODE)
+               return CE_MODE_SPARSE_DIRECTORY;
        if (S_ISDIR(mode) || S_ISGITLINK(mode))
                return S_IFGITLINK;
        return S_IFREG | ce_permissions(mode);

@@ -319,7 +325,8 @@ struct index_state {
                 drop_cache_tree : 1,
                 updated_workdir : 1,
                 updated_skipworktree : 1,
-                fsmonitor_has_run_once : 1;
+                fsmonitor_has_run_once : 1,
+                sparse_index : 1;
        struct hashmap name_hash;
        struct hashmap dir_hash;
        struct object_id oid;

@@ -721,6 +728,8 @@ int read_index_from(struct index_state *, const char *path,
                    const char *gitdir);
 int is_index_unborn(struct index_state *);
 
+void ensure_full_index(struct index_state *istate);
+
 /* For use with `write_locked_index()`. */
 #define COMMIT_LOCK            (1 << 0)
 #define SKIP_IF_UNCHANGED      (1 << 1)

read-cache.c +9 -0

index ecf6f689940..1097ecbf132 100644
@@ -101,6 +101,9 @@ static const char *alternate_index_output;
 
 static void set_index_entry(struct index_state *istate, int nr, struct cache_entry *ce)
 {
+       if (S_ISSPARSEDIR(ce))
+               istate->sparse_index = 1;
+
        istate->cache[nr] = ce;
        add_name_hash(istate, ce);
 }

@@ -2255,6 +2258,12 @@ int do_read_index(struct index_state *istate, const char *path, int must_exist)
        trace2_data_intmax("index", the_repository, "read/cache_nr",
                           istate->cache_nr);
 
+       if (!istate->repo)
+               istate->repo = the_repository;
+       prepare_repo_settings(istate->repo);
+       if (istate->repo->settings.command_requires_full_index)
+               ensure_full_index(istate);
+
        return istate->cache_nr;
 
 unmap:

sparse-index.c +77 -1

index 82183ead563..1e70244dc13 100644
@@ -1,8 +1,100 @@
 #include "cache.h"
 #include "repository.h"
 #include "sparse-index.h"
+#include "tree.h"
+#include "pathspec.h"
+#include "trace2.h"
+
+static void set_index_entry(struct index_state *istate, int nr, struct cache_entry *ce)
+{
+       ALLOC_GROW(istate->cache, nr + 1, istate->cache_alloc);
+
+       istate->cache[nr] = ce;
+       add_name_hash(istate, ce);
+}
+
+static int add_path_to_index(const struct object_id *oid,
+                               struct strbuf *base, const char *path,
+                               unsigned int mode, int stage, void *context)
+{
+       struct index_state *istate = (struct index_state *)context;
+       struct cache_entry *ce;
+       size_t len = base->len;
+
+       if (S_ISDIR(mode))
+               return READ_TREE_RECURSIVE;
+
+       strbuf_addstr(base, path);
+
+       ce = make_cache_entry(istate, mode, oid, base->buf, 0, 0);
+       ce->ce_flags |= CE_SKIP_WORKTREE;
+       set_index_entry(istate, istate->cache_nr++, ce);
+
+       strbuf_setlen(base, len);
+       return 0;
+}
 
 void ensure_full_index(struct index_state *istate)
 {
-       /* intentionally left blank */
+       int i;
+       struct index_state *full;
+
+       if (!istate || !istate->sparse_index)
+               return;
+
+       if (!istate->repo)
+               istate->repo = the_repository;
+
+       trace2_region_enter("index", "ensure_full_index", istate->repo);
+
+       /* initialize basics of new index */
+       full = xcalloc(1, sizeof(struct index_state));
+       memcpy(full, istate, sizeof(struct index_state));
+
+       /* then change the necessary things */
+       full->sparse_index = 0;
+       full->cache_alloc = (3 * istate->cache_alloc) / 2;
+       full->cache_nr = 0;
+       ALLOC_ARRAY(full->cache, full->cache_alloc);
+
+       for (i = 0; i < istate->cache_nr; i++) {
+               struct cache_entry *ce = istate->cache[i];
+               struct tree *tree;
+               struct pathspec ps;
+
+               if (!S_ISSPARSEDIR(ce)) {
+                       set_index_entry(full, full->cache_nr++, ce);
+                       continue;
+               }
+               if (!(ce->ce_flags & CE_SKIP_WORKTREE))
+                       warning(_("index entry is a directory, but not sparse (%08x)"),
+                               ce->ce_flags);
+
+               /* recursively walk into cd->name */
+               tree = lookup_tree(istate->repo, &ce->oid);
+
+               memset(&ps, 0, sizeof(ps));
+               ps.recursive = 1;
+               ps.has_wildcard = 1;
+               ps.max_depth = -1;
+
+               read_tree_recursive(istate->repo, tree,
+                                   ce->name, strlen(ce->name),
+                                   0, &ps,

sparse-index.c +6 -0

+       istate->cache_nr = full->cache_nr;
+       istate->cache_alloc = full->cache_alloc;
+
+       free(full);
+
+       trace2_region_leave("index", "ensure_full_index", istate->repo);
 }

sparse-index.h +0 -0

index 8dda92032e2..a2777dcac59 100644

sparse-index.h +0 -1

 #endif
\ No newline at end of file
--

gitgitgadget

On Mon, Jan 25, 2021 at 9:42 AM Derrick Stolee via GitGitGadget <[email protected]> wrote:

Quoted 6 lines

Sparse directory entries have a specific 'ce_mode' value. The macro S_ISSPARSEDIR(ce) can check if a cache_entry 'ce' has this type. This ce_mode is not possible with the existing index formats, so we don't also verify all properties of a sparse-directory entry, which are:

1. ce->ce_mode == 01000755

This is a weird number. What's the reason for choosing it? It looks deceptively close to 0100755, normal executable files, but has the extra 0, meaning that ce->ce_mode & S_IFMT is 0, suggesting it has no file type.

Since it's a directory, why not use S_IFDIR (040000)?

(GITLINK does use the weird 0160000 value, but it happens to be S_IFLNK | S_IFDIR == 0120000 | 040000, which conveys "it's both a directory and a symlink")

Quoted 1 line

2. ce->flags & CE_SKIP_WORKTREE is true

Makes sense.

Quoted 1 line

3. ce->name[ce->namelen - 1] == '/' (ends in dir separator)

Is there a particular reason for this? I'm used to seeing names without the trailing slash, both in the index and in tree objects. I don't know enough to be for or against this idea; just curious at this point.

Quoted 1 line

4. ce->oid references a tree object.

Makes sense...but doesn't that suggest we'd want to use ce->ce_mode = 040000?

cache.h struct cache_entry { · line 208

Quoted 2 lines

cache.h +2 -0

+#define CE_MODE_SPARSE_DIRECTORY 01000755
+#define SPARSE_DIR_MODE 0100

Another magic value. Feels like the commit message should reference this one and why it was picked. Seems odd to me, and possibly problematic to re-use file permission bits that might collide with files recorded by really old versions of git. Maybe that's not a concern, though.

Quoted 1 line

+#define S_ISSPARSEDIR(m) ((m)->ce_mode == CE_MODE_SPARSE_DIRECTORY)

Should the special sauce apply to ce_flags rather than ce_mode? Thus, instead of an S_ISSPARSEDIR, perhaps have a ce_sparse_dir macro (similar to ce_skip_worktree) based on a CE_SPARSE_DIR value (similar to CE_SKIP_WORKTREE)?

Or, alternatively, do we need a single special state here? Could we check for a combination of ce_mode == 040000 && ce_skip_worktree(ce)?

sparse-index.c line 93

Quoted 4 lines

sparse-index.c +4 -0

+       /* Copy back into original index. */
+       memcpy(&istate->name_hash, &full->name_hash, sizeof(full->name_hash));
+       istate->sparse_index = 0;
+       istate->cache = full->cache;

Haven't you leaked the original istate->cache here?

Quoted 2 lines · Elijah Newren

On Mon, Jan 25, 2021 at 9:42 AM Derrick Stolee via GitGitGadget <[email protected]> wrote:

...

Quoted 12 lines

Quoted 6 lines

Sparse directory entries have a specific 'ce_mode' value. The macro S_ISSPARSEDIR(ce) can check if a cache_entry 'ce' has this type. This ce_mode is not possible with the existing index formats, so we don't also verify all properties of a sparse-directory entry, which are:

1. ce->ce_mode == 01000755

This is a weird number. What's the reason for choosing it? It looks deceptively close to 0100755, normal executable files, but has the extra 0, meaning that ce->ce_mode & S_IFMT is 0, suggesting it has no file type.

Quoted 5 lines

Since it's a directory, why not use S_IFDIR (040000)?

(GITLINK does use the weird 0160000 value, but it happens to be S_IFLNK | S_IFDIR == 0120000 | 040000, which conveys "it's both a directory and a symlink")

I forget how exactly I came up with these magic constants, but then completely forgot to think of them critically because I haven't had to look at them in a while. They _are_ important, especially because these values affect the file format itself.

I'll think harder on this before submitting a series intended for merging.

Quoted 10 lines

Quoted 1 line

2. ce->flags & CE_SKIP_WORKTREE is true

Makes sense.

Quoted 1 line

3. ce->name[ce->namelen - 1] == '/' (ends in dir separator)

Is there a particular reason for this? I'm used to seeing names without the trailing slash, both in the index and in tree objects. I don't know enough to be for or against this idea; just curious at this point.

It's yet another way to distinguish directories from files, but there are cases where we do string searches up to a prefix, and having these directory separators did help, IIRC.

Quoted 3 lines

Quoted 1 line

4. ce->oid references a tree object.

Makes sense...but doesn't that suggest we'd want to use ce->ce_mode = 040000?

...

Quoted 16 lines

cache.h struct cache_entry { · line 208

Quoted 2 lines

cache.h +2 -0

+#define CE_MODE_SPARSE_DIRECTORY 01000755
+#define SPARSE_DIR_MODE 0100

Another magic value. Feels like the commit message should reference this one and why it was picked. Seems odd to me, and possibly problematic to re-use file permission bits that might collide with files recorded by really old versions of git. Maybe that's not a concern, though.

Quoted 1 line

+#define S_ISSPARSEDIR(m) ((m)->ce_mode == CE_MODE_SPARSE_DIRECTORY)

Should the special sauce apply to ce_flags rather than ce_mode? Thus, instead of an S_ISSPARSEDIR, perhaps have a ce_sparse_dir macro (similar to ce_skip_worktree) based on a CE_SPARSE_DIR value (similar to CE_SKIP_WORKTREE)?

Quoted 2 lines

Or, alternatively, do we need a single special state here? Could we check for a combination of ce_mode == 040000 && ce_skip_worktree(ce)?

The intention was that ce_mode be a unique value that could only be assigned to a directory entry, which would then by necessity be sparse. Checking both ce_mode and ce_flags seemed wasteful with the given assumptions

...

Quoted 6 lines

sparse-index.c line 93

Quoted 4 lines

sparse-index.c +4 -0

+       /* Copy back into original index. */
+       memcpy(&istate->name_hash, &full->name_hash, sizeof(full->name_hash));
+       istate->sparse_index = 0;
+       istate->cache = full->cache;

Haven't you leaked the original istate->cache here?

Yes, seems so. Will fix.

Thanks,
-Stolee

Quoted 1 line

1. ce->ce_mode == 01000755

This is a weird number. What's the reason for choosing it? It looks deceptively close to 0100755, normal executable files, but has the extra 0, meaning that ce->ce_mode & S_IFMT is 0, suggesting it has no file type.

Since it's a directory, why not use S_IFDIR (040000)?

(GITLINK does use the weird 0160000 value, but it happens to be S_IFLNK | S_IFDIR == 0120000 | 040000, which conveys "it's both a directory and a symlink")

From: Derrick Stolee <[email protected]>

Add a new 'sparse-index' repo alongside the 'full-checkout' and 'sparse-checkout' repos in t1092-sparse-checkout-compatibility.sh. Also add run_on_sparse and test_sparse_match helpers. These helpers will be used when the sparse index is implemented.

Signed-off-by Derrick Stolee <[email protected]>

---
 t/t1092-sparse-checkout-compatibility.sh | 29 ++++++++++++++++++++----
 1 file changed, 25 insertions(+), 4 deletions(-)

t/t1092-sparse-checkout-compatibility.sh +0 -0

index 8cd3e5a8d22..8876eae0fe3 100755

t/t1092-sparse-checkout-compatibility.sh +24 -5

                cd initial-repo &&
                echo a >a &&
                echo "after deep" >e &&

@@ -87,23 +88,32 @@ init_repos () {
 
        cp -r initial-repo sparse-checkout &&
        git -C sparse-checkout reset --hard &&
-       git -C sparse-checkout sparse-checkout init --cone &&
+
+       cp -r initial-repo sparse-index &&
+       git -C sparse-index reset --hard &&
 
        # initialize sparse-checkout definitions
-       git -C sparse-checkout sparse-checkout set deep
+       git -C sparse-checkout sparse-checkout init --cone &&
+       git -C sparse-checkout sparse-checkout set deep &&
+       GIT_TEST_SPARSE_INDEX=1 git -C sparse-index sparse-checkout init --cone &&
+       GIT_TEST_SPARSE_INDEX=1 git -C sparse-index sparse-checkout set deep
 }
 
 run_on_sparse () {
        (
                cd sparse-checkout &&
-               $* >../sparse-checkout-out 2>../sparse-checkout-err
+               GIT_TEST_SPARSE_INDEX=0 $* >../sparse-checkout-out 2>../sparse-checkout-err
+       ) &&
+       (
+               cd sparse-index &&
+               $* >../sparse-index-out 2>../sparse-index-err
        )
 }
 
 run_on_all () {
        (
                cd full-checkout &&
-               $* >../full-checkout-out 2>../full-checkout-err
+               GIT_TEST_SPARSE_INDEX=0 $* >../full-checkout-out 2>../full-checkout-err
        ) &&
        run_on_sparse $*
 }

@@ -114,6 +124,17 @@ test_all_match () {
        test_cmp full-checkout-err sparse-checkout-err
 }
 
+test_sparse_match () {
+       run_on_sparse $* &&
+       test_cmp sparse-checkout-out sparse-index-out &&
+       test_cmp sparse-checkout-err sparse-index-err
+}
+
+test_expect_success 'expanded in-memory index matches full index' '
+       init_repos &&
+       test_sparse_match test-tool read-cache --expand --table-no-stat
+'
+
 test_expect_success 'status with options' '
        init_repos &&
        test_all_match git status --porcelain=v2 &&
--

gitgitgadget

Quoted 13 lines · Derrick Stolee via GitGitGadget

From: Derrick Stolee <[email protected]>

Add a new 'sparse-index' repo alongside the 'full-checkout' and 'sparse-checkout' repos in t1092-sparse-checkout-compatibility.sh. Also add run_on_sparse and test_sparse_match helpers. These helpers will be used when the sparse index is implemented.

Signed-off-by Derrick Stolee <[email protected]>

---
 t/t1092-sparse-checkout-compatibility.sh | 29 ++++++++++++++++++++----
 1 file changed, 25 insertions(+), 4 deletions(-)

t/t1092-sparse-checkout-compatibility.sh test_description='compare full workdir to sparse workdir' · line 10

Quoted 9 lines

t/t1092-sparse-checkout-compatibility.sh +1 -0

index 8cd3e5a8d22..8876eae0fe3 100755
@@ -7,6 +7,7 @@ test_description='compare full workdir to sparse workdir'
 test_expect_success 'setup' '
        git init initial-repo &&
        (
+               (GIT_TEST_SPARSE_INDEX=0 && export GIT_TEST_SPARSE_INDEX) &&

I thought parentheses started a subshell; once the subshell ends, wouldn't the setting of GIT_TEST_SPARSE_INDEX be thrown away?

Quoted 15 lines · Elijah Newren

Quoted 13 lines · Derrick Stolee via GitGitGadget

From: Derrick Stolee <[email protected]>

Add a new 'sparse-index' repo alongside the 'full-checkout' and 'sparse-checkout' repos in t1092-sparse-checkout-compatibility.sh. Also add run_on_sparse and test_sparse_match helpers. These helpers will be used when the sparse index is implemented.

Signed-off-by Derrick Stolee <[email protected]>

---
 t/t1092-sparse-checkout-compatibility.sh | 29 ++++++++++++++++++++----
 1 file changed, 25 insertions(+), 4 deletions(-)

Quoted 12 lines

t/t1092-sparse-checkout-compatibility.sh test_description='compare full workdir to sparse workdir' · line 10

Quoted 9 lines

t/t1092-sparse-checkout-compatibility.sh +1 -0

index 8cd3e5a8d22..8876eae0fe3 100755
@@ -7,6 +7,7 @@ test_description='compare full workdir to sparse workdir'
 test_expect_success 'setup' '
        git init initial-repo &&
        (
+               (GIT_TEST_SPARSE_INDEX=0 && export GIT_TEST_SPARSE_INDEX) &&

I thought parentheses started a subshell; once the subshell ends, wouldn't the setting of GIT_TEST_SPARSE_INDEX be thrown away?

I think the "export" specifically pushes the setting out of the first level of subshell. This is the recommendation that comes up

if one runs

        export GIT_TEST_SPARSE_INDEX=1 &&

inside a test on macOS, since this isn't completely portable.

From: Derrick Stolee <[email protected]>

This table is helpful for discovering data in the index to ensure it is being written correctly, especially as we build and test the sparse-index.

To make the option parsing slightly more robust, wrap the string comparisons in a loop adapted from test-dir-iterator.c.

Care must be taken with the final check for the 'cnt' variable. We continue the expectation that the numerical value is the final argument.

Signed-off-by Derrick Stolee <[email protected]>

---
 t/helper/test-read-cache.c | 49 ++++++++++++++++++++++++++++++++++----
 1 file changed, 44 insertions(+), 5 deletions(-)

t/helper/test-read-cache.c +0 -0

index 244977a29bd..cd7d106a675 100644
@@ -2,18 +2,55 @@
 #include "cache.h"
 #include "config.h"
 

t/helper/test-read-cache.c +23 -6

+       printf("%s ", oid_to_hex(&ce->oid));
+
+       printf("%s\n", ce->name);
+}
+
+static void print_cache(struct index_state *cache)
+{
+       int i;
+       for (i = 0; i < the_index.cache_nr; i++)
+               print_cache_entry(the_index.cache[i]);
+}
+
 int cmd__read_cache(int argc, const char **argv)
 {
        int i, cnt = 1;
        const char *name = NULL;
+       int table = 0;
 
-       if (argc > 1 && skip_prefix(argv[1], "--print-and-refresh=", &name)) {
-               argc--;
-               argv++;
+       for (++argv, --argc; *argv && starts_with(*argv, "--"); ++argv, --argc) {
+               if (skip_prefix(*argv, "--print-and-refresh=", &name))
+                       continue;
+               if (!strcmp(*argv, "--table")) {
+                       table = 1;
+               }
        }
 
-       if (argc == 2)
-               cnt = strtol(argv[1], NULL, 0);
+       if (argc == 1)
+               cnt = strtol(argv[0], NULL, 0);
        setup_git_directory();
        git_config(git_default_config, NULL);
        for (i = 0; i < cnt; i++) {

@@ -30,6 +67,8 @@ int cmd__read_cache(int argc, const char **argv)
                               ce_uptodate(the_index.cache[pos]) ? "" : " not");
                        write_file(name, "%d\n", i);
                }
+               if (table)
+                       print_cache(&the_index);
                discard_cache();
        }
        return 0;
--

gitgitgadget

From: Derrick Stolee <[email protected]>

The next change will translate full indexes into sparse indexes at write time. The existing logic provides a way for every sparse index to be expanded to a full index at read time. However, there are cases where an index is written and then continues to be used in-memory to perform further updates.

unpack_trees() is frequently called after such a write. In particular, commands like 'git reset' do this double-update of the index.

Ensure that we have a full index when entering unpack_trees(), but only when command_requires_full_index is true. This is always true at the moment, but we will later relax that after unpack_trees() is updated to handle sparse directory entries.

Signed-off-by Derrick Stolee <[email protected]>

---
 unpack-trees.c | 7 +++++++
 1 file changed, 7 insertions(+)

unpack-trees.c +1 -0

index f5f668f532d..4dd99219073 100644
@@ -1567,6 +1567,7 @@ static int verify_absent(const struct cache_entry *,
  */
 int unpack_trees(unsigned len, struct tree_desc *t, struct unpack_trees_options *o)
 {
+       struct repository *repo = the_repository;
        int i, ret;
        static struct cache_entry *dfc;
        struct pattern_list pl;

unpack-trees.c +2 -1

+       }
+
        if (!core_apply_sparse_checkout || !o->update)
                o->skip_sparse_checkout = 1;
        if (!o->skip_sparse_checkout && !o->pl) {
--

gitgitgadget

From: Derrick Stolee <[email protected]>

As we modify the sparse-checkout definition, we perform index operations on a pattern_list that only exists in-memory. This allows easy backing out in case the index update fails.

However, if the index write itself cares about the sparse-checkout pattern set, we need access to that in-memory copy. Place a pointer to a 'struct pattern_list' in the index so we can access this on-demand. This will be used in the next change which uses the sparse-checkout definition to filter out directories that are outsie the sparse cone.

Signed-off-by Derrick Stolee <[email protected]>

---
 builtin/sparse-checkout.c | 17 ++++++++++-------
 cache.h                   |  2 ++
 2 files changed, 12 insertions(+), 7 deletions(-)

builtin/sparse-checkout.c +10 -7

index 2306a9ad98e..e00b82af727 100644
@@ -110,6 +110,8 @@ static int update_working_directory(struct pattern_list *pl)
        if (is_index_unborn(r->index))
                return UPDATE_SPARSITY_SUCCESS;
 
+       r->index->sparse_checkout_patterns = pl;
+
        memset(&o, 0, sizeof(o));
        o.verbose_update = isatty(2);
        o.update = 1;

@@ -138,6 +140,7 @@ static int update_working_directory(struct pattern_list *pl)
        else
                rollback_lock_file(&lock_file);
 
+       r->index->sparse_checkout_patterns = NULL;
        return result;
 }
 

@@ -517,19 +520,18 @@ static int modify_pattern_list(int argc, const char **argv, enum modify_type m)
 {
        int result;
        int changed_config = 0;
-       struct pattern_list pl;
-       memset(&pl, 0, sizeof(pl));
+       struct pattern_list *pl = xcalloc(1, sizeof(*pl));
 
        switch (m) {
        case ADD:
                if (core_sparse_checkout_cone)
-                       add_patterns_cone_mode(argc, argv, &pl);
+                       add_patterns_cone_mode(argc, argv, pl);
                else
-                       add_patterns_literal(argc, argv, &pl);
+                       add_patterns_literal(argc, argv, pl);
                break;
 
        case REPLACE:
-               add_patterns_from_input(&pl, argc, argv);
+               add_patterns_from_input(pl, argc, argv);
                break;
        }
 

@@ -539,12 +541,13 @@ static int modify_pattern_list(int argc, const char **argv, enum modify_type m)
                changed_config = 1;
        }
 
-       result = write_patterns_and_update(&pl);
+       result = write_patterns_and_update(pl);
 
        if (result && changed_config)
                set_config(MODE_NO_PATTERNS);
 
-       clear_pattern_list(&pl);
+       clear_pattern_list(pl);
+       free(pl);
        return result;
 }
 

cache.h +2 -1

index 884046ca5b8..b05341cc687 100644
@@ -311,6 +311,7 @@ static inline unsigned int canon_mode(unsigned int mode)
 struct split_index;
 struct untracked_cache;
 struct progress;
+struct pattern_list;
 
 struct index_state {
        struct cache_entry **cache;

@@ -336,6 +337,7 @@ struct index_state {
        struct mem_pool *ce_mem_pool;
        struct progress *progress;
        struct repository *repo;
+       struct pattern_list *sparse_checkout_patterns;
 };
 
 /* Name hashing */
--

gitgitgadget

Quoted 7 lines · Derrick Stolee via GitGitGadget

From: Derrick Stolee <[email protected]>

As we modify the sparse-checkout definition, we perform index operations on a pattern_list that only exists in-memory. This allows easy backing out in case the index update fails.

Quoted 5 lines

However, if the index write itself cares about the sparse-checkout pattern set, we need access to that in-memory copy. Place a pointer to a 'struct pattern_list' in the index so we can access this on-demand. This will be used in the next change which uses the sparse-checkout definition to filter out directories that are outsie the sparse cone.

s/outsie/outside/

Isn't this the same patch you put in your index cleanup series, or am I getting confused? It looks very familiar.

From: Derrick Stolee <[email protected]>

If we have a full index, then we can convert it to a sparse index by replacing directories outside of the sparse cone with sparse directory entries. The convert_to_sparse() method does this, when the situation is appropriate.

For now, we avoid converting the index to a sparse index if:

1. the index is split.
2. the index is already sparse.
3. sparse-checkout is disabled.
4. sparse-checkout does not use cone mode.

Finally, we currently limit the conversion to when the GIT_TEST_SPARSE_INDEX environment variable is enabled. A mode using Git config will be added in a later change.

The trickiest thing about this conversion is that we might not be able to mark a directory as a sparse directory just because it is outside the sparse cone. There might be unmerged files within that directory, so we need to look for those. Also, if there is some strange reason why a file is not marked with CE_SKIP_WORKTREE, then we should give up on converting that directory. There is still hope that some of its subdirectories might be able to convert to sparse, so we keep looking deeper.

The conversion process is assisted by the cache-tree extension. This is calculated from the full index if it does not already exist. We then abandon the cache-tree as it no longer applies to the newly-sparse index. Thus, this cache-tree will be recalculated in every sparse-full-sparse round-trip until we integrate the cache-tree extension with the sparse index.

We can compare the behavior of the sparse-index in t1092-sparse-checkout-compability.sh by using GIT_TEST_SPARSE_INDEX=1 when operating on the 'sparse-index' repo. We can also compare the two sparse repos directly, such as comparing their indexes (when expanded to full in the case of the 'sparse-index' repo). We also verify that the index is actually populated with sparse directory entries.

The 'checkout and reset (mixed)' test is marked for failure when comparing a sparse repo to a full repo, but we can compare the two sparse-checkout cases directly to ensure that we are not changing the behavior when using a sparse index.

Signed-off-by Derrick Stolee <[email protected]>

---
 cache-tree.c                             |   3 +
 read-cache.c                             |  18 ++-
 sparse-index.c                           | 139 +++++++++++++++++++++++
 t/t1092-sparse-checkout-compatibility.sh |  63 +++++++++-
 4 files changed, 218 insertions(+), 5 deletions(-)

cache-tree.c +3 -0

index 2fb483d3c08..5f07a39e501 100644
@@ -6,6 +6,7 @@
 #include "object-store.h"
 #include "replace-object.h"
 #include "promisor-remote.h"
+#include "sparse-index.h"
 
 #ifndef DEBUG_CACHE_TREE
 #define DEBUG_CACHE_TREE 0

@@ -442,6 +443,8 @@ int cache_tree_update(struct index_state *istate, int flags)
        if (i)
                return i;
 
+       ensure_full_index(istate);
+
        if (!istate->cache_tree)
                istate->cache_tree = cache_tree();
 

read-cache.c +1 -0

index 1097ecbf132..0522260416e 100644
@@ -25,6 +25,7 @@
 #include "fsmonitor.h"
 #include "thread-utils.h"
 #include "progress.h"
+#include "sparse-index.h"
 
 /* Mask for the name length in ce_flags in the on-disk index */
 

read-cache.c +8 -0

                } else if (c == '\\' && protect_ntfs) {
                        if (is_ntfs_dotgit(path))
                                return 0;

@@ -3062,6 +3070,13 @@ static int do_write_locked_index(struct index_state *istate, struct lock_file *l
 {
        int ret;
 
+       ret = convert_to_sparse(istate);
+
+       if (ret) {
+               warning(_("failed to convert to a sparse-index"));
+               return ret;
+       }
+
        /*
         * TODO trace2: replace "the_repository" with the actual repo instance
         * that is associated with the given "istate".

@@ -3165,6 +3180,7 @@ static int write_shared_index(struct index_state *istate,
        int ret;
 
        move_cache_to_base_index(istate);
+       convert_to_sparse(istate);
 
        trace2_region_enter_printf("index", "shared/do_write_index",
                                   the_repository, "%s", (*temp)->filename.buf);

sparse-index.c +33 -0

index 1e70244dc13..d8f1a5a13d7 100644
@@ -4,6 +4,145 @@
 #include "tree.h"
 #include "pathspec.h"
 #include "trace2.h"
+#include "cache-tree.h"
+#include "config.h"
+#include "dir.h"
+#include "fsmonitor.h"
+
+static struct cache_entry *construct_sparse_dir_entry(
+                               struct index_state *istate,
+                               const char *sparse_dir,
+                               struct cache_tree *tree)
+{
+       struct cache_entry *de;
+
+       de = make_cache_entry(istate, SPARSE_DIR_MODE, &tree->oid, sparse_dir, 0, 0);
+
+       de->ce_flags |= CE_SKIP_WORKTREE;
+       return de;
+}
+
+/*
+ * Returns the number of entries "inserted" into the index.
+ */
+static int convert_to_sparse_rec(struct index_state *istate,
+                                int num_converted,
+                                int start, int end,
+                                const char *ct_path, size_t ct_pathlen,
+                                struct cache_tree *ct)
+{
+       int i, can_convert = 1;
+       int start_converted = num_converted;
+       enum pattern_match_result match;
+       int dtype;
+       struct strbuf child_path = STRBUF_INIT;
+       struct pattern_list *pl = istate->sparse_checkout_patterns;

sparse-index.c +7 -0

+       }
+
+       if (can_convert) {
+               struct cache_entry *se;
+               se = construct_sparse_dir_entry(istate, ct_path, ct);
+
+               istate->cache[num_converted++] = se;

sparse-index.c +14 -0

+                * entry.
+                */
+               base = ce->name + ct_pathlen;
+               slash = strchr(base, '/');
+
+               if (slash)
+                       pos = cache_tree_subtree_pos(ct, base, slash - base);
+
+               if (pos < 0) {
+                       istate->cache[num_converted++] = ce;
+                       i++;
+                       continue;
+               }
+

sparse-index.c +48 -0

+       }
+
+       strbuf_release(&child_path);
+       return num_converted - start_converted;
+}
+
+int convert_to_sparse(struct index_state *istate)
+{
+       if (istate->split_index || istate->sparse_index ||
+           !core_apply_sparse_checkout || !core_sparse_checkout_cone)
+               return 0;
+
+       /*
+        * For now, only create a sparse index with the
+        * GIT_TEST_SPARSE_INDEX environment variable. We will relax
+        * this once we have a proper way to opt-in (and later still,
+        * opt-out).
+        */
+       if (!git_env_bool("GIT_TEST_SPARSE_INDEX", 0))
+               return 0;
+
+       if (!istate->sparse_checkout_patterns) {
+               istate->sparse_checkout_patterns = xcalloc(1, sizeof(struct pattern_list));
+               if (get_sparse_checkout_patterns(istate->sparse_checkout_patterns) < 0)
+                       return 0;
+       }
+
+       if (!istate->sparse_checkout_patterns->use_cone_patterns) {
+               warning(_("attempting to use sparse-index without cone mode"));
+               return -1;
+       }
+
+       if (cache_tree_update(istate, 0)) {
+               warning(_("unable to update cache-tree, staying full"));
+               return -1;
+       }
+
+       remove_fsmonitor(istate);
+
+       trace2_region_enter("index", "convert_to_sparse", istate->repo);
+       istate->cache_nr = convert_to_sparse_rec(istate,
+                                                0, 0, istate->cache_nr,
+                                                "", 0, istate->cache_tree);
+       istate->drop_cache_tree = 1;
+       istate->sparse_index = 1;
+       trace2_region_leave("index", "convert_to_sparse", istate->repo);
+       return 0;
+}
 
 static void set_index_entry(struct index_state *istate, int nr, struct cache_entry *ce)
 {

t/t1092-sparse-checkout-compatibility.sh +0 -0

index 3aa9b0d21b4..22becbaca2e 100755

t/t1092-sparse-checkout-compatibility.sh +14 -2

+GIT_TEST_SPLIT_INDEX=0
+
 . ./test-lib.sh
 
 test_expect_success 'setup' '

@@ -106,7 +109,7 @@ run_on_sparse () {
        ) &&
        (
                cd sparse-index &&
-               $* >../sparse-index-out 2>../sparse-index-err
+               GIT_TEST_SPARSE_INDEX=1 $* >../sparse-index-out 2>../sparse-index-err
        )
 }
 

@@ -121,7 +124,9 @@ run_on_all () {
 test_all_match () {
        run_on_all $* &&
        test_cmp full-checkout-out sparse-checkout-out &&
-       test_cmp full-checkout-err sparse-checkout-err
+       test_cmp full-checkout-out sparse-index-out &&
+       test_cmp full-checkout-err sparse-checkout-err &&
+       test_cmp full-checkout-err sparse-index-err
 }
 
 test_sparse_match () {

@@ -130,6 +135,38 @@ test_sparse_match () {
        test_cmp sparse-checkout-err sparse-index-err
 }
 
+test_expect_success 'sparse-index contents' '
+       init_repos &&
+
+       test-tool -C sparse-index read-cache --table --no-stat >cache &&
+       for dir in folder1 folder2 x
+       do
+               TREE=$(git -C sparse-index rev-parse HEAD:$dir) &&
+               grep "0b0000 0755 $TREE $dir/" cache \

t/t1092-sparse-checkout-compatibility.sh +15 -0

+                       || return 1
+       done &&
+
+       GIT_TEST_SPARSE_INDEX=1 git -C sparse-index sparse-checkout set deep/deeper1 &&
+
+       test-tool -C sparse-index read-cache --table --no-stat >cache &&
+       for dir in deep/deeper2 folder1 folder2 x
+       do
+               TREE=$(git -C sparse-index rev-parse HEAD:$dir) &&
+               grep "0b0000 0755 $TREE $dir/" cache \
+                       || return 1
+       done
+'
+
 test_expect_success 'expanded in-memory index matches full index' '
        init_repos &&
        test_sparse_match test-tool read-cache --expand --table --no-stat

@@ -137,6 +174,7 @@ test_expect_success 'expanded in-memory index matches full index' '
 
 test_expect_success 'status with options' '
        init_repos &&
+       test_sparse_match ls &&
        test_all_match git status --porcelain=v2 &&
        test_all_match git status --porcelain=v2 -z -u &&
        test_all_match git status --porcelain=v2 -uno &&

t/t1092-sparse-checkout-compatibility.sh +18 -2

        test_all_match git checkout HEAD~1 &&
        test_all_match git checkout - &&

@@ -273,6 +311,17 @@ test_expect_failure 'checkout and reset (mixed)' '
        test_all_match git reset update-folder2
 '
 
+# Ensure that sparse-index behaves identically to
+# sparse-checkout with a full index.
+test_expect_success 'checkout and reset (mixed) [sparse]' '
+       init_repos &&
+
+       test_sparse_match git checkout -b reset-test update-deep &&
+       test_sparse_match git reset deepest &&
+       test_sparse_match git reset update-folder1 &&
+       test_sparse_match git reset update-folder2
+'
+
 test_expect_success 'merge' '
        init_repos &&
 

@@ -309,14 +358,20 @@ test_expect_success 'clean' '
        test_all_match git status --porcelain=v2 &&
        test_all_match git clean -f &&
        test_all_match git status --porcelain=v2 &&
+       test_sparse_match ls &&
+       test_sparse_match ls folder1 &&
 
        test_all_match git clean -xf &&
        test_all_match git status --porcelain=v2 &&
+       test_sparse_match ls &&
+       test_sparse_match ls folder1 &&
 
        test_all_match git clean -xdf &&
        test_all_match git status --porcelain=v2 &&
+       test_sparse_match ls &&
+       test_sparse_match ls folder1 &&
 
-       test_path_is_dir sparse-checkout/folder1
+       test_sparse_match test_path_is_dir folder1
 '
 
 test_done
--

gitgitgadget

From: Derrick Stolee <[email protected]>

The index_pos_by_traverse_info() currently throws a BUG() when a directory entry exists exactly in the index. We need to consider that it is possible to have a directory in a sparse index as long as that entry is itself marked with the skip-worktree bit.

The negation of the 'pos' variable must be conditioned to only when it starts as negative. This is identical behavior as before when the index is full.

The starts_with() condition matches because our name.buf terminates with a directory separator, just like our sparse directory entries.

Signed-off-by Derrick Stolee <[email protected]>

---
 unpack-trees.c | 9 ++++++---
 1 file changed, 6 insertions(+), 3 deletions(-)

unpack-trees.c +6 -4

index 4dd99219073..b324eec2a5d 100644
@@ -746,9 +746,12 @@ static int index_pos_by_traverse_info(struct name_entry *names,
        strbuf_make_traverse_path(&name, info, names->path, names->pathlen);
        strbuf_addch(&name, '/');
        pos = index_name_pos(o->src_index, name.buf, name.len);
-       if (pos >= 0)
-               BUG("This is a directory and should not exist in index");
-       pos = -pos - 1;
+       if (pos >= 0) {
+               if (!o->src_index->sparse_index ||
+                   !(o->src_index->cache[pos]->ce_flags & CE_SKIP_WORKTREE))
+                       BUG("This is a directory and should not exist in index");
+       } else
+               pos = -pos - 1;
        if (pos >= o->src_index->cache_nr ||
            !starts_with(o->src_index->cache[pos]->name, name.buf) ||
            (pos > 0 && starts_with(o->src_index->cache[pos-1]->name, name.buf)))
--

gitgitgadget

From: Derrick Stolee <[email protected]>

Add a test case that uses test_region to ensure that we are truly expanding a sparse index to a full one, then converting back to sparse when writing the index. As we integrate more Git commands with the sparse index, we will convert these commands to check that we do _not_ convert the sparse index to a full index and instead stay sparse the entire time.

Signed-off-by Derrick Stolee <[email protected]>

---
 t/t1092-sparse-checkout-compatibility.sh | 17 +++++++++++++++++
 1 file changed, 17 insertions(+)

t/t1092-sparse-checkout-compatibility.sh +0 -0

index 22becbaca2e..a22def89e37 100755

t/t1092-sparse-checkout-compatibility.sh +12 -1

+               GIT_TRACE2_EVENT="$(pwd)/trace2.txt" GIT_TRACE2_EVENT_NESTING=10 \
+                       git -C sparse-index -c core.fsmonitor="" reset --hard &&
+               test_region index convert_to_sparse trace2.txt &&
+               test_region index ensure_full_index trace2.txt &&
+
+               rm trace2.txt &&
+               GIT_TRACE2_EVENT="$(pwd)/trace2.txt" GIT_TRACE2_EVENT_NESTING=10 \
+                       git -C sparse-index -c core.fsmonitor="" status -uno &&
+               test_region index ensure_full_index trace2.txt
+       )
+'
+
 test_done
--

gitgitgadget

From: Derrick Stolee <[email protected]>

Previously, we enabled the sparse index format only using GIT_TEST_SPARSE_INDEX=1. This is not a feasible direction for users to actually select this mode. Further, sparse directory entries are not understood by the index formats as advertised.

We _could_ add a new index version that explicitly adds these capabilities, but there are nuances to index formats 2, 3, and 4 that are still valuable to select as options. For now, create a repo extension, "extensions.sparseIndex", that specifies that the tool reading this repository must understand sparse directory entries.

This change only encodes the extension and enables it when GIT_TEST_SPARSE_INDEX=1. Later, we will add a more user-friendly CLI mechanism.

Signed-off-by Derrick Stolee <[email protected]>

---
 Documentation/config/extensions.txt |  7 ++++++
 cache.h                             |  1 +
 repo-settings.c                     |  7 ++++++
 repository.h                        |  3 ++-
 setup.c                             |  3 +++
 sparse-index.c                      | 38 +++++++++++++++++++++++++----
 6 files changed, 53 insertions(+), 6 deletions(-)

Documentation/config/extensions.txt +0 -0

index 4e23d73cdca..5c86b364873 100644

cache.h +1 -0

index b05341cc687..dcf089b7006 100644
@@ -1054,6 +1054,7 @@ struct repository_format {
        int worktree_config;
        int is_bare;
        int hash_algo;
+       int sparse_index;
        char *work_tree;
        struct string_list unknown_extensions;
        struct string_list v1_only_extensions;

repo-settings.c +7 -0

index d63569e4041..9677d50f923 100644
@@ -85,4 +85,11 @@ void prepare_repo_settings(struct repository *r)
         * removed.
         */
        r->settings.command_requires_full_index = 1;
+
+       /*
+        * Initialize this as off.
+        */
+       r->settings.sparse_index = 0;
+       if (!repo_config_get_bool(r, "extensions.sparseindex", &value) && value)
+               r->settings.sparse_index = 1;
 }

repository.h +2 -1

index e06a2301569..a45f7520fd9 100644
@@ -42,7 +42,8 @@ struct repo_settings {
 
        int core_multi_pack_index;
 
-       unsigned command_requires_full_index:1;
+       unsigned command_requires_full_index:1,
+                sparse_index:1;
 };
 
 struct repository {

setup.c +3 -0

index c04cd25a30d..cd839456461 100644
@@ -500,6 +500,9 @@ static enum extension_result handle_extension(const char *var,
                        return error("invalid value for 'extensions.objectformat'");
                data->hash_algo = format;
                return EXTENSION_OK;
+       } else if (!strcmp(ext, "sparseindex")) {
+               data->sparse_index = 1;
+               return EXTENSION_OK;
        }
        return EXTENSION_UNKNOWN;
 }

sparse-index.c +33 -6

index 5dd0b835b9d..71544095267 100644
@@ -102,19 +102,47 @@ static int convert_to_sparse_rec(struct index_state *istate,
        return num_converted - start_converted;
 }
 
+static int enable_sparse_index(struct repository *repo)
+{
+       const char *config_path = repo_git_path(repo, "config.worktree");
+
+       if (upgrade_repository_format(1) < 0) {
+               warning(_("unable to upgrade repository format to enable sparse-index"));
+               return -1;
+       }
+       git_config_set_in_file_gently(config_path,
+                                     "extensions.sparseIndex",
+                                     "true");
+
+       prepare_repo_settings(repo);
+       repo->settings.sparse_index = 1;
+       return 0;
+}
+
 int convert_to_sparse(struct index_state *istate)
 {
        if (istate->split_index || istate->sparse_index ||
            !core_apply_sparse_checkout || !core_sparse_checkout_cone)
                return 0;
 
+       if (!istate->repo)
+               istate->repo = the_repository;
+
+       /*
+        * The GIT_TEST_SPARSE_INDEX environment variable triggers the
+        * extensions.sparseIndex config variable to be on.
+        */
+       if (git_env_bool("GIT_TEST_SPARSE_INDEX", 0)) {
+               int err = enable_sparse_index(istate->repo);
+               if (err < 0)
+                       return err;
+       }
+
        /*
-        * For now, only create a sparse index with the
-        * GIT_TEST_SPARSE_INDEX environment variable. We will relax
-        * this once we have a proper way to opt-in (and later still,
-        * opt-out).
+        * Only convert to sparse if extensions.sparseIndex is set.
         */
-       if (!git_env_bool("GIT_TEST_SPARSE_INDEX", 0))
+       prepare_repo_settings(istate->repo);
+       if (!istate->repo->settings.sparse_index)
                return 0;
 
        if (!istate->sparse_checkout_patterns) {
--

gitgitgadget

From: Derrick Stolee <[email protected]>

The sparse index extension is used to signal that index writes should be in sparse mode. This was only updated using GIT_TEST_SPARSE_INDEX=1.

Add a '--[no-]sparse-index' option to 'git sparse-checkout init' that specifies if the sparse index should be used. It also updates the index to use the correct format, either way. Add a warning in the documentation that the use of a repository extension might reduce compatibility with third-party tools. 'git sparse-checkout init' already sets extension.worktreeConfig, which places most sparse-checkout users outside of the scope of most third-party tools.

Update t1092-sparse-checkout-compatibility.sh to use this CLI instead of GIT_TEST_SPARSE_INDEX=1.

Signed-off-by Derrick Stolee <[email protected]>

---
 Documentation/git-sparse-checkout.txt    | 14 +++++++++
 builtin/sparse-checkout.c                | 17 ++++++++++-
 sparse-index.c                           | 38 ++++++++++++++++--------
 sparse-index.h                           |  3 ++
 t/t1092-sparse-checkout-compatibility.sh | 33 ++++++++++----------
 5 files changed, 75 insertions(+), 30 deletions(-)

Documentation/git-sparse-checkout.txt +1 -0

index a0eeaeb02ee..b51b8450cfd 100644
@@ -45,6 +45,20 @@ To avoid interfering with other worktrees, it first enables the
 When `--cone` is provided, the `core.sparseCheckoutCone` setting is
 also set, allowing for better performance with a limited set of
 patterns (see 'CONE PATTERN SET' below).
++

Documentation/git-sparse-checkout.txt +3 -0

+other tools to stop working with your repository. If you have trouble with
+this compatibility, then run `git sparse-checkout sparse-index disable` to
+remove this config and rewrite your index to not be sparse.
 
 'set'::
        Write a set of patterns to the sparse-checkout file, as given as

builtin/sparse-checkout.c +1 -0

index e00b82af727..ca63e2c64e9 100644
@@ -14,6 +14,7 @@
 #include "unpack-trees.h"
 #include "wt-status.h"
 #include "quote.h"
+#include "sparse-index.h"
 
 static const char *empty_base = "";
 

builtin/sparse-checkout.c +14 -0

        NULL
 };
 
 static struct sparse_checkout_init_opts {
        int cone_mode;
+       int sparse_index;
 } init_opts;
 
 static int sparse_checkout_init(int argc, const char **argv)

@@ -303,11 +305,15 @@ static int sparse_checkout_init(int argc, const char **argv)
        static struct option builtin_sparse_checkout_init_options[] = {
                OPT_BOOL(0, "cone", &init_opts.cone_mode,
                         N_("initialize the sparse-checkout in cone mode")),
+               OPT_BOOL(0, "sparse-index", &init_opts.sparse_index,
+                        N_("toggle the use of a sparse index")),
                OPT_END(),
        };
 
        repo_read_index(the_repository);
 
+       init_opts.sparse_index = -1;
+
        argc = parse_options(argc, argv, NULL,
                             builtin_sparse_checkout_init_options,
                             builtin_sparse_checkout_init_usage, 0);

@@ -326,6 +332,15 @@ static int sparse_checkout_init(int argc, const char **argv)
        sparse_filename = get_sparse_checkout_filename();
        res = add_patterns_from_file_to_list(sparse_filename, "", 0, &pl, NULL);
 
+       if (init_opts.sparse_index >= 0) {
+               if (set_sparse_index_config(the_repository, init_opts.sparse_index) < 0)
+                       die(_("failed to modify sparse-index config"));
+
+               /* force an index rewrite */
+               repo_read_index(the_repository);
+               the_repository->index->updated_workdir = 1;
+       }
+
        /* If we already have a sparse-checkout file, use it. */
        if (res >= 0) {
                free(sparse_filename);

sparse-index.c +26 -12

index 71544095267..3552f88fb03 100644
@@ -104,23 +104,38 @@ static int convert_to_sparse_rec(struct index_state *istate,
 
 static int enable_sparse_index(struct repository *repo)
 {
-       const char *config_path = repo_git_path(repo, "config.worktree");
+       int res;
 
        if (upgrade_repository_format(1) < 0) {
                warning(_("unable to upgrade repository format to enable sparse-index"));
                return -1;
        }
-       git_config_set_in_file_gently(config_path,
-                                     "extensions.sparseIndex",
-                                     "true");
+       res = git_config_set_gently("extensions.sparseindex", "true");
 
        prepare_repo_settings(repo);
        repo->settings.sparse_index = 1;
-       return 0;
+       return res;
+}
+
+int set_sparse_index_config(struct repository *repo, int enable)
+{
+       int res;
+
+       if (enable)
+               return enable_sparse_index(repo);
+
+       /* Don't downgrade repository format, just remove the extension. */
+       res = git_config_set_multivar_gently("extensions.sparseindex", NULL, "",
+                                            CONFIG_FLAGS_MULTI_REPLACE);
+
+       prepare_repo_settings(repo);
+       repo->settings.sparse_index = 0;
+       return res;
 }
 
 int convert_to_sparse(struct index_state *istate)
 {
+       int test_env;
        if (istate->split_index || istate->sparse_index ||
            !core_apply_sparse_checkout || !core_sparse_checkout_cone)
                return 0;

@@ -129,14 +144,13 @@ int convert_to_sparse(struct index_state *istate)
                istate->repo = the_repository;
 
        /*
-        * The GIT_TEST_SPARSE_INDEX environment variable triggers the
-        * extensions.sparseIndex config variable to be on.
+        * If GIT_TEST_SPARSE_INDEX=1, then trigger extensions.sparseIndex
+        * to be fully enabled. If GIT_TEST_SPARSE_INDEX=0 (set explicitly),
+        * then purposefully disable the setting.
         */
-       if (git_env_bool("GIT_TEST_SPARSE_INDEX", 0)) {
-               int err = enable_sparse_index(istate->repo);
-               if (err < 0)
-                       return err;
-       }
+       test_env = git_env_bool("GIT_TEST_SPARSE_INDEX", -1);
+       if (test_env >= 0)
+               set_sparse_index_config(istate->repo, test_env);
 
        /*
         * Only convert to sparse if extensions.sparseIndex is set.

sparse-index.h +3 -0

index a2777dcac59..ca936e95d11 100644
@@ -5,4 +5,7 @@ struct index_state;
 void ensure_full_index(struct index_state *istate);
 int convert_to_sparse(struct index_state *istate);
 
+struct repository;
+int set_sparse_index_config(struct repository *repo, int enable);
+
 #endif
\ No newline at end of file

t/t1092-sparse-checkout-compatibility.sh +16 -18

index a22def89e37..c6b7e8b8891 100755
@@ -4,6 +4,7 @@ test_description='compare full workdir to sparse workdir'
 
 GIT_TEST_CHECK_CACHE_TREE=0
 GIT_TEST_SPLIT_INDEX=0
+GIT_TEST_SPARSE_INDEX=
 
 . ./test-lib.sh
 

@@ -98,8 +99,9 @@ init_repos () {
        # initialize sparse-checkout definitions
        git -C sparse-checkout sparse-checkout init --cone &&
        git -C sparse-checkout sparse-checkout set deep &&
-       GIT_TEST_SPARSE_INDEX=1 git -C sparse-index sparse-checkout init --cone &&
-       GIT_TEST_SPARSE_INDEX=1 git -C sparse-index sparse-checkout set deep
+       git -C sparse-index sparse-checkout init --cone --sparse-index &&
+       test_cmp_config -C sparse-index true extensions.sparseindex &&
+       git -C sparse-index sparse-checkout set deep
 }
 
 run_on_sparse () {

@@ -109,7 +111,7 @@ run_on_sparse () {
        ) &&
        (
                cd sparse-index &&
-               GIT_TEST_SPARSE_INDEX=1 $* >../sparse-index-out 2>../sparse-index-err
+               $* >../sparse-index-out 2>../sparse-index-err
        )
 }
 

@@ -146,7 +148,7 @@ test_expect_success 'sparse-index contents' '
                        || return 1
        done &&
 
-       GIT_TEST_SPARSE_INDEX=1 git -C sparse-index sparse-checkout set folder1 &&
+       git -C sparse-index sparse-checkout set folder1 &&
 
        test-tool -C sparse-index read-cache --table --no-stat >cache &&
        for dir in deep folder2 x

@@ -156,7 +158,7 @@ test_expect_success 'sparse-index contents' '
                        || return 1
        done &&
 
-       GIT_TEST_SPARSE_INDEX=1 git -C sparse-index sparse-checkout set deep/deeper1 &&
+       git -C sparse-index sparse-checkout set deep/deeper1 &&
 
        test-tool -C sparse-index read-cache --table --no-stat >cache &&
        for dir in deep/deeper2 folder1 folder2 x

@@ -377,18 +379,15 @@ test_expect_success 'clean' '
 test_expect_success 'sparse-index is expanded and converted back' '
        init_repos &&
 
-       (
-               (GIT_TEST_SPARSE_INDEX=1 && export GIT_TEST_SPARSE_INDEX) &&
-               GIT_TRACE2_EVENT="$(pwd)/trace2.txt" GIT_TRACE2_EVENT_NESTING=10 \
-                       git -C sparse-index -c core.fsmonitor="" reset --hard &&
-               test_region index convert_to_sparse trace2.txt &&
-               test_region index ensure_full_index trace2.txt &&
-
-               rm trace2.txt &&
-               GIT_TRACE2_EVENT="$(pwd)/trace2.txt" GIT_TRACE2_EVENT_NESTING=10 \
-                       git -C sparse-index -c core.fsmonitor="" status -uno &&
-               test_region index ensure_full_index trace2.txt
-       )
+       GIT_TRACE2_EVENT="$(pwd)/trace2.txt" GIT_TRACE2_EVENT_NESTING=10 \
+               git -C sparse-index -c core.fsmonitor="" reset --hard &&
+       test_region index convert_to_sparse trace2.txt &&
+       test_region index ensure_full_index trace2.txt &&
+
+       rm trace2.txt &&
+       GIT_TRACE2_EVENT="$(pwd)/trace2.txt" GIT_TRACE2_EVENT_NESTING=10 \
+               git -C sparse-index -c core.fsmonitor="" status -uno &&
+       test_region index ensure_full_index trace2.txt
 '
 
 test_done
--

gitgitgadget

On Mon, Jan 25, 2021 at 9:42 AM Derrick Stolee via GitGitGadget <[email protected]> wrote:

I need to take a break from reviewing again at this point and work on some other tasks. I'll resume reviewing the series later, perhaps tomorrow afternoon.