Audit and document Scalar config

In September [1], we discussed that the Scalar config options could use some documented justification as well as some comments to the config file that they were set by Scalar. I was then immediately distracted by other work things and am finally here with a series to do just that.

[1]
https://lore.kernel.org/git/[email protected]/

I have indeed used Patrick's idea to add '# set by scalar' to each line added by Scalar, it took a little more work for all the kinds of config set. I made myself a co-author.

While working to justify each config option, I found some stale or incorrect config options. I also relaxed the override setting in most cases which gave me an opportunity to alphabetize the settings.

There was at least one case (I'm thinking of core.fscache here) where the config doesn't even exist in core Git, but instead in Git for Windows. We'll need to adjust in that fork to reinclude it in the right place.

Thanks, -Stolee

Derrick Stolee (5):
scalar: annotate config file with "set by scalar"
scalar: use index.skipHash=true for performance
scalar: remove stale config values
scalar: alphabetize and simplify config
scalar: document config settings

 Documentation/scalar.adoc | 158 ++++++++++++++++++++++++++++++++++++++
 scalar.c                  |  81 ++++++++++---------
 t/t9210-scalar.sh         |  26 ++++---
 3 files changed, 218 insertions(+), 47 deletions(-)

base-commit: 6ab38b7e9cc7adafc304f3204616a4debd49c6e9
Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-2010%2Fderrickstolee%2Fscalar-config-v1
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-2010/derrickstolee/scalar-config-v1
Pull-Request: https://github.com/gitgitgadget/git/pull/2010

gitgitgadget

From: Derrick Stolee <[email protected]>

A repo may have config options set by 'scalar clone' or 'scalar register' and then updated by 'scalar reconfigure'. It can be helpful to point out which of those options were set by the latest scalar recommendations.

Add "# set by scalar" to the end of each config option to assist users in identifying why these config options were set in their repo.

---
 scalar.c          | 16 +++++++++++++---
 t/t9210-scalar.sh |  3 +++
 2 files changed, 16 insertions(+), 3 deletions(-)

index f754311627..431b631585 100644
@@ -19,6 +19,7 @@
 #include "help.h"
 #include "setup.h"
 #include "trace2.h"
+#include "path.h"
 
 static void setup_enlistment_directory(int argc, const char **argv,
 				       const char * const *usagestr,

@@ -99,16 +100,20 @@ static int set_scalar_config(const struct scalar_config *config, int reconfigure
 {
 	char *value = NULL;
 	int res;
+	char *file = repo_git_path(the_repository, "config");
 
 	if ((reconfigure && config->overwrite_on_reconfigure) ||
 	    repo_config_get_string(the_repository, config->key, &value)) {
 		trace2_data_string("scalar", the_repository, config->key, "created");
-		res = repo_config_set_gently(the_repository, config->key, config->value);
+		res = repo_config_set_multivar_in_file_gently(the_repository, file, config->key,
+							      config->value, NULL,
+							      " # set by scalar", 0);
 	} else {
 		trace2_data_string("scalar", the_repository, config->key, "exists");
 		res = 0;
 	}
 
+	free(file);
 	free(value);
 	return res;
 }

@@ -195,13 +200,18 @@ static int set_recommended_config(int reconfigure)
 	 * for multiple values.
 	 */
 	if (repo_config_get_string(the_repository, "log.excludeDecoration", &value)) {
+		char *file = repo_git_path(the_repository, "config");
 		trace2_data_string("scalar", the_repository,
 				   "log.excludeDecoration", "created");
-		if (repo_config_set_multivar_gently(the_repository, "log.excludeDecoration",
+		if (repo_config_set_multivar_in_file_gently(the_repository, file,
+						    "log.excludeDecoration",
 						    "refs/prefetch/*",
-						    CONFIG_REGEX_NONE, 0))
+						    CONFIG_REGEX_NONE,
+						    " # set by scalar",
+						    0))
 			return error(_("could not configure "
 				       "log.excludeDecoration"));
+		free(file);
 	} else {
 		trace2_data_string("scalar", the_repository,
 				   "log.excludeDecoration", "exists");

index bd6f0c40d2..43c210a23d 100755
@@ -210,6 +210,9 @@ test_expect_success 'scalar reconfigure' '
 	GIT_TRACE2_EVENT="$(pwd)/reconfigure" scalar reconfigure -a &&
 	test_path_is_file one/src/cron.txt &&
 	test true = "$(git -C one/src config core.preloadIndex)" &&
+	test_grep "preloadIndex = true # set by scalar" one/src/.git/config &&
+	test_grep "excludeDecoration = refs/prefetch/\* # set by scalar" one/src/.git/config &&
+
 	test_subcommand git maintenance start <reconfigure &&
 	test_subcommand ! git maintenance unregister --force <reconfigure &&
 

gitgitgadget

The implementation is quite straight-forward, inlining expansion of repo_config_set_gently() in the places that we want to add comment to.

If we had (a lot) more than two callsites, I would have suggested to add a simple helper function, something like

    static int scalar_config_set(struct repository *r, const char *key, const char *value)
    {
	char *file = repo_git_path(r, "config");
        int res = repo_config_set_multivar_in_file_gently(r, file,
		key, value, NULL, " # set by scalar", 0);
	free(file);
	return res;
    }

and then the updates to the callers would have been absolute minimum.

Well, even with only two callsites, perhaps such a refactoring may still have value in reducing the risk of typo in the comment.

Agreed, I think it's a good idea to provide such a function. The calls to repo_config_set_multivar_in_file_gently() are quite verbose.

Patrick

From: Derrick Stolee <[email protected]>

The index.skipHash config option has been set to 'false' by Scalar since 4933152cbb (scalar: enable path-walk during push via config, 2025-05-16) but that commit message is trying to communicate the exact opposite:
that the 'true' value is what we want instead. This means that we've been disabling this performance benefit for Scalar repos unintentionally.

Fix this issue before we add justification for the config options set in this list.

Oddly, enabling index.skipHash causes a test issue during 'test_commit' in one of the Scalar tests when GIT_TEST_SPLIT_INDEX is enabled (as caught by the linux-test-vars build). I'm fixing the test by disabling the environment variable, but the issue should be resolved in a series focused on the split index.

---
 scalar.c          | 2 +-
 t/t9210-scalar.sh | 5 +++++
 2 files changed, 6 insertions(+), 1 deletion(-)

index 431b631585..75fd292be7 100644
@@ -154,7 +154,7 @@ static int set_recommended_config(int reconfigure)
 		{ "credential.validate", "false", 1 }, /* GCM4W-only */
 		{ "gc.auto", "0", 1 },
 		{ "gui.GCWarning", "false", 1 },
-		{ "index.skipHash", "false", 1 },
+		{ "index.skipHash", "true", 1 },
 		{ "index.threads", "true", 1 },
 		{ "index.version", "4", 1 },
 		{ "merge.stat", "false", 1 },

index 43c210a23d..91d5964b73 100755
@@ -246,6 +246,11 @@ test_expect_success 'scalar reconfigure --all with includeIf.onbranch' '
 '
 
 test_expect_success 'scalar reconfigure --all with detached HEADs' '
+	# This test demonstrates an issue with index.skipHash=true and
+	# this test variable for the split index. Disable the test variable.
+	GIT_TEST_SPLIT_INDEX= &&
+	export GIT_TEST_SPLIT_INDEX &&
+
 	repos="two three four" &&
 	for num in $repos
 	do

gitgitgadget

That's indeed a better way to do it. Will do in v2.

Thanks,
-Stolee

From: Derrick Stolee <[email protected]>

These config values were added in the original Scalar contribution, d0feac4e8c (scalar: 'register' sets recommended config and starts maintenance, 2021-12-03), but were never fully checked for validity in the upstream Git project. At the time, Scalar was only intended for the contrib/ directory so did not have as rigorous of an investigation.

Each config option has its own justification for removal:

* core.preloadIndex: This value is true by default, now. Removing this causes some changes required to the tests that checked this config value. Use gui.gcwarning=false instead.

* core.fscache: This config does not exist in the core Git project, but is instead a config option for a Git for Windows feature.

* core.multiPackIndex: This config value is now enabled by default, so does not need to be called out specifically. It was originally included to make sure the background maintenance that created multi-pack-indexes would result in the expected performance improvements.

* credential.validate: This option is not something specific to Git but instead an older version of Git Credential Manager for Windows. That software was replaced several years ago by the cross-platform Git Credential Manger so this option is no longer needed to help users who were on that older software.

* pack.useSparse=true: This value is now Git's default as of de3a864114 (config: set pack.useSparse=true by default, 2020-03-20) so we don't need it set by Scalar.

---
 scalar.c          |  5 -----
 t/t9210-scalar.sh | 20 ++++++++++----------
 2 files changed, 10 insertions(+), 15 deletions(-)

index 75fd292be7..fb595fab3a 100644
@@ -129,9 +129,6 @@ static int set_recommended_config(int reconfigure)
 	struct scalar_config config[] = {
 		/* Required */
 		{ "am.keepCR", "true", 1 },
-		{ "core.FSCache", "true", 1 },
-		{ "core.multiPackIndex", "true", 1 },
-		{ "core.preloadIndex", "true", 1 },
 #ifndef WIN32
 		{ "core.untrackedCache", "true", 1 },
 #else

@@ -151,7 +148,6 @@ static int set_recommended_config(int reconfigure)
 #endif
 		{ "core.logAllRefUpdates", "true", 1 },
 		{ "credential.https://dev.azure.com.useHttpPath", "true", 1 },
-		{ "credential.validate", "false", 1 }, /* GCM4W-only */
 		{ "gc.auto", "0", 1 },
 		{ "gui.GCWarning", "false", 1 },
 		{ "index.skipHash", "true", 1 },

@@ -160,7 +156,6 @@ static int set_recommended_config(int reconfigure)
 		{ "merge.stat", "false", 1 },
 		{ "merge.renames", "true", 1 },
 		{ "pack.useBitmaps", "false", 1 },
-		{ "pack.useSparse", "true", 1 },
 		{ "receive.autoGC", "false", 1 },
 		{ "feature.manyFiles", "false", 1 },
 		{ "feature.experimental", "false", 1 },

index 91d5964b73..63224d2062 100755
@@ -202,15 +202,15 @@ test_expect_success 'scalar clone --no-... opts' '
 test_expect_success 'scalar reconfigure' '
 	git init one/src &&
 	scalar register one &&
-	git -C one/src config core.preloadIndex false &&
+	git -C one/src config unset gui.gcwarning &&
 	scalar reconfigure one &&
-	test true = "$(git -C one/src config core.preloadIndex)" &&
-	git -C one/src config core.preloadIndex false &&
+	test false = "$(git -C one/src config gui.gcwarning)" &&
+	git -C one/src config unset gui.gcwarning &&
 	rm one/src/cron.txt &&
 	GIT_TRACE2_EVENT="$(pwd)/reconfigure" scalar reconfigure -a &&
 	test_path_is_file one/src/cron.txt &&
-	test true = "$(git -C one/src config core.preloadIndex)" &&
-	test_grep "preloadIndex = true # set by scalar" one/src/.git/config &&
+	test false = "$(git -C one/src config gui.gcwarning)" &&
+	test_grep "GCWarning = false # set by scalar" one/src/.git/config &&
 	test_grep "excludeDecoration = refs/prefetch/\* # set by scalar" one/src/.git/config &&
 
 	test_subcommand git maintenance start <reconfigure &&

@@ -234,14 +234,14 @@ test_expect_success 'scalar reconfigure --all with includeIf.onbranch' '
 		git init $num/src &&
 		scalar register $num/src &&
 		git -C $num/src config includeif."onbranch:foo".path something &&
-		git -C $num/src config core.preloadIndex false || return 1
+		git -C $num/src config unset gui.gcwarning || return 1
 	done &&
 
 	scalar reconfigure --all &&
 
 	for num in $repos
 	do
-		test true = "$(git -C $num/src config core.preloadIndex)" || return 1
+		test false = "$(git -C $num/src config gui.gcwarning)" || return 1
 	done
 '
 

@@ -257,7 +257,7 @@ test_expect_success 'scalar reconfigure --all with detached HEADs' '
 		rm -rf $num/src &&
 		git init $num/src &&
 		scalar register $num/src &&
-		git -C $num/src config core.preloadIndex false &&
+		git -C $num/src config unset gui.gcwarning &&
 		test_commit -C $num/src initial &&
 		git -C $num/src switch --detach HEAD || return 1
 	done &&

@@ -266,7 +266,7 @@ test_expect_success 'scalar reconfigure --all with detached HEADs' '
 
 	for num in $repos
 	do
-		test true = "$(git -C $num/src config core.preloadIndex)" || return 1
+		test false = "$(git -C $num/src config gui.gcwarning)" || return 1
 	done
 '
 

@@ -298,7 +298,7 @@ test_expect_success 'scalar supports -c/-C' '
 	git init sub &&
 	scalar -C sub -c status.aheadBehind=bogus register &&
 	test -z "$(git -C sub config --local status.aheadBehind)" &&
-	test true = "$(git -C sub config core.preloadIndex)"
+	test false = "$(git -C sub config gui.gcwarning)"
 '
 
 test_expect_success '`scalar [...] <dir>` errors out when dir is missing' '

gitgitgadget

Thanks for a conprehensive list. Very well described.

From: Derrick Stolee <[email protected]>

The config values set by Scalar went through an audit in the previous changes, so now reorganize the settings and simplify their purpose.

First, alphabetize the config options, except put the platform-specific options at the end. This groups two Windows-specific settings and only one non-Windows setting.

Also, this removes the 'overwrite_on_reconfigure' setting for many of these options. That setting made nearly all of these options "required" for scalar enlistments, restricting use for users. Instead, now nearly all options have removed this setting.

However, there is one setting that still has this, which is index.skipHash, which was previously being set to _false_ when we actually prefer the value of true. Keep the overwrite here to help Scalar users upgrade to the new version. We may remove that overwrite in the future once we belive that most of the users who have the false value have upgraded to a version that overwrites that to 'true'.

---
 scalar.c | 60 ++++++++++++++++++++++++++++----------------------------
 1 file changed, 30 insertions(+), 30 deletions(-)

index fb595fab3a..d73f9d85c7 100644
@@ -127,10 +127,33 @@ static int have_fsmonitor_support(void)
 static int set_recommended_config(int reconfigure)
 {
 	struct scalar_config config[] = {
-		/* Required */
-		{ "am.keepCR", "true", 1 },
+		{ "am.keepCR", "true" },
+		{ "commitGraph.changedPaths", "true" },
+		{ "commitGraph.generationVersion", "1" },
+		{ "core.autoCRLF", "false" },
+		{ "core.logAllRefUpdates", "true" },
+		{ "core.safeCRLF", "false" },
+		{ "credential.https://dev.azure.com.useHttpPath", "true" },
+		{ "feature.experimental", "false" },
+		{ "feature.manyFiles", "false" },
+		{ "fetch.showForcedUpdates", "false" },
+		{ "fetch.unpackLimit", "1" },
+		{ "fetch.writeCommitGraph", "false" },
+		{ "gc.auto", "0" },
+		{ "gui.GCWarning", "false" },
+		{ "index.skipHash", "true", 1 /* Fix previous setting. */ },
+		{ "index.threads", "true"},
+		{ "index.version", "4" },
+		{ "merge.renames", "true" },
+		{ "merge.stat", "false" },
+		{ "pack.useBitmaps", "false" },
+		{ "pack.usePathWalk", "true" },
+		{ "receive.autoGC", "false" },
+		{ "status.aheadBehind", "false" },
+
+		/* platform-specific */
 #ifndef WIN32
-		{ "core.untrackedCache", "true", 1 },
+		{ "core.untrackedCache", "true" },
 #else
 		/*
 		 * Unfortunately, Scalar's Functional Tests demonstrated

@@ -144,34 +167,11 @@ static int set_recommended_config(int reconfigure)
 		 * Therefore, with a sad heart, we disable this very useful
 		 * feature on Windows.
 		 */
-		{ "core.untrackedCache", "false", 1 },
-#endif
-		{ "core.logAllRefUpdates", "true", 1 },
-		{ "credential.https://dev.azure.com.useHttpPath", "true", 1 },
-		{ "gc.auto", "0", 1 },
-		{ "gui.GCWarning", "false", 1 },
-		{ "index.skipHash", "true", 1 },
-		{ "index.threads", "true", 1 },
-		{ "index.version", "4", 1 },
-		{ "merge.stat", "false", 1 },
-		{ "merge.renames", "true", 1 },
-		{ "pack.useBitmaps", "false", 1 },
-		{ "receive.autoGC", "false", 1 },
-		{ "feature.manyFiles", "false", 1 },
-		{ "feature.experimental", "false", 1 },
-		{ "fetch.unpackLimit", "1", 1 },
-		{ "fetch.writeCommitGraph", "false", 1 },
-#ifdef WIN32
-		{ "http.sslBackend", "schannel", 1 },
+		{ "core.untrackedCache", "false" },
+
+		/* Other Windows-specific required settings: */
+		{ "http.sslBackend", "schannel" },
 #endif
-		/* Optional */
-		{ "status.aheadBehind", "false" },
-		{ "commitGraph.changedPaths", "true" },
-		{ "commitGraph.generationVersion", "1" },
-		{ "core.autoCRLF", "false" },
-		{ "core.safeCRLF", "false" },
-		{ "fetch.showForcedUpdates", "false" },
-		{ "pack.usePathWalk", "true" },
 		{ NULL, NULL },
 	};
 	int i;

gitgitgadget

As far as I understand, this setting causes us to overwrite any preexisting config values when reconfiguring Scalar? So with your changes the effect is that we now don't do that anymore, which allows the user to tune some of the configuration values to their liking after having run scalar init for the first time. I guess that makes sense, as it gives the user more flexibility.

It does make me wonder though: is it really the most sensible thing to overwrite any keys that already exist in the configuration? We may end up overwriting configuration specified by the user both in the case of scalar init and scalar reconfigure. But arguably, we might want to only ever write configuration that does _not_ yet have an explicit value in the configuration file, regardless of whether or not we reconfigure.

Makes sense. This has likely been a bug, and we now want to rectify that bug.

Thanks!

Patrick

Yes, that is correct.

I agree that this notion of forcing config is not optimal, and is a leftover from VFS for Git where some of these config things were actually required for the virtualization to work. Once that idea was in place, it was easy to think "we'll make sure the repo is configured correctly" but that makes much less sense in Scalar these days.

And hopefully this is the only reason we'd need this "overwrite" feature from this point on.

Thanks,
-Stolee

From: Derrick Stolee <[email protected]>

Add user-facing documentation that justifies the values being set by 'scalar clone', 'scalar register', and 'scalar reconfigure'.

---
 Documentation/scalar.adoc | 158 ++++++++++++++++++++++++++++++++++++++
 1 file changed, 158 insertions(+)

index f81b2832f8..b34af225e6 100644
@@ -197,6 +197,164 @@ delete <enlistment>::
 	This subcommand lets you delete an existing Scalar enlistment from your
 	local file system, unregistering the repository.
 
+REQUIRED AND RECOMMENDED CONFIG
+-------------------------------
+
+As part of both `scalar clone` and `scalar register`, certain Git config
+values are set to optimize for large repositories or cross-platform support.
+These options are updated in new Git versions according to the best known
+advice for large repositories, and users can get the latest recommendations
+by running `scalar reconfigure [--all]`.
+
+This section lists justifications for the config values that are set in the
+latest version.
+
+am.keepCR=true::
+	This setting is important for cross-platform development across Windows
+	and non-Windows platforms and keeping carriage return (`\r`) characters
+	in certain workflows.
+
+commitGraph.changedPaths=true::
+	This setting helps the background maintenance steps that compute the
+	serialized commit-graph to also store changed-path Bloom filters. This
+	accelerates file history commands and allows users to automatically
+	benefit without running a foreground command.
+
+commitGraph.generationVersion=1::
+	While the preferred version is 2 for performance reasons, existing users
+	that had version 1 by default will need special care in upgrading to
+	version 2. This is likely to change in the future as the upgrade story
+	is solidifies.
+
+core.autoCRLF=false::
+	This removes the transformation of worktree files to add CRLF line
+	endings when only LF line endings exist. This is removed for performance
+	reasons. Repositories that use tools that care about CRLF line endings
+	should commit the necessary files with those line endings instead.
+
+core.logAllRefUpdates=true::
+	This enables the reflog on all branches. While this is a performance
+	cost for large repositories, it is frequently an important data source
+	for users to get out of bad situations or to seek support from experts.
+
+core.safeCRLF=false::
+	Similar to `core.autoCRLF=false`, this disables checks around whether
+	the CRLF conversion is reversible. This is a performance improvement,
+	but can be dangerous if `core.autoCRLF` is reenabled by the user.
+
+credential.https://dev.azure.com.useHttpPath=true::
+	This setting enables the `credential.useHttpPath` feature only for web
+	URLs for Azure DevOps. This is important for users interacting with that
+	service using multiple organizations and thus multiple credential
+	tokens.
+
+feature.experimental=false::
+	This disables the "experimental" optimizations grouped under this
+	feature config. The expectation is that all valuable optimizations are
+	also set explicitly by Scalar config, and any differences are
+	intentional. Notable differences include several bitmap-related config
+	options which are disabled for client-focused Scalar repos.
+
+feature.manyFiles=false::
+	This disables the "many files" optimizations grouped under this feature
+	config. The expectation is that all valuable optimizations are also set
+	explicitly by Scalar config, and any differences are intentional.
+
+fetch.showForcedUpdates=false::
+	This disables the check at the end of `git fetch` that notifies the user
+	if the ref update was a forced update (one where the previous position
+	is not reachable from the latest position). This check can be very
+	expensive in large repositories, so is disabled and replaced with an
+	advice message. Set `advice.fetchShowForcedUpdates=false` to disable
+	this advice message.
+
+fetch.unpackLimit=1::
+	This setting prevents Git from unpacking packfiles into loose objects
+	as they are downloaded from the server. This feature was intended as a
+	way to prevent performance issues from too many packfiles, but Scalar
+	uses background maintenance to group packfiles and cover them with a
+	multi-pack-index, removing this issue.
+
+fetch.writeCommitGraph=false::
+	This config setting was created to help users automatically udpate their
+	commit-graph files as they perform fetches. However, this takes time
+	from foreground fetches and pulls and Scalar uses background maintenance
+	for this function instead.
+
+gc.auto=0::
+	This disables automatic garbage collection, since Scalar uses background
+	maintenance to keep the repository data in good shape.
+
+gui.GCWarning=false::
+	Since Scalar disables garbage collection by setting `gc.auto=0`, the
+	`git-gui` tool may start to warn about this setting. Disable this
+	warning as Scalar's background maintenance configuration makes the
+	warning irrelevant.
+
+index.skipHash=true::
+	Disable computing the hash of the index contents as it is being written.
+	This assists with performance, especially for large index files.
+
+index.threads=true::
+	This tells Git to automatically detect how many threads it should use
+	when reading the index in parallel due to the `core.preloadIndex=true`
+	setting.
+
+index.version=4::
+	This index version adds compression to the path names, reducing the size
+	of the index in a significant way for large repos. This is an important
+	performance boost.
+
+merge.renames=true::
+	When computing merges in large repos, it is particularly important to
+	detect renames to maximize the potential for a result that will validate
+	correctly. Users performing merges locally are more likely to be doing
+	so because a server-side merge (via pull request or similar) resulted in
+	conflicts. While this is the default setting, it is set specifically to
+	override a potential change to `diff.renames` which a user may set for
+	performance reasons.
+
+merge.stat=false::
+	This disables a diff output after computing a merge. This improves
+	performance of `git merge` for large repos while reducing noisy output.
+
+pack.useBitmaps=false::
+	This disables the use of `.bitmap` files attached to packfiles. Bitmap
+	files are optimized for server-side use, not client-side use. Scalar
+	disables this to avoid some performance issues that can occur if a user
+	accidentally creates `.bitmap` files.
+
+pack.usePathWalk=true::
+	This enables the `--path-walk` option to `git pack-objects` by default.
+	This can accelerate the computation and compression of packfiles created
+	by `git push` and other repack operations.
+
+receive.autoGC=false::
+	Similar to `gc.auto`, this setting is disabled in preference of
+	background maintenance.
+
+status.aheadBehind=false::
+	This disables the ahead/behind calculation that would normally happen
+	during a `git status` command. This information is frequently ignored by
+	users but can be expensive to calculate in large repos that receive
+	thousands of commits per day. The calculation is replaced with an advice
+	message that can be disabled by disabling the `advice.statusAheadBehind`
+	config.
+
+The following settings are different based on which platform is in use:
+
+core.untrackedCache=(true|false)::
+	The untracked cache feature is important for performance benefits on
+	large repositories, but has demonstrated some bugs on Windows
+	filesystems. Thus, this is set for other platforms but disabled on
+	Windows.
+
+http.sslBackend=schannel::
+	On Windows, the `openssl` backend has some issues with certain types of
+	remote providers and certificate types. Override the default setting to
+	avoid these common problems.
+
+
 SEE ALSO
 --------
 linkgit:git-clone[1], linkgit:git-maintenance[1].

gitgitgadget

That's better than what I was going for which was "is solidified". Will fix.

Yes. thanks.

I should still include this, but mention that it is enabled by default and still recommended.

Thanks,
-Stolee

I'm not sure, as this is a significant cost to the computation time. It will impact foreground commands, as well. It increases the size of the file, too.

It's worth considering, but I don't think the answer is very simple.

This is very likely, but I haven't validated myself. I'd be interested to double-check and update this setting in a later series. If we update to 2, then this would be a good reason to overwrite the old config for a while.

Good catch. Thanks!

-Stolee

Hi Stolee,

Thank you for doing this, in particular the (quite long!) list of explanations are excellent, especially when some user wonders why a particular setting was chosen and wants to understand the reason.

I am glad that the work I put in to optionally add comments pays off.

It's a bit sad that there is no well-designed bulk-edit "API" function which therefore requires constructing and free()ing that file variable many times, but that's not the fault of this series.

Thank you for calling this out! I will take care of this in Git for Windows and also in Microsoft Git (which inherits this flag from Git for Windows).

To be honest, I am not so certain that we want the FSCache to be enabled, it does have long-standing bugs (introduced by the partial clone feature, for example, where the FSCache continues to retain stale information about which loose objects are present even after the missing ones have been fetched). I guess we'll have to measure the actual performance benefits to reassess whether the feature is worth the trouble.

Thank you for your diligent work, as always,
Johannes

In September [1], we discussed that the Scalar config options could use some documented justification as well as some comments to the config file that they were set by Scalar. I was then immediately distracted by other work things and am finally here with a series to do just that.

[1]
https://lore.kernel.org/git/[email protected]/

I have indeed used Patrick's idea to add '# set by scalar' to each line added by Scalar, it took a little more work for all the kinds of config set. I made myself a co-author.

While working to justify each config option, I found some stale or incorrect config options. I also relaxed the override setting in most cases which gave me an opportunity to alphabetize the settings.

There was at least one case (I'm thinking of core.fscache here) where the config doesn't even exist in core Git, but instead in Git for Windows. We'll need to adjust in that fork to reinclude it in the right place.

Updates in V2
=============

* The config-setting code is simplified somewhat.
* Use 'sane_unset' instead of 'export' in test.
* Documentation is improved for typos, grammar, and clarity.

Thanks, -Stolee

Derrick Stolee (5):
scalar: annotate config file with "set by scalar"
scalar: use index.skipHash=true for performance
scalar: remove stale config values
scalar: alphabetize and simplify config
scalar: document config settings

 Documentation/scalar.adoc | 158 ++++++++++++++++++++++++++++++++++++++
 scalar.c                  |  83 ++++++++++----------
 t/t9210-scalar.sh         |  25 +++---
 3 files changed, 218 insertions(+), 48 deletions(-)

base-commit: 6ab38b7e9cc7adafc304f3204616a4debd49c6e9
Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-2010%2Fderrickstolee%2Fscalar-config-v2
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-2010/derrickstolee/scalar-config-v2
Pull-Request: https://github.com/gitgitgadget/git/pull/2010

Range-diff vs v1:

1: a4ad8f80d0 ! 1: 639ff98c44 scalar: annotate config file with "set by scalar"

     @@ Commit message
          recommendations.
      
          Add "# set by scalar" to the end of each config option to assist users
     -    in identifying why these config options were set in their repo.
     +    in identifying why these config options were set in their repo. Use a new
     +    helper method to simplify the two callsites.
      
          Co-authored-by: Patrick Steinhardt <[email protected]>
          Signed-off-by: Patrick Steinhardt <[email protected]>
     @@ scalar.c
       
       static void setup_enlistment_directory(int argc, const char **argv,
       				       const char * const *usagestr,
     -@@ scalar.c: static int set_scalar_config(const struct scalar_config *config, int reconfigure
     +@@ scalar.c: struct scalar_config {
     + 	int overwrite_on_reconfigure;
     + };
     + 
     ++static int set_config_with_comment(const char *key, const char *value)
     ++{
     ++	char *file = repo_git_path(the_repository, "config");
     ++	int res = repo_config_set_multivar_in_file_gently(the_repository, file,
     ++							  key, value, NULL,
     ++							  " # set by scalar", 0);
     ++	free(file);
     ++	return res;
     ++}
     ++
     + static int set_scalar_config(const struct scalar_config *config, int reconfigure)
       {
       	char *value = NULL;
     - 	int res;
     -+	char *file = repo_git_path(the_repository, "config");
     - 
     +@@ scalar.c: static int set_scalar_config(const struct scalar_config *config, int reconfigure
       	if ((reconfigure && config->overwrite_on_reconfigure) ||
       	    repo_config_get_string(the_repository, config->key, &value)) {
       		trace2_data_string("scalar", the_repository, config->key, "created");
      -		res = repo_config_set_gently(the_repository, config->key, config->value);
     -+		res = repo_config_set_multivar_in_file_gently(the_repository, file, config->key,
     -+							      config->value, NULL,
     -+							      " # set by scalar", 0);
     ++		res = set_config_with_comment(config->key, config->value);
       	} else {
       		trace2_data_string("scalar", the_repository, config->key, "exists");
       		res = 0;
     - 	}
     - 
     -+	free(file);
     - 	free(value);
     - 	return res;
     - }
      @@ scalar.c: static int set_recommended_config(int reconfigure)
     - 	 * for multiple values.
     - 	 */
       	if (repo_config_get_string(the_repository, "log.excludeDecoration", &value)) {
     -+		char *file = repo_git_path(the_repository, "config");
       		trace2_data_string("scalar", the_repository,
       				   "log.excludeDecoration", "created");
      -		if (repo_config_set_multivar_gently(the_repository, "log.excludeDecoration",
     -+		if (repo_config_set_multivar_in_file_gently(the_repository, file,
     -+						    "log.excludeDecoration",
     - 						    "refs/prefetch/*",
     +-						    "refs/prefetch/*",
      -						    CONFIG_REGEX_NONE, 0))
     -+						    CONFIG_REGEX_NONE,
     -+						    " # set by scalar",
     -+						    0))
     ++		if (set_config_with_comment("log.excludeDecoration",
     ++					    "refs/prefetch/*"))
       			return error(_("could not configure "
       				       "log.excludeDecoration"));
     -+		free(file);
       	} else {
     - 		trace2_data_string("scalar", the_repository,
     - 				   "log.excludeDecoration", "exists");
      
       ## t/t9210-scalar.sh ##
      @@ t/t9210-scalar.sh: test_expect_success 'scalar reconfigure' '
 2:  1c51dbb814 ! 2:  10e9548955 scalar: use index.skipHash=true for performance
     @@ t/t9210-scalar.sh: test_expect_success 'scalar reconfigure --all with includeIf.
       test_expect_success 'scalar reconfigure --all with detached HEADs' '
      +	# This test demonstrates an issue with index.skipHash=true and
      +	# this test variable for the split index. Disable the test variable.
     -+	GIT_TEST_SPLIT_INDEX= &&
     -+	export GIT_TEST_SPLIT_INDEX &&
     ++	sane_unset GIT_TEST_SPLIT_INDEX &&
      +
       	repos="two three four" &&
       	for num in $repos
 3:  156be69a79 = 3:  8783db6153 scalar: remove stale config values
 4:  9b8ce6ba2b = 4:  edc0254770 scalar: alphabetize and simplify config
 5:  18580f020d ! 5:  ac1627dbd9 scalar: document config settings

@@ Commit message
Add user-facing documentation that justifies the values being set by 'scalar clone', 'scalar register', and 'scalar reconfigure'.

+ Helped-by: Junio C Hamano <[email protected]>
+ Helped-by: Patrick Steinhardt <[email protected]>

## Documentation/scalar.adoc ##
@@ Documentation/scalar.adoc: delete <enlistment>::
+ While the preferred version is 2 for performance reasons, existing users
+ that had version 1 by default will need special care in upgrading to
+ version 2. This is likely to change in the future as the upgrade story
-+ is solidifies.
++ solidifies.
+
+core.autoCRLF=false::
+ This removes the transformation of worktree files to add CRLF line
@@ Documentation/scalar.adoc: delete <enlistment>::
+
+fetch.unpackLimit=1::
+ This setting prevents Git from unpacking packfiles into loose objects
-+ as they are downloaded from the server. This feature was intended as a
-+ way to prevent performance issues from too many packfiles, but Scalar
-+ uses background maintenance to group packfiles and cover them with a
-+ multi-pack-index, removing this issue.
++ as they are downloaded from the server. The default limit of 100 was
++ intended as a way to prevent performance issues from too many packfiles,
++ but Scalar uses background maintenance to group packfiles and cover them
++ with a multi-pack-index, removing this issue.
+
+fetch.writeCommitGraph=false::
-+ This config setting was created to help users automatically udpate their
++ This config setting was created to help users automatically update their
+ commit-graph files as they perform fetches. However, this takes time
+ from foreground fetches and pulls and Scalar uses background maintenance
+ for this function instead.
@@ Documentation/scalar.adoc: delete <enlistment>::
+
+index.threads=true::
+ This tells Git to automatically detect how many threads it should use
-+ when reading the index in parallel due to the core.preloadIndex=true
-+ setting.
++ when reading the index due to the default value of core.preloadIndex,
++ which enables parallel index reads.
+
+index.version=4::
+ This index version adds compression to the path names, reducing the size

gitgitgadget

From: Derrick Stolee <[email protected]>

A repo may have config options set by 'scalar clone' or 'scalar register' and then updated by 'scalar reconfigure'. It can be helpful to point out which of those options were set by the latest scalar recommendations.

Add "# set by scalar" to the end of each config option to assist users in identifying why these config options were set in their repo. Use a new helper method to simplify the two callsites.

---
 scalar.c          | 18 ++++++++++++++----
 t/t9210-scalar.sh |  3 +++
 2 files changed, 17 insertions(+), 4 deletions(-)

index f754311627..be3878948d 100644
@@ -19,6 +19,7 @@
 #include "help.h"
 #include "setup.h"
 #include "trace2.h"
+#include "path.h"
 
 static void setup_enlistment_directory(int argc, const char **argv,
 				       const char * const *usagestr,

@@ -95,6 +96,16 @@ struct scalar_config {
 	int overwrite_on_reconfigure;
 };
 
+static int set_config_with_comment(const char *key, const char *value)
+{
+	char *file = repo_git_path(the_repository, "config");
+	int res = repo_config_set_multivar_in_file_gently(the_repository, file,
+							  key, value, NULL,
+							  " # set by scalar", 0);
+	free(file);
+	return res;
+}
+
 static int set_scalar_config(const struct scalar_config *config, int reconfigure)
 {
 	char *value = NULL;

@@ -103,7 +114,7 @@ static int set_scalar_config(const struct scalar_config *config, int reconfigure
 	if ((reconfigure && config->overwrite_on_reconfigure) ||
 	    repo_config_get_string(the_repository, config->key, &value)) {
 		trace2_data_string("scalar", the_repository, config->key, "created");
-		res = repo_config_set_gently(the_repository, config->key, config->value);
+		res = set_config_with_comment(config->key, config->value);
 	} else {
 		trace2_data_string("scalar", the_repository, config->key, "exists");
 		res = 0;

@@ -197,9 +208,8 @@ static int set_recommended_config(int reconfigure)
 	if (repo_config_get_string(the_repository, "log.excludeDecoration", &value)) {
 		trace2_data_string("scalar", the_repository,
 				   "log.excludeDecoration", "created");
-		if (repo_config_set_multivar_gently(the_repository, "log.excludeDecoration",
-						    "refs/prefetch/*",
-						    CONFIG_REGEX_NONE, 0))
+		if (set_config_with_comment("log.excludeDecoration",
+					    "refs/prefetch/*"))
 			return error(_("could not configure "
 				       "log.excludeDecoration"));
 	} else {

index bd6f0c40d2..43c210a23d 100755
@@ -210,6 +210,9 @@ test_expect_success 'scalar reconfigure' '
 	GIT_TRACE2_EVENT="$(pwd)/reconfigure" scalar reconfigure -a &&
 	test_path_is_file one/src/cron.txt &&
 	test true = "$(git -C one/src config core.preloadIndex)" &&
+	test_grep "preloadIndex = true # set by scalar" one/src/.git/config &&
+	test_grep "excludeDecoration = refs/prefetch/\* # set by scalar" one/src/.git/config &&
+
 	test_subcommand git maintenance start <reconfigure &&
 	test_subcommand ! git maintenance unregister --force <reconfigure &&
 

gitgitgadget

From: Derrick Stolee <[email protected]>

The index.skipHash config option has been set to 'false' by Scalar since 4933152cbb (scalar: enable path-walk during push via config, 2025-05-16) but that commit message is trying to communicate the exact opposite:
that the 'true' value is what we want instead. This means that we've been disabling this performance benefit for Scalar repos unintentionally.

Fix this issue before we add justification for the config options set in this list.

Oddly, enabling index.skipHash causes a test issue during 'test_commit' in one of the Scalar tests when GIT_TEST_SPLIT_INDEX is enabled (as caught by the linux-test-vars build). I'm fixing the test by disabling the environment variable, but the issue should be resolved in a series focused on the split index.

---
 scalar.c          | 2 +-
 t/t9210-scalar.sh | 4 ++++
 2 files changed, 5 insertions(+), 1 deletion(-)

index be3878948d..1cb8ce3723 100644
@@ -160,7 +160,7 @@ static int set_recommended_config(int reconfigure)
 		{ "credential.validate", "false", 1 }, /* GCM4W-only */
 		{ "gc.auto", "0", 1 },
 		{ "gui.GCWarning", "false", 1 },
-		{ "index.skipHash", "false", 1 },
+		{ "index.skipHash", "true", 1 },
 		{ "index.threads", "true", 1 },
 		{ "index.version", "4", 1 },
 		{ "merge.stat", "false", 1 },

index 43c210a23d..923c243c13 100755
@@ -246,6 +246,10 @@ test_expect_success 'scalar reconfigure --all with includeIf.onbranch' '
 '
 
 test_expect_success 'scalar reconfigure --all with detached HEADs' '
+	# This test demonstrates an issue with index.skipHash=true and
+	# this test variable for the split index. Disable the test variable.
+	sane_unset GIT_TEST_SPLIT_INDEX &&
+
 	repos="two three four" &&
 	for num in $repos
 	do

gitgitgadget

From: Derrick Stolee <[email protected]>

These config values were added in the original Scalar contribution, d0feac4e8c (scalar: 'register' sets recommended config and starts maintenance, 2021-12-03), but were never fully checked for validity in the upstream Git project. At the time, Scalar was only intended for the contrib/ directory so did not have as rigorous of an investigation.

Each config option has its own justification for removal:

* core.preloadIndex: This value is true by default, now. Removing this causes some changes required to the tests that checked this config value. Use gui.gcwarning=false instead.

* core.fscache: This config does not exist in the core Git project, but is instead a config option for a Git for Windows feature.

* core.multiPackIndex: This config value is now enabled by default, so does not need to be called out specifically. It was originally included to make sure the background maintenance that created multi-pack-indexes would result in the expected performance improvements.

* credential.validate: This option is not something specific to Git but instead an older version of Git Credential Manager for Windows. That software was replaced several years ago by the cross-platform Git Credential Manger so this option is no longer needed to help users who were on that older software.

* pack.useSparse=true: This value is now Git's default as of de3a864114 (config: set pack.useSparse=true by default, 2020-03-20) so we don't need it set by Scalar.

---
 scalar.c          |  5 -----
 t/t9210-scalar.sh | 20 ++++++++++----------
 2 files changed, 10 insertions(+), 15 deletions(-)

index 1cb8ce3723..5fef9a4104 100644
@@ -135,9 +135,6 @@ static int set_recommended_config(int reconfigure)
 	struct scalar_config config[] = {
 		/* Required */
 		{ "am.keepCR", "true", 1 },
-		{ "core.FSCache", "true", 1 },
-		{ "core.multiPackIndex", "true", 1 },
-		{ "core.preloadIndex", "true", 1 },
 #ifndef WIN32
 		{ "core.untrackedCache", "true", 1 },
 #else

@@ -157,7 +154,6 @@ static int set_recommended_config(int reconfigure)
 #endif
 		{ "core.logAllRefUpdates", "true", 1 },
 		{ "credential.https://dev.azure.com.useHttpPath", "true", 1 },
-		{ "credential.validate", "false", 1 }, /* GCM4W-only */
 		{ "gc.auto", "0", 1 },
 		{ "gui.GCWarning", "false", 1 },
 		{ "index.skipHash", "true", 1 },

@@ -166,7 +162,6 @@ static int set_recommended_config(int reconfigure)
 		{ "merge.stat", "false", 1 },
 		{ "merge.renames", "true", 1 },
 		{ "pack.useBitmaps", "false", 1 },
-		{ "pack.useSparse", "true", 1 },
 		{ "receive.autoGC", "false", 1 },
 		{ "feature.manyFiles", "false", 1 },
 		{ "feature.experimental", "false", 1 },

index 923c243c13..009437a5f3 100755
@@ -202,15 +202,15 @@ test_expect_success 'scalar clone --no-... opts' '
 test_expect_success 'scalar reconfigure' '
 	git init one/src &&
 	scalar register one &&
-	git -C one/src config core.preloadIndex false &&
+	git -C one/src config unset gui.gcwarning &&
 	scalar reconfigure one &&
-	test true = "$(git -C one/src config core.preloadIndex)" &&
-	git -C one/src config core.preloadIndex false &&
+	test false = "$(git -C one/src config gui.gcwarning)" &&
+	git -C one/src config unset gui.gcwarning &&
 	rm one/src/cron.txt &&
 	GIT_TRACE2_EVENT="$(pwd)/reconfigure" scalar reconfigure -a &&
 	test_path_is_file one/src/cron.txt &&
-	test true = "$(git -C one/src config core.preloadIndex)" &&
-	test_grep "preloadIndex = true # set by scalar" one/src/.git/config &&
+	test false = "$(git -C one/src config gui.gcwarning)" &&
+	test_grep "GCWarning = false # set by scalar" one/src/.git/config &&
 	test_grep "excludeDecoration = refs/prefetch/\* # set by scalar" one/src/.git/config &&
 
 	test_subcommand git maintenance start <reconfigure &&

@@ -234,14 +234,14 @@ test_expect_success 'scalar reconfigure --all with includeIf.onbranch' '
 		git init $num/src &&
 		scalar register $num/src &&
 		git -C $num/src config includeif."onbranch:foo".path something &&
-		git -C $num/src config core.preloadIndex false || return 1
+		git -C $num/src config unset gui.gcwarning || return 1
 	done &&
 
 	scalar reconfigure --all &&
 
 	for num in $repos
 	do
-		test true = "$(git -C $num/src config core.preloadIndex)" || return 1
+		test false = "$(git -C $num/src config gui.gcwarning)" || return 1
 	done
 '
 

@@ -256,7 +256,7 @@ test_expect_success 'scalar reconfigure --all with detached HEADs' '
 		rm -rf $num/src &&
 		git init $num/src &&
 		scalar register $num/src &&
-		git -C $num/src config core.preloadIndex false &&
+		git -C $num/src config unset gui.gcwarning &&
 		test_commit -C $num/src initial &&
 		git -C $num/src switch --detach HEAD || return 1
 	done &&

@@ -265,7 +265,7 @@ test_expect_success 'scalar reconfigure --all with detached HEADs' '
 
 	for num in $repos
 	do
-		test true = "$(git -C $num/src config core.preloadIndex)" || return 1
+		test false = "$(git -C $num/src config gui.gcwarning)" || return 1
 	done
 '
 

@@ -297,7 +297,7 @@ test_expect_success 'scalar supports -c/-C' '
 	git init sub &&
 	scalar -C sub -c status.aheadBehind=bogus register &&
 	test -z "$(git -C sub config --local status.aheadBehind)" &&
-	test true = "$(git -C sub config core.preloadIndex)"
+	test false = "$(git -C sub config gui.gcwarning)"
 '
 
 test_expect_success '`scalar [...] <dir>` errors out when dir is missing' '

gitgitgadget

I was going to ask about if we could also rely on the default value of index.threads like we do here, but then went and did some reading and realised some config values, like index.recordOffsetTable, have their value set according to whether index.threads was explicitly set, so I guess there's an implicit reliance on that behaviour that we want to keep?

Thanks for the detail on all of these, very helpful

Wait. Are you saying that "index.recordOffsetTable" behaves differently based on whether "index.threads" is implicitly enabled due to the default value or explicitly enabled via the configuration? If so, that smells like a plain bug to me.

Patrick

That was my understanding from a cursory read of the results of searching for 'index.threads' in git-config:

Hm, true. At least that's a concious decision then.

The logic around this was introduced in 2a9dedef2e (index: make index.threads=true enable ieot and eoie, 2018-11-19), and the ultimate reason for it seems to be backwards compatibility:

index.threads and index.recordOffsetTable unspecified: do not write the offset table yet (to avoid alarming the user with "ignoring IEOT extension" messages when an older version of Git accesses the repository) but do make use of multiple threads to read the index if the supporting offset table is present.

Older versions of Git complained when they see unknown extensions, and we didn't want to expose users to such warnings. That makes me wonder whether it's time now to revisit that decision -- it's been 7 years since then, I guess that many clients nowadays would understand the extension.

The only (documented) downside should thus not be that important anymore, but the upside is that reading the index would be faster if we default-enable writing the extension.

Patrick

Not "Older versions".

Any version of Git should complain mandatory index extension that it does not understand. And any version of Git, even today's Git, gives a note when it ignores an optional index extension it does not understand.

I do not think 7 years matters. The only reason you might see the "ignoring" message is after using a newer version of Git that is aware of that index extension, and then reverting back to an older version. As the index file is a purely local matter, it is not very likely situation to begin with, and when it happens, the user should be made aware of it. Not understanding an optional index extension is not a breaking sin; but being in such a situation, i.e., the user is using older version of Git than they once used to use, is a note worthy vent.

So yes, even this logic was introduced last week, if the only reason is to avoid showing the note, that design decision should be revisited.

That's what I wanted to say, but I did so poorly :)

Yeah, this is of course something that can happen. I'm a bit torn on the usefulness of the warning. Sure, it warns the user about something Git does not understand. But that's exactly why we have optional extensions in the first place: so that we can introduce something without old clients understanding it.

But I can also see arguments why we want to warn. The old client would always discard the information contained in the extension, and that may be unfortunate depending on the extension itself. Ultimately, it's only the user that can decide, but they are most likely not informed enough to judge whether the warning really should be ignored or not.

Maybe there should still be a configuration that allows us to disable this warning, for example via our advice infra?

I think the number of years it has been since the new extension was introduced does matter to a certain degree: the longer we wait, the less likely it is that users will have mixed versions of Git clients where one of the versions doesn't understand the extension.

Thanks!

Patrick

Thanks for identifying this subtle behavior. I will mention it explicitly so any future change that removes that behavior could also lead to a removal of this explicit setting.

Thanks,
-Stolee

From: Derrick Stolee <[email protected]>

The config values set by Scalar went through an audit in the previous changes, so now reorganize the settings and simplify their purpose.

First, alphabetize the config options, except put the platform-specific options at the end. This groups two Windows-specific settings and only one non-Windows setting.

Also, this removes the 'overwrite_on_reconfigure' setting for many of these options. That setting made nearly all of these options "required" for scalar enlistments, restricting use for users. Instead, now nearly all options have removed this setting.

However, there is one setting that still has this, which is index.skipHash, which was previously being set to _false_ when we actually prefer the value of true. Keep the overwrite here to help Scalar users upgrade to the new version. We may remove that overwrite in the future once we belive that most of the users who have the false value have upgraded to a version that overwrites that to 'true'.

---
 scalar.c | 60 ++++++++++++++++++++++++++++----------------------------
 1 file changed, 30 insertions(+), 30 deletions(-)

index 5fef9a4104..1fbd02e03a 100644
@@ -133,10 +133,33 @@ static int have_fsmonitor_support(void)
 static int set_recommended_config(int reconfigure)
 {
 	struct scalar_config config[] = {
-		/* Required */
-		{ "am.keepCR", "true", 1 },
+		{ "am.keepCR", "true" },
+		{ "commitGraph.changedPaths", "true" },
+		{ "commitGraph.generationVersion", "1" },
+		{ "core.autoCRLF", "false" },
+		{ "core.logAllRefUpdates", "true" },
+		{ "core.safeCRLF", "false" },
+		{ "credential.https://dev.azure.com.useHttpPath", "true" },
+		{ "feature.experimental", "false" },
+		{ "feature.manyFiles", "false" },
+		{ "fetch.showForcedUpdates", "false" },
+		{ "fetch.unpackLimit", "1" },
+		{ "fetch.writeCommitGraph", "false" },
+		{ "gc.auto", "0" },
+		{ "gui.GCWarning", "false" },
+		{ "index.skipHash", "true", 1 /* Fix previous setting. */ },
+		{ "index.threads", "true"},
+		{ "index.version", "4" },
+		{ "merge.renames", "true" },
+		{ "merge.stat", "false" },
+		{ "pack.useBitmaps", "false" },
+		{ "pack.usePathWalk", "true" },
+		{ "receive.autoGC", "false" },
+		{ "status.aheadBehind", "false" },
+
+		/* platform-specific */
 #ifndef WIN32
-		{ "core.untrackedCache", "true", 1 },
+		{ "core.untrackedCache", "true" },
 #else
 		/*
 		 * Unfortunately, Scalar's Functional Tests demonstrated

@@ -150,34 +173,11 @@ static int set_recommended_config(int reconfigure)
 		 * Therefore, with a sad heart, we disable this very useful
 		 * feature on Windows.
 		 */
-		{ "core.untrackedCache", "false", 1 },
-#endif
-		{ "core.logAllRefUpdates", "true", 1 },
-		{ "credential.https://dev.azure.com.useHttpPath", "true", 1 },
-		{ "gc.auto", "0", 1 },
-		{ "gui.GCWarning", "false", 1 },
-		{ "index.skipHash", "true", 1 },
-		{ "index.threads", "true", 1 },
-		{ "index.version", "4", 1 },
-		{ "merge.stat", "false", 1 },
-		{ "merge.renames", "true", 1 },
-		{ "pack.useBitmaps", "false", 1 },
-		{ "receive.autoGC", "false", 1 },
-		{ "feature.manyFiles", "false", 1 },
-		{ "feature.experimental", "false", 1 },
-		{ "fetch.unpackLimit", "1", 1 },
-		{ "fetch.writeCommitGraph", "false", 1 },
-#ifdef WIN32
-		{ "http.sslBackend", "schannel", 1 },
+		{ "core.untrackedCache", "false" },
+
+		/* Other Windows-specific required settings: */
+		{ "http.sslBackend", "schannel" },
 #endif
-		/* Optional */
-		{ "status.aheadBehind", "false" },
-		{ "commitGraph.changedPaths", "true" },
-		{ "commitGraph.generationVersion", "1" },
-		{ "core.autoCRLF", "false" },
-		{ "core.safeCRLF", "false" },
-		{ "fetch.showForcedUpdates", "false" },
-		{ "pack.usePathWalk", "true" },
 		{ NULL, NULL },
 	};
 	int i;

gitgitgadget

From: Derrick Stolee <[email protected]>

Add user-facing documentation that justifies the values being set by 'scalar clone', 'scalar register', and 'scalar reconfigure'.

---
 Documentation/scalar.adoc | 158 ++++++++++++++++++++++++++++++++++++++
 1 file changed, 158 insertions(+)

index f81b2832f8..f1d776ae0b 100644
@@ -197,6 +197,164 @@ delete <enlistment>::
 	This subcommand lets you delete an existing Scalar enlistment from your
 	local file system, unregistering the repository.
 
+REQUIRED AND RECOMMENDED CONFIG
+-------------------------------
+
+As part of both `scalar clone` and `scalar register`, certain Git config
+values are set to optimize for large repositories or cross-platform support.
+These options are updated in new Git versions according to the best known
+advice for large repositories, and users can get the latest recommendations
+by running `scalar reconfigure [--all]`.
+
+This section lists justifications for the config values that are set in the
+latest version.
+
+am.keepCR=true::
+	This setting is important for cross-platform development across Windows
+	and non-Windows platforms and keeping carriage return (`\r`) characters
+	in certain workflows.
+
+commitGraph.changedPaths=true::
+	This setting helps the background maintenance steps that compute the
+	serialized commit-graph to also store changed-path Bloom filters. This
+	accelerates file history commands and allows users to automatically
+	benefit without running a foreground command.
+
+commitGraph.generationVersion=1::
+	While the preferred version is 2 for performance reasons, existing users
+	that had version 1 by default will need special care in upgrading to
+	version 2. This is likely to change in the future as the upgrade story
+	solidifies.
+
+core.autoCRLF=false::
+	This removes the transformation of worktree files to add CRLF line
+	endings when only LF line endings exist. This is removed for performance
+	reasons. Repositories that use tools that care about CRLF line endings
+	should commit the necessary files with those line endings instead.
+
+core.logAllRefUpdates=true::
+	This enables the reflog on all branches. While this is a performance
+	cost for large repositories, it is frequently an important data source
+	for users to get out of bad situations or to seek support from experts.
+
+core.safeCRLF=false::
+	Similar to `core.autoCRLF=false`, this disables checks around whether
+	the CRLF conversion is reversible. This is a performance improvement,
+	but can be dangerous if `core.autoCRLF` is reenabled by the user.
+
+credential.https://dev.azure.com.useHttpPath=true::
+	This setting enables the `credential.useHttpPath` feature only for web
+	URLs for Azure DevOps. This is important for users interacting with that
+	service using multiple organizations and thus multiple credential
+	tokens.
+
+feature.experimental=false::
+	This disables the "experimental" optimizations grouped under this
+	feature config. The expectation is that all valuable optimizations are
+	also set explicitly by Scalar config, and any differences are
+	intentional. Notable differences include several bitmap-related config
+	options which are disabled for client-focused Scalar repos.
+
+feature.manyFiles=false::
+	This disables the "many files" optimizations grouped under this feature
+	config. The expectation is that all valuable optimizations are also set
+	explicitly by Scalar config, and any differences are intentional.
+
+fetch.showForcedUpdates=false::
+	This disables the check at the end of `git fetch` that notifies the user
+	if the ref update was a forced update (one where the previous position
+	is not reachable from the latest position). This check can be very
+	expensive in large repositories, so is disabled and replaced with an
+	advice message. Set `advice.fetchShowForcedUpdates=false` to disable
+	this advice message.
+
+fetch.unpackLimit=1::
+	This setting prevents Git from unpacking packfiles into loose objects
+	as they are downloaded from the server. The default limit of 100 was
+	intended as a way to prevent performance issues from too many packfiles,
+	but Scalar uses background maintenance to group packfiles and cover them
+	with a multi-pack-index, removing this issue.
+
+fetch.writeCommitGraph=false::
+	This config setting was created to help users automatically update their
+	commit-graph files as they perform fetches. However, this takes time
+	from foreground fetches and pulls and Scalar uses background maintenance
+	for this function instead.
+
+gc.auto=0::
+	This disables automatic garbage collection, since Scalar uses background
+	maintenance to keep the repository data in good shape.
+
+gui.GCWarning=false::
+	Since Scalar disables garbage collection by setting `gc.auto=0`, the
+	`git-gui` tool may start to warn about this setting. Disable this
+	warning as Scalar's background maintenance configuration makes the
+	warning irrelevant.
+
+index.skipHash=true::
+	Disable computing the hash of the index contents as it is being written.
+	This assists with performance, especially for large index files.
+
+index.threads=true::
+	This tells Git to automatically detect how many threads it should use
+	when reading the index due to the default value of `core.preloadIndex`,
+	which enables parallel index reads.
+
+index.version=4::
+	This index version adds compression to the path names, reducing the size
+	of the index in a significant way for large repos. This is an important
+	performance boost.
+
+merge.renames=true::
+	When computing merges in large repos, it is particularly important to
+	detect renames to maximize the potential for a result that will validate
+	correctly. Users performing merges locally are more likely to be doing
+	so because a server-side merge (via pull request or similar) resulted in
+	conflicts. While this is the default setting, it is set specifically to
+	override a potential change to `diff.renames` which a user may set for
+	performance reasons.
+
+merge.stat=false::
+	This disables a diff output after computing a merge. This improves
+	performance of `git merge` for large repos while reducing noisy output.
+
+pack.useBitmaps=false::
+	This disables the use of `.bitmap` files attached to packfiles. Bitmap
+	files are optimized for server-side use, not client-side use. Scalar
+	disables this to avoid some performance issues that can occur if a user
+	accidentally creates `.bitmap` files.
+
+pack.usePathWalk=true::
+	This enables the `--path-walk` option to `git pack-objects` by default.
+	This can accelerate the computation and compression of packfiles created
+	by `git push` and other repack operations.
+
+receive.autoGC=false::
+	Similar to `gc.auto`, this setting is disabled in preference of
+	background maintenance.
+
+status.aheadBehind=false::
+	This disables the ahead/behind calculation that would normally happen
+	during a `git status` command. This information is frequently ignored by
+	users but can be expensive to calculate in large repos that receive
+	thousands of commits per day. The calculation is replaced with an advice
+	message that can be disabled by disabling the `advice.statusAheadBehind`
+	config.
+
+The following settings are different based on which platform is in use:
+
+core.untrackedCache=(true|false)::
+	The untracked cache feature is important for performance benefits on
+	large repositories, but has demonstrated some bugs on Windows
+	filesystems. Thus, this is set for other platforms but disabled on
+	Windows.
+
+http.sslBackend=schannel::
+	On Windows, the `openssl` backend has some issues with certain types of
+	remote providers and certificate types. Override the default setting to
+	avoid these common problems.
+
+
 SEE ALSO
 --------
 linkgit:git-clone[1], linkgit:git-maintenance[1].

gitgitgadget

Thanks! This is exactly what I was hoping for.

Would it be worth noting in scalar.c that the config options listed there are documented here, So that a dev changing the list in the source will know to also update this? I assume there's an understanding that if e.g. you update a flag you should know to also update relevant docs, but perhaps this is a bit more niche.

Yes, auto-garbage-collection is completely disabled in repositories managed by Scalar. And I guess that made sense in the past:
auto-maintenance did not know about maintenance strategies at all, and consequently it would still run git-gc(1). And that's not really compatible with the "incremental" strategy that Scalar wants to use.

I changed that in Git 2.52 so that maintenance strategies now apply to both scheduled and normal maintenance. But I was worried about backwards compatibility for the "incremental" strategy, so I made the change in a backwards compatible way so that normal maintenance still ends up using git-gc(1).

Arguably though, we can now iterate on our infrastructure: if we were to introduce an "incremental-v2" strategy we could adapt it to have proper strategies for both scheduled and normal maintenance. And if so, we can adapt Scalar in such a way that it doesn't have to disable auto maintenance anymore.

I think that would be a reasonable thing to do. Scheduled maintenance only runs once per hour, and in a high-activity repo a user may easily generate tons of objects in that hour that make the repository perform badly.

Patrick

Hi Derrick. I was planning to contribute a patch removing some config options so I'll take this opportunity to just discuss those here.

My motivation is that some of the options seem to be related to things other than performance and the list is huge, so I believe that some options don't belong to scalar.

There's no mention on which configs are required and which are recommended, and it looks like none are actually required so maybe just remove REQUIRED.

You're absolutely right. Good eye! I started this documentation before going back and removing the "required" configs.

We did find that index.threads=true gives something more when explicitly set, so there is currently value in keeping it explicit.

The CRLF configs are sometimes set globally on Windows systems, but we want the local repository to override those global settings for performance reasons.

I suppose that the default reason is that registering a repo with Scalar already enables some config in an "indirect" way and having it rely on features.manyFiles would be another layer of indirection.

The historical reason is that we initially didn't want changes to the features.* config settings to automatically be assigned to Scalar. I think this is more important on the features.experimental side, as the intention of features.manyFiles is very similar to the intention of cloning/registering with Scalar.

For now, I'm going to leave this as-is, because we have enough changes to the config settings and documentation. You can submit a change on top of this one to demonstrate the value of setting features.manyFiles=true and how that impacts the code in its new shape.

Thanks,
-Stolee

It appears you missed my question above.

Given that this is just a possibility, and also that it only affects a subset of users, wouldn't it make more sense to instead check for those settings in the global config, and only if they are enabled override their value or just show a warning?

OK. I'll do that, thanks!

I do not care too deeply as this is a file-scope static that is called only twice, but I would have preferred scalar_set_config() which is a lot more specificto the purpose of this function (and the comment "# set by scalar" is hardcoded constant in this function that its callers cannot affect, so "with_comment" is not even a statement that "the callers can add comment to their config settings") which would have taken a bit shorter line to call.

Nicely explained.

Will replace (when I land).

Thanks.

In September [1], we discussed that the Scalar config options could use some documented justification as well as some comments to the config file that they were set by Scalar. I was then immediately distracted by other work things and am finally here with a series to do just that.

[1]
https://lore.kernel.org/git/[email protected]/

I have indeed used Patrick's idea to add '# set by scalar' to each line added by Scalar, it took a little more work for all the kinds of config set. I made myself a co-author.

While working to justify each config option, I found some stale or incorrect config options. I also relaxed the override setting in most cases which gave me an opportunity to alphabetize the settings.

There was at least one case (I'm thinking of core.fscache here) where the config doesn't even exist in core Git, but instead in Git for Windows. We'll need to adjust in that fork to reinclude it in the right place.

Updates in V2
=============

* The config-setting code is simplified somewhat.
* Use 'sane_unset' instead of 'export' in test.
* Documentation is improved for typos, grammar, and clarity.

Updates in V3
=============

* Updated method names when setting recommended config.
* Updated documentation section title now that nothing is "required".
* Made distinction for index.threads=true as explicit setting.
* Added documentation for log.exludeDecoration.

Thanks, -Stolee

Derrick Stolee (5):
scalar: annotate config file with "set by scalar"
scalar: use index.skipHash=true for performance
scalar: remove stale config values
scalar: alphabetize and simplify config
scalar: document config settings

 Documentation/scalar.adoc | 164 ++++++++++++++++++++++++++++++++++++++
 scalar.c                  |  93 +++++++++++----------
 t/t9210-scalar.sh         |  25 +++---
 3 files changed, 231 insertions(+), 51 deletions(-)

base-commit: 6ab38b7e9cc7adafc304f3204616a4debd49c6e9
Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-2010%2Fderrickstolee%2Fscalar-config-v3
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-2010/derrickstolee/scalar-config-v3
Pull-Request: https://github.com/gitgitgadget/git/pull/2010

Range-diff vs v2:

1: 639ff98c44 ! 1: 7a2f919d7c scalar: annotate config file with "set by scalar"
@@ scalar.c: struct scalar_config {
int overwrite_on_reconfigure;
};

-+static int set_config_with_comment(const char *key, const char *value) +-static int set_scalar_config(const struct scalar_config *config, int reconfigure) ++static int set_scalar_config(const char *key, const char *value)
+{
+ char *file = repo_git_path(the_repository, "config");
+ int res = repo_config_set_multivar_in_file_gently(the_repository, file,
@@ scalar.c: struct scalar_config {
+ return res;
+}
+
- static int set_scalar_config(const struct scalar_config *config, int reconfigure) ++static int set_config_if_missing(const struct scalar_config *config, int reconfigure) {
char *value = NULL;
+ int res;
@@ scalar.c: static int set_scalar_config(const struct scalar_config *config, int reconfigure
if ((reconfigure && config->overwrite_on_reconfigure) ||
repo_config_get_string(the_repository, config->key, &value)) {
trace2_data_string("scalar", the_repository, config->key, "created");
- res = repo_config_set_gently(the_repository, config->key, config->value);
-+ res = set_config_with_comment(config->key, config->value);
++ res = set_scalar_config(config->key, config->value);
} else {
trace2_data_string("scalar", the_repository, config->key, "exists"); res = 0;
+@@ scalar.c: static int set_recommended_config(int reconfigure)
+ char *value;
+
+ for (i = 0; config[i].key; i++) {
+- if (set_scalar_config(config + i, reconfigure))
++ if (set_config_if_missing(config + i, reconfigure))
+ return error(_("could not configure %s=%s"),
+ config[i].key, config[i].value);
+ }
+
+ if (have_fsmonitor_support()) {
+ struct scalar_config fsmonitor = { "core.fsmonitor", "true" };
+- if (set_scalar_config(&fsmonitor, reconfigure))
++ if (set_config_if_missing(&fsmonitor, reconfigure))
+ return error(_("could not configure %s=%s"),
+ fsmonitor.key, fsmonitor.value);
+ }
@@ scalar.c: static int set_recommended_config(int reconfigure)
if (repo_config_get_string(the_repository, "log.excludeDecoration", &value)) {
trace2_data_string("scalar", the_repository,
@@ scalar.c: static int set_recommended_config(int reconfigure)
- if (repo_config_set_multivar_gently(the_repository, "log.excludeDecoration",
- "refs/prefetch/*",
- CONFIG_REGEX_NONE, 0))
-+ if (set_config_with_comment("log.excludeDecoration",
++ if (set_scalar_config("log.excludeDecoration",
+ "refs/prefetch/*"))
return error(_("could not configure "
"log.excludeDecoration"));

 2:  10e9548955 = 2:  573929ecdb scalar: use index.skipHash=true for performance
 3:  8783db6153 = 3:  85b499a616 scalar: remove stale config values
 4:  edc0254770 = 4:  c30ffc87dc scalar: alphabetize and simplify config
 5:  ac1627dbd9 ! 5:  f062b0e077 scalar: document config settings

@@ Documentation/scalar.adoc: delete <enlistment>::
This subcommand lets you delete an existing Scalar enlistment from your local file system, unregistering the repository.

-+REQUIRED AND RECOMMENDED CONFIG
-+-------------------------------
++RECOMMENDED CONFIG VALUES
++-------------------------
+
+As part of both scalar clone and scalar register, certain Git config +values are set to optimize for large repositories or cross-platform support.
@@ Documentation/scalar.adoc: delete <enlistment>::
+
+index.threads=true::
+ This tells Git to automatically detect how many threads it should use
-+ when reading the index due to the default value of core.preloadIndex,
-+ which enables parallel index reads.
++ when reading the index due the default value of core.preloadIndex,
++ which enables parallel index reads. This explicit setting also enables
++ index.recordOffsetTable=true to speed up parallel index reads.
+
+index.version=4::
+ This index version adds compression to the path names, reducing the size
+ of the index in a significant way for large repos. This is an important
+ performance boost.
+
++log.excludeDecoration=refs/prefetch/*::
++ Since Scalar enables background maintenance with the incremental
++ strategy, this setting avoids polluting git log output with refs
++ stored by the background prefetch operations.
++
+merge.renames=true::
+ When computing merges in large repos, it is particularly important to
+ detect renames to maximize the potential for a result that will validate
@@ Documentation/scalar.adoc: delete <enlistment>::
SEE ALSO
--------
linkgit:git-clone[1], linkgit:git-maintenance[1].
+
+ ## scalar.c ##
+@@ scalar.c: static int have_fsmonitor_support(void)
+
+ static int set_recommended_config(int reconfigure)
+ {
++ /*
++ * Be sure to update Documentation/scalar.adoc if you add, update,
++ * or remove any of these recommended settings.
++ */
+ struct scalar_config config[] = {
+ { "am.keepCR", "true" },
+ { "commitGraph.changedPaths", "true" },

gitgitgadget

From: Derrick Stolee <[email protected]>

A repo may have config options set by 'scalar clone' or 'scalar register' and then updated by 'scalar reconfigure'. It can be helpful to point out which of those options were set by the latest scalar recommendations.

Add "# set by scalar" to the end of each config option to assist users in identifying why these config options were set in their repo. Use a new helper method to simplify the two callsites.

---
 scalar.c          | 24 +++++++++++++++++-------
 t/t9210-scalar.sh |  3 +++
 2 files changed, 20 insertions(+), 7 deletions(-)

index f754311627..1c7bd1a8f8 100644
@@ -19,6 +19,7 @@
 #include "help.h"
 #include "setup.h"
 #include "trace2.h"
+#include "path.h"
 
 static void setup_enlistment_directory(int argc, const char **argv,
 				       const char * const *usagestr,

@@ -95,7 +96,17 @@ struct scalar_config {
 	int overwrite_on_reconfigure;
 };
 
-static int set_scalar_config(const struct scalar_config *config, int reconfigure)
+static int set_scalar_config(const char *key, const char *value)
+{
+	char *file = repo_git_path(the_repository, "config");
+	int res = repo_config_set_multivar_in_file_gently(the_repository, file,
+							  key, value, NULL,
+							  " # set by scalar", 0);
+	free(file);
+	return res;
+}
+
+static int set_config_if_missing(const struct scalar_config *config, int reconfigure)
 {
 	char *value = NULL;
 	int res;

@@ -103,7 +114,7 @@ static int set_scalar_config(const struct scalar_config *config, int reconfigure
 	if ((reconfigure && config->overwrite_on_reconfigure) ||
 	    repo_config_get_string(the_repository, config->key, &value)) {
 		trace2_data_string("scalar", the_repository, config->key, "created");
-		res = repo_config_set_gently(the_repository, config->key, config->value);
+		res = set_scalar_config(config->key, config->value);
 	} else {
 		trace2_data_string("scalar", the_repository, config->key, "exists");
 		res = 0;

@@ -178,14 +189,14 @@ static int set_recommended_config(int reconfigure)
 	char *value;
 
 	for (i = 0; config[i].key; i++) {
-		if (set_scalar_config(config + i, reconfigure))
+		if (set_config_if_missing(config + i, reconfigure))
 			return error(_("could not configure %s=%s"),
 				     config[i].key, config[i].value);
 	}
 
 	if (have_fsmonitor_support()) {
 		struct scalar_config fsmonitor = { "core.fsmonitor", "true" };
-		if (set_scalar_config(&fsmonitor, reconfigure))
+		if (set_config_if_missing(&fsmonitor, reconfigure))
 			return error(_("could not configure %s=%s"),
 				     fsmonitor.key, fsmonitor.value);
 	}

@@ -197,9 +208,8 @@ static int set_recommended_config(int reconfigure)
 	if (repo_config_get_string(the_repository, "log.excludeDecoration", &value)) {
 		trace2_data_string("scalar", the_repository,
 				   "log.excludeDecoration", "created");
-		if (repo_config_set_multivar_gently(the_repository, "log.excludeDecoration",
-						    "refs/prefetch/*",
-						    CONFIG_REGEX_NONE, 0))
+		if (set_scalar_config("log.excludeDecoration",
+					    "refs/prefetch/*"))
 			return error(_("could not configure "
 				       "log.excludeDecoration"));
 	} else {

index bd6f0c40d2..43c210a23d 100755
@@ -210,6 +210,9 @@ test_expect_success 'scalar reconfigure' '
 	GIT_TRACE2_EVENT="$(pwd)/reconfigure" scalar reconfigure -a &&
 	test_path_is_file one/src/cron.txt &&
 	test true = "$(git -C one/src config core.preloadIndex)" &&
+	test_grep "preloadIndex = true # set by scalar" one/src/.git/config &&
+	test_grep "excludeDecoration = refs/prefetch/\* # set by scalar" one/src/.git/config &&
+
 	test_subcommand git maintenance start <reconfigure &&
 	test_subcommand ! git maintenance unregister --force <reconfigure &&
 

gitgitgadget

From: Derrick Stolee <[email protected]>

The index.skipHash config option has been set to 'false' by Scalar since 4933152cbb (scalar: enable path-walk during push via config, 2025-05-16) but that commit message is trying to communicate the exact opposite:
that the 'true' value is what we want instead. This means that we've been disabling this performance benefit for Scalar repos unintentionally.

Fix this issue before we add justification for the config options set in this list.

Oddly, enabling index.skipHash causes a test issue during 'test_commit' in one of the Scalar tests when GIT_TEST_SPLIT_INDEX is enabled (as caught by the linux-test-vars build). I'm fixing the test by disabling the environment variable, but the issue should be resolved in a series focused on the split index.

---
 scalar.c          | 2 +-
 t/t9210-scalar.sh | 4 ++++
 2 files changed, 5 insertions(+), 1 deletion(-)

index 1c7bd1a8f8..55b8542770 100644
@@ -160,7 +160,7 @@ static int set_recommended_config(int reconfigure)
 		{ "credential.validate", "false", 1 }, /* GCM4W-only */
 		{ "gc.auto", "0", 1 },
 		{ "gui.GCWarning", "false", 1 },
-		{ "index.skipHash", "false", 1 },
+		{ "index.skipHash", "true", 1 },
 		{ "index.threads", "true", 1 },
 		{ "index.version", "4", 1 },
 		{ "merge.stat", "false", 1 },

index 43c210a23d..923c243c13 100755
@@ -246,6 +246,10 @@ test_expect_success 'scalar reconfigure --all with includeIf.onbranch' '
 '
 
 test_expect_success 'scalar reconfigure --all with detached HEADs' '
+	# This test demonstrates an issue with index.skipHash=true and
+	# this test variable for the split index. Disable the test variable.
+	sane_unset GIT_TEST_SPLIT_INDEX &&
+
 	repos="two three four" &&
 	for num in $repos
 	do

gitgitgadget

From: Derrick Stolee <[email protected]>

These config values were added in the original Scalar contribution, d0feac4e8c (scalar: 'register' sets recommended config and starts maintenance, 2021-12-03), but were never fully checked for validity in the upstream Git project. At the time, Scalar was only intended for the contrib/ directory so did not have as rigorous of an investigation.

Each config option has its own justification for removal:

* core.preloadIndex: This value is true by default, now. Removing this causes some changes required to the tests that checked this config value. Use gui.gcwarning=false instead.

* core.fscache: This config does not exist in the core Git project, but is instead a config option for a Git for Windows feature.

* core.multiPackIndex: This config value is now enabled by default, so does not need to be called out specifically. It was originally included to make sure the background maintenance that created multi-pack-indexes would result in the expected performance improvements.

* credential.validate: This option is not something specific to Git but instead an older version of Git Credential Manager for Windows. That software was replaced several years ago by the cross-platform Git Credential Manger so this option is no longer needed to help users who were on that older software.

* pack.useSparse=true: This value is now Git's default as of de3a864114 (config: set pack.useSparse=true by default, 2020-03-20) so we don't need it set by Scalar.

---
 scalar.c          |  5 -----
 t/t9210-scalar.sh | 20 ++++++++++----------
 2 files changed, 10 insertions(+), 15 deletions(-)

index 55b8542770..aeebea41fa 100644
@@ -135,9 +135,6 @@ static int set_recommended_config(int reconfigure)
 	struct scalar_config config[] = {
 		/* Required */
 		{ "am.keepCR", "true", 1 },
-		{ "core.FSCache", "true", 1 },
-		{ "core.multiPackIndex", "true", 1 },
-		{ "core.preloadIndex", "true", 1 },
 #ifndef WIN32
 		{ "core.untrackedCache", "true", 1 },
 #else

@@ -157,7 +154,6 @@ static int set_recommended_config(int reconfigure)
 #endif
 		{ "core.logAllRefUpdates", "true", 1 },
 		{ "credential.https://dev.azure.com.useHttpPath", "true", 1 },
-		{ "credential.validate", "false", 1 }, /* GCM4W-only */
 		{ "gc.auto", "0", 1 },
 		{ "gui.GCWarning", "false", 1 },
 		{ "index.skipHash", "true", 1 },

@@ -166,7 +162,6 @@ static int set_recommended_config(int reconfigure)
 		{ "merge.stat", "false", 1 },
 		{ "merge.renames", "true", 1 },
 		{ "pack.useBitmaps", "false", 1 },
-		{ "pack.useSparse", "true", 1 },
 		{ "receive.autoGC", "false", 1 },
 		{ "feature.manyFiles", "false", 1 },
 		{ "feature.experimental", "false", 1 },

index 923c243c13..009437a5f3 100755
@@ -202,15 +202,15 @@ test_expect_success 'scalar clone --no-... opts' '
 test_expect_success 'scalar reconfigure' '
 	git init one/src &&
 	scalar register one &&
-	git -C one/src config core.preloadIndex false &&
+	git -C one/src config unset gui.gcwarning &&
 	scalar reconfigure one &&
-	test true = "$(git -C one/src config core.preloadIndex)" &&
-	git -C one/src config core.preloadIndex false &&
+	test false = "$(git -C one/src config gui.gcwarning)" &&
+	git -C one/src config unset gui.gcwarning &&
 	rm one/src/cron.txt &&
 	GIT_TRACE2_EVENT="$(pwd)/reconfigure" scalar reconfigure -a &&
 	test_path_is_file one/src/cron.txt &&
-	test true = "$(git -C one/src config core.preloadIndex)" &&
-	test_grep "preloadIndex = true # set by scalar" one/src/.git/config &&
+	test false = "$(git -C one/src config gui.gcwarning)" &&
+	test_grep "GCWarning = false # set by scalar" one/src/.git/config &&
 	test_grep "excludeDecoration = refs/prefetch/\* # set by scalar" one/src/.git/config &&
 
 	test_subcommand git maintenance start <reconfigure &&

@@ -234,14 +234,14 @@ test_expect_success 'scalar reconfigure --all with includeIf.onbranch' '
 		git init $num/src &&
 		scalar register $num/src &&
 		git -C $num/src config includeif."onbranch:foo".path something &&
-		git -C $num/src config core.preloadIndex false || return 1
+		git -C $num/src config unset gui.gcwarning || return 1
 	done &&
 
 	scalar reconfigure --all &&
 
 	for num in $repos
 	do
-		test true = "$(git -C $num/src config core.preloadIndex)" || return 1
+		test false = "$(git -C $num/src config gui.gcwarning)" || return 1
 	done
 '
 

@@ -256,7 +256,7 @@ test_expect_success 'scalar reconfigure --all with detached HEADs' '
 		rm -rf $num/src &&
 		git init $num/src &&
 		scalar register $num/src &&
-		git -C $num/src config core.preloadIndex false &&
+		git -C $num/src config unset gui.gcwarning &&
 		test_commit -C $num/src initial &&
 		git -C $num/src switch --detach HEAD || return 1
 	done &&

@@ -265,7 +265,7 @@ test_expect_success 'scalar reconfigure --all with detached HEADs' '
 
 	for num in $repos
 	do
-		test true = "$(git -C $num/src config core.preloadIndex)" || return 1
+		test false = "$(git -C $num/src config gui.gcwarning)" || return 1
 	done
 '
 

@@ -297,7 +297,7 @@ test_expect_success 'scalar supports -c/-C' '
 	git init sub &&
 	scalar -C sub -c status.aheadBehind=bogus register &&
 	test -z "$(git -C sub config --local status.aheadBehind)" &&
-	test true = "$(git -C sub config core.preloadIndex)"
+	test false = "$(git -C sub config gui.gcwarning)"
 '
 
 test_expect_success '`scalar [...] <dir>` errors out when dir is missing' '

gitgitgadget

From: Derrick Stolee <[email protected]>

The config values set by Scalar went through an audit in the previous changes, so now reorganize the settings and simplify their purpose.

First, alphabetize the config options, except put the platform-specific options at the end. This groups two Windows-specific settings and only one non-Windows setting.

Also, this removes the 'overwrite_on_reconfigure' setting for many of these options. That setting made nearly all of these options "required" for scalar enlistments, restricting use for users. Instead, now nearly all options have removed this setting.

However, there is one setting that still has this, which is index.skipHash, which was previously being set to _false_ when we actually prefer the value of true. Keep the overwrite here to help Scalar users upgrade to the new version. We may remove that overwrite in the future once we belive that most of the users who have the false value have upgraded to a version that overwrites that to 'true'.

---
 scalar.c | 60 ++++++++++++++++++++++++++++----------------------------
 1 file changed, 30 insertions(+), 30 deletions(-)

index aeebea41fa..3b25fd3f35 100644
@@ -133,10 +133,33 @@ static int have_fsmonitor_support(void)
 static int set_recommended_config(int reconfigure)
 {
 	struct scalar_config config[] = {
-		/* Required */
-		{ "am.keepCR", "true", 1 },
+		{ "am.keepCR", "true" },
+		{ "commitGraph.changedPaths", "true" },
+		{ "commitGraph.generationVersion", "1" },
+		{ "core.autoCRLF", "false" },
+		{ "core.logAllRefUpdates", "true" },
+		{ "core.safeCRLF", "false" },
+		{ "credential.https://dev.azure.com.useHttpPath", "true" },
+		{ "feature.experimental", "false" },
+		{ "feature.manyFiles", "false" },
+		{ "fetch.showForcedUpdates", "false" },
+		{ "fetch.unpackLimit", "1" },
+		{ "fetch.writeCommitGraph", "false" },
+		{ "gc.auto", "0" },
+		{ "gui.GCWarning", "false" },
+		{ "index.skipHash", "true", 1 /* Fix previous setting. */ },
+		{ "index.threads", "true"},
+		{ "index.version", "4" },
+		{ "merge.renames", "true" },
+		{ "merge.stat", "false" },
+		{ "pack.useBitmaps", "false" },
+		{ "pack.usePathWalk", "true" },
+		{ "receive.autoGC", "false" },
+		{ "status.aheadBehind", "false" },
+
+		/* platform-specific */
 #ifndef WIN32
-		{ "core.untrackedCache", "true", 1 },
+		{ "core.untrackedCache", "true" },
 #else
 		/*
 		 * Unfortunately, Scalar's Functional Tests demonstrated

@@ -150,34 +173,11 @@ static int set_recommended_config(int reconfigure)
 		 * Therefore, with a sad heart, we disable this very useful
 		 * feature on Windows.
 		 */
-		{ "core.untrackedCache", "false", 1 },
-#endif
-		{ "core.logAllRefUpdates", "true", 1 },
-		{ "credential.https://dev.azure.com.useHttpPath", "true", 1 },
-		{ "gc.auto", "0", 1 },
-		{ "gui.GCWarning", "false", 1 },
-		{ "index.skipHash", "true", 1 },
-		{ "index.threads", "true", 1 },
-		{ "index.version", "4", 1 },
-		{ "merge.stat", "false", 1 },
-		{ "merge.renames", "true", 1 },
-		{ "pack.useBitmaps", "false", 1 },
-		{ "receive.autoGC", "false", 1 },
-		{ "feature.manyFiles", "false", 1 },
-		{ "feature.experimental", "false", 1 },
-		{ "fetch.unpackLimit", "1", 1 },
-		{ "fetch.writeCommitGraph", "false", 1 },
-#ifdef WIN32
-		{ "http.sslBackend", "schannel", 1 },
+		{ "core.untrackedCache", "false" },
+
+		/* Other Windows-specific required settings: */
+		{ "http.sslBackend", "schannel" },
 #endif
-		/* Optional */
-		{ "status.aheadBehind", "false" },
-		{ "commitGraph.changedPaths", "true" },
-		{ "commitGraph.generationVersion", "1" },
-		{ "core.autoCRLF", "false" },
-		{ "core.safeCRLF", "false" },
-		{ "fetch.showForcedUpdates", "false" },
-		{ "pack.usePathWalk", "true" },
 		{ NULL, NULL },
 	};
 	int i;

gitgitgadget

From: Derrick Stolee <[email protected]>

Add user-facing documentation that justifies the values being set by 'scalar clone', 'scalar register', and 'scalar reconfigure'.

---
 Documentation/scalar.adoc | 164 ++++++++++++++++++++++++++++++++++++++
 scalar.c                  |   4 +
 2 files changed, 168 insertions(+)

index f81b2832f8..ee9fc0ac0c 100644
@@ -197,6 +197,170 @@ delete <enlistment>::
 	This subcommand lets you delete an existing Scalar enlistment from your
 	local file system, unregistering the repository.
 
+RECOMMENDED CONFIG VALUES
+-------------------------
+
+As part of both `scalar clone` and `scalar register`, certain Git config
+values are set to optimize for large repositories or cross-platform support.
+These options are updated in new Git versions according to the best known
+advice for large repositories, and users can get the latest recommendations
+by running `scalar reconfigure [--all]`.
+
+This section lists justifications for the config values that are set in the
+latest version.
+
+am.keepCR=true::
+	This setting is important for cross-platform development across Windows
+	and non-Windows platforms and keeping carriage return (`\r`) characters
+	in certain workflows.
+
+commitGraph.changedPaths=true::
+	This setting helps the background maintenance steps that compute the
+	serialized commit-graph to also store changed-path Bloom filters. This
+	accelerates file history commands and allows users to automatically
+	benefit without running a foreground command.
+
+commitGraph.generationVersion=1::
+	While the preferred version is 2 for performance reasons, existing users
+	that had version 1 by default will need special care in upgrading to
+	version 2. This is likely to change in the future as the upgrade story
+	solidifies.
+
+core.autoCRLF=false::
+	This removes the transformation of worktree files to add CRLF line
+	endings when only LF line endings exist. This is removed for performance
+	reasons. Repositories that use tools that care about CRLF line endings
+	should commit the necessary files with those line endings instead.
+
+core.logAllRefUpdates=true::
+	This enables the reflog on all branches. While this is a performance
+	cost for large repositories, it is frequently an important data source
+	for users to get out of bad situations or to seek support from experts.
+
+core.safeCRLF=false::
+	Similar to `core.autoCRLF=false`, this disables checks around whether
+	the CRLF conversion is reversible. This is a performance improvement,
+	but can be dangerous if `core.autoCRLF` is reenabled by the user.
+
+credential.https://dev.azure.com.useHttpPath=true::
+	This setting enables the `credential.useHttpPath` feature only for web
+	URLs for Azure DevOps. This is important for users interacting with that
+	service using multiple organizations and thus multiple credential
+	tokens.
+
+feature.experimental=false::
+	This disables the "experimental" optimizations grouped under this
+	feature config. The expectation is that all valuable optimizations are
+	also set explicitly by Scalar config, and any differences are
+	intentional. Notable differences include several bitmap-related config
+	options which are disabled for client-focused Scalar repos.
+
+feature.manyFiles=false::
+	This disables the "many files" optimizations grouped under this feature
+	config. The expectation is that all valuable optimizations are also set
+	explicitly by Scalar config, and any differences are intentional.
+
+fetch.showForcedUpdates=false::
+	This disables the check at the end of `git fetch` that notifies the user
+	if the ref update was a forced update (one where the previous position
+	is not reachable from the latest position). This check can be very
+	expensive in large repositories, so is disabled and replaced with an
+	advice message. Set `advice.fetchShowForcedUpdates=false` to disable
+	this advice message.
+
+fetch.unpackLimit=1::
+	This setting prevents Git from unpacking packfiles into loose objects
+	as they are downloaded from the server. The default limit of 100 was
+	intended as a way to prevent performance issues from too many packfiles,
+	but Scalar uses background maintenance to group packfiles and cover them
+	with a multi-pack-index, removing this issue.
+
+fetch.writeCommitGraph=false::
+	This config setting was created to help users automatically update their
+	commit-graph files as they perform fetches. However, this takes time
+	from foreground fetches and pulls and Scalar uses background maintenance
+	for this function instead.
+
+gc.auto=0::
+	This disables automatic garbage collection, since Scalar uses background
+	maintenance to keep the repository data in good shape.
+
+gui.GCWarning=false::
+	Since Scalar disables garbage collection by setting `gc.auto=0`, the
+	`git-gui` tool may start to warn about this setting. Disable this
+	warning as Scalar's background maintenance configuration makes the
+	warning irrelevant.
+
+index.skipHash=true::
+	Disable computing the hash of the index contents as it is being written.
+	This assists with performance, especially for large index files.
+
+index.threads=true::
+	This tells Git to automatically detect how many threads it should use
+	when reading the index due the default value of	`core.preloadIndex`,
+	which enables parallel index reads. This explicit setting also enables
+	`index.recordOffsetTable=true` to speed up parallel index reads.
+
+index.version=4::
+	This index version adds compression to the path names, reducing the size
+	of the index in a significant way for large repos. This is an important
+	performance boost.
+
+log.excludeDecoration=refs/prefetch/*::
+	Since Scalar enables background maintenance with the `incremental`
+	strategy, this setting avoids polluting `git log` output with refs
+	stored by the background prefetch operations.
+
+merge.renames=true::
+	When computing merges in large repos, it is particularly important to
+	detect renames to maximize the potential for a result that will validate
+	correctly. Users performing merges locally are more likely to be doing
+	so because a server-side merge (via pull request or similar) resulted in
+	conflicts. While this is the default setting, it is set specifically to
+	override a potential change to `diff.renames` which a user may set for
+	performance reasons.
+
+merge.stat=false::
+	This disables a diff output after computing a merge. This improves
+	performance of `git merge` for large repos while reducing noisy output.
+
+pack.useBitmaps=false::
+	This disables the use of `.bitmap` files attached to packfiles. Bitmap
+	files are optimized for server-side use, not client-side use. Scalar
+	disables this to avoid some performance issues that can occur if a user
+	accidentally creates `.bitmap` files.
+
+pack.usePathWalk=true::
+	This enables the `--path-walk` option to `git pack-objects` by default.
+	This can accelerate the computation and compression of packfiles created
+	by `git push` and other repack operations.
+
+receive.autoGC=false::
+	Similar to `gc.auto`, this setting is disabled in preference of
+	background maintenance.
+
+status.aheadBehind=false::
+	This disables the ahead/behind calculation that would normally happen
+	during a `git status` command. This information is frequently ignored by
+	users but can be expensive to calculate in large repos that receive
+	thousands of commits per day. The calculation is replaced with an advice
+	message that can be disabled by disabling the `advice.statusAheadBehind`
+	config.
+
+The following settings are different based on which platform is in use:
+
+core.untrackedCache=(true|false)::
+	The untracked cache feature is important for performance benefits on
+	large repositories, but has demonstrated some bugs on Windows
+	filesystems. Thus, this is set for other platforms but disabled on
+	Windows.
+
+http.sslBackend=schannel::
+	On Windows, the `openssl` backend has some issues with certain types of
+	remote providers and certificate types. Override the default setting to
+	avoid these common problems.
+
+
 SEE ALSO
 --------
 linkgit:git-clone[1], linkgit:git-maintenance[1].

index 3b25fd3f35..21ab1dba89 100644
@@ -132,6 +132,10 @@ static int have_fsmonitor_support(void)
 
 static int set_recommended_config(int reconfigure)
 {
+	/*
+	 * Be sure to update Documentation/scalar.adoc if you add, update,
+	 * or remove any of these recommended settings.
+	 */
 	struct scalar_config config[] = {
 		{ "am.keepCR", "true" },
 		{ "commitGraph.changedPaths", "true" },

gitgitgadget

Looking good.

I understood the previous "due to" version (from the middle school English class I took eons ago), but not the updated one.

Other than that, there weren't anything iffy in the new version. Will replace. Thanks.

Indeed, I don't know how I dropped that "to".

Thanks,
-Stolee

Will locally amend, then. Also on the same line, "value of core.preloadIndex" has a tab (instead of a space) after of, which looked funny.

I wonder if there a need for a new whitespace error class that spots a HT at the 7th column and has non-whitespace characters on both sides. #leftoverbits