Re: [PATCH v3 7/8] sparse-index: add SPARSE_INDEX_IGNORE_CONFIG flag

[Derrick Stolee <stolee@gmail.com>]

On 8/17/2021 9:23 AM, Derrick Stolee via GitGitGadget wrote:
> From: Derrick Stolee <dstolee@microsoft.com>
> 
> The convert_to_sparse() method checks for the GIT_TEST_SPARSE_INDEX
> environment variable or the "index.sparse" config setting before
> converting the index to a sparse one. This is for ease of use since all
> current consumers are preparing to compress the index before writing it
> to disk. If these settings are not enabled, then convert_to_sparse()
> silently returns without doing anything.
> 
> We will add a consumer in the next change that wants to use the sparse
> index as an in-memory data structure, regardless of whether the on-disk
> format should be sparse.
> 
> To that end, create the SPARSE_INDEX_IGNORE_CONFIG flag that will skip
> these config checks when enabled. All current consumers are modified to
> pass '0' in the new 'flags' parameter.

...

> -int convert_to_sparse(struct index_state *istate)
> +int convert_to_sparse(struct index_state *istate, int flags)
>  {
>  	int test_env;
>  
> @@ -135,20 +135,22 @@ int convert_to_sparse(struct index_state *istate)
>  	if (!istate->repo)
>  		istate->repo = the_repository;
>  

Above this hunk, we fail automatically if the index has a split index.

The purpose of this flag is instead to say "convert to sparse for the
purpose of in-memory computations, not for writing to disk". For such
a case, we could move the split index check to be within the hunk
below. It would be appropriate to rename the flag to something like
SPARSE_INDEX_IN_MEMORY or SPARSE_INDEX_NO_DISK_WRITE to make the
intention more clear.

Thanks to SZEDER for pointing out this failure. I will fix it in the
next version.

> -	/*
> -	 * The GIT_TEST_SPARSE_INDEX environment variable triggers the
> -	 * index.sparse config variable to be on.
> -	 */
> -	test_env = git_env_bool("GIT_TEST_SPARSE_INDEX", -1);
> -	if (test_env >= 0)
> -		set_sparse_index_config(istate->repo, test_env);
> +	if (!(flags & SPARSE_INDEX_IGNORE_CONFIG)) {
> +		/*
> +		 * The GIT_TEST_SPARSE_INDEX environment variable triggers the
> +		 * index.sparse config variable to be on.
> +		 */
> +		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 index.sparse is set.
> -	 */
> -	prepare_repo_settings(istate->repo);
> -	if (!istate->repo->settings.sparse_index)
> -		return 0;
> +		/*
> +		 * Only convert to sparse if index.sparse is set.
> +		 */
> +		prepare_repo_settings(istate->repo);
> +		if (!istate->repo->settings.sparse_index)
> +			return 0;
> +	}

If you want to try this, here is a diff that can help:

--- >8 ---

diff --git a/builtin/sparse-checkout.c b/builtin/sparse-checkout.c
index b06c8f885ac..c6a512a2107 100644
--- a/builtin/sparse-checkout.c
+++ b/builtin/sparse-checkout.c
@@ -131,7 +131,7 @@ static void clean_tracked_sparse_directories(struct repository *r)
 		 * prevents us from converting to a sparse index, then do
 		 * not try deleting files.
 		 */
-		if (convert_to_sparse(r->index, SPARSE_INDEX_IGNORE_CONFIG))
+		if (convert_to_sparse(r->index, SPARSE_INDEX_MEMORY_ONLY))
 			return;
 		was_full = 1;
 	}
diff --git a/sparse-index.c b/sparse-index.c
index e0a854f9fc3..267503b21fa 100644
--- a/sparse-index.c
+++ b/sparse-index.c
@@ -128,14 +128,20 @@ int convert_to_sparse(struct index_state *istate, int flags)
 {
 	int test_env;
 
-	if (istate->split_index || istate->sparse_index || !istate->cache_nr ||
+	if (istate->sparse_index || !istate->cache_nr ||
 	    !core_apply_sparse_checkout || !core_sparse_checkout_cone)
 		return 0;
 
 	if (!istate->repo)
 		istate->repo = the_repository;
 
-	if (!(flags & SPARSE_INDEX_IGNORE_CONFIG)) {
+	if (!(flags & SPARSE_INDEX_MEMORY_ONLY)) {
+		/*
+		 * The sparse index is not integrated with a split index.
+		 */
+		if (istate->sparse_index)
+			return 0;
+
 		/*
 		 * The GIT_TEST_SPARSE_INDEX environment variable triggers the
 		 * index.sparse config variable to be on.
diff --git a/sparse-index.h b/sparse-index.h
index 475f4f0f8da..9f3d7bc7faf 100644
--- a/sparse-index.h
+++ b/sparse-index.h
@@ -2,7 +2,7 @@
 #define SPARSE_INDEX_H__
 
 struct index_state;
-#define SPARSE_INDEX_IGNORE_CONFIG (1 << 0)
+#define SPARSE_INDEX_MEMORY_ONLY (1 << 0)
 int convert_to_sparse(struct index_state *istate, int flags);
 
 /*


--- >8 ---

Thanks,
-Stolee