Create 'feature.*' config area and some centralized config parsing

This is a brand-new thread to replace ds/early-access. The discussion on that thread was very helpful [1].

With this in mind, I propose instead a set of "feature.*" config settings that form groups of "community recommended" settings (with some caveats). In the space below, I'll list a set of possible feature names and the implied config options.

First, the main two categories we've discussed so far: many commits and many files. These two feature sets are for when your repo is large in one of these dimensions. Perhaps there are other settings to include in these?

feature.manyFiles:
index.version = 4
core.untrackedCache = true

feature.manyCommits:
core.commitGraph = true
gc.writeCommitGraph = true
(future: fetch.writeSplitCommitGraph = true)

Note: the fetch.writeSplitCommitGraph does not exist yet, but could be introduced in a later release to write a new commit-graph (with --split) on fetch.

The other category that has been discussed already is that of "experimental features that we generally think are helpful but change behavior slightly in some cases".

feature.experimental:
pack.useSparse = true
merge.directoryRenames = true
fetch.negotiationAlgorithm = skipping

Specifically, this setting is for config values we are not sure will ever be on by default, but additional testing is needed to be sure. This is different than a possible 'feature.preview' setting that would include config settings that we are committed to updating the defaults in a future release. There are many ways we can take this idea in the future (including more additions to these categories).

Thanks, -Stolee

[1] https://public-inbox.org/git/[email protected]/

Derrick Stolee (5):
repo-settings: consolidate some config settings
repo-settings: add feature.manyCommits setting
repo-settings: parse core.untrackedCache
repo-settings: create feature.manyFiles setting
repo-settings: create feature.experimental setting

 Documentation/config.txt             |   2 +
 Documentation/config/core.txt        |   7 +-
 Documentation/config/feature.txt     |  42 +++++++++++
 Documentation/config/fetch.txt       |   3 +-
 Documentation/config/gc.txt          |   4 +-
 Documentation/config/index.txt       |   1 +
 Documentation/config/merge.txt       |   3 +-
 Documentation/config/pack.txt        |   3 +-
 Makefile                             |   1 +
 builtin/am.c                         |   4 +-
 builtin/gc.c                         |  13 ++--
 builtin/pack-objects.c               |   9 ++-
 builtin/update-index.c               |   7 +-
 commit-graph.c                       |   7 +-
 config.c                             |  24 ------
 fetch-negotiator.c                   |  26 ++++---
 fetch-negotiator.h                   |   5 +-
 fetch-pack.c                         |  11 ++-
 merge-recursive.c                    |  32 ++++----
 merge-recursive.h                    |   1 -
 read-cache.c                         |  31 ++++----
 repo-settings.c                      | 108 +++++++++++++++++++++++++++
 repo-settings.h                      |  29 +++++++
 repository.h                         |   3 +
 t/t1600-index.sh                     |  31 ++++++--
 t/t5552-skipping-fetch-negotiator.sh |  23 ------
 26 files changed, 303 insertions(+), 127 deletions(-)
 create mode 100644 Documentation/config/feature.txt
 create mode 100644 repo-settings.c
 create mode 100644 repo-settings.h

base-commit: 9c9b961d7eb15fb583a2a812088713a68a85f1c0
Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-292%2Fderrickstolee%2Frepo-settings%2Fhead-v1
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-292/derrickstolee/repo-settings/head-v1
Pull-Request: https://github.com/gitgitgadget/git/pull/292

gitgitgadget

From: Derrick Stolee <[email protected]>

There are a few important config settings that are not loaded during git_default_config. These are instead loaded on-demand.

Centralize these config options to a single scan, and store all of the values in a repo_settings struct. The values for each setting are initialized as negative to indicate "unset".

This centralization will be particularly important in a later change to introduce "meta" config settings that change the defaults for these config settings.

---
 Makefile               |  1 +
 builtin/gc.c           | 13 ++++++-------
 builtin/pack-objects.c |  9 +++++----
 commit-graph.c         |  7 ++++---
 read-cache.c           | 12 +++++++-----
 repo-settings.c        | 44 ++++++++++++++++++++++++++++++++++++++++++
 repo-settings.h        | 15 ++++++++++++++
 repository.h           |  3 +++
 8 files changed, 85 insertions(+), 19 deletions(-)
 create mode 100644 repo-settings.c
 create mode 100644 repo-settings.h

index 11ccea4071..032fe9b5f9 100644
@@ -964,6 +964,7 @@ LIB_OBJS += refspec.o
 LIB_OBJS += ref-filter.o
 LIB_OBJS += remote.o
 LIB_OBJS += replace-object.o
+LIB_OBJS += repo-settings.o
 LIB_OBJS += repository.o
 LIB_OBJS += rerere.o
 LIB_OBJS += resolve-undo.o

index c18efadda5..243be2907b 100644
@@ -27,6 +27,7 @@
 #include "pack-objects.h"
 #include "blob.h"
 #include "tree.h"
+#include "repo-settings.h"
 
 #define FAILED_RUN "failed to run %s"
 

@@ -41,7 +42,6 @@ static int aggressive_depth = 50;
 static int aggressive_window = 250;
 static int gc_auto_threshold = 6700;
 static int gc_auto_pack_limit = 50;
-static int gc_write_commit_graph;
 static int detach_auto = 1;
 static timestamp_t gc_log_expire_time;
 static const char *gc_log_expire = "1.day.ago";

@@ -148,7 +148,6 @@ static void gc_config(void)
 	git_config_get_int("gc.aggressivedepth", &aggressive_depth);
 	git_config_get_int("gc.auto", &gc_auto_threshold);
 	git_config_get_int("gc.autopacklimit", &gc_auto_pack_limit);
-	git_config_get_bool("gc.writecommitgraph", &gc_write_commit_graph);
 	git_config_get_bool("gc.autodetach", &detach_auto);
 	git_config_get_expiry("gc.pruneexpire", &prune_expire);
 	git_config_get_expiry("gc.worktreepruneexpire", &prune_worktrees_expire);

@@ -685,11 +684,11 @@ int cmd_gc(int argc, const char **argv, const char *prefix)
 		clean_pack_garbage();
 	}
 
-	if (gc_write_commit_graph &&
-	    write_commit_graph_reachable(get_object_directory(),
-					 !quiet && !daemonized ? COMMIT_GRAPH_PROGRESS : 0,
-					 NULL))
-		return 1;
+	prepare_repo_settings(the_repository);
+	if (the_repository->settings->gc_write_commit_graph == 1)
+		write_commit_graph_reachable(get_object_directory(),
+					     !quiet && !daemonized ? COMMIT_GRAPH_PROGRESS : 0,
+					     NULL);
 
 	if (auto_gc && too_many_loose_objects())
 		warning(_("There are too many unreachable loose objects; "

index 267c562b1f..223038b309 100644
@@ -34,6 +34,7 @@
 #include "dir.h"
 #include "midx.h"
 #include "trace2.h"
+#include "repo-settings.h"
 
 #define IN_PACK(obj) oe_in_pack(&to_pack, obj)
 #define SIZE(obj) oe_size(&to_pack, obj)

@@ -2709,10 +2710,6 @@ static int git_pack_config(const char *k, const char *v, void *cb)
 		use_bitmap_index_default = git_config_bool(k, v);
 		return 0;
 	}
-	if (!strcmp(k, "pack.usesparse")) {
-		sparse = git_config_bool(k, v);
-		return 0;
-	}
 	if (!strcmp(k, "pack.threads")) {
 		delta_search_threads = git_config_int(k, v);
 		if (delta_search_threads < 0)

@@ -3332,6 +3329,10 @@ int cmd_pack_objects(int argc, const char **argv, const char *prefix)
 	read_replace_refs = 0;
 
 	sparse = git_env_bool("GIT_TEST_PACK_SPARSE", 0);
+	prepare_repo_settings(the_repository);
+	if (!sparse && the_repository->settings->pack_use_sparse != -1)
+		sparse = the_repository->settings->pack_use_sparse;
+
 	reset_pack_idx_option(&pack_idx_opts);
 	git_config(git_pack_config, NULL);
 

index b3c4de79b6..48e03aa285 100644
@@ -16,6 +16,7 @@
 #include "hashmap.h"
 #include "replace-object.h"
 #include "progress.h"
+#include "repo-settings.h"
 
 #define GRAPH_SIGNATURE 0x43475048 /* "CGPH" */
 #define GRAPH_CHUNKID_OIDFANOUT 0x4f494446 /* "OIDF" */

@@ -466,7 +467,6 @@ static void prepare_commit_graph_one(struct repository *r, const char *obj_dir)
 static int prepare_commit_graph(struct repository *r)
 {
 	struct object_directory *odb;
-	int config_value;
 
 	if (git_env_bool(GIT_TEST_COMMIT_GRAPH_DIE_ON_LOAD, 0))
 		die("dying as requested by the '%s' variable on commit-graph load!",

@@ -476,9 +476,10 @@ static int prepare_commit_graph(struct repository *r)
 		return !!r->objects->commit_graph;
 	r->objects->commit_graph_attempted = 1;
 
+	prepare_repo_settings(r);
+
 	if (!git_env_bool(GIT_TEST_COMMIT_GRAPH, 0) &&
-	    (repo_config_get_bool(r, "core.commitgraph", &config_value) ||
-	    !config_value))
+	    r->settings->core_commit_graph != 1)
 		/*
 		 * This repository is not configured to use commit graphs, so
 		 * do not load one. (But report commit_graph_attempted anyway

index c701f7f8b8..ee1aaa8917 100644
@@ -25,6 +25,7 @@
 #include "fsmonitor.h"
 #include "thread-utils.h"
 #include "progress.h"
+#include "repo-settings.h"
 
 /* Mask for the name length in ce_flags in the on-disk index */
 

@@ -1599,16 +1600,17 @@ struct cache_entry *refresh_cache_entry(struct index_state *istate,
 
 #define INDEX_FORMAT_DEFAULT 3
 
-static unsigned int get_index_format_default(void)
+static unsigned int get_index_format_default(struct repository *r)
 {
 	char *envversion = getenv("GIT_INDEX_VERSION");
 	char *endp;
-	int value;
 	unsigned int version = INDEX_FORMAT_DEFAULT;
 
 	if (!envversion) {
-		if (!git_config_get_int("index.version", &value))
-			version = value;
+		prepare_repo_settings(r);
+
+		if (r->settings->index_version >= 0)
+			version = r->settings->index_version;
 		if (version < INDEX_FORMAT_LB || INDEX_FORMAT_UB < version) {
 			warning(_("index.version set, but the value is invalid.\n"
 				  "Using version %i"), INDEX_FORMAT_DEFAULT);

@@ -2765,7 +2767,7 @@ static int do_write_index(struct index_state *istate, struct tempfile *tempfile,
 	}
 
 	if (!istate->version) {
-		istate->version = get_index_format_default();
+		istate->version = get_index_format_default(the_repository);
 		if (git_env_bool("GIT_TEST_SPLIT_INDEX", 0))
 			init_split_index(istate);
 	}

new file mode 100644
index 0000000000..1151c2193a
@@ -0,0 +1,15 @@
+#ifndef REPO_SETTINGS_H
+#define REPO_SETTINGS_H
+
+struct repo_settings {
+	int core_commit_graph;
+	int gc_write_commit_graph;
+	int pack_use_sparse;
+	int index_version;
+};
+
+struct repository;
+
+void prepare_repo_settings(struct repository *r);
+
+#endif /* REPO_SETTINGS_H */

index 4fb6a5885f..352afc9cd8 100644
@@ -4,6 +4,7 @@
 #include "path.h"
 
 struct config_set;
+struct repo_settings;
 struct git_hash_algo;
 struct index_state;
 struct lock_file;

@@ -72,6 +73,8 @@ struct repository {
 	 */
 	char *submodule_prefix;
 
+	struct repo_settings *settings;
+
 	/* Subsystems */
 	/*
 	 * Repository's config which contains key-value pairs from the usual

gitgitgadget

Hi Stolee,

From: Derrick Stolee <[email protected]>

When a repo has many commits, it is helpful to write and read the commit-graph file. Future changes to Git may include new config settings that are benefitial in this scenario.

Create the 'feature.manyCommits' config setting that changes the default values of 'core.commitGraph' and 'gc.writeCommitGraph' to true.

---
 Documentation/config.txt         |  2 ++
 Documentation/config/core.txt    |  3 ++-
 Documentation/config/feature.txt | 15 +++++++++++++++
 Documentation/config/gc.txt      |  4 ++--
 repo-settings.c                  |  7 +++++++
 5 files changed, 28 insertions(+), 3 deletions(-)
 create mode 100644 Documentation/config/feature.txt

index e3f5bc3396..77f3b1486b 100644
@@ -345,6 +345,8 @@ include::config/difftool.txt[]
 
 include::config/fastimport.txt[]
 
+include::config/feature.txt[]
+
 include::config/fetch.txt[]
 
 include::config/format.txt[]

index 75538d27e7..d80162681a 100644
@@ -577,7 +577,8 @@ the `GIT_NOTES_REF` environment variable.  See linkgit:git-notes[1].
 
 core.commitGraph::
 	If true, then git will read the commit-graph file (if it exists)
-	to parse the graph structure of commits. Defaults to false. See
+	to parse the graph structure of commits. Defaults to false, unless
+	`feature.manyCommits` is enabled. See
 	linkgit:git-commit-graph[1] for more information.
 
 core.useReplaceRefs::

new file mode 100644
index 0000000000..f74314ae90
@@ -0,0 +1,15 @@
+feature.*::
+	The config settings that start with `feature.` modify the defaults of
+	a group of other config settings. These groups are created by the Git
+	developer community as recommended defaults and are subject to change.
+	In particular, new config options may be added with different defaults.
+
+feature.manyCommits::
+	Enable config options that optimize for repos with many commits. This
+	setting is recommended for repos with at least 100,000 commits. The
+	new default values are:
++
+* `core.commitGraph=true` enables reading the commit-graph file.
++
+* `gc.writeCommitGraph=true` enables writing the commit-graph file during
+garbage collection.
\ No newline at end of file

index 02b92b18b5..31a5fc4f75 100644
@@ -63,8 +63,8 @@ gc.writeCommitGraph::
 	If true, then gc will rewrite the commit-graph file when
 	linkgit:git-gc[1] is run. When using `git gc --auto`
 	the commit-graph will be updated if housekeeping is
-	required. Default is false. See linkgit:git-commit-graph[1]
-	for details.
+	required. Default is false, unless `feature.manyCommits`
+	is enabled. See linkgit:git-commit-graph[1] for details.
 
 gc.logExpiry::
 	If the file gc.log exists, then `git gc --auto` will print

index 13a9128f62..f328602fd7 100644
@@ -3,10 +3,17 @@
 #include "config.h"
 #include "repo-settings.h"
 
+#define UPDATE_DEFAULT(s,v) do { if (s == -1) { s = v; } } while(0)
+
 static int git_repo_config(const char *key, const char *value, void *cb)
 {
 	struct repo_settings *rs = (struct repo_settings *)cb;
 
+	if (!strcmp(key, "feature.manycommits")) {
+		UPDATE_DEFAULT(rs->core_commit_graph, 1);
+		UPDATE_DEFAULT(rs->gc_write_commit_graph, 1);
+		return 0;
+	}
 	if (!strcmp(key, "core.commitgraph")) {
 		rs->core_commit_graph = git_config_bool(key, value);
 		return 0;

gitgitgadget

Hi Stolee,

s/benefitial/beneficial/

Great!

	if (!repo_config_get_bool(r, "feature.manycommits", &b) && b) {
		rs->core_commit_graph = 1;
		rs->gc_write_commit_graph = 1;
	}

	[...]

	repo_config_get_bool(r, "core.commitgraph", &rs->core_commit_graph);

I had forgotten about this interaction. Thanks for finding it!

The way I would fix it is to add an "int feature_many_commits" to the repo_settings struct, then check its value at the very end of prepare_repo_settings() and then UPDATE_DEFAULT() for the settings we want.

Thanks!
-Stolee

From: Derrick Stolee <[email protected]>

The core.untrackedCache config setting is slightly complicated, so clarify its use and centralize its parsing into the repo settings.

The default value is "keep" (returned as -1), which persists the untracked cache if it exists.

If the value is set as "false" (returned as 0), then remove the untracked cache if it exists.

If the value is set as "true" (returned as 1), then write the untracked cache and persist it.

Instead of relying on magic values of -1, 0, and 1, split these options into bitflags CORE_UNTRACKED_CACHE_KEEP and CORE_UNTRACKED_CACHE_WRITE. This allows the use of "-1" as a default value. After parsing the config options, if the value is unset we can initialize it to CORE_UNTRACKED_CACHE_KEEP.

---
 builtin/update-index.c |  7 +++++--
 config.c               | 24 ------------------------
 read-cache.c           | 19 +++++++++----------
 repo-settings.c        | 21 +++++++++++++++++++++
 repo-settings.h        |  4 ++++
 5 files changed, 39 insertions(+), 36 deletions(-)

index dff2f4b837..6c26bbae80 100644
@@ -18,6 +18,7 @@
 #include "dir.h"
 #include "split-index.h"
 #include "fsmonitor.h"
+#include "repo-settings.h"
 
 /*
  * Default to not allowing changes to the list of files. The

@@ -966,6 +967,7 @@ int cmd_update_index(int argc, const char **argv, const char *prefix)
 	struct parse_opt_ctx_t ctx;
 	strbuf_getline_fn getline_fn;
 	int parseopt_state = PARSE_OPT_UNKNOWN;
+	struct repository *r = the_repository;
 	struct option options[] = {
 		OPT_BIT('q', NULL, &refresh_args.flags,
 			N_("continue refresh even when index needs update"),

@@ -1180,11 +1182,12 @@ int cmd_update_index(int argc, const char **argv, const char *prefix)
 		remove_split_index(&the_index);
 	}
 
+	prepare_repo_settings(r);
 	switch (untracked_cache) {
 	case UC_UNSPECIFIED:
 		break;
 	case UC_DISABLE:
-		if (git_config_get_untracked_cache() == 1)
+		if (r->settings->core_untracked_cache & CORE_UNTRACKED_CACHE_WRITE)
 			warning(_("core.untrackedCache is set to true; "
 				  "remove or change it, if you really want to "
 				  "disable the untracked cache"));

@@ -1196,7 +1199,7 @@ int cmd_update_index(int argc, const char **argv, const char *prefix)
 		return !test_if_untracked_cache_is_supported();
 	case UC_ENABLE:
 	case UC_FORCE:
-		if (git_config_get_untracked_cache() == 0)
+		if (r->settings->core_untracked_cache == 0)
 			warning(_("core.untrackedCache is set to false; "
 				  "remove or change it, if you really want to "
 				  "enable the untracked cache"));

index faa57e436c..3241dbc54d 100644
@@ -2277,30 +2277,6 @@ int git_config_get_expiry_in_days(const char *key, timestamp_t *expiry, timestam
 	return -1; /* thing exists but cannot be parsed */
 }
 
-int git_config_get_untracked_cache(void)
-{
-	int val = -1;
-	const char *v;
-
-	/* Hack for test programs like test-dump-untracked-cache */
-	if (ignore_untracked_cache_config)
-		return -1;
-
-	if (!git_config_get_maybe_bool("core.untrackedcache", &val))
-		return val;
-
-	if (!git_config_get_value("core.untrackedcache", &v)) {
-		if (!strcasecmp(v, "keep"))
-			return -1;
-
-		error(_("unknown core.untrackedCache value '%s'; "
-			"using 'keep' default value"), v);
-		return -1;
-	}
-
-	return -1; /* default value */
-}
-
 int git_config_get_split_index(void)
 {
 	int val;

index ee1aaa8917..e67e6f6e3e 100644
@@ -1846,18 +1846,17 @@ static void check_ce_order(struct index_state *istate)
 
 static void tweak_untracked_cache(struct index_state *istate)
 {
-	switch (git_config_get_untracked_cache()) {
-	case -1: /* keep: do nothing */
-		break;
-	case 0: /* false */
+	struct repository *r = the_repository;
+
+	prepare_repo_settings(r);
+
+	if (!(r->settings->core_untracked_cache & CORE_UNTRACKED_CACHE_KEEP)) {
 		remove_untracked_cache(istate);
-		break;
-	case 1: /* true */
-		add_untracked_cache(istate);
-		break;
-	default: /* unknown value: do nothing */
-		break;
+		return;
 	}
+
+	if (r->settings->core_untracked_cache & CORE_UNTRACKED_CACHE_WRITE)
+		add_untracked_cache(istate);
 }
 
 static void tweak_split_index(struct index_state *istate)

index f328602fd7..807c5a29d6 100644
@@ -30,6 +30,20 @@ static int git_repo_config(const char *key, const char *value, void *cb)
 		rs->index_version = git_config_int(key, value);
 		return 0;
 	}
+	if (!strcmp(key, "core.untrackedcache")) {
+		int bool_value = git_parse_maybe_bool(value);
+		if (bool_value == 0)
+			rs->core_untracked_cache = 0;
+		else if (bool_value == 1)
+			rs->core_untracked_cache = CORE_UNTRACKED_CACHE_KEEP |
+						   CORE_UNTRACKED_CACHE_WRITE;
+		else if (!strcasecmp(value, "keep"))
+			rs->core_untracked_cache = CORE_UNTRACKED_CACHE_KEEP;
+		else
+			error(_("unknown core.untrackedCache value '%s'; "
+				"using 'keep' default value"), value);
+		return 0;
+	}
 
 	return 1;
 }

@@ -46,6 +60,13 @@ void prepare_repo_settings(struct repository *r)
 	r->settings->gc_write_commit_graph = -1;
 	r->settings->pack_use_sparse = -1;
 	r->settings->index_version = -1;
+	r->settings->core_untracked_cache = -1;
 
 	repo_config(r, git_repo_config, r->settings);
+
+	/* Hack for test programs like test-dump-untracked-cache */
+	if (ignore_untracked_cache_config)
+		r->settings->core_untracked_cache = CORE_UNTRACKED_CACHE_KEEP;
+	else
+		UPDATE_DEFAULT(r->settings->core_untracked_cache, CORE_UNTRACKED_CACHE_KEEP);
 }

index 1151c2193a..bac9b87d49 100644
@@ -1,11 +1,15 @@
 #ifndef REPO_SETTINGS_H
 #define REPO_SETTINGS_H
 
+#define CORE_UNTRACKED_CACHE_WRITE (1 << 0)
+#define CORE_UNTRACKED_CACHE_KEEP (1 << 1)
+
 struct repo_settings {
 	int core_commit_graph;
 	int gc_write_commit_graph;
 	int pack_use_sparse;
 	int index_version;
+	int core_untracked_cache;
 };
 
 struct repository;

gitgitgadget

Hi Stolee,

Nice!

I believe the code as written is correct, but confusing. The value is not an enum, but instead a bitflag. When the config setting is given as "true", then both _KEEP and _WRITE are set, so the flow is identical.

However, you already suggested switching to an enum, in which case using _REMOVE would be clearer.

Yes, there is no reason to have _WRITE without also _KEEP. An enum is better. Thanks!

From: Derrick Stolee <[email protected]>

The feature.manyFiles setting is suitable for repos with many files in the working directory. By setting index.version=4 and core.untrackedCache=true, commands such as 'git status' should improve.

While adding this setting, modify the index version precedence tests to check how this setting overrides the default for index.version is unset.

---
 Documentation/config/core.txt    |  4 +++-
 Documentation/config/feature.txt | 12 +++++++++++-
 Documentation/config/index.txt   |  1 +
 repo-settings.c                  |  6 ++++++
 t/t1600-index.sh                 | 31 ++++++++++++++++++++++++++-----
 5 files changed, 47 insertions(+), 7 deletions(-)

index d80162681a..7a2a33bc8c 100644
@@ -86,7 +86,9 @@ core.untrackedCache::
 	it will automatically be removed, if set to `false`. Before
 	setting it to `true`, you should check that mtime is working
 	properly on your system.
-	See linkgit:git-update-index[1]. `keep` by default.
+	See linkgit:git-update-index[1]. `keep` by default, unless
+	`feature.manyFiles` is enabled which sets this setting to
+	`true` by default.
 
 core.checkStat::
 	When missing or is set to `default`, many fields in the stat

index f74314ae90..c2d9ef7473 100644
@@ -12,4 +12,14 @@ feature.manyCommits::
 * `core.commitGraph=true` enables reading the commit-graph file.
 +
 * `gc.writeCommitGraph=true` enables writing the commit-graph file during
-garbage collection.
\ No newline at end of file
+garbage collection.
+
+feature.manyFiles::
+	Enable config options that optimize for repos with many files in the
+	working directory. With many files, commands such as `git status` and
+	`git checkout` may be slow and these new defaults improve performance:
++
+* `index.version=4` enables path-prefix compression in the index.
++
+* `core.untrackedCache=true` enables the untracked cache. This setting assumes
+that mtime is working on your machine.
\ No newline at end of file

index f181503041..7cb50b37e9 100644
@@ -24,3 +24,4 @@ index.threads::
 index.version::
 	Specify the version with which new index files should be
 	initialized.  This does not affect existing repositories.
+	If `feature.manyFiles` is enabled, then the default is 4.

index 807c5a29d6..9e4b8e6268 100644
@@ -14,6 +14,12 @@ static int git_repo_config(const char *key, const char *value, void *cb)
 		UPDATE_DEFAULT(rs->gc_write_commit_graph, 1);
 		return 0;
 	}
+	if (!strcmp(key, "feature.manyfiles")) {
+		UPDATE_DEFAULT(rs->index_version, 4);
+		UPDATE_DEFAULT(rs->core_untracked_cache,
+			       CORE_UNTRACKED_CACHE_KEEP | CORE_UNTRACKED_CACHE_WRITE);
+		return 0;
+	}
 	if (!strcmp(key, "core.commitgraph")) {
 		rs->core_commit_graph = git_config_bool(key, value);
 		return 0;

index 42962ed7d4..c77721b580 100755
@@ -59,17 +59,38 @@ test_expect_success 'out of bounds index.version issues warning' '
 	)
 '
 
-test_expect_success 'GIT_INDEX_VERSION takes precedence over config' '
+test_index_version () {
+	INDEX_VERSION_CONFIG=$1 &&
+	FEATURE_MANY_FILES=$2 &&
+	ENV_VAR_VERSION=$3
+	EXPECTED_OUTPUT_VERSION=$4 &&
 	(
 		rm -f .git/index &&
-		GIT_INDEX_VERSION=4 &&
-		export GIT_INDEX_VERSION &&
-		git config --add index.version 2 &&
+		rm -f .git/config &&
+		if test "$INDEX_VERSION_CONFIG" -ne 0
+		then
+			git config --add index.version $INDEX_VERSION_CONFIG
+		fi &&
+		git config --add feature.manyFiles $FEATURE_MANY_FILES
+		if test "$ENV_VAR_VERSION" -ne 0
+		then
+			GIT_INDEX_VERSION=$ENV_VAR_VERSION &&
+			export GIT_INDEX_VERSION
+		else
+			unset GIT_INDEX_VERSION
+		fi &&
 		git add a 2>&1 &&
-		echo 4 >expect &&
+		echo $EXPECTED_OUTPUT_VERSION >expect &&
 		test-tool index-version <.git/index >actual &&
 		test_cmp expect actual
 	)
+}
+
+test_expect_success 'index version config precedence' '
+	test_index_version 2 false 4 4 &&
+	test_index_version 2 true 0 2 &&
+	test_index_version 0 true 0 4 &&
+	test_index_version 0 true 2 2
 '
 
 test_done

gitgitgadget

From: Derrick Stolee <[email protected]>

The 'feature.experimental' setting includes config options that are not committed to become defaults, but could use additional testing.

Update the following config settings to take new defaults, and to use the repo_settings struct if not already using them:

* 'pack.useSparse=true'

* 'merge.directoryRenames=true'

* 'fetch.negotiationAlgorithm=skipping'

In the case of fetch.negotiationAlgorithm, the existing logic would load the config option only when about to use the setting, so had a die() statement on an unknown string value. This is removed as now the config is parsed under prepare_repo_settings(). In general, this die() is probably misplaced and not valuable. A test was removed that checked this die() statement executed.

---
 Documentation/config/feature.txt     | 17 +++++++++++++++
 Documentation/config/fetch.txt       |  3 ++-
 Documentation/config/merge.txt       |  3 ++-
 Documentation/config/pack.txt        |  3 ++-
 builtin/am.c                         |  4 +++-
 fetch-negotiator.c                   | 26 +++++++++++-----------
 fetch-negotiator.h                   |  5 +++--
 fetch-pack.c                         | 11 +++++-----
 merge-recursive.c                    | 32 +++++++++++++++-------------
 merge-recursive.h                    |  1 -
 repo-settings.c                      | 32 +++++++++++++++++++++++++++-
 repo-settings.h                      | 12 ++++++++++-
 t/t5552-skipping-fetch-negotiator.sh | 23 --------------------
 13 files changed, 107 insertions(+), 65 deletions(-)

index c2d9ef7473..287621b1e8 100644
@@ -4,6 +4,23 @@ feature.*::
 	developer community as recommended defaults and are subject to change.
 	In particular, new config options may be added with different defaults.
 
+feature.experimental::
+	Enable config options that are new to Git, and are being considered for
+	future defaults. Config settings included here may be added or removed
+	with each release, including minor version updates. These settings may
+	have unintended interactions since they are so new. Please enable this
+	setting if you are interested in providing feedback on experimental
+	features. The new default values are:
++
+* `pack.useSparse=true` uses a new algorithm when constructing a pack-file
+which can improve `git push` performance in repos with many files.
++
+* `merge.directoryRenames=true` uses a new algorithm for detecting renames by
+using entire directories at a time instead of single files at a time.
++
+* `fetch.negotiationAlgorithm=skipping` may improve fetch negotiation times by
+skipping more commits at a time, reducing the number of round trips.
+
 feature.manyCommits::
 	Enable config options that optimize for repos with many commits. This
 	setting is recommended for repos with at least 100,000 commits. The

index ba890b5884..d402110638 100644
@@ -59,7 +59,8 @@ fetch.negotiationAlgorithm::
 	effort to converge faster, but may result in a larger-than-necessary
 	packfile; The default is "default" which instructs Git to use the default algorithm
 	that never skips commits (unless the server has acknowledged it or one
-	of its descendants).
+	of its descendants). If `feature.experimental` is enabled, then this
+	setting defaults to "skipping".
 	Unknown values will cause 'git fetch' to error out.
 +
 See also the `--negotiation-tip` option for linkgit:git-fetch[1].

index 6a313937f8..e8def2d63c 100644
@@ -54,7 +54,8 @@ merge.directoryRenames::
 	moved into the new directory.  If set to "conflict", a conflict
 	will be reported for such paths.  If merge.renames is false,
 	merge.directoryRenames is ignored and treated as false.  Defaults
-	to "conflict".
+	to "conflict" unless `feature.experimental` is enabled and the
+	default is "true".
 
 merge.renormalize::
 	Tell Git that canonical representation of files in the

index 9cdcfa7324..1d66f0c992 100644
@@ -112,7 +112,8 @@ pack.useSparse::
 	objects. This can have significant performance benefits when
 	computing a pack to send a small change. However, it is possible
 	that extra objects are added to the pack-file if the included
-	commits contain certain types of direct renames.
+	commits contain certain types of direct renames. Default is `false`
+	unless `feature.experimental` is enabled.
 
 pack.writeBitmaps (deprecated)::
 	This is a deprecated synonym for `repack.writeBitmaps`.

index 1aea657a7f..5f3ee3e6cd 100644
@@ -34,6 +34,7 @@
 #include "string-list.h"
 #include "packfile.h"
 #include "repository.h"
+#include "repo-settings.h"
 
 /**
  * Returns the length of the first line of msg.

@@ -1538,7 +1539,8 @@ static int fall_back_threeway(const struct am_state *state, const char *index_pa
 	o.branch1 = "HEAD";
 	their_tree_name = xstrfmt("%.*s", linelen(state->msg), state->msg);
 	o.branch2 = their_tree_name;
-	o.detect_directory_renames = 0;
+	prepare_repo_settings(the_repository);
+	the_repository->settings->merge_directory_renames = 0;
 
 	if (state->quiet)
 		o.verbosity = 0;

index d6d685cba0..e2990cf46e 100644
@@ -2,19 +2,21 @@
 #include "fetch-negotiator.h"
 #include "negotiator/default.h"
 #include "negotiator/skipping.h"
+#include "repository.h"
+#include "repo-settings.h"
 
-void fetch_negotiator_init(struct fetch_negotiator *negotiator,
-			   const char *algorithm)
+void fetch_negotiator_init(struct repository *r,
+			   struct fetch_negotiator *negotiator)
 {
-	if (algorithm) {
-		if (!strcmp(algorithm, "skipping")) {
-			skipping_negotiator_init(negotiator);
-			return;
-		} else if (!strcmp(algorithm, "default")) {
-			/* Fall through to default initialization */
-		} else {
-			die("unknown fetch negotiation algorithm '%s'", algorithm);
-		}
+	prepare_repo_settings(r);
+	switch(r->settings->fetch_negotiation_algorithm) {
+	case FETCH_NEGOTIATION_SKIPPING:
+		skipping_negotiator_init(negotiator);
+		return;
+
+	case FETCH_NEGOTIATION_DEFAULT:
+	default:
+		default_negotiator_init(negotiator);
+		return;
 	}
-	default_negotiator_init(negotiator);
 }

index 9e3967ce66..ea78868504 100644
@@ -2,6 +2,7 @@
 #define FETCH_NEGOTIATOR_H
 
 struct commit;
+struct repository;
 
 /*
  * An object that supplies the information needed to negotiate the contents of

@@ -52,7 +53,7 @@ struct fetch_negotiator {
 	void *data;
 };
 
-void fetch_negotiator_init(struct fetch_negotiator *negotiator,
-			   const char *algorithm);
+void fetch_negotiator_init(struct repository *r,
+			   struct fetch_negotiator *negotiator);
 
 #endif

index 65be043f2a..d81f47c07b 100644
@@ -36,7 +36,6 @@ static int agent_supported;
 static int server_supports_filtering;
 static struct lock_file shallow_lock;
 static const char *alternate_shallow_file;
-static char *negotiation_algorithm;
 static struct strbuf fsck_msg_types = STRBUF_INIT;
 
 /* Remember to update object flag allocation in object.h */

@@ -892,12 +891,13 @@ static struct ref *do_fetch_pack(struct fetch_pack_args *args,
 				 struct shallow_info *si,
 				 char **pack_lockfile)
 {
+	struct repository *r = the_repository;
 	struct ref *ref = copy_ref_list(orig_ref);
 	struct object_id oid;
 	const char *agent_feature;
 	int agent_len;
 	struct fetch_negotiator negotiator;
-	fetch_negotiator_init(&negotiator, negotiation_algorithm);
+	fetch_negotiator_init(r, &negotiator);
 
 	sort_ref_list(&ref, ref_compare_name);
 	QSORT(sought, nr_sought, cmp_ref_by_name);

@@ -911,7 +911,7 @@ static struct ref *do_fetch_pack(struct fetch_pack_args *args,
 
 	if (server_supports("shallow"))
 		print_verbose(args, _("Server supports %s"), "shallow");
-	else if (args->depth > 0 || is_repository_shallow(the_repository))
+	else if (args->depth > 0 || is_repository_shallow(r))
 		die(_("Server does not support shallow clients"));
 	if (args->depth > 0 || args->deepen_since || args->deepen_not)
 		args->deepen = 1;

@@ -1379,6 +1379,7 @@ static struct ref *do_fetch_pack_v2(struct fetch_pack_args *args,
 				    struct shallow_info *si,
 				    char **pack_lockfile)
 {
+	struct repository *r = the_repository;
 	struct ref *ref = copy_ref_list(orig_ref);
 	enum fetch_state state = FETCH_CHECK_LOCAL;
 	struct oidset common = OIDSET_INIT;

@@ -1386,7 +1387,7 @@ static struct ref *do_fetch_pack_v2(struct fetch_pack_args *args,
 	int in_vain = 0;
 	int haves_to_send = INITIAL_FLUSH;
 	struct fetch_negotiator negotiator;
-	fetch_negotiator_init(&negotiator, negotiation_algorithm);
+	fetch_negotiator_init(r, &negotiator);
 	packet_reader_init(&reader, fd[0], NULL, 0,
 			   PACKET_READ_CHOMP_NEWLINE |
 			   PACKET_READ_DIE_ON_ERR_PACKET);

@@ -1505,8 +1506,6 @@ static void fetch_pack_config(void)
 	git_config_get_bool("repack.usedeltabaseoffset", &prefer_ofs_delta);
 	git_config_get_bool("fetch.fsckobjects", &fetch_fsck_objects);
 	git_config_get_bool("transfer.fsckobjects", &transfer_fsck_objects);
-	git_config_get_string("fetch.negotiationalgorithm",
-			      &negotiation_algorithm);
 
 	git_config(fetch_pack_config_cb, NULL);
 }

index 12300131fc..162d5a4753 100644
@@ -28,6 +28,7 @@
 #include "submodule.h"
 #include "revision.h"
 #include "commit-reach.h"
+#include "repo-settings.h"
 
 struct path_hashmap_entry {
 	struct hashmap_entry e;

@@ -1375,10 +1376,14 @@ static int handle_rename_via_dir(struct merge_options *opt,
 	 * there is no content merge to do; just move the file into the
 	 * desired final location.
 	 */
+	struct repository *r = the_repository;
 	const struct rename *ren = ci->ren1;
 	const struct diff_filespec *dest = ren->pair->two;
 	char *file_path = dest->path;
-	int mark_conflicted = (opt->detect_directory_renames == 1);
+	int mark_conflicted;
+
+	prepare_repo_settings(r);
+	mark_conflicted = (r->settings->merge_directory_renames == MERGE_DIRECTORY_RENAMES_CONFLICT);
 	assert(ren->dir_rename_original_dest);
 
 	if (!opt->call_depth && would_lose_untracked(opt, dest->path)) {

@@ -2850,6 +2855,7 @@ static int detect_and_process_renames(struct merge_options *opt,
 				      struct string_list *entries,
 				      struct rename_info *ri)
 {
+	struct repository *r = the_repository;
 	struct diff_queue_struct *head_pairs, *merge_pairs;
 	struct hashmap *dir_re_head, *dir_re_merge;
 	int clean = 1;

@@ -2863,7 +2869,8 @@ static int detect_and_process_renames(struct merge_options *opt,
 	head_pairs = get_diffpairs(opt, common, head);
 	merge_pairs = get_diffpairs(opt, common, merge);
 
-	if (opt->detect_directory_renames) {
+	prepare_repo_settings(r);
+	if (r->settings->merge_directory_renames) {
 		dir_re_head = get_directory_renames(head_pairs);
 		dir_re_merge = get_directory_renames(merge_pairs);
 

@@ -3112,6 +3119,7 @@ static int handle_rename_normal(struct merge_options *opt,
 				const struct diff_filespec *b,
 				struct rename_conflict_info *ci)
 {
+	struct repository *r = the_repository;
 	struct rename *ren = ci->ren1;
 	struct merge_file_info mfi;
 	int clean;

@@ -3121,7 +3129,9 @@ static int handle_rename_normal(struct merge_options *opt,
 	clean = handle_content_merge(&mfi, opt, path, was_dirty(opt, path),
 				     o, a, b, ci);
 
-	if (clean && opt->detect_directory_renames == 1 &&
+	prepare_repo_settings(r);
+	if (clean &&
+	    r->settings->merge_directory_renames == MERGE_DIRECTORY_RENAMES_CONFLICT &&
 	    ren->dir_rename_original_dest) {
 		if (update_stages(opt, path,
 				  NULL,

@@ -3155,6 +3165,7 @@ static void dir_rename_warning(const char *msg,
 static int warn_about_dir_renamed_entries(struct merge_options *opt,
 					  struct rename *ren)
 {
+	struct repository *r = the_repository;
 	const char *msg;
 	int clean = 1, is_add;
 

@@ -3166,12 +3177,13 @@ static int warn_about_dir_renamed_entries(struct merge_options *opt,
 		return clean;
 
 	/* Sanity checks */
-	assert(opt->detect_directory_renames > 0);
+	prepare_repo_settings(r);
+	assert(r->settings->merge_directory_renames > 0);
 	assert(ren->dir_rename_original_type == 'A' ||
 	       ren->dir_rename_original_type == 'R');
 
 	/* Check whether to treat directory renames as a conflict */
-	clean = (opt->detect_directory_renames == 2);
+	clean = (r->settings->merge_directory_renames == MERGE_DIRECTORY_RENAMES_TRUE);
 
 	is_add = (ren->dir_rename_original_type == 'A');
 	if (ren->dir_rename_original_type == 'A' && clean) {

@@ -3662,15 +3674,6 @@ static void merge_recursive_config(struct merge_options *opt)
 		opt->merge_detect_rename = git_config_rename("merge.renames", value);
 		free(value);
 	}
-	if (!git_config_get_string("merge.directoryrenames", &value)) {
-		int boolval = git_parse_maybe_bool(value);
-		if (0 <= boolval) {
-			opt->detect_directory_renames = boolval ? 2 : 0;
-		} else if (!strcasecmp(value, "conflict")) {
-			opt->detect_directory_renames = 1;
-		} /* avoid erroring on values from future versions of git */
-		free(value);
-	}
 	git_config(git_xmerge_config, NULL);
 }
 

@@ -3687,7 +3690,6 @@ void init_merge_options(struct merge_options *opt,
 	opt->renormalize = 0;
 	opt->diff_detect_rename = -1;
 	opt->merge_detect_rename = -1;
-	opt->detect_directory_renames = 1;
 	merge_recursive_config(opt);
 	merge_verbosity = getenv("GIT_MERGE_VERBOSITY");
 	if (merge_verbosity)

index c2b7bb65c6..b8eba244ee 100644
@@ -22,7 +22,6 @@ struct merge_options {
 	unsigned renormalize : 1;
 	long xdl_opts;
 	int verbosity;
-	int detect_directory_renames;
 	int diff_detect_rename;
 	int merge_detect_rename;
 	int diff_rename_limit;

index 9e4b8e6268..5e9249c437 100644
@@ -9,6 +9,12 @@ static int git_repo_config(const char *key, const char *value, void *cb)
 {
 	struct repo_settings *rs = (struct repo_settings *)cb;
 
+	if (!strcmp(key, "feature.experimental")) {
+		UPDATE_DEFAULT(rs->pack_use_sparse, 1);
+		UPDATE_DEFAULT(rs->merge_directory_renames, MERGE_DIRECTORY_RENAMES_TRUE);
+		UPDATE_DEFAULT(rs->fetch_negotiation_algorithm, FETCH_NEGOTIATION_SKIPPING);
+		return 0;
+	}
 	if (!strcmp(key, "feature.manycommits")) {
 		UPDATE_DEFAULT(rs->core_commit_graph, 1);
 		UPDATE_DEFAULT(rs->gc_write_commit_graph, 1);

@@ -50,6 +56,23 @@ static int git_repo_config(const char *key, const char *value, void *cb)
 				"using 'keep' default value"), value);
 		return 0;
 	}
+	if (!strcmp(key, "merge.directoryrenames")) {
+		int bool_value = git_parse_maybe_bool(value);
+		if (0 <= bool_value) {
+			rs->merge_directory_renames = bool_value ? MERGE_DIRECTORY_RENAMES_TRUE : 0;
+		} else if (!strcasecmp(value, "conflict")) {
+			rs->merge_directory_renames = MERGE_DIRECTORY_RENAMES_CONFLICT;
+		}
+		return 0;
+	}
+	if (!strcmp(key, "fetch.negotiationalgorithm"))	{
+		if (!strcasecmp(value, "skipping")) {
+			rs->fetch_negotiation_algorithm = FETCH_NEGOTIATION_SKIPPING;
+		} else {
+			rs->fetch_negotiation_algorithm = FETCH_NEGOTIATION_DEFAULT;
+		}
+		return 0;
+	}
 
 	return 1;
 }

@@ -64,10 +87,14 @@ void prepare_repo_settings(struct repository *r)
 	/* Defaults */
 	r->settings->core_commit_graph = -1;
 	r->settings->gc_write_commit_graph = -1;
-	r->settings->pack_use_sparse = -1;
+
 	r->settings->index_version = -1;
 	r->settings->core_untracked_cache = -1;
 
+	r->settings->pack_use_sparse = -1;
+	r->settings->merge_directory_renames = -1;
+	r->settings->fetch_negotiation_algorithm = -1;
+
 	repo_config(r, git_repo_config, r->settings);
 
 	/* Hack for test programs like test-dump-untracked-cache */

@@ -75,4 +102,7 @@ void prepare_repo_settings(struct repository *r)
 		r->settings->core_untracked_cache = CORE_UNTRACKED_CACHE_KEEP;
 	else
 		UPDATE_DEFAULT(r->settings->core_untracked_cache, CORE_UNTRACKED_CACHE_KEEP);
+
+	UPDATE_DEFAULT(r->settings->merge_directory_renames, MERGE_DIRECTORY_RENAMES_CONFLICT);
+	UPDATE_DEFAULT(r->settings->fetch_negotiation_algorithm, FETCH_NEGOTIATION_DEFAULT);
 }

index bac9b87d49..cecf7d0028 100644
@@ -4,12 +4,22 @@
 #define CORE_UNTRACKED_CACHE_WRITE (1 << 0)
 #define CORE_UNTRACKED_CACHE_KEEP (1 << 1)
 
+#define MERGE_DIRECTORY_RENAMES_CONFLICT 1
+#define MERGE_DIRECTORY_RENAMES_TRUE 2
+
+#define FETCH_NEGOTIATION_DEFAULT 1
+#define FETCH_NEGOTIATION_SKIPPING 2
+
 struct repo_settings {
 	int core_commit_graph;
 	int gc_write_commit_graph;
-	int pack_use_sparse;
+
 	int index_version;
 	int core_untracked_cache;
+
+	int pack_use_sparse;
+	int merge_directory_renames;
+	int fetch_negotiation_algorithm;
 };
 
 struct repository;

index 8a14be51a1..f70cbcc9ca 100755
@@ -60,29 +60,6 @@ test_expect_success 'commits with no parents are sent regardless of skip distanc
 	have_not_sent c6 c4 c3
 '
 
-test_expect_success 'unknown fetch.negotiationAlgorithm values error out' '
-	rm -rf server client trace &&
-	git init server &&
-	test_commit -C server to_fetch &&
-
-	git init client &&
-	test_commit -C client on_client &&
-	git -C client checkout on_client &&
-
-	test_config -C client fetch.negotiationAlgorithm invalid &&
-	test_must_fail git -C client fetch "$(pwd)/server" 2>err &&
-	test_i18ngrep "unknown fetch negotiation algorithm" err &&
-
-	# Explicit "default" value
-	test_config -C client fetch.negotiationAlgorithm default &&
-	git -C client -c fetch.negotiationAlgorithm=default fetch "$(pwd)/server" &&
-
-	# Implementation detail: If there is nothing to fetch, we will not error out
-	test_config -C client fetch.negotiationAlgorithm invalid &&
-	git -C client fetch "$(pwd)/server" 2>err &&
-	test_i18ngrep ! "unknown fetch negotiation algorithm" err
-'
-
 test_expect_success 'when two skips collide, favor the larger one' '
 	rm -rf server client trace &&
 	git init server &&

gitgitgadget

Hi Stolee,

Good.

[snip]

Yes, thanks! Your suggestion makes much more sense and makes the diff much simpler. No reason to couple too closely to the config storage here.

I think you gave me enough feedback to merit a v2 before others chime in, so I'll send one after I validate my current version.

Thanks,
-Stolee

This is a brand-new thread to replace ds/early-access. The discussion on that thread was very helpful [1].

With this in mind, I propose instead a set of "feature.*" config settings that form groups of "community recommended" settings (with some caveats). In the space below, I'll list a set of possible feature names and the implied config options.

First, the main two categories we've discussed so far: many commits and many files. These two feature sets are for when your repo is large in one of these dimensions. Perhaps there are other settings to include in these?

feature.manyFiles:
index.version = 4
core.untrackedCache = true

feature.manyCommits:
core.commitGraph = true
gc.writeCommitGraph = true
(future: fetch.writeSplitCommitGraph = true)

Note: the fetch.writeSplitCommitGraph does not exist yet, but could be introduced in a later release to write a new commit-graph (with --split) on fetch.

The other category that has been discussed already is that of "experimental features that we generally think are helpful but change behavior slightly in some cases".

feature.experimental:
pack.useSparse = true
merge.directoryRenames = true
fetch.negotiationAlgorithm = skipping

Specifically, this setting is for config values we are not sure will ever be on by default, but additional testing is needed to be sure. This is different than a possible 'feature.preview' setting that would include config settings that we are committed to updating the defaults in a future release. There are many ways we can take this idea in the future (including more additions to these categories).

Thanks, -Stolee

Updates in V2: I'm responding to Dscho's comments early, as they were very valuable.

* Rearranged how we are checking for the feature.* settings, so if one layer turns the setting on and a later layer turns it off, we do not adjust the defaults.

* Switched to using enums for the non-boolean config options.

* Placed the repo_settings struct directly in the repository struct.

* All struct members are initialized to -1 using memset.

* The config changes around directory rename detection is greatly simplified.

[1] https://public-inbox.org/git/[email protected]/

Derrick Stolee (5):
repo-settings: consolidate some config settings
repo-settings: add feature.manyCommits setting
repo-settings: parse core.untrackedCache
repo-settings: create feature.manyFiles setting
repo-settings: create feature.experimental setting

 Documentation/config.txt             |  2 +
 Documentation/config/core.txt        |  7 ++-
 Documentation/config/feature.txt     | 42 +++++++++++++++
 Documentation/config/fetch.txt       |  3 +-
 Documentation/config/gc.txt          |  4 +-
 Documentation/config/index.txt       |  1 +
 Documentation/config/merge.txt       |  3 +-
 Documentation/config/pack.txt        |  3 +-
 Makefile                             |  1 +
 builtin/am.c                         |  1 +
 builtin/gc.c                         | 13 +++--
 builtin/pack-objects.c               |  9 ++--
 builtin/update-index.c               |  7 ++-
 commit-graph.c                       |  7 +--
 config.c                             | 24 ---------
 fetch-negotiator.c                   | 26 +++++-----
 fetch-negotiator.h                   |  5 +-
 fetch-pack.c                         | 11 ++--
 merge-recursive.c                    | 15 +++---
 read-cache.c                         | 31 +++++------
 repo-settings.c                      | 78 ++++++++++++++++++++++++++++
 repo-settings.h                      | 41 +++++++++++++++
 repository.h                         |  5 ++
 t/t1600-index.sh                     | 31 +++++++++--
 t/t5552-skipping-fetch-negotiator.sh | 23 --------
 25 files changed, 274 insertions(+), 119 deletions(-)
 create mode 100644 Documentation/config/feature.txt
 create mode 100644 repo-settings.c
 create mode 100644 repo-settings.h

base-commit: 9c9b961d7eb15fb583a2a812088713a68a85f1c0
Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-292%2Fderrickstolee%2Frepo-settings%2Fhead-v2
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-292/derrickstolee/repo-settings/head-v2
Pull-Request: https://github.com/gitgitgadget/git/pull/292

Range-diff vs v1:

1: 72f652b89c ! 1: 597ab7d621 repo-settings: consolidate some config settings

     @@ -64,7 +64,7 @@
      -					 NULL))
      -		return 1;
      +	prepare_repo_settings(the_repository);
     -+	if (the_repository->settings->gc_write_commit_graph == 1)
     ++	if (the_repository->settings.gc_write_commit_graph == 1)
      +		write_commit_graph_reachable(get_object_directory(),
      +					     !quiet && !daemonized ? COMMIT_GRAPH_PROGRESS : 0,
      +					     NULL);
     @@ -99,8 +99,8 @@
       
       	sparse = git_env_bool("GIT_TEST_PACK_SPARSE", 0);
      +	prepare_repo_settings(the_repository);
     -+	if (!sparse && the_repository->settings->pack_use_sparse != -1)
     -+		sparse = the_repository->settings->pack_use_sparse;
     ++	if (!sparse && the_repository->settings.pack_use_sparse != -1)
     ++		sparse = the_repository->settings.pack_use_sparse;
      +
       	reset_pack_idx_option(&pack_idx_opts);
       	git_config(git_pack_config, NULL);
     @@ -134,7 +134,7 @@
       	if (!git_env_bool(GIT_TEST_COMMIT_GRAPH, 0) &&
      -	    (repo_config_get_bool(r, "core.commitgraph", &config_value) ||
      -	    !config_value))
     -+	    r->settings->core_commit_graph != 1)
     ++	    r->settings.core_commit_graph != 1)
       		/*
       		 * This repository is not configured to use commit graphs, so
       		 * do not load one. (But report commit_graph_attempted anyway
     @@ -167,8 +167,8 @@
      -			version = value;
      +		prepare_repo_settings(r);
      +
     -+		if (r->settings->index_version >= 0)
     -+			version = r->settings->index_version;
     ++		if (r->settings.index_version >= 0)
     ++			version = r->settings.index_version;
       		if (version < INDEX_FORMAT_LB || INDEX_FORMAT_UB < version) {
       			warning(_("index.version set, but the value is invalid.\n"
       				  "Using version %i"), INDEX_FORMAT_DEFAULT);
     @@ -192,44 +192,28 @@
      +#include "config.h"
      +#include "repo-settings.h"
      +
     -+static int git_repo_config(const char *key, const char *value, void *cb)
     ++void prepare_repo_settings(struct repository *r)
      +{
     -+	struct repo_settings *rs = (struct repo_settings *)cb;
     ++	int value;
      +
     -+	if (!strcmp(key, "core.commitgraph")) {
     -+		rs->core_commit_graph = git_config_bool(key, value);
     -+		return 0;
     -+	}
     -+	if (!strcmp(key, "gc.writecommitgraph")) {
     -+		rs->gc_write_commit_graph = git_config_bool(key, value);
     -+		return 0;
     -+	}
     -+	if (!strcmp(key, "pack.usesparse")) {
     -+		rs->pack_use_sparse = git_config_bool(key, value);
     -+		return 0;
     -+	}
     -+	if (!strcmp(key, "index.version")) {
     -+		rs->index_version = git_config_int(key, value);
     -+		return 0;
     -+	}
     ++	if (r->settings_initialized)
     ++		return;
      +
     -+	return 1;
     -+}
     ++	/* Defaults */
     ++	memset(&r->settings, -1, sizeof(r->settings));
      +
     -+void prepare_repo_settings(struct repository *r)
     -+{
     -+	if (r->settings)
     -+		return;
     ++	if (!repo_config_get_bool(r, "core.commitgraph", &value))
     ++		r->settings.core_commit_graph = value;
     ++	if (!repo_config_get_bool(r, "gc.writecommitgraph", &value))
     ++		r->settings.gc_write_commit_graph = value;
      +
     -+	r->settings = xmalloc(sizeof(*r->settings));
     ++	if (!repo_config_get_bool(r, "index.version", &value))
     ++		r->settings.index_version = value;
      +
     -+	/* Defaults */
     -+	r->settings->core_commit_graph = -1;
     -+	r->settings->gc_write_commit_graph = -1;
     -+	r->settings->pack_use_sparse = -1;
     -+	r->settings->index_version = -1;
     ++	if (!repo_config_get_bool(r, "pack.usesparse", &value))
     ++		r->settings.pack_use_sparse = value;
      +
     -+	repo_config(r, git_repo_config, r->settings);
     ++	r->settings_initialized = 1;
      +}
      
       diff --git a/repo-settings.h b/repo-settings.h
     @@ -243,8 +227,10 @@
      +struct repo_settings {
      +	int core_commit_graph;
      +	int gc_write_commit_graph;
     -+	int pack_use_sparse;
     ++
      +	int index_version;
     ++
     ++	int pack_use_sparse;
      +};
      +
      +struct repository;
     @@ -257,7 +243,10 @@
       --- a/repository.h
       +++ b/repository.h
      @@
     + #define REPOSITORY_H
     + 
       #include "path.h"
     ++#include "repo-settings.h"
       
       struct config_set;
      +struct repo_settings;
     @@ -268,7 +257,8 @@
       	 */
       	char *submodule_prefix;
       
     -+	struct repo_settings *settings;
     ++	int settings_initialized;
     ++	struct repo_settings settings;
      +
       	/* Subsystems */
       	/*

2: e00a1be75b ! 2: 86380c7832 repo-settings: add feature.manyCommits setting
@@ -4,7 +4,7 @@

When a repo has many commits, it is helpful to write and read the commit-graph file. Future changes to Git may include new config
- settings that are benefitial in this scenario.
+ settings that are beneficial in this scenario.

Create the 'feature.manyCommits' config setting that changes the default values of 'core.commitGraph' and 'gc.writeCommitGraph' to
@@ -85,15 +85,17 @@

+#define UPDATE_DEFAULT(s,v) do { if (s == -1) { s = v; } } while(0) +
- static int git_repo_config(const char *key, const char *value, void *cb)
+ void prepare_repo_settings(struct repository *r)
{
- struct repo_settings *rs = (struct repo_settings *)cb;
+ int value;
+@@
+ if (!repo_config_get_bool(r, "pack.usesparse", &value))
+ r->settings.pack_use_sparse = value;

-+ if (!strcmp(key, "feature.manycommits")) {
-+ UPDATE_DEFAULT(rs->core_commit_graph, 1);
-+ UPDATE_DEFAULT(rs->gc_write_commit_graph, 1);
-+ return 0;
++ if (!repo_config_get_bool(r, "feature.manycommits", &value) && value) {
++ UPDATE_DEFAULT(r->settings.core_commit_graph, 1);
++ UPDATE_DEFAULT(r->settings.gc_write_commit_graph, 1);
+ }
- if (!strcmp(key, "core.commitgraph")) {
- rs->core_commit_graph = git_config_bool(key, value);
- return 0;
++
+ r->settings_initialized = 1;
+ }
3: 47ae3e7d4d ! 3: 49be7a7345 repo-settings: parse core.untrackedCache

     @@ -16,10 +16,9 @@
          untracked cache and persist it.
      
          Instead of relying on magic values of -1, 0, and 1, split these
     -    options into bitflags CORE_UNTRACKED_CACHE_KEEP and
     -    CORE_UNTRACKED_CACHE_WRITE. This allows the use of "-1" as a
     +    options into an enum. This allows the use of "-1" as a
          default value. After parsing the config options, if the value is
     -    unset we can initialize it to CORE_UNTRACKED_CACHE_KEEP.
     +    unset we can initialize it to UNTRACKED_CACHE_KEEP.
      
          Signed-off-by: Derrick Stolee <[email protected]>
      
     @@ -52,7 +51,7 @@
       		break;
       	case UC_DISABLE:
      -		if (git_config_get_untracked_cache() == 1)
     -+		if (r->settings->core_untracked_cache & CORE_UNTRACKED_CACHE_WRITE)
     ++		if (r->settings.core_untracked_cache == UNTRACKED_CACHE_WRITE)
       			warning(_("core.untrackedCache is set to true; "
       				  "remove or change it, if you really want to "
       				  "disable the untracked cache"));
     @@ -61,7 +60,7 @@
       	case UC_ENABLE:
       	case UC_FORCE:
      -		if (git_config_get_untracked_cache() == 0)
     -+		if (r->settings->core_untracked_cache == 0)
     ++		if (r->settings.core_untracked_cache == UNTRACKED_CACHE_REMOVE)
       			warning(_("core.untrackedCache is set to false; "
       				  "remove or change it, if you really want to "
       				  "enable the untracked cache"));
     @@ -116,7 +115,7 @@
      +
      +	prepare_repo_settings(r);
      +
     -+	if (!(r->settings->core_untracked_cache & CORE_UNTRACKED_CACHE_KEEP)) {
     ++	if (r->settings.core_untracked_cache  == UNTRACKED_CACHE_REMOVE) {
       		remove_untracked_cache(istate);
      -		break;
      -	case 1: /* true */
     @@ -127,7 +126,7 @@
      +		return;
       	}
      +
     -+	if (r->settings->core_untracked_cache & CORE_UNTRACKED_CACHE_WRITE)
     ++	if (r->settings.core_untracked_cache == UNTRACKED_CACHE_WRITE)
      +		add_untracked_cache(istate);
       }
       
     @@ -137,39 +136,46 @@
       --- a/repo-settings.c
       +++ b/repo-settings.c
      @@
     - 		rs->index_version = git_config_int(key, value);
     - 		return 0;
     - 	}
     -+	if (!strcmp(key, "core.untrackedcache")) {
     -+		int bool_value = git_parse_maybe_bool(value);
     -+		if (bool_value == 0)
     -+			rs->core_untracked_cache = 0;
     -+		else if (bool_value == 1)
     -+			rs->core_untracked_cache = CORE_UNTRACKED_CACHE_KEEP |
     -+						   CORE_UNTRACKED_CACHE_WRITE;
     -+		else if (!strcasecmp(value, "keep"))
     -+			rs->core_untracked_cache = CORE_UNTRACKED_CACHE_KEEP;
     -+		else
     -+			error(_("unknown core.untrackedCache value '%s'; "
     -+				"using 'keep' default value"), value);
     -+		return 0;
     -+	}
     + void prepare_repo_settings(struct repository *r)
     + {
     + 	int value;
     ++	char *strval;
       
     - 	return 1;
     - }
     + 	if (r->settings_initialized)
     + 		return;
      @@
     - 	r->settings->gc_write_commit_graph = -1;
     - 	r->settings->pack_use_sparse = -1;
     - 	r->settings->index_version = -1;
     -+	r->settings->core_untracked_cache = -1;
       
     - 	repo_config(r, git_repo_config, r->settings);
     + 	if (!repo_config_get_bool(r, "index.version", &value))
     + 		r->settings.index_version = value;
     ++	if (!repo_config_get_maybe_bool(r, "core.untrackedcache", &value)) {
     ++		if (value == 0)
     ++			r->settings.core_untracked_cache = UNTRACKED_CACHE_REMOVE;
     ++		else
     ++			r->settings.core_untracked_cache = UNTRACKED_CACHE_WRITE;
     ++	} else if (!repo_config_get_string(r, "core.untrackedcache", &strval)) {
     ++		if (!strcasecmp(strval, "keep"))
     ++			r->settings.core_untracked_cache = UNTRACKED_CACHE_KEEP;
     ++
     ++		free(strval);
     ++	}
      +
     + 
     + 	if (!repo_config_get_bool(r, "pack.usesparse", &value))
     + 		r->settings.pack_use_sparse = value;
     +-
     ++	
     + 	if (!repo_config_get_bool(r, "feature.manycommits", &value) && value) {
     + 		UPDATE_DEFAULT(r->settings.core_commit_graph, 1);
     + 		UPDATE_DEFAULT(r->settings.gc_write_commit_graph, 1);
     + 	}
     + 
      +	/* Hack for test programs like test-dump-untracked-cache */
      +	if (ignore_untracked_cache_config)
     -+		r->settings->core_untracked_cache = CORE_UNTRACKED_CACHE_KEEP;
     ++		r->settings.core_untracked_cache = UNTRACKED_CACHE_KEEP;
      +	else
     -+		UPDATE_DEFAULT(r->settings->core_untracked_cache, CORE_UNTRACKED_CACHE_KEEP);
     ++		UPDATE_DEFAULT(r->settings.core_untracked_cache, UNTRACKED_CACHE_KEEP);
     ++
     + 	r->settings_initialized = 1;
       }
      
       diff --git a/repo-settings.h b/repo-settings.h
     @@ -179,15 +185,19 @@
       #ifndef REPO_SETTINGS_H
       #define REPO_SETTINGS_H
       
     -+#define CORE_UNTRACKED_CACHE_WRITE (1 << 0)
     -+#define CORE_UNTRACKED_CACHE_KEEP (1 << 1)
     ++enum untracked_cache_setting {
     ++	UNTRACKED_CACHE_UNSET = -1,
     ++	UNTRACKED_CACHE_REMOVE = 0,
     ++	UNTRACKED_CACHE_KEEP = 1,
     ++	UNTRACKED_CACHE_WRITE = 2
     ++};
      +
       struct repo_settings {
       	int core_commit_graph;
       	int gc_write_commit_graph;
     - 	int pack_use_sparse;
     + 
       	int index_version;
     -+	int core_untracked_cache;
     - };
     ++	enum untracked_cache_setting core_untracked_cache;
       
     - struct repository;
     + 	int pack_use_sparse;
     + };
 4:  63b522a858 ! 4:  86a5a0c589 repo-settings: create feature.manyFiles setting
     @@ -63,18 +63,16 @@
       --- a/repo-settings.c
       +++ b/repo-settings.c
      @@
     - 		UPDATE_DEFAULT(rs->gc_write_commit_graph, 1);
     - 		return 0;
     + 		UPDATE_DEFAULT(r->settings.core_commit_graph, 1);
     + 		UPDATE_DEFAULT(r->settings.gc_write_commit_graph, 1);
       	}
     -+	if (!strcmp(key, "feature.manyfiles")) {
     -+		UPDATE_DEFAULT(rs->index_version, 4);
     -+		UPDATE_DEFAULT(rs->core_untracked_cache,
     -+			       CORE_UNTRACKED_CACHE_KEEP | CORE_UNTRACKED_CACHE_WRITE);
     -+		return 0;
     ++	if (!repo_config_get_bool(r, "feature.manyfiles", &value) && value) {
     ++		UPDATE_DEFAULT(r->settings.index_version, 4);
     ++		UPDATE_DEFAULT(r->settings.core_untracked_cache, UNTRACKED_CACHE_WRITE);
      +	}
     - 	if (!strcmp(key, "core.commitgraph")) {
     - 		rs->core_commit_graph = git_config_bool(key, value);
     - 		return 0;
     + 
     + 	/* Hack for test programs like test-dump-untracked-cache */
     + 	if (ignore_untracked_cache_config)
      
       diff --git a/t/t1600-index.sh b/t/t1600-index.sh
       --- a/t/t1600-index.sh
 5:  da7685936f ! 5:  b8a631683a repo-settings: create feature.experimental setting
     @@ -104,16 +104,6 @@
       
       /**
        * Returns the length of the first line of msg.
     -@@
     - 	o.branch1 = "HEAD";
     - 	their_tree_name = xstrfmt("%.*s", linelen(state->msg), state->msg);
     - 	o.branch2 = their_tree_name;
     --	o.detect_directory_renames = 0;
     -+	prepare_repo_settings(the_repository);
     -+	the_repository->settings->merge_directory_renames = 0;
     - 
     - 	if (state->quiet)
     - 		o.verbosity = 0;
      
       diff --git a/fetch-negotiator.c b/fetch-negotiator.c
       --- a/fetch-negotiator.c
     @@ -140,7 +130,7 @@
      -			die("unknown fetch negotiation algorithm '%s'", algorithm);
      -		}
      +	prepare_repo_settings(r);
     -+	switch(r->settings->fetch_negotiation_algorithm) {
     ++	switch(r->settings.fetch_negotiation_algorithm) {
      +	case FETCH_NEGOTIATION_SKIPPING:
      +		skipping_negotiator_init(negotiator);
      +		return;
     @@ -248,83 +238,6 @@
       
       struct path_hashmap_entry {
       	struct hashmap_entry e;
     -@@
     - 	 * there is no content merge to do; just move the file into the
     - 	 * desired final location.
     - 	 */
     -+	struct repository *r = the_repository;
     - 	const struct rename *ren = ci->ren1;
     - 	const struct diff_filespec *dest = ren->pair->two;
     - 	char *file_path = dest->path;
     --	int mark_conflicted = (opt->detect_directory_renames == 1);
     -+	int mark_conflicted;
     -+
     -+	prepare_repo_settings(r);
     -+	mark_conflicted = (r->settings->merge_directory_renames == MERGE_DIRECTORY_RENAMES_CONFLICT);
     - 	assert(ren->dir_rename_original_dest);
     - 
     - 	if (!opt->call_depth && would_lose_untracked(opt, dest->path)) {
     -@@
     - 				      struct string_list *entries,
     - 				      struct rename_info *ri)
     - {
     -+	struct repository *r = the_repository;
     - 	struct diff_queue_struct *head_pairs, *merge_pairs;
     - 	struct hashmap *dir_re_head, *dir_re_merge;
     - 	int clean = 1;
     -@@
     - 	head_pairs = get_diffpairs(opt, common, head);
     - 	merge_pairs = get_diffpairs(opt, common, merge);
     - 
     --	if (opt->detect_directory_renames) {
     -+	prepare_repo_settings(r);
     -+	if (r->settings->merge_directory_renames) {
     - 		dir_re_head = get_directory_renames(head_pairs);
     - 		dir_re_merge = get_directory_renames(merge_pairs);
     - 
     -@@
     - 				const struct diff_filespec *b,
     - 				struct rename_conflict_info *ci)
     - {
     -+	struct repository *r = the_repository;
     - 	struct rename *ren = ci->ren1;
     - 	struct merge_file_info mfi;
     - 	int clean;
     -@@
     - 	clean = handle_content_merge(&mfi, opt, path, was_dirty(opt, path),
     - 				     o, a, b, ci);
     - 
     --	if (clean && opt->detect_directory_renames == 1 &&
     -+	prepare_repo_settings(r);
     -+	if (clean &&
     -+	    r->settings->merge_directory_renames == MERGE_DIRECTORY_RENAMES_CONFLICT &&
     - 	    ren->dir_rename_original_dest) {
     - 		if (update_stages(opt, path,
     - 				  NULL,
     -@@
     - static int warn_about_dir_renamed_entries(struct merge_options *opt,
     - 					  struct rename *ren)
     - {
     -+	struct repository *r = the_repository;
     - 	const char *msg;
     - 	int clean = 1, is_add;
     - 
     -@@
     - 		return clean;
     - 
     - 	/* Sanity checks */
     --	assert(opt->detect_directory_renames > 0);
     -+	prepare_repo_settings(r);
     -+	assert(r->settings->merge_directory_renames > 0);
     - 	assert(ren->dir_rename_original_type == 'A' ||
     - 	       ren->dir_rename_original_type == 'R');
     - 
     - 	/* Check whether to treat directory renames as a conflict */
     --	clean = (opt->detect_directory_renames == 2);
     -+	clean = (r->settings->merge_directory_renames == MERGE_DIRECTORY_RENAMES_TRUE);
     - 
     - 	is_add = (ren->dir_rename_original_type == 'A');
     - 	if (ren->dir_rename_original_type == 'A' && clean) {
      @@
       		opt->merge_detect_rename = git_config_rename("merge.renames", value);
       		free(value);
     @@ -342,115 +255,92 @@
       }
       
      @@
     - 	opt->renormalize = 0;
       	opt->diff_detect_rename = -1;
       	opt->merge_detect_rename = -1;
     --	opt->detect_directory_renames = 1;
     + 	opt->detect_directory_renames = 1;
     ++
     ++	prepare_repo_settings(repo);
     ++	if (repo->settings.merge_directory_renames >= 0)
     ++		opt->detect_directory_renames = repo->settings.merge_directory_renames;
     ++
       	merge_recursive_config(opt);
       	merge_verbosity = getenv("GIT_MERGE_VERBOSITY");
       	if (merge_verbosity)
      
     - diff --git a/merge-recursive.h b/merge-recursive.h
     - --- a/merge-recursive.h
     - +++ b/merge-recursive.h
     -@@
     - 	unsigned renormalize : 1;
     - 	long xdl_opts;
     - 	int verbosity;
     --	int detect_directory_renames;
     - 	int diff_detect_rename;
     - 	int merge_detect_rename;
     - 	int diff_rename_limit;
     -
       diff --git a/repo-settings.c b/repo-settings.c
       --- a/repo-settings.c
       +++ b/repo-settings.c
      @@
     - {
     - 	struct repo_settings *rs = (struct repo_settings *)cb;
     - 
     -+	if (!strcmp(key, "feature.experimental")) {
     -+		UPDATE_DEFAULT(rs->pack_use_sparse, 1);
     -+		UPDATE_DEFAULT(rs->merge_directory_renames, MERGE_DIRECTORY_RENAMES_TRUE);
     -+		UPDATE_DEFAULT(rs->fetch_negotiation_algorithm, FETCH_NEGOTIATION_SKIPPING);
     -+		return 0;
     -+	}
     - 	if (!strcmp(key, "feature.manycommits")) {
     - 		UPDATE_DEFAULT(rs->core_commit_graph, 1);
     - 		UPDATE_DEFAULT(rs->gc_write_commit_graph, 1);
     -@@
     - 				"using 'keep' default value"), value);
     - 		return 0;
     + 		free(strval);
       	}
     -+	if (!strcmp(key, "merge.directoryrenames")) {
     -+		int bool_value = git_parse_maybe_bool(value);
     -+		if (0 <= bool_value) {
     -+			rs->merge_directory_renames = bool_value ? MERGE_DIRECTORY_RENAMES_TRUE : 0;
     -+		} else if (!strcasecmp(value, "conflict")) {
     -+			rs->merge_directory_renames = MERGE_DIRECTORY_RENAMES_CONFLICT;
     -+		}
     -+		return 0;
     + 
     ++	if (!repo_config_get_maybe_bool(r, "merge.directoryrenames", &value))
     ++		r->settings.merge_directory_renames = value ? MERGE_DIRECTORY_RENAMES_TRUE : 0;
     ++	else if (!repo_config_get_string(r, "merge.directoryrenames", &strval)) {
     ++		if (!strcasecmp(strval, "conflict"))
     ++			r->settings.merge_directory_renames = MERGE_DIRECTORY_RENAMES_CONFLICT;
      +	}
     -+	if (!strcmp(key, "fetch.negotiationalgorithm"))	{
     -+		if (!strcasecmp(value, "skipping")) {
     -+			rs->fetch_negotiation_algorithm = FETCH_NEGOTIATION_SKIPPING;
     -+		} else {
     -+			rs->fetch_negotiation_algorithm = FETCH_NEGOTIATION_DEFAULT;
     -+		}
     -+		return 0;
     ++	if (!repo_config_get_string(r, "fetch.negotiationalgorithm", &strval)) {
     ++		if (!strcasecmp(strval, "skipping"))
     ++			r->settings.fetch_negotiation_algorithm = FETCH_NEGOTIATION_SKIPPING;
     ++		else
     ++			r->settings.fetch_negotiation_algorithm = FETCH_NEGOTIATION_DEFAULT;
      +	}
       
     - 	return 1;
     - }
     + 	if (!repo_config_get_bool(r, "pack.usesparse", &value))
     + 		r->settings.pack_use_sparse = value;
      @@
     - 	/* Defaults */
     - 	r->settings->core_commit_graph = -1;
     - 	r->settings->gc_write_commit_graph = -1;
     --	r->settings->pack_use_sparse = -1;
     -+
     - 	r->settings->index_version = -1;
     - 	r->settings->core_untracked_cache = -1;
     - 
     -+	r->settings->pack_use_sparse = -1;
     -+	r->settings->merge_directory_renames = -1;
     -+	r->settings->fetch_negotiation_algorithm = -1;
     -+
     - 	repo_config(r, git_repo_config, r->settings);
     + 		UPDATE_DEFAULT(r->settings.index_version, 4);
     + 		UPDATE_DEFAULT(r->settings.core_untracked_cache, UNTRACKED_CACHE_WRITE);
     + 	}
     ++	if (!repo_config_get_bool(r, "feature.experimental", &value) && value) {
     ++		UPDATE_DEFAULT(r->settings.pack_use_sparse, 1);
     ++		UPDATE_DEFAULT(r->settings.merge_directory_renames, MERGE_DIRECTORY_RENAMES_TRUE);
     ++		UPDATE_DEFAULT(r->settings.fetch_negotiation_algorithm, FETCH_NEGOTIATION_SKIPPING);
     ++	}
       
       	/* Hack for test programs like test-dump-untracked-cache */
     + 	if (ignore_untracked_cache_config)
      @@
     - 		r->settings->core_untracked_cache = CORE_UNTRACKED_CACHE_KEEP;
       	else
     - 		UPDATE_DEFAULT(r->settings->core_untracked_cache, CORE_UNTRACKED_CACHE_KEEP);
     + 		UPDATE_DEFAULT(r->settings.core_untracked_cache, UNTRACKED_CACHE_KEEP);
     + 
     ++	UPDATE_DEFAULT(r->settings.merge_directory_renames, MERGE_DIRECTORY_RENAMES_CONFLICT);
     ++	UPDATE_DEFAULT(r->settings.fetch_negotiation_algorithm, FETCH_NEGOTIATION_DEFAULT);
      +
     -+	UPDATE_DEFAULT(r->settings->merge_directory_renames, MERGE_DIRECTORY_RENAMES_CONFLICT);
     -+	UPDATE_DEFAULT(r->settings->fetch_negotiation_algorithm, FETCH_NEGOTIATION_DEFAULT);
     + 	r->settings_initialized = 1;
       }
      
       diff --git a/repo-settings.h b/repo-settings.h
       --- a/repo-settings.h
       +++ b/repo-settings.h
      @@
     - #define CORE_UNTRACKED_CACHE_WRITE (1 << 0)
     - #define CORE_UNTRACKED_CACHE_KEEP (1 << 1)
     + 	UNTRACKED_CACHE_WRITE = 2
     + };
       
     -+#define MERGE_DIRECTORY_RENAMES_CONFLICT 1
     -+#define MERGE_DIRECTORY_RENAMES_TRUE 2
     ++enum merge_directory_renames_setting {
     ++	MERGE_DIRECTORY_RENAMES_UNSET = -1,
     ++	MERGE_DIRECTORY_RENAMES_NONE = 0,
     ++	MERGE_DIRECTORY_RENAMES_CONFLICT = 1,
     ++	MERGE_DIRECTORY_RENAMES_TRUE = 2,
     ++};
      +
     -+#define FETCH_NEGOTIATION_DEFAULT 1
     -+#define FETCH_NEGOTIATION_SKIPPING 2
     ++enum fetch_negotiation_setting {
     ++	FETCH_NEGOTIATION_UNSET = -1,
     ++	FETCH_NEGOTIATION_NONE = 0,
     ++	FETCH_NEGOTIATION_DEFAULT = 1,
     ++	FETCH_NEGOTIATION_SKIPPING = 2,
     ++};
      +
       struct repo_settings {
       	int core_commit_graph;
       	int gc_write_commit_graph;
     --	int pack_use_sparse;
     -+
     - 	int index_version;
     - 	int core_untracked_cache;
     -+
     -+	int pack_use_sparse;
     -+	int merge_directory_renames;
     -+	int fetch_negotiation_algorithm;
     +@@
     + 	enum untracked_cache_setting core_untracked_cache;
     + 
     + 	int pack_use_sparse;
     ++	enum merge_directory_renames_setting merge_directory_renames;
     ++	enum fetch_negotiation_setting fetch_negotiation_algorithm;
       };
       
       struct repository;

gitgitgadget

From: Derrick Stolee <[email protected]>

There are a few important config settings that are not loaded during git_default_config. These are instead loaded on-demand.

Centralize these config options to a single scan, and store all of the values in a repo_settings struct. The values for each setting are initialized as negative to indicate "unset".

This centralization will be particularly important in a later change to introduce "meta" config settings that change the defaults for these config settings.

---
 Makefile               |  1 +
 builtin/gc.c           | 13 ++++++-------
 builtin/pack-objects.c |  9 +++++----
 commit-graph.c         |  7 ++++---
 read-cache.c           | 12 +++++++-----
 repo-settings.c        | 28 ++++++++++++++++++++++++++++
 repo-settings.h        | 17 +++++++++++++++++
 repository.h           |  5 +++++
 8 files changed, 73 insertions(+), 19 deletions(-)
 create mode 100644 repo-settings.c
 create mode 100644 repo-settings.h

index 11ccea4071..032fe9b5f9 100644
@@ -964,6 +964,7 @@ LIB_OBJS += refspec.o
 LIB_OBJS += ref-filter.o
 LIB_OBJS += remote.o
 LIB_OBJS += replace-object.o
+LIB_OBJS += repo-settings.o
 LIB_OBJS += repository.o
 LIB_OBJS += rerere.o
 LIB_OBJS += resolve-undo.o

index c18efadda5..5e365049aa 100644
@@ -27,6 +27,7 @@
 #include "pack-objects.h"
 #include "blob.h"
 #include "tree.h"
+#include "repo-settings.h"
 
 #define FAILED_RUN "failed to run %s"
 

@@ -41,7 +42,6 @@ static int aggressive_depth = 50;
 static int aggressive_window = 250;
 static int gc_auto_threshold = 6700;
 static int gc_auto_pack_limit = 50;
-static int gc_write_commit_graph;
 static int detach_auto = 1;
 static timestamp_t gc_log_expire_time;
 static const char *gc_log_expire = "1.day.ago";

@@ -148,7 +148,6 @@ static void gc_config(void)
 	git_config_get_int("gc.aggressivedepth", &aggressive_depth);
 	git_config_get_int("gc.auto", &gc_auto_threshold);
 	git_config_get_int("gc.autopacklimit", &gc_auto_pack_limit);
-	git_config_get_bool("gc.writecommitgraph", &gc_write_commit_graph);
 	git_config_get_bool("gc.autodetach", &detach_auto);
 	git_config_get_expiry("gc.pruneexpire", &prune_expire);
 	git_config_get_expiry("gc.worktreepruneexpire", &prune_worktrees_expire);

@@ -685,11 +684,11 @@ int cmd_gc(int argc, const char **argv, const char *prefix)
 		clean_pack_garbage();
 	}
 
-	if (gc_write_commit_graph &&
-	    write_commit_graph_reachable(get_object_directory(),
-					 !quiet && !daemonized ? COMMIT_GRAPH_PROGRESS : 0,
-					 NULL))
-		return 1;
+	prepare_repo_settings(the_repository);
+	if (the_repository->settings.gc_write_commit_graph == 1)
+		write_commit_graph_reachable(get_object_directory(),
+					     !quiet && !daemonized ? COMMIT_GRAPH_PROGRESS : 0,
+					     NULL);
 
 	if (auto_gc && too_many_loose_objects())
 		warning(_("There are too many unreachable loose objects; "

index 267c562b1f..20bcd57033 100644
@@ -34,6 +34,7 @@
 #include "dir.h"
 #include "midx.h"
 #include "trace2.h"
+#include "repo-settings.h"
 
 #define IN_PACK(obj) oe_in_pack(&to_pack, obj)
 #define SIZE(obj) oe_size(&to_pack, obj)

@@ -2709,10 +2710,6 @@ static int git_pack_config(const char *k, const char *v, void *cb)
 		use_bitmap_index_default = git_config_bool(k, v);
 		return 0;
 	}
-	if (!strcmp(k, "pack.usesparse")) {
-		sparse = git_config_bool(k, v);
-		return 0;
-	}
 	if (!strcmp(k, "pack.threads")) {
 		delta_search_threads = git_config_int(k, v);
 		if (delta_search_threads < 0)

@@ -3332,6 +3329,10 @@ int cmd_pack_objects(int argc, const char **argv, const char *prefix)
 	read_replace_refs = 0;
 
 	sparse = git_env_bool("GIT_TEST_PACK_SPARSE", 0);
+	prepare_repo_settings(the_repository);
+	if (!sparse && the_repository->settings.pack_use_sparse != -1)
+		sparse = the_repository->settings.pack_use_sparse;
+
 	reset_pack_idx_option(&pack_idx_opts);
 	git_config(git_pack_config, NULL);
 

index b3c4de79b6..0c2dadab9b 100644
@@ -16,6 +16,7 @@
 #include "hashmap.h"
 #include "replace-object.h"
 #include "progress.h"
+#include "repo-settings.h"
 
 #define GRAPH_SIGNATURE 0x43475048 /* "CGPH" */
 #define GRAPH_CHUNKID_OIDFANOUT 0x4f494446 /* "OIDF" */

@@ -466,7 +467,6 @@ static void prepare_commit_graph_one(struct repository *r, const char *obj_dir)
 static int prepare_commit_graph(struct repository *r)
 {
 	struct object_directory *odb;
-	int config_value;
 
 	if (git_env_bool(GIT_TEST_COMMIT_GRAPH_DIE_ON_LOAD, 0))
 		die("dying as requested by the '%s' variable on commit-graph load!",

@@ -476,9 +476,10 @@ static int prepare_commit_graph(struct repository *r)
 		return !!r->objects->commit_graph;
 	r->objects->commit_graph_attempted = 1;
 
+	prepare_repo_settings(r);
+
 	if (!git_env_bool(GIT_TEST_COMMIT_GRAPH, 0) &&
-	    (repo_config_get_bool(r, "core.commitgraph", &config_value) ||
-	    !config_value))
+	    r->settings.core_commit_graph != 1)
 		/*
 		 * This repository is not configured to use commit graphs, so
 		 * do not load one. (But report commit_graph_attempted anyway

index c701f7f8b8..bda665a25a 100644
@@ -25,6 +25,7 @@
 #include "fsmonitor.h"
 #include "thread-utils.h"
 #include "progress.h"
+#include "repo-settings.h"
 
 /* Mask for the name length in ce_flags in the on-disk index */
 

@@ -1599,16 +1600,17 @@ struct cache_entry *refresh_cache_entry(struct index_state *istate,
 
 #define INDEX_FORMAT_DEFAULT 3
 
-static unsigned int get_index_format_default(void)
+static unsigned int get_index_format_default(struct repository *r)
 {
 	char *envversion = getenv("GIT_INDEX_VERSION");
 	char *endp;
-	int value;
 	unsigned int version = INDEX_FORMAT_DEFAULT;
 
 	if (!envversion) {
-		if (!git_config_get_int("index.version", &value))
-			version = value;
+		prepare_repo_settings(r);
+
+		if (r->settings.index_version >= 0)
+			version = r->settings.index_version;
 		if (version < INDEX_FORMAT_LB || INDEX_FORMAT_UB < version) {
 			warning(_("index.version set, but the value is invalid.\n"
 				  "Using version %i"), INDEX_FORMAT_DEFAULT);

@@ -2765,7 +2767,7 @@ static int do_write_index(struct index_state *istate, struct tempfile *tempfile,
 	}
 
 	if (!istate->version) {
-		istate->version = get_index_format_default();
+		istate->version = get_index_format_default(the_repository);
 		if (git_env_bool("GIT_TEST_SPLIT_INDEX", 0))
 			init_split_index(istate);
 	}

new file mode 100644
index 0000000000..2c33810cc6
@@ -0,0 +1,28 @@
+#include "cache.h"
+#include "repository.h"
+#include "config.h"
+#include "repo-settings.h"
+
+void prepare_repo_settings(struct repository *r)
+{
+	int value;
+
+	if (r->settings_initialized)
+		return;
+
+	/* Defaults */
+	memset(&r->settings, -1, sizeof(r->settings));
+
+	if (!repo_config_get_bool(r, "core.commitgraph", &value))
+		r->settings.core_commit_graph = value;
+	if (!repo_config_get_bool(r, "gc.writecommitgraph", &value))
+		r->settings.gc_write_commit_graph = value;
+
+	if (!repo_config_get_bool(r, "index.version", &value))
+		r->settings.index_version = value;
+
+	if (!repo_config_get_bool(r, "pack.usesparse", &value))
+		r->settings.pack_use_sparse = value;
+
+	r->settings_initialized = 1;
+}

new file mode 100644
index 0000000000..89fb0159bf
@@ -0,0 +1,17 @@
+#ifndef REPO_SETTINGS_H
+#define REPO_SETTINGS_H
+
+struct repo_settings {
+	int core_commit_graph;
+	int gc_write_commit_graph;
+
+	int index_version;
+
+	int pack_use_sparse;
+};
+
+struct repository;
+
+void prepare_repo_settings(struct repository *r);
+
+#endif /* REPO_SETTINGS_H */

index 4fb6a5885f..a817486825 100644
@@ -2,8 +2,10 @@
 #define REPOSITORY_H
 
 #include "path.h"
+#include "repo-settings.h"
 
 struct config_set;
+struct repo_settings;
 struct git_hash_algo;
 struct index_state;
 struct lock_file;

@@ -72,6 +74,9 @@ struct repository {
 	 */
 	char *submodule_prefix;
 
+	int settings_initialized;
+	struct repo_settings settings;
+
 	/* Subsystems */
 	/*
 	 * Repository's config which contains key-value pairs from the usual

gitgitgadget

Hi Stolee,

	unsigned settings_initialized:1;

From: Derrick Stolee <[email protected]>

When a repo has many commits, it is helpful to write and read the commit-graph file. Future changes to Git may include new config settings that are beneficial in this scenario.

Create the 'feature.manyCommits' config setting that changes the default values of 'core.commitGraph' and 'gc.writeCommitGraph' to true.

---
 Documentation/config.txt         |  2 ++
 Documentation/config/core.txt    |  3 ++-
 Documentation/config/feature.txt | 15 +++++++++++++++
 Documentation/config/gc.txt      |  4 ++--
 repo-settings.c                  |  7 +++++++
 5 files changed, 28 insertions(+), 3 deletions(-)
 create mode 100644 Documentation/config/feature.txt

index e3f5bc3396..77f3b1486b 100644
@@ -345,6 +345,8 @@ include::config/difftool.txt[]
 
 include::config/fastimport.txt[]
 
+include::config/feature.txt[]
+
 include::config/fetch.txt[]
 
 include::config/format.txt[]

index 75538d27e7..d80162681a 100644
@@ -577,7 +577,8 @@ the `GIT_NOTES_REF` environment variable.  See linkgit:git-notes[1].
 
 core.commitGraph::
 	If true, then git will read the commit-graph file (if it exists)
-	to parse the graph structure of commits. Defaults to false. See
+	to parse the graph structure of commits. Defaults to false, unless
+	`feature.manyCommits` is enabled. See
 	linkgit:git-commit-graph[1] for more information.
 
 core.useReplaceRefs::

new file mode 100644
index 0000000000..f74314ae90
@@ -0,0 +1,15 @@
+feature.*::
+	The config settings that start with `feature.` modify the defaults of
+	a group of other config settings. These groups are created by the Git
+	developer community as recommended defaults and are subject to change.
+	In particular, new config options may be added with different defaults.
+
+feature.manyCommits::
+	Enable config options that optimize for repos with many commits. This
+	setting is recommended for repos with at least 100,000 commits. The
+	new default values are:
++
+* `core.commitGraph=true` enables reading the commit-graph file.
++
+* `gc.writeCommitGraph=true` enables writing the commit-graph file during
+garbage collection.
\ No newline at end of file

index 02b92b18b5..31a5fc4f75 100644
@@ -63,8 +63,8 @@ gc.writeCommitGraph::
 	If true, then gc will rewrite the commit-graph file when
 	linkgit:git-gc[1] is run. When using `git gc --auto`
 	the commit-graph will be updated if housekeeping is
-	required. Default is false. See linkgit:git-commit-graph[1]
-	for details.
+	required. Default is false, unless `feature.manyCommits`
+	is enabled. See linkgit:git-commit-graph[1] for details.
 
 gc.logExpiry::
 	If the file gc.log exists, then `git gc --auto` will print

index 2c33810cc6..3face7d8b9 100644
@@ -3,6 +3,8 @@
 #include "config.h"
 #include "repo-settings.h"
 
+#define UPDATE_DEFAULT(s,v) do { if (s == -1) { s = v; } } while(0)
+
 void prepare_repo_settings(struct repository *r)
 {
 	int value;

@@ -24,5 +26,10 @@ void prepare_repo_settings(struct repository *r)
 	if (!repo_config_get_bool(r, "pack.usesparse", &value))
 		r->settings.pack_use_sparse = value;
 
+	if (!repo_config_get_bool(r, "feature.manycommits", &value) && value) {
+		UPDATE_DEFAULT(r->settings.core_commit_graph, 1);
+		UPDATE_DEFAULT(r->settings.gc_write_commit_graph, 1);
+	}
+
 	r->settings_initialized = 1;
 }

gitgitgadget

From: Derrick Stolee <[email protected]>

The core.untrackedCache config setting is slightly complicated, so clarify its use and centralize its parsing into the repo settings.

The default value is "keep" (returned as -1), which persists the untracked cache if it exists.

If the value is set as "false" (returned as 0), then remove the untracked cache if it exists.

If the value is set as "true" (returned as 1), then write the untracked cache and persist it.

Instead of relying on magic values of -1, 0, and 1, split these options into an enum. This allows the use of "-1" as a default value. After parsing the config options, if the value is unset we can initialize it to UNTRACKED_CACHE_KEEP.

---
 builtin/update-index.c |  7 +++++--
 config.c               | 24 ------------------------
 read-cache.c           | 19 +++++++++----------
 repo-settings.c        | 21 ++++++++++++++++++++-
 repo-settings.h        |  8 ++++++++
 5 files changed, 42 insertions(+), 37 deletions(-)

index dff2f4b837..5ced51c1ee 100644
@@ -18,6 +18,7 @@
 #include "dir.h"
 #include "split-index.h"
 #include "fsmonitor.h"
+#include "repo-settings.h"
 
 /*
  * Default to not allowing changes to the list of files. The

@@ -966,6 +967,7 @@ int cmd_update_index(int argc, const char **argv, const char *prefix)
 	struct parse_opt_ctx_t ctx;
 	strbuf_getline_fn getline_fn;
 	int parseopt_state = PARSE_OPT_UNKNOWN;
+	struct repository *r = the_repository;
 	struct option options[] = {
 		OPT_BIT('q', NULL, &refresh_args.flags,
 			N_("continue refresh even when index needs update"),

@@ -1180,11 +1182,12 @@ int cmd_update_index(int argc, const char **argv, const char *prefix)
 		remove_split_index(&the_index);
 	}
 
+	prepare_repo_settings(r);
 	switch (untracked_cache) {
 	case UC_UNSPECIFIED:
 		break;
 	case UC_DISABLE:
-		if (git_config_get_untracked_cache() == 1)
+		if (r->settings.core_untracked_cache == UNTRACKED_CACHE_WRITE)
 			warning(_("core.untrackedCache is set to true; "
 				  "remove or change it, if you really want to "
 				  "disable the untracked cache"));

@@ -1196,7 +1199,7 @@ int cmd_update_index(int argc, const char **argv, const char *prefix)
 		return !test_if_untracked_cache_is_supported();
 	case UC_ENABLE:
 	case UC_FORCE:
-		if (git_config_get_untracked_cache() == 0)
+		if (r->settings.core_untracked_cache == UNTRACKED_CACHE_REMOVE)
 			warning(_("core.untrackedCache is set to false; "
 				  "remove or change it, if you really want to "
 				  "enable the untracked cache"));

index faa57e436c..3241dbc54d 100644
@@ -2277,30 +2277,6 @@ int git_config_get_expiry_in_days(const char *key, timestamp_t *expiry, timestam
 	return -1; /* thing exists but cannot be parsed */
 }
 
-int git_config_get_untracked_cache(void)
-{
-	int val = -1;
-	const char *v;
-
-	/* Hack for test programs like test-dump-untracked-cache */
-	if (ignore_untracked_cache_config)
-		return -1;
-
-	if (!git_config_get_maybe_bool("core.untrackedcache", &val))
-		return val;
-
-	if (!git_config_get_value("core.untrackedcache", &v)) {
-		if (!strcasecmp(v, "keep"))
-			return -1;
-
-		error(_("unknown core.untrackedCache value '%s'; "
-			"using 'keep' default value"), v);
-		return -1;
-	}
-
-	return -1; /* default value */
-}
-
 int git_config_get_split_index(void)
 {
 	int val;

index bda665a25a..3761b9a171 100644
@@ -1846,18 +1846,17 @@ static void check_ce_order(struct index_state *istate)
 
 static void tweak_untracked_cache(struct index_state *istate)
 {
-	switch (git_config_get_untracked_cache()) {
-	case -1: /* keep: do nothing */
-		break;
-	case 0: /* false */
+	struct repository *r = the_repository;
+
+	prepare_repo_settings(r);
+
+	if (r->settings.core_untracked_cache  == UNTRACKED_CACHE_REMOVE) {
 		remove_untracked_cache(istate);
-		break;
-	case 1: /* true */
-		add_untracked_cache(istate);
-		break;
-	default: /* unknown value: do nothing */
-		break;
+		return;
 	}
+
+	if (r->settings.core_untracked_cache == UNTRACKED_CACHE_WRITE)
+		add_untracked_cache(istate);
 }
 
 static void tweak_split_index(struct index_state *istate)

index 3face7d8b9..65b980403c 100644
@@ -8,6 +8,7 @@
 void prepare_repo_settings(struct repository *r)
 {
 	int value;
+	char *strval;
 
 	if (r->settings_initialized)
 		return;

@@ -22,14 +23,32 @@ void prepare_repo_settings(struct repository *r)
 
 	if (!repo_config_get_bool(r, "index.version", &value))
 		r->settings.index_version = value;
+	if (!repo_config_get_maybe_bool(r, "core.untrackedcache", &value)) {
+		if (value == 0)
+			r->settings.core_untracked_cache = UNTRACKED_CACHE_REMOVE;
+		else
+			r->settings.core_untracked_cache = UNTRACKED_CACHE_WRITE;
+	} else if (!repo_config_get_string(r, "core.untrackedcache", &strval)) {
+		if (!strcasecmp(strval, "keep"))
+			r->settings.core_untracked_cache = UNTRACKED_CACHE_KEEP;
+
+		free(strval);
+	}
+
 
 	if (!repo_config_get_bool(r, "pack.usesparse", &value))
 		r->settings.pack_use_sparse = value;
-
+	
 	if (!repo_config_get_bool(r, "feature.manycommits", &value) && value) {
 		UPDATE_DEFAULT(r->settings.core_commit_graph, 1);
 		UPDATE_DEFAULT(r->settings.gc_write_commit_graph, 1);
 	}
 
+	/* Hack for test programs like test-dump-untracked-cache */
+	if (ignore_untracked_cache_config)
+		r->settings.core_untracked_cache = UNTRACKED_CACHE_KEEP;
+	else
+		UPDATE_DEFAULT(r->settings.core_untracked_cache, UNTRACKED_CACHE_KEEP);
+
 	r->settings_initialized = 1;
 }

index 89fb0159bf..e9b028e1a9 100644
@@ -1,11 +1,19 @@
 #ifndef REPO_SETTINGS_H
 #define REPO_SETTINGS_H
 
+enum untracked_cache_setting {
+	UNTRACKED_CACHE_UNSET = -1,
+	UNTRACKED_CACHE_REMOVE = 0,
+	UNTRACKED_CACHE_KEEP = 1,
+	UNTRACKED_CACHE_WRITE = 2
+};
+
 struct repo_settings {
 	int core_commit_graph;
 	int gc_write_commit_graph;
 
 	int index_version;
+	enum untracked_cache_setting core_untracked_cache;
 
 	int pack_use_sparse;
 };

gitgitgadget

Hi Stolee,

From: Derrick Stolee <[email protected]>

The feature.manyFiles setting is suitable for repos with many files in the working directory. By setting index.version=4 and core.untrackedCache=true, commands such as 'git status' should improve.

While adding this setting, modify the index version precedence tests to check how this setting overrides the default for index.version is unset.

---
 Documentation/config/core.txt    |  4 +++-
 Documentation/config/feature.txt | 12 +++++++++++-
 Documentation/config/index.txt   |  1 +
 repo-settings.c                  |  4 ++++
 t/t1600-index.sh                 | 31 ++++++++++++++++++++++++++-----
 5 files changed, 45 insertions(+), 7 deletions(-)

index d80162681a..7a2a33bc8c 100644
@@ -86,7 +86,9 @@ core.untrackedCache::
 	it will automatically be removed, if set to `false`. Before
 	setting it to `true`, you should check that mtime is working
 	properly on your system.
-	See linkgit:git-update-index[1]. `keep` by default.
+	See linkgit:git-update-index[1]. `keep` by default, unless
+	`feature.manyFiles` is enabled which sets this setting to
+	`true` by default.
 
 core.checkStat::
 	When missing or is set to `default`, many fields in the stat

index f74314ae90..c2d9ef7473 100644
@@ -12,4 +12,14 @@ feature.manyCommits::
 * `core.commitGraph=true` enables reading the commit-graph file.
 +
 * `gc.writeCommitGraph=true` enables writing the commit-graph file during
-garbage collection.
\ No newline at end of file
+garbage collection.
+
+feature.manyFiles::
+	Enable config options that optimize for repos with many files in the
+	working directory. With many files, commands such as `git status` and
+	`git checkout` may be slow and these new defaults improve performance:
++
+* `index.version=4` enables path-prefix compression in the index.
++
+* `core.untrackedCache=true` enables the untracked cache. This setting assumes
+that mtime is working on your machine.
\ No newline at end of file

index f181503041..7cb50b37e9 100644
@@ -24,3 +24,4 @@ index.threads::
 index.version::
 	Specify the version with which new index files should be
 	initialized.  This does not affect existing repositories.
+	If `feature.manyFiles` is enabled, then the default is 4.

index 65b980403c..3a5cf33814 100644
@@ -43,6 +43,10 @@ void prepare_repo_settings(struct repository *r)
 		UPDATE_DEFAULT(r->settings.core_commit_graph, 1);
 		UPDATE_DEFAULT(r->settings.gc_write_commit_graph, 1);
 	}
+	if (!repo_config_get_bool(r, "feature.manyfiles", &value) && value) {
+		UPDATE_DEFAULT(r->settings.index_version, 4);
+		UPDATE_DEFAULT(r->settings.core_untracked_cache, UNTRACKED_CACHE_WRITE);
+	}
 
 	/* Hack for test programs like test-dump-untracked-cache */
 	if (ignore_untracked_cache_config)

index 42962ed7d4..c77721b580 100755
@@ -59,17 +59,38 @@ test_expect_success 'out of bounds index.version issues warning' '
 	)
 '
 
-test_expect_success 'GIT_INDEX_VERSION takes precedence over config' '
+test_index_version () {
+	INDEX_VERSION_CONFIG=$1 &&
+	FEATURE_MANY_FILES=$2 &&
+	ENV_VAR_VERSION=$3
+	EXPECTED_OUTPUT_VERSION=$4 &&
 	(
 		rm -f .git/index &&
-		GIT_INDEX_VERSION=4 &&
-		export GIT_INDEX_VERSION &&
-		git config --add index.version 2 &&
+		rm -f .git/config &&
+		if test "$INDEX_VERSION_CONFIG" -ne 0
+		then
+			git config --add index.version $INDEX_VERSION_CONFIG
+		fi &&
+		git config --add feature.manyFiles $FEATURE_MANY_FILES
+		if test "$ENV_VAR_VERSION" -ne 0
+		then
+			GIT_INDEX_VERSION=$ENV_VAR_VERSION &&
+			export GIT_INDEX_VERSION
+		else
+			unset GIT_INDEX_VERSION
+		fi &&
 		git add a 2>&1 &&
-		echo 4 >expect &&
+		echo $EXPECTED_OUTPUT_VERSION >expect &&
 		test-tool index-version <.git/index >actual &&
 		test_cmp expect actual
 	)
+}
+
+test_expect_success 'index version config precedence' '
+	test_index_version 2 false 4 4 &&
+	test_index_version 2 true 0 2 &&
+	test_index_version 0 true 0 4 &&
+	test_index_version 0 true 2 2
 '
 
 test_done

gitgitgadget

From: Derrick Stolee <[email protected]>

The 'feature.experimental' setting includes config options that are not committed to become defaults, but could use additional testing.

Update the following config settings to take new defaults, and to use the repo_settings struct if not already using them:

* 'pack.useSparse=true'

* 'merge.directoryRenames=true'

* 'fetch.negotiationAlgorithm=skipping'

In the case of fetch.negotiationAlgorithm, the existing logic would load the config option only when about to use the setting, so had a die() statement on an unknown string value. This is removed as now the config is parsed under prepare_repo_settings(). In general, this die() is probably misplaced and not valuable. A test was removed that checked this die() statement executed.

---
 Documentation/config/feature.txt     | 17 +++++++++++++++++
 Documentation/config/fetch.txt       |  3 ++-
 Documentation/config/merge.txt       |  3 ++-
 Documentation/config/pack.txt        |  3 ++-
 builtin/am.c                         |  1 +
 fetch-negotiator.c                   | 26 ++++++++++++++------------
 fetch-negotiator.h                   |  5 +++--
 fetch-pack.c                         | 11 +++++------
 merge-recursive.c                    | 15 ++++++---------
 repo-settings.c                      | 20 ++++++++++++++++++++
 repo-settings.h                      | 16 ++++++++++++++++
 t/t5552-skipping-fetch-negotiator.sh | 23 -----------------------
 12 files changed, 88 insertions(+), 55 deletions(-)

index c2d9ef7473..287621b1e8 100644
@@ -4,6 +4,23 @@ feature.*::
 	developer community as recommended defaults and are subject to change.
 	In particular, new config options may be added with different defaults.
 
+feature.experimental::
+	Enable config options that are new to Git, and are being considered for
+	future defaults. Config settings included here may be added or removed
+	with each release, including minor version updates. These settings may
+	have unintended interactions since they are so new. Please enable this
+	setting if you are interested in providing feedback on experimental
+	features. The new default values are:
++
+* `pack.useSparse=true` uses a new algorithm when constructing a pack-file
+which can improve `git push` performance in repos with many files.
++
+* `merge.directoryRenames=true` uses a new algorithm for detecting renames by
+using entire directories at a time instead of single files at a time.
++
+* `fetch.negotiationAlgorithm=skipping` may improve fetch negotiation times by
+skipping more commits at a time, reducing the number of round trips.
+
 feature.manyCommits::
 	Enable config options that optimize for repos with many commits. This
 	setting is recommended for repos with at least 100,000 commits. The

index ba890b5884..d402110638 100644
@@ -59,7 +59,8 @@ fetch.negotiationAlgorithm::
 	effort to converge faster, but may result in a larger-than-necessary
 	packfile; The default is "default" which instructs Git to use the default algorithm
 	that never skips commits (unless the server has acknowledged it or one
-	of its descendants).
+	of its descendants). If `feature.experimental` is enabled, then this
+	setting defaults to "skipping".
 	Unknown values will cause 'git fetch' to error out.
 +
 See also the `--negotiation-tip` option for linkgit:git-fetch[1].

index 6a313937f8..e8def2d63c 100644
@@ -54,7 +54,8 @@ merge.directoryRenames::
 	moved into the new directory.  If set to "conflict", a conflict
 	will be reported for such paths.  If merge.renames is false,
 	merge.directoryRenames is ignored and treated as false.  Defaults
-	to "conflict".
+	to "conflict" unless `feature.experimental` is enabled and the
+	default is "true".
 
 merge.renormalize::
 	Tell Git that canonical representation of files in the

index 9cdcfa7324..1d66f0c992 100644
@@ -112,7 +112,8 @@ pack.useSparse::
 	objects. This can have significant performance benefits when
 	computing a pack to send a small change. However, it is possible
 	that extra objects are added to the pack-file if the included
-	commits contain certain types of direct renames.
+	commits contain certain types of direct renames. Default is `false`
+	unless `feature.experimental` is enabled.
 
 pack.writeBitmaps (deprecated)::
 	This is a deprecated synonym for `repack.writeBitmaps`.

index 1aea657a7f..912c6155b8 100644
@@ -34,6 +34,7 @@
 #include "string-list.h"
 #include "packfile.h"
 #include "repository.h"
+#include "repo-settings.h"
 
 /**
  * Returns the length of the first line of msg.

index d6d685cba0..1a68be1a2e 100644
@@ -2,19 +2,21 @@
 #include "fetch-negotiator.h"
 #include "negotiator/default.h"
 #include "negotiator/skipping.h"
+#include "repository.h"
+#include "repo-settings.h"
 
-void fetch_negotiator_init(struct fetch_negotiator *negotiator,
-			   const char *algorithm)
+void fetch_negotiator_init(struct repository *r,
+			   struct fetch_negotiator *negotiator)
 {
-	if (algorithm) {
-		if (!strcmp(algorithm, "skipping")) {
-			skipping_negotiator_init(negotiator);
-			return;
-		} else if (!strcmp(algorithm, "default")) {
-			/* Fall through to default initialization */
-		} else {
-			die("unknown fetch negotiation algorithm '%s'", algorithm);
-		}
+	prepare_repo_settings(r);
+	switch(r->settings.fetch_negotiation_algorithm) {
+	case FETCH_NEGOTIATION_SKIPPING:
+		skipping_negotiator_init(negotiator);
+		return;
+
+	case FETCH_NEGOTIATION_DEFAULT:
+	default:
+		default_negotiator_init(negotiator);
+		return;
 	}
-	default_negotiator_init(negotiator);
 }

index 9e3967ce66..ea78868504 100644
@@ -2,6 +2,7 @@
 #define FETCH_NEGOTIATOR_H
 
 struct commit;
+struct repository;
 
 /*
  * An object that supplies the information needed to negotiate the contents of

@@ -52,7 +53,7 @@ struct fetch_negotiator {
 	void *data;
 };
 
-void fetch_negotiator_init(struct fetch_negotiator *negotiator,
-			   const char *algorithm);
+void fetch_negotiator_init(struct repository *r,
+			   struct fetch_negotiator *negotiator);
 
 #endif

index 65be043f2a..d81f47c07b 100644
@@ -36,7 +36,6 @@ static int agent_supported;
 static int server_supports_filtering;
 static struct lock_file shallow_lock;
 static const char *alternate_shallow_file;
-static char *negotiation_algorithm;
 static struct strbuf fsck_msg_types = STRBUF_INIT;
 
 /* Remember to update object flag allocation in object.h */

@@ -892,12 +891,13 @@ static struct ref *do_fetch_pack(struct fetch_pack_args *args,
 				 struct shallow_info *si,
 				 char **pack_lockfile)
 {
+	struct repository *r = the_repository;
 	struct ref *ref = copy_ref_list(orig_ref);
 	struct object_id oid;
 	const char *agent_feature;
 	int agent_len;
 	struct fetch_negotiator negotiator;
-	fetch_negotiator_init(&negotiator, negotiation_algorithm);
+	fetch_negotiator_init(r, &negotiator);
 
 	sort_ref_list(&ref, ref_compare_name);
 	QSORT(sought, nr_sought, cmp_ref_by_name);

@@ -911,7 +911,7 @@ static struct ref *do_fetch_pack(struct fetch_pack_args *args,
 
 	if (server_supports("shallow"))
 		print_verbose(args, _("Server supports %s"), "shallow");
-	else if (args->depth > 0 || is_repository_shallow(the_repository))
+	else if (args->depth > 0 || is_repository_shallow(r))
 		die(_("Server does not support shallow clients"));
 	if (args->depth > 0 || args->deepen_since || args->deepen_not)
 		args->deepen = 1;

@@ -1379,6 +1379,7 @@ static struct ref *do_fetch_pack_v2(struct fetch_pack_args *args,
 				    struct shallow_info *si,
 				    char **pack_lockfile)
 {
+	struct repository *r = the_repository;
 	struct ref *ref = copy_ref_list(orig_ref);
 	enum fetch_state state = FETCH_CHECK_LOCAL;
 	struct oidset common = OIDSET_INIT;

@@ -1386,7 +1387,7 @@ static struct ref *do_fetch_pack_v2(struct fetch_pack_args *args,
 	int in_vain = 0;
 	int haves_to_send = INITIAL_FLUSH;
 	struct fetch_negotiator negotiator;
-	fetch_negotiator_init(&negotiator, negotiation_algorithm);
+	fetch_negotiator_init(r, &negotiator);
 	packet_reader_init(&reader, fd[0], NULL, 0,
 			   PACKET_READ_CHOMP_NEWLINE |
 			   PACKET_READ_DIE_ON_ERR_PACKET);

@@ -1505,8 +1506,6 @@ static void fetch_pack_config(void)
 	git_config_get_bool("repack.usedeltabaseoffset", &prefer_ofs_delta);
 	git_config_get_bool("fetch.fsckobjects", &fetch_fsck_objects);
 	git_config_get_bool("transfer.fsckobjects", &transfer_fsck_objects);
-	git_config_get_string("fetch.negotiationalgorithm",
-			      &negotiation_algorithm);
 
 	git_config(fetch_pack_config_cb, NULL);
 }

index 12300131fc..f39c31c9c4 100644
@@ -28,6 +28,7 @@
 #include "submodule.h"
 #include "revision.h"
 #include "commit-reach.h"
+#include "repo-settings.h"
 
 struct path_hashmap_entry {
 	struct hashmap_entry e;

@@ -3662,15 +3663,6 @@ static void merge_recursive_config(struct merge_options *opt)
 		opt->merge_detect_rename = git_config_rename("merge.renames", value);
 		free(value);
 	}
-	if (!git_config_get_string("merge.directoryrenames", &value)) {
-		int boolval = git_parse_maybe_bool(value);
-		if (0 <= boolval) {
-			opt->detect_directory_renames = boolval ? 2 : 0;
-		} else if (!strcasecmp(value, "conflict")) {
-			opt->detect_directory_renames = 1;
-		} /* avoid erroring on values from future versions of git */
-		free(value);
-	}
 	git_config(git_xmerge_config, NULL);
 }
 

@@ -3688,6 +3680,11 @@ void init_merge_options(struct merge_options *opt,
 	opt->diff_detect_rename = -1;
 	opt->merge_detect_rename = -1;
 	opt->detect_directory_renames = 1;
+
+	prepare_repo_settings(repo);
+	if (repo->settings.merge_directory_renames >= 0)
+		opt->detect_directory_renames = repo->settings.merge_directory_renames;
+
 	merge_recursive_config(opt);
 	merge_verbosity = getenv("GIT_MERGE_VERBOSITY");
 	if (merge_verbosity)

index 3a5cf33814..bfb3288fda 100644
@@ -35,6 +35,18 @@ void prepare_repo_settings(struct repository *r)
 		free(strval);
 	}
 
+	if (!repo_config_get_maybe_bool(r, "merge.directoryrenames", &value))
+		r->settings.merge_directory_renames = value ? MERGE_DIRECTORY_RENAMES_TRUE : 0;
+	else if (!repo_config_get_string(r, "merge.directoryrenames", &strval)) {
+		if (!strcasecmp(strval, "conflict"))
+			r->settings.merge_directory_renames = MERGE_DIRECTORY_RENAMES_CONFLICT;
+	}
+	if (!repo_config_get_string(r, "fetch.negotiationalgorithm", &strval)) {
+		if (!strcasecmp(strval, "skipping"))
+			r->settings.fetch_negotiation_algorithm = FETCH_NEGOTIATION_SKIPPING;
+		else
+			r->settings.fetch_negotiation_algorithm = FETCH_NEGOTIATION_DEFAULT;
+	}
 
 	if (!repo_config_get_bool(r, "pack.usesparse", &value))
 		r->settings.pack_use_sparse = value;

@@ -47,6 +59,11 @@ void prepare_repo_settings(struct repository *r)
 		UPDATE_DEFAULT(r->settings.index_version, 4);
 		UPDATE_DEFAULT(r->settings.core_untracked_cache, UNTRACKED_CACHE_WRITE);
 	}
+	if (!repo_config_get_bool(r, "feature.experimental", &value) && value) {
+		UPDATE_DEFAULT(r->settings.pack_use_sparse, 1);
+		UPDATE_DEFAULT(r->settings.merge_directory_renames, MERGE_DIRECTORY_RENAMES_TRUE);
+		UPDATE_DEFAULT(r->settings.fetch_negotiation_algorithm, FETCH_NEGOTIATION_SKIPPING);
+	}
 
 	/* Hack for test programs like test-dump-untracked-cache */
 	if (ignore_untracked_cache_config)

@@ -54,5 +71,8 @@ void prepare_repo_settings(struct repository *r)
 	else
 		UPDATE_DEFAULT(r->settings.core_untracked_cache, UNTRACKED_CACHE_KEEP);
 
+	UPDATE_DEFAULT(r->settings.merge_directory_renames, MERGE_DIRECTORY_RENAMES_CONFLICT);
+	UPDATE_DEFAULT(r->settings.fetch_negotiation_algorithm, FETCH_NEGOTIATION_DEFAULT);
+
 	r->settings_initialized = 1;
 }

index e9b028e1a9..95d3392df2 100644
@@ -8,6 +8,20 @@ enum untracked_cache_setting {
 	UNTRACKED_CACHE_WRITE = 2
 };
 
+enum merge_directory_renames_setting {
+	MERGE_DIRECTORY_RENAMES_UNSET = -1,
+	MERGE_DIRECTORY_RENAMES_NONE = 0,
+	MERGE_DIRECTORY_RENAMES_CONFLICT = 1,
+	MERGE_DIRECTORY_RENAMES_TRUE = 2,
+};
+
+enum fetch_negotiation_setting {
+	FETCH_NEGOTIATION_UNSET = -1,
+	FETCH_NEGOTIATION_NONE = 0,
+	FETCH_NEGOTIATION_DEFAULT = 1,
+	FETCH_NEGOTIATION_SKIPPING = 2,
+};
+
 struct repo_settings {
 	int core_commit_graph;
 	int gc_write_commit_graph;

@@ -16,6 +30,8 @@ struct repo_settings {
 	enum untracked_cache_setting core_untracked_cache;
 
 	int pack_use_sparse;
+	enum merge_directory_renames_setting merge_directory_renames;
+	enum fetch_negotiation_setting fetch_negotiation_algorithm;
 };
 
 struct repository;

index 8a14be51a1..f70cbcc9ca 100755
@@ -60,29 +60,6 @@ test_expect_success 'commits with no parents are sent regardless of skip distanc
 	have_not_sent c6 c4 c3
 '
 
-test_expect_success 'unknown fetch.negotiationAlgorithm values error out' '
-	rm -rf server client trace &&
-	git init server &&
-	test_commit -C server to_fetch &&
-
-	git init client &&
-	test_commit -C client on_client &&
-	git -C client checkout on_client &&
-
-	test_config -C client fetch.negotiationAlgorithm invalid &&
-	test_must_fail git -C client fetch "$(pwd)/server" 2>err &&
-	test_i18ngrep "unknown fetch negotiation algorithm" err &&
-
-	# Explicit "default" value
-	test_config -C client fetch.negotiationAlgorithm default &&
-	git -C client -c fetch.negotiationAlgorithm=default fetch "$(pwd)/server" &&
-
-	# Implementation detail: If there is nothing to fetch, we will not error out
-	test_config -C client fetch.negotiationAlgorithm invalid &&
-	git -C client fetch "$(pwd)/server" 2>err &&
-	test_i18ngrep ! "unknown fetch negotiation algorithm" err
-'
-
 test_expect_success 'when two skips collide, favor the larger one' '
 	rm -rf server client trace &&
 	git init server &&

gitgitgadget

Hi Stolee,

I left only a couple of comments, and quite honestly, I am really happy with the direction this is going.

It even leaves the door open to let users define their custom feature groups, at least I have a couple of ideas how that could be accomplished building on top of repo-settings.c.

Thanks!
Dscho

This is a brand-new thread to replace ds/early-access. The discussion on that thread was very helpful [1].

With this in mind, I propose instead a set of "feature.*" config settings that form groups of "community recommended" settings (with some caveats). In the space below, I'll list a set of possible feature names and the implied config options.

First, the main two categories we've discussed so far: many commits and many files. These two feature sets are for when your repo is large in one of these dimensions. Perhaps there are other settings to include in these?

feature.manyFiles:
index.version = 4
core.untrackedCache = true

feature.manyCommits:
core.commitGraph = true
gc.writeCommitGraph = true
(future: fetch.writeSplitCommitGraph = true)

Note: the fetch.writeSplitCommitGraph does not exist yet, but could be introduced in a later release to write a new commit-graph (with --split) on fetch.

The other category that has been discussed already is that of "experimental features that we generally think are helpful but change behavior slightly in some cases".

feature.experimental:
pack.useSparse = true
merge.directoryRenames = true
fetch.negotiationAlgorithm = skipping

Specifically, this setting is for config values we are not sure will ever be on by default, but additional testing is needed to be sure. This is different than a possible 'feature.preview' setting that would include config settings that we are committed to updating the defaults in a future release. There are many ways we can take this idea in the future (including more additions to these categories).

Thanks, -Stolee

Updates in V2: I'm responding to Dscho's comments early, as they were very valuable.

* Rearranged how we are checking for the feature.* settings, so if one layer turns the setting on and a later layer turns it off, we do not adjust the defaults.

* Switched to using enums for the non-boolean config options.

* Placed the repo_settings struct directly in the repository struct.

* All struct members are initialized to -1 using memset.

* The config changes around directory rename detection is greatly simplified.

Updates in V3: These are more Dscho comments, and are quite small.

* Remove repo-settings.h by placing the struct and function definitions in repository.h.

* Move the initialization int into "struct repo_settings".

[1] https://public-inbox.org/git/[email protected]/

Derrick Stolee (5):
repo-settings: consolidate some config settings
repo-settings: add feature.manyCommits setting
repo-settings: parse core.untrackedCache
repo-settings: create feature.manyFiles setting
repo-settings: create feature.experimental setting

 Documentation/config.txt             |  2 +
 Documentation/config/core.txt        |  7 ++-
 Documentation/config/feature.txt     | 42 ++++++++++++++++
 Documentation/config/fetch.txt       |  3 +-
 Documentation/config/gc.txt          |  4 +-
 Documentation/config/index.txt       |  1 +
 Documentation/config/merge.txt       |  3 +-
 Documentation/config/pack.txt        |  3 +-
 Makefile                             |  1 +
 builtin/gc.c                         | 12 ++---
 builtin/pack-objects.c               |  8 +--
 builtin/update-index.c               |  6 ++-
 commit-graph.c                       |  6 +--
 config.c                             | 24 ---------
 fetch-negotiator.c                   | 25 +++++-----
 fetch-negotiator.h                   |  5 +-
 fetch-pack.c                         | 11 ++--
 merge-recursive.c                    | 14 ++----
 read-cache.c                         | 30 +++++------
 repo-settings.c                      | 75 ++++++++++++++++++++++++++++
 repository.h                         | 39 +++++++++++++++
 t/t1600-index.sh                     | 31 ++++++++++--
 t/t5552-skipping-fetch-negotiator.sh | 23 ---------
 23 files changed, 256 insertions(+), 119 deletions(-)
 create mode 100644 Documentation/config/feature.txt
 create mode 100644 repo-settings.c

base-commit: 9c9b961d7eb15fb583a2a812088713a68a85f1c0
Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-292%2Fderrickstolee%2Frepo-settings%2Fhead-v3
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-292/derrickstolee/repo-settings/head-v3
Pull-Request: https://github.com/gitgitgadget/git/pull/292

Range-diff vs v2:

1: 597ab7d621 ! 1: 4d0db57ecb repo-settings: consolidate some config settings

     @@ -30,14 +30,6 @@
       diff --git a/builtin/gc.c b/builtin/gc.c
       --- a/builtin/gc.c
       +++ b/builtin/gc.c
     -@@
     - #include "pack-objects.h"
     - #include "blob.h"
     - #include "tree.h"
     -+#include "repo-settings.h"
     - 
     - #define FAILED_RUN "failed to run %s"
     - 
      @@
       static int aggressive_window = 250;
       static int gc_auto_threshold = 6700;
     @@ -75,14 +67,6 @@
       diff --git a/builtin/pack-objects.c b/builtin/pack-objects.c
       --- a/builtin/pack-objects.c
       +++ b/builtin/pack-objects.c
     -@@
     - #include "dir.h"
     - #include "midx.h"
     - #include "trace2.h"
     -+#include "repo-settings.h"
     - 
     - #define IN_PACK(obj) oe_in_pack(&to_pack, obj)
     - #define SIZE(obj) oe_size(&to_pack, obj)
      @@
       		use_bitmap_index_default = git_config_bool(k, v);
       		return 0;
     @@ -109,14 +93,6 @@
       diff --git a/commit-graph.c b/commit-graph.c
       --- a/commit-graph.c
       +++ b/commit-graph.c
     -@@
     - #include "hashmap.h"
     - #include "replace-object.h"
     - #include "progress.h"
     -+#include "repo-settings.h"
     - 
     - #define GRAPH_SIGNATURE 0x43475048 /* "CGPH" */
     - #define GRAPH_CHUNKID_OIDFANOUT 0x4f494446 /* "OIDF" */
      @@
       static int prepare_commit_graph(struct repository *r)
       {
     @@ -142,14 +118,6 @@
       diff --git a/read-cache.c b/read-cache.c
       --- a/read-cache.c
       +++ b/read-cache.c
     -@@
     - #include "fsmonitor.h"
     - #include "thread-utils.h"
     - #include "progress.h"
     -+#include "repo-settings.h"
     - 
     - /* Mask for the name length in ce_flags in the on-disk index */
     - 
      @@
       
       #define INDEX_FORMAT_DEFAULT 3
     @@ -188,15 +156,14 @@
       +++ b/repo-settings.c
      @@
      +#include "cache.h"
     -+#include "repository.h"
      +#include "config.h"
     -+#include "repo-settings.h"
     ++#include "repository.h"
      +
      +void prepare_repo_settings(struct repository *r)
      +{
      +	int value;
      +
     -+	if (r->settings_initialized)
     ++	if (r->settings.initialized)
      +		return;
      +
      +	/* Defaults */
     @@ -212,54 +179,50 @@
      +
      +	if (!repo_config_get_bool(r, "pack.usesparse", &value))
      +		r->settings.pack_use_sparse = value;
     -+
     -+	r->settings_initialized = 1;
      +}
      
     - diff --git a/repo-settings.h b/repo-settings.h
     - new file mode 100644
     - --- /dev/null
     - +++ b/repo-settings.h
     -@@
     -+#ifndef REPO_SETTINGS_H
     -+#define REPO_SETTINGS_H
     -+
     -+struct repo_settings {
     -+	int core_commit_graph;
     -+	int gc_write_commit_graph;
     -+
     -+	int index_version;
     -+
     -+	int pack_use_sparse;
     -+};
     -+
     -+struct repository;
     -+
     -+void prepare_repo_settings(struct repository *r);
     -+
     -+#endif /* REPO_SETTINGS_H */
     -
       diff --git a/repository.h b/repository.h
       --- a/repository.h
       +++ b/repository.h
      @@
     - #define REPOSITORY_H
     - 
       #include "path.h"
     -+#include "repo-settings.h"
       
       struct config_set;
      +struct repo_settings;
       struct git_hash_algo;
       struct index_state;
       struct lock_file;
     +@@
     + struct raw_object_store;
     + struct submodule_cache;
     + 
     ++struct repo_settings {
     ++	int initialized;
     ++
     ++	int core_commit_graph;
     ++	int gc_write_commit_graph;
     ++
     ++	int index_version;
     ++
     ++	int pack_use_sparse;
     ++};
     ++
     + struct repository {
     + 	/* Environment */
     + 	/*
      @@
       	 */
       	char *submodule_prefix;
       
     -+	int settings_initialized;
      +	struct repo_settings settings;
      +
       	/* Subsystems */
       	/*
       	 * Repository's config which contains key-value pairs from the usual
     +@@
     +  */
     + void repo_update_index_if_able(struct repository *, struct lock_file *);
     + 
     ++void prepare_repo_settings(struct repository *r);
     + 
     + #endif /* REPOSITORY_H */
 2:  86380c7832 ! 2:  c0129066a0 repo-settings: add feature.manyCommits setting
     @@ -81,7 +81,7 @@
       +++ b/repo-settings.c
      @@
       #include "config.h"
     - #include "repo-settings.h"
     + #include "repository.h"
       
      +#define UPDATE_DEFAULT(s,v) do { if (s == -1) { s = v; } } while(0)
      +
     @@ -89,13 +89,12 @@
       {
       	int value;
      @@
     + 
       	if (!repo_config_get_bool(r, "pack.usesparse", &value))
       		r->settings.pack_use_sparse = value;
     - 
     ++
      +	if (!repo_config_get_bool(r, "feature.manycommits", &value) && value) {
      +		UPDATE_DEFAULT(r->settings.core_commit_graph, 1);
      +		UPDATE_DEFAULT(r->settings.gc_write_commit_graph, 1);
      +	}
     -+
     - 	r->settings_initialized = 1;
       }
 3:  49be7a7345 ! 3:  b0a2d0e188 repo-settings: parse core.untrackedCache
     @@ -25,14 +25,6 @@
       diff --git a/builtin/update-index.c b/builtin/update-index.c
       --- a/builtin/update-index.c
       +++ b/builtin/update-index.c
     -@@
     - #include "dir.h"
     - #include "split-index.h"
     - #include "fsmonitor.h"
     -+#include "repo-settings.h"
     - 
     - /*
     -  * Default to not allowing changes to the list of files. The
      @@
       	struct parse_opt_ctx_t ctx;
       	strbuf_getline_fn getline_fn;
     @@ -141,7 +133,7 @@
       	int value;
      +	char *strval;
       
     - 	if (r->settings_initialized)
     + 	if (r->settings.initialized)
       		return;
      @@
       
     @@ -168,22 +160,21 @@
       		UPDATE_DEFAULT(r->settings.core_commit_graph, 1);
       		UPDATE_DEFAULT(r->settings.gc_write_commit_graph, 1);
       	}
     - 
     ++
      +	/* Hack for test programs like test-dump-untracked-cache */
      +	if (ignore_untracked_cache_config)
      +		r->settings.core_untracked_cache = UNTRACKED_CACHE_KEEP;
      +	else
      +		UPDATE_DEFAULT(r->settings.core_untracked_cache, UNTRACKED_CACHE_KEEP);
      +
     - 	r->settings_initialized = 1;
       }
      
     - diff --git a/repo-settings.h b/repo-settings.h
     - --- a/repo-settings.h
     - +++ b/repo-settings.h
     + diff --git a/repository.h b/repository.h
     + --- a/repository.h
     + +++ b/repository.h
      @@
     - #ifndef REPO_SETTINGS_H
     - #define REPO_SETTINGS_H
     + struct raw_object_store;
     + struct submodule_cache;
       
      +enum untracked_cache_setting {
      +	UNTRACKED_CACHE_UNSET = -1,
     @@ -193,7 +184,9 @@
      +};
      +
       struct repo_settings {
     - 	int core_commit_graph;
     + 	int initialized;
     + 
     +@@
       	int gc_write_commit_graph;
       
       	int index_version;
 4:  86a5a0c589 = 4:  0d4774d45f repo-settings: create feature.manyFiles setting
 5:  b8a631683a ! 5:  2e153fac22 repo-settings: create feature.experimental setting
     @@ -93,18 +93,6 @@
       pack.writeBitmaps (deprecated)::
       	This is a deprecated synonym for `repack.writeBitmaps`.
      
     - diff --git a/builtin/am.c b/builtin/am.c
     - --- a/builtin/am.c
     - +++ b/builtin/am.c
     -@@
     - #include "string-list.h"
     - #include "packfile.h"
     - #include "repository.h"
     -+#include "repo-settings.h"
     - 
     - /**
     -  * Returns the length of the first line of msg.
     -
       diff --git a/fetch-negotiator.c b/fetch-negotiator.c
       --- a/fetch-negotiator.c
       +++ b/fetch-negotiator.c
     @@ -113,7 +101,6 @@
       #include "negotiator/default.h"
       #include "negotiator/skipping.h"
      +#include "repository.h"
     -+#include "repo-settings.h"
       
      -void fetch_negotiator_init(struct fetch_negotiator *negotiator,
      -			   const char *algorithm)
     @@ -230,14 +217,6 @@
       diff --git a/merge-recursive.c b/merge-recursive.c
       --- a/merge-recursive.c
       +++ b/merge-recursive.c
     -@@
     - #include "submodule.h"
     - #include "revision.h"
     - #include "commit-reach.h"
     -+#include "repo-settings.h"
     - 
     - struct path_hashmap_entry {
     - 	struct hashmap_entry e;
      @@
       		opt->merge_detect_rename = git_config_rename("merge.renames", value);
       		free(value);
     @@ -307,13 +286,11 @@
       
      +	UPDATE_DEFAULT(r->settings.merge_directory_renames, MERGE_DIRECTORY_RENAMES_CONFLICT);
      +	UPDATE_DEFAULT(r->settings.fetch_negotiation_algorithm, FETCH_NEGOTIATION_DEFAULT);
     -+
     - 	r->settings_initialized = 1;
       }
      
     - diff --git a/repo-settings.h b/repo-settings.h
     - --- a/repo-settings.h
     - +++ b/repo-settings.h
     + diff --git a/repository.h b/repository.h
     + --- a/repository.h
     + +++ b/repository.h
      @@
       	UNTRACKED_CACHE_WRITE = 2
       };
     @@ -333,8 +310,8 @@
      +};
      +
       struct repo_settings {
     - 	int core_commit_graph;
     - 	int gc_write_commit_graph;
     + 	int initialized;
     + 
      @@
       	enum untracked_cache_setting core_untracked_cache;
       
     @@ -343,7 +320,7 @@
      +	enum fetch_negotiation_setting fetch_negotiation_algorithm;
       };
       
     - struct repository;
     + struct repository {
      
       diff --git a/t/t5552-skipping-fetch-negotiator.sh b/t/t5552-skipping-fetch-negotiator.sh
       --- a/t/t5552-skipping-fetch-negotiator.sh

gitgitgadget

From: Derrick Stolee <[email protected]>

There are a few important config settings that are not loaded during git_default_config. These are instead loaded on-demand.

Centralize these config options to a single scan, and store all of the values in a repo_settings struct. The values for each setting are initialized as negative to indicate "unset".

This centralization will be particularly important in a later change to introduce "meta" config settings that change the defaults for these config settings.

---
 Makefile               |  1 +
 builtin/gc.c           | 12 +++++-------
 builtin/pack-objects.c |  8 ++++----
 commit-graph.c         |  6 +++---
 read-cache.c           | 11 ++++++-----
 repo-settings.c        | 25 +++++++++++++++++++++++++
 repository.h           | 15 +++++++++++++++
 7 files changed, 59 insertions(+), 19 deletions(-)
 create mode 100644 repo-settings.c

index 11ccea4071..032fe9b5f9 100644
@@ -964,6 +964,7 @@ LIB_OBJS += refspec.o
 LIB_OBJS += ref-filter.o
 LIB_OBJS += remote.o
 LIB_OBJS += replace-object.o
+LIB_OBJS += repo-settings.o
 LIB_OBJS += repository.o
 LIB_OBJS += rerere.o
 LIB_OBJS += resolve-undo.o

index c18efadda5..4b8fbb9965 100644
@@ -41,7 +41,6 @@ static int aggressive_depth = 50;
 static int aggressive_window = 250;
 static int gc_auto_threshold = 6700;
 static int gc_auto_pack_limit = 50;
-static int gc_write_commit_graph;
 static int detach_auto = 1;
 static timestamp_t gc_log_expire_time;
 static const char *gc_log_expire = "1.day.ago";

@@ -148,7 +147,6 @@ static void gc_config(void)
 	git_config_get_int("gc.aggressivedepth", &aggressive_depth);
 	git_config_get_int("gc.auto", &gc_auto_threshold);
 	git_config_get_int("gc.autopacklimit", &gc_auto_pack_limit);
-	git_config_get_bool("gc.writecommitgraph", &gc_write_commit_graph);
 	git_config_get_bool("gc.autodetach", &detach_auto);
 	git_config_get_expiry("gc.pruneexpire", &prune_expire);
 	git_config_get_expiry("gc.worktreepruneexpire", &prune_worktrees_expire);

@@ -685,11 +683,11 @@ int cmd_gc(int argc, const char **argv, const char *prefix)
 		clean_pack_garbage();
 	}
 
-	if (gc_write_commit_graph &&
-	    write_commit_graph_reachable(get_object_directory(),
-					 !quiet && !daemonized ? COMMIT_GRAPH_PROGRESS : 0,
-					 NULL))
-		return 1;
+	prepare_repo_settings(the_repository);
+	if (the_repository->settings.gc_write_commit_graph == 1)
+		write_commit_graph_reachable(get_object_directory(),
+					     !quiet && !daemonized ? COMMIT_GRAPH_PROGRESS : 0,
+					     NULL);
 
 	if (auto_gc && too_many_loose_objects())
 		warning(_("There are too many unreachable loose objects; "

index 267c562b1f..f9f7181f3f 100644
@@ -2709,10 +2709,6 @@ static int git_pack_config(const char *k, const char *v, void *cb)
 		use_bitmap_index_default = git_config_bool(k, v);
 		return 0;
 	}
-	if (!strcmp(k, "pack.usesparse")) {
-		sparse = git_config_bool(k, v);
-		return 0;
-	}
 	if (!strcmp(k, "pack.threads")) {
 		delta_search_threads = git_config_int(k, v);
 		if (delta_search_threads < 0)

@@ -3332,6 +3328,10 @@ int cmd_pack_objects(int argc, const char **argv, const char *prefix)
 	read_replace_refs = 0;
 
 	sparse = git_env_bool("GIT_TEST_PACK_SPARSE", 0);
+	prepare_repo_settings(the_repository);
+	if (!sparse && the_repository->settings.pack_use_sparse != -1)
+		sparse = the_repository->settings.pack_use_sparse;
+
 	reset_pack_idx_option(&pack_idx_opts);
 	git_config(git_pack_config, NULL);
 

index b3c4de79b6..7854e491b2 100644
@@ -466,7 +466,6 @@ static void prepare_commit_graph_one(struct repository *r, const char *obj_dir)
 static int prepare_commit_graph(struct repository *r)
 {
 	struct object_directory *odb;
-	int config_value;
 
 	if (git_env_bool(GIT_TEST_COMMIT_GRAPH_DIE_ON_LOAD, 0))
 		die("dying as requested by the '%s' variable on commit-graph load!",

@@ -476,9 +475,10 @@ static int prepare_commit_graph(struct repository *r)
 		return !!r->objects->commit_graph;
 	r->objects->commit_graph_attempted = 1;
 
+	prepare_repo_settings(r);
+
 	if (!git_env_bool(GIT_TEST_COMMIT_GRAPH, 0) &&
-	    (repo_config_get_bool(r, "core.commitgraph", &config_value) ||
-	    !config_value))
+	    r->settings.core_commit_graph != 1)
 		/*
 		 * This repository is not configured to use commit graphs, so
 		 * do not load one. (But report commit_graph_attempted anyway

index c701f7f8b8..59dbebc15d 100644
@@ -1599,16 +1599,17 @@ struct cache_entry *refresh_cache_entry(struct index_state *istate,
 
 #define INDEX_FORMAT_DEFAULT 3
 
-static unsigned int get_index_format_default(void)
+static unsigned int get_index_format_default(struct repository *r)
 {
 	char *envversion = getenv("GIT_INDEX_VERSION");
 	char *endp;
-	int value;
 	unsigned int version = INDEX_FORMAT_DEFAULT;
 
 	if (!envversion) {
-		if (!git_config_get_int("index.version", &value))
-			version = value;
+		prepare_repo_settings(r);
+
+		if (r->settings.index_version >= 0)
+			version = r->settings.index_version;
 		if (version < INDEX_FORMAT_LB || INDEX_FORMAT_UB < version) {
 			warning(_("index.version set, but the value is invalid.\n"
 				  "Using version %i"), INDEX_FORMAT_DEFAULT);

@@ -2765,7 +2766,7 @@ static int do_write_index(struct index_state *istate, struct tempfile *tempfile,
 	}
 
 	if (!istate->version) {
-		istate->version = get_index_format_default();
+		istate->version = get_index_format_default(the_repository);
 		if (git_env_bool("GIT_TEST_SPLIT_INDEX", 0))
 			init_split_index(istate);
 	}

new file mode 100644
index 0000000000..309577f6bc
@@ -0,0 +1,25 @@
+#include "cache.h"
+#include "config.h"
+#include "repository.h"
+
+void prepare_repo_settings(struct repository *r)
+{
+	int value;
+
+	if (r->settings.initialized)
+		return;
+
+	/* Defaults */
+	memset(&r->settings, -1, sizeof(r->settings));
+
+	if (!repo_config_get_bool(r, "core.commitgraph", &value))
+		r->settings.core_commit_graph = value;
+	if (!repo_config_get_bool(r, "gc.writecommitgraph", &value))
+		r->settings.gc_write_commit_graph = value;
+
+	if (!repo_config_get_bool(r, "index.version", &value))
+		r->settings.index_version = value;
+
+	if (!repo_config_get_bool(r, "pack.usesparse", &value))
+		r->settings.pack_use_sparse = value;
+}

index 4fb6a5885f..2bb2bc3eea 100644
@@ -4,6 +4,7 @@
 #include "path.h"
 
 struct config_set;
+struct repo_settings;
 struct git_hash_algo;
 struct index_state;
 struct lock_file;

@@ -11,6 +12,17 @@ struct pathspec;
 struct raw_object_store;
 struct submodule_cache;
 
+struct repo_settings {
+	int initialized;
+
+	int core_commit_graph;
+	int gc_write_commit_graph;
+
+	int index_version;
+
+	int pack_use_sparse;
+};
+
 struct repository {
 	/* Environment */
 	/*

@@ -72,6 +84,8 @@ struct repository {
 	 */
 	char *submodule_prefix;
 
+	struct repo_settings settings;
+
 	/* Subsystems */
 	/*
 	 * Repository's config which contains key-value pairs from the usual

@@ -157,5 +171,6 @@ int repo_read_index_unmerged(struct repository *);
  */
 void repo_update_index_if_able(struct repository *, struct lock_file *);
 
+void prepare_repo_settings(struct repository *r);
 
 #endif /* REPOSITORY_H */

gitgitgadget

From: Derrick Stolee <[email protected]>

When a repo has many commits, it is helpful to write and read the commit-graph file. Future changes to Git may include new config settings that are beneficial in this scenario.

Create the 'feature.manyCommits' config setting that changes the default values of 'core.commitGraph' and 'gc.writeCommitGraph' to true.

---
 Documentation/config.txt         |  2 ++
 Documentation/config/core.txt    |  3 ++-
 Documentation/config/feature.txt | 15 +++++++++++++++
 Documentation/config/gc.txt      |  4 ++--
 repo-settings.c                  |  7 +++++++
 5 files changed, 28 insertions(+), 3 deletions(-)
 create mode 100644 Documentation/config/feature.txt

index e3f5bc3396..77f3b1486b 100644
@@ -345,6 +345,8 @@ include::config/difftool.txt[]
 
 include::config/fastimport.txt[]
 
+include::config/feature.txt[]
+
 include::config/fetch.txt[]
 
 include::config/format.txt[]

index 75538d27e7..d80162681a 100644
@@ -577,7 +577,8 @@ the `GIT_NOTES_REF` environment variable.  See linkgit:git-notes[1].
 
 core.commitGraph::
 	If true, then git will read the commit-graph file (if it exists)
-	to parse the graph structure of commits. Defaults to false. See
+	to parse the graph structure of commits. Defaults to false, unless
+	`feature.manyCommits` is enabled. See
 	linkgit:git-commit-graph[1] for more information.
 
 core.useReplaceRefs::

new file mode 100644
index 0000000000..f74314ae90
@@ -0,0 +1,15 @@
+feature.*::
+	The config settings that start with `feature.` modify the defaults of
+	a group of other config settings. These groups are created by the Git
+	developer community as recommended defaults and are subject to change.
+	In particular, new config options may be added with different defaults.
+
+feature.manyCommits::
+	Enable config options that optimize for repos with many commits. This
+	setting is recommended for repos with at least 100,000 commits. The
+	new default values are:
++
+* `core.commitGraph=true` enables reading the commit-graph file.
++
+* `gc.writeCommitGraph=true` enables writing the commit-graph file during
+garbage collection.
\ No newline at end of file

index 02b92b18b5..31a5fc4f75 100644
@@ -63,8 +63,8 @@ gc.writeCommitGraph::
 	If true, then gc will rewrite the commit-graph file when
 	linkgit:git-gc[1] is run. When using `git gc --auto`
 	the commit-graph will be updated if housekeeping is
-	required. Default is false. See linkgit:git-commit-graph[1]
-	for details.
+	required. Default is false, unless `feature.manyCommits`
+	is enabled. See linkgit:git-commit-graph[1] for details.
 
 gc.logExpiry::
 	If the file gc.log exists, then `git gc --auto` will print

index 309577f6bc..fc05f4fbc4 100644
@@ -2,6 +2,8 @@
 #include "config.h"
 #include "repository.h"
 
+#define UPDATE_DEFAULT(s,v) do { if (s == -1) { s = v; } } while(0)
+
 void prepare_repo_settings(struct repository *r)
 {
 	int value;

@@ -22,4 +24,9 @@ void prepare_repo_settings(struct repository *r)
 
 	if (!repo_config_get_bool(r, "pack.usesparse", &value))
 		r->settings.pack_use_sparse = value;
+
+	if (!repo_config_get_bool(r, "feature.manycommits", &value) && value) {
+		UPDATE_DEFAULT(r->settings.core_commit_graph, 1);
+		UPDATE_DEFAULT(r->settings.gc_write_commit_graph, 1);
+	}
 }

gitgitgadget

Hi Junio,

On the other hand, as we never promised any kind of API (and this is not even an internal API to begin with), it will be _easy_ to rename it in the unlikely event that we would ever introduce non-boolean defaults to override, wouldn't you agree?

We have plenty of precedent where patch series start by refactoring, whether it is to rename functions or variables or files or extracting functions. Preparing for a future that might never come strikes me as premature optimization.

Ciao,
Dscho