sparse-index: require commands to declare compatibility

3c31623 (sparse-index: add guard to ensure full index, 2021-01-08)
introduced command_requires_full_index as scaffolding: every command
starts by expanding the sparse index, and commands opt out one by one
by setting the field to 0.  Because the default is 1, a command that
never touches the field silently gets full expansion -- there is no
signal that it was forgotten rather than intentionally left.

Change the default to a sentinel (-1) and add an accessor that dies
if a command reaches index reading without declaring a value.  An
environment variable GIT_ALLOW_SPARSE_INDEX_WITHOUT_DECLARATION=1
bypasses the check as a safety net.

Both the safety net and the field itself are meant to be removed
once no commands require full expansion from the start.

Four commands that never declared this field are surfaced and
explicitly set to 1 to preserve existing behavior:

  - git-am
  - git-mv
  - git-submodule--helper
  - grep's submodule path

This is a pure refactoring with no behavior change.

Signed-off-by: Kristofer Karlsson <krka@spotify.com>

Author: spkrka

builtin/am.c

@@ -23,6 +23,7 @@
 #include "lockfile.h"
 #include "cache-tree.h"
 #include "refs.h"
+#include "repo-settings.h"
 #include "commit.h"
 #include "diff.h"
 #include "unpack-trees.h"
@@ -2464,6 +2465,9 @@ int cmd_am(int argc,
 	/* Ensure a valid committer ident can be constructed */
 	git_committer_info(IDENT_STRICT);
 
+	prepare_repo_settings(the_repository);
+	the_repository->settings.command_requires_full_index = 1;
+
 	if (repo_read_index_preload(the_repository, NULL, 0) < 0)
 		die(_("failed to read the index"));
 

builtin/grep.c

@@ -500,6 +500,10 @@ static int grep_submodule(struct grep_opt *opt,
 	 *
 	 * Note that this list is not exhaustive.
 	 */
+	/* not yet verified whether subrepo can use the sparse index */
+	prepare_repo_settings(subrepo);
+	subrepo->settings.command_requires_full_index = 1;
+
 	repo_read_gitmodules(subrepo, 0);
 
 	/*

builtin/mv.c

@@ -22,6 +22,7 @@
 #include "string-list.h"
 #include "parse-options.h"
 #include "read-cache-ll.h"
+#include "repo-settings.h"
 
 #include "setup.h"
 #include "strvec.h"
@@ -247,6 +248,9 @@ int cmd_mv(int argc,
 	if (--argc < 1)
 		usage_with_options(builtin_mv_usage, builtin_mv_options);
 
+	prepare_repo_settings(the_repository);
+	the_repository->settings.command_requires_full_index = 1;
+
 	repo_hold_locked_index(the_repository, &lock_file, LOCK_DIE_ON_ERROR);
 	if (repo_read_index(the_repository) < 0)
 		die(_("index file corrupt"));

builtin/submodule--helper.c

@@ -16,6 +16,7 @@
 #include "read-cache.h"
 #include "setup.h"
 #include "sparse-index.h"
+#include "repo-settings.h"
 #include "submodule.h"
 #include "submodule-config.h"
 #include "string-list.h"
@@ -3832,5 +3833,9 @@ int cmd_submodule__helper(int argc,
 	};
 	argc = parse_options(argc, argv, prefix, options, usage, 0);
 
+	/* not yet verified whether this can use the sparse index */
+	prepare_repo_settings(the_repository);
+	the_repository->settings.command_requires_full_index = 1;
+
 	return fn(argc, argv, prefix, repo);
 }

read-cache.c

@@ -2189,7 +2189,7 @@ static void set_new_index_sparsity(struct index_state *istate)
 	 * repo settings.
 	 */
 	prepare_repo_settings(istate->repo);
-	if (!istate->repo->settings.command_requires_full_index &&
+	if (!repo_settings_get_command_requires_full_index(istate->repo) &&
 	    is_sparse_index_allowed(istate, 0))
 		istate->sparse_index = 1;
 }
@@ -2321,7 +2321,7 @@ int do_read_index(struct index_state *istate, const char *path, int must_exist)
 	 * settings and other properties of the index (if necessary).
 	 */
 	prepare_repo_settings(istate->repo);
-	if (istate->repo->settings.command_requires_full_index)
+	if (repo_settings_get_command_requires_full_index(istate->repo))
 		ensure_full_index(istate);
 	else
 		ensure_correct_sparsity(istate);

repo-settings.c

@@ -1,5 +1,6 @@
 #include "git-compat-util.h"
 #include "config.h"
+#include "gettext.h"
 #include "repo-settings.h"
 #include "repository.h"
 #include "midx.h"
@@ -131,14 +132,6 @@ void prepare_repo_settings(struct repository *r)
 			die("unknown fetch negotiation algorithm '%s'", strval);
 	}
 
-	/*
-	 * This setting guards all index reads to require a full index
-	 * over a sparse index. After suitable guards are placed in the
-	 * codebase around uses of the index, this setting will be
-	 * removed.
-	 */
-	r->settings.command_requires_full_index = 1;
-
 	if (!repo_config_get_ulong(r, "core.deltabasecachelimit", &ulongval))
 		r->settings.delta_base_cache_limit = ulongval;
 
@@ -156,6 +149,20 @@ void prepare_repo_settings(struct repository *r)
 		r->settings.packed_git_limit = ulongval;
 }
 
+int repo_settings_get_command_requires_full_index(struct repository *r)
+{
+	if (r->settings.command_requires_full_index >= 0)
+		return r->settings.command_requires_full_index;
+
+	if (git_env_bool("GIT_ALLOW_SPARSE_INDEX_WITHOUT_DECLARATION", 0)) {
+		r->settings.command_requires_full_index = 1;
+		return 1;
+	}
+
+	die(_("command has not declared sparse-index compatibility;\n"
+	      "set GIT_ALLOW_SPARSE_INDEX_WITHOUT_DECLARATION=1 to bypass"));
+}
+
 void repo_settings_clear(struct repository *r)
 {
 	struct repo_settings empty = REPO_SETTINGS_INIT;

repo-settings.h

@@ -72,6 +72,7 @@ struct repo_settings {
 	char *hooks_path;
 };
 #define REPO_SETTINGS_INIT { \
+	.command_requires_full_index = -1, \
 	.shared_repository = -1, \
 	.index_version = -1, \
 	.core_untracked_cache = UNTRACKED_CACHE_KEEP, \
@@ -85,6 +86,7 @@ struct repo_settings {
 
 void prepare_repo_settings(struct repository *r);
 void repo_settings_clear(struct repository *r);
+int repo_settings_get_command_requires_full_index(struct repository *r);
 
 /* Read the value for "core.logAllRefUpdates". */
 enum log_refs_config repo_settings_get_log_all_ref_updates(struct repository *repo);

repository.c

@@ -465,7 +465,7 @@ int repo_read_index(struct repository *repo)
 	res = read_index_from(repo->index, repo->index_file, repo->gitdir);
 
 	prepare_repo_settings(repo);
-	if (repo->settings.command_requires_full_index)
+	if (repo_settings_get_command_requires_full_index(repo))
 		ensure_full_index(repo->index);
 
 	/*

unpack-trees.c

@@ -1906,7 +1906,7 @@ int unpack_trees(unsigned len, struct tree_desc *t, struct unpack_trees_options
 	trace2_region_enter("unpack_trees", "unpack_trees", the_repository);
 
 	prepare_repo_settings(repo);
-	if (repo->settings.command_requires_full_index) {
+	if (repo_settings_get_command_requires_full_index(repo)) {
 		ensure_full_index(o->src_index);
 		if (o->dst_index)
 			ensure_full_index(o->dst_index);
@@ -1964,7 +1964,7 @@ int unpack_trees(unsigned len, struct tree_desc *t, struct unpack_trees_options
 	o->internal.result.fsmonitor_has_run_once = o->src_index->fsmonitor_has_run_once;
 
 	if (!o->src_index->initialized &&
-	    !repo->settings.command_requires_full_index &&
+	    !repo_settings_get_command_requires_full_index(repo) &&
 	    is_sparse_index_allowed(&o->internal.result, 0))
 		o->internal.result.sparse_index = 1;