[PATCH 0/5] Audit and document Scalar config

[PATCH 0/5] Audit and document Scalar config

[Derrick Stolee via GitGitGadget]

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/ffa61066-7004-48dd-9096-85b305373bc7@gmail.com/

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

[PATCH 1/5] scalar: annotate config file with "set by scalar"

[Derrick Stolee via GitGitGadget]

From: Derrick Stolee <stolee@gmail.com>

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.

Co-authored-by: Patrick Steinhardt <ps@pks.im>
Signed-off-by: Patrick Steinhardt <ps@pks.im>
Signed-off-by: Derrick Stolee <stolee@gmail.com>
---
 scalar.c          | 16 +++++++++++++---
 t/t9210-scalar.sh |  3 +++
 2 files changed, 16 insertions(+), 3 deletions(-)

diff --git a/scalar.c b/scalar.c
index f754311627..431b631585 100644
--- a/scalar.c
+++ b/scalar.c
@@ -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");
diff --git a/t/t9210-scalar.sh b/t/t9210-scalar.sh
index bd6f0c40d2..43c210a23d 100755
--- a/t/t9210-scalar.sh
+++ b/t/t9210-scalar.sh
@@ -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

Re: [PATCH 1/5] scalar: annotate config file with "set by scalar"

[Junio C Hamano]

"Derrick Stolee via GitGitGadget" <gitgitgadget@gmail.com> writes:

> 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.

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.

> diff --git a/t/t9210-scalar.sh b/t/t9210-scalar.sh
> index bd6f0c40d2..43c210a23d 100755
> --- a/t/t9210-scalar.sh
> +++ b/t/t9210-scalar.sh
> @@ -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 &&

Looks good.

Re: [PATCH 1/5] scalar: annotate config file with "set by scalar"

[Patrick Steinhardt]

On Wed, Nov 26, 2025 at 03:55:10PM -0800, Junio C Hamano wrote:
> "Derrick Stolee via GitGitGadget" <gitgitgadget@gmail.com> writes:
> 
> > 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.
> 
> 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

[PATCH 2/5] scalar: use index.skipHash=true for performance

[Derrick Stolee via GitGitGadget]

From: Derrick Stolee <stolee@gmail.com>

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.

Signed-off-by: Derrick Stolee <stolee@gmail.com>
---
 scalar.c          | 2 +-
 t/t9210-scalar.sh | 5 +++++
 2 files changed, 6 insertions(+), 1 deletion(-)

diff --git a/scalar.c b/scalar.c
index 431b631585..75fd292be7 100644
--- a/scalar.c
+++ b/scalar.c
@@ -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 },
diff --git a/t/t9210-scalar.sh b/t/t9210-scalar.sh
index 43c210a23d..91d5964b73 100755
--- a/t/t9210-scalar.sh
+++ b/t/t9210-scalar.sh
@@ -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

Re: [PATCH 2/5] scalar: use index.skipHash=true for performance

[Junio C Hamano]

"Derrick Stolee via GitGitGadget" <gitgitgadget@gmail.com> writes:

> diff --git a/t/t9210-scalar.sh b/t/t9210-scalar.sh
> index 43c210a23d..91d5964b73 100755
> --- a/t/t9210-scalar.sh
> +++ b/t/t9210-scalar.sh
> @@ -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 &&

Interesting.  I would have expected to see a simple "sane_unset",
instead of exporting an empty setting explicitly.

>  	repos="two three four" &&
>  	for num in $repos
>  	do

Re: [PATCH 2/5] scalar: use index.skipHash=true for performance

[Derrick Stolee]

On 11/26/2025 6:57 PM, Junio C Hamano wrote:
> "Derrick Stolee via GitGitGadget" <gitgitgadget@gmail.com> writes:
> 
>> diff --git a/t/t9210-scalar.sh b/t/t9210-scalar.sh
>> index 43c210a23d..91d5964b73 100755
>> --- a/t/t9210-scalar.sh
>> +++ b/t/t9210-scalar.sh
>> @@ -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 &&
> 
> Interesting.  I would have expected to see a simple "sane_unset",
> instead of exporting an empty setting explicitly.

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

Thanks,
-Stolee

[PATCH 3/5] scalar: remove stale config values

[Derrick Stolee via GitGitGadget]

From: Derrick Stolee <stolee@gmail.com>

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.

Signed-off-by: Derrick Stolee <stolee@gmail.com>
---
 scalar.c          |  5 -----
 t/t9210-scalar.sh | 20 ++++++++++----------
 2 files changed, 10 insertions(+), 15 deletions(-)

diff --git a/scalar.c b/scalar.c
index 75fd292be7..fb595fab3a 100644
--- a/scalar.c
+++ b/scalar.c
@@ -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 },
diff --git a/t/t9210-scalar.sh b/t/t9210-scalar.sh
index 91d5964b73..63224d2062 100755
--- a/t/t9210-scalar.sh
+++ b/t/t9210-scalar.sh
@@ -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

Re: [PATCH 3/5] scalar: remove stale config values

[Junio C Hamano]

"Derrick Stolee via GitGitGadget" <gitgitgadget@gmail.com> writes:

> From: Derrick Stolee <stolee@gmail.com>
>
> 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.

Thanks for a conprehensive list.  Very well described.

[PATCH 4/5] scalar: alphabetize and simplify config

[Derrick Stolee via GitGitGadget]

From: Derrick Stolee <stolee@gmail.com>

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'.

Signed-off-by: Derrick Stolee <stolee@gmail.com>
---
 scalar.c | 60 ++++++++++++++++++++++++++++----------------------------
 1 file changed, 30 insertions(+), 30 deletions(-)

diff --git a/scalar.c b/scalar.c
index fb595fab3a..d73f9d85c7 100644
--- a/scalar.c
+++ b/scalar.c
@@ -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

Re: [PATCH 4/5] scalar: alphabetize and simplify config

[Patrick Steinhardt]

On Wed, Nov 26, 2025 at 10:18:35PM +0000, Derrick Stolee via GitGitGadget wrote:
> From: Derrick Stolee <stolee@gmail.com>
> 
> 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.

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.

> 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'.

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

Thanks!

Patrick

Re: [PATCH 4/5] scalar: alphabetize and simplify config

[Derrick Stolee]

On 12/1/25 3:55 AM, Patrick Steinhardt wrote:
> On Wed, Nov 26, 2025 at 10:18:35PM +0000, Derrick Stolee via GitGitGadget wrote:
>> From: Derrick Stolee <stolee@gmail.com>
>>
>> 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.
> 
> 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.

Yes, that is correct.

> 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.

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.

>> 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'.
> 
> Makes sense. This has likely been a bug, and we now want to rectify that
> bug.

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

Thanks,
-Stolee

[PATCH 5/5] scalar: document config settings

[Derrick Stolee via GitGitGadget]

From: Derrick Stolee <stolee@gmail.com>

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

Signed-off-by: Derrick Stolee <stolee@gmail.com>
---
 Documentation/scalar.adoc | 158 ++++++++++++++++++++++++++++++++++++++
 1 file changed, 158 insertions(+)

diff --git a/Documentation/scalar.adoc b/Documentation/scalar.adoc
index f81b2832f8..b34af225e6 100644
--- a/Documentation/scalar.adoc
+++ b/Documentation/scalar.adoc
@@ -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

Re: [PATCH 5/5] scalar: document config settings

[Junio C Hamano]

"Derrick Stolee via GitGitGadget" <gitgitgadget@gmail.com> writes:

> +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.

"as the upgrade story solidifies"?

> +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.

"update their 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.

Is "due to the `core.preloadIndex=true` setting" part of this
sentence still relevant?


Other than that, superbly written.  Thanks, will queue.

Re: [PATCH 5/5] scalar: document config settings

[Derrick Stolee]

On 11/26/2025 7:09 PM, Junio C Hamano wrote:
> "Derrick Stolee via GitGitGadget" <gitgitgadget@gmail.com> writes:
> 
>> +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.
> 
> "as the upgrade story solidifies"?

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

>> +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.
> 
> "update their files".

Yes. thanks.

>> +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.
> 
> Is "due to the `core.preloadIndex=true` setting" part of this
> sentence still relevant?

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

> Other than that, superbly written.  Thanks, will queue.

Thanks,
-Stolee

Re: [PATCH 5/5] scalar: document config settings

[Patrick Steinhardt]

On Wed, Nov 26, 2025 at 10:18:36PM +0000, Derrick Stolee via GitGitGadget wrote:
> diff --git a/Documentation/scalar.adoc b/Documentation/scalar.adoc
> index f81b2832f8..b34af225e6 100644
> --- a/Documentation/scalar.adoc
> +++ b/Documentation/scalar.adoc
> @@ -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.

Is this something we also want to promote to "default" eventually? The
downside of course is that maintenance takes a bit longer, but given
that it runs in the background anyway this shouldn't really impact our
users all that much.

> +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.

Is that still the case? We _did_ have some bugs in the upgrade path in
the past, but I thought it got all sorted out by now?

[snip]
> +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.

The second sentence here reads as if "fetch.unpackLimit=1" was the
feature you are talking about, which led to some puzzlement at first.
But what you are talking about is the _default_ unpack limit of 100.
Maybe something like this reads better?

    This setting prevents Git from unpacking packfiles into loose objects
    as they are downloaded from the server. The default limit of 100
    objects 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.

Patrick

Re: [PATCH 5/5] scalar: document config settings

[Derrick Stolee]

On 12/1/25 3:55 AM, Patrick Steinhardt wrote:
> On Wed, Nov 26, 2025 at 10:18:36PM +0000, Derrick Stolee via GitGitGadget wrote:
>> diff --git a/Documentation/scalar.adoc b/Documentation/scalar.adoc
>> index f81b2832f8..b34af225e6 100644
>> --- a/Documentation/scalar.adoc
>> +++ b/Documentation/scalar.adoc
>> @@ -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.
> 
> Is this something we also want to promote to "default" eventually? The
> downside of course is that maintenance takes a bit longer, but given
> that it runs in the background anyway this shouldn't really impact our
> users all that much.

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.

>> +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.
> 
> Is that still the case? We _did_ have some bugs in the upgrade path in
> the past, but I thought it got all sorted out by now?

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.

> [snip]
>> +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.
> 
> The second sentence here reads as if "fetch.unpackLimit=1" was the
> feature you are talking about, which led to some puzzlement at first.
> But what you are talking about is the _default_ unpack limit of 100.
> Maybe something like this reads better?
> 
>      This setting prevents Git from unpacking packfiles into loose objects
>      as they are downloaded from the server. The default limit of 100
>      objects 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.

Good catch. Thanks!

-Stolee

Re: [PATCH 0/5] Audit and document Scalar config

[Johannes Schindelin]

Hi Stolee,

On Wed, 26 Nov 2025, Derrick Stolee via GitGitGadget wrote:

> 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.

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.

> 
> [1]
> https://lore.kernel.org/git/ffa61066-7004-48dd-9096-85b305373bc7@gmail.com/
> 
> 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 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.

> 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.

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

> 
> 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
> 

[PATCH v2 0/5] Audit and document Scalar config

[Derrick Stolee via GitGitGadget]

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/ffa61066-7004-48dd-9096-85b305373bc7@gmail.com/

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 <ps@pks.im>
          Signed-off-by: Patrick Steinhardt <ps@pks.im>
     @@ 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 <gitster@pobox.com>
     +    Helped-by: Patrick Steinhardt <ps@pks.im>
          Signed-off-by: Derrick Stolee <stolee@gmail.com>
      
       ## 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

[PATCH v2 1/5] scalar: annotate config file with "set by scalar"

[Derrick Stolee via GitGitGadget]

From: Derrick Stolee <stolee@gmail.com>

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.

Co-authored-by: Patrick Steinhardt <ps@pks.im>
Signed-off-by: Patrick Steinhardt <ps@pks.im>
Signed-off-by: Derrick Stolee <stolee@gmail.com>
---
 scalar.c          | 18 ++++++++++++++----
 t/t9210-scalar.sh |  3 +++
 2 files changed, 17 insertions(+), 4 deletions(-)

diff --git a/scalar.c b/scalar.c
index f754311627..be3878948d 100644
--- a/scalar.c
+++ b/scalar.c
@@ -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 {
diff --git a/t/t9210-scalar.sh b/t/t9210-scalar.sh
index bd6f0c40d2..43c210a23d 100755
--- a/t/t9210-scalar.sh
+++ b/t/t9210-scalar.sh
@@ -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

Re: [PATCH v2 1/5] scalar: annotate config file with "set by scalar"

[Patrick Steinhardt]

On Mon, Dec 01, 2025 at 04:50:43PM +0000, Derrick Stolee via GitGitGadget wrote:
> diff --git a/t/t9210-scalar.sh b/t/t9210-scalar.sh
> index bd6f0c40d2..43c210a23d 100755
> --- a/t/t9210-scalar.sh
> +++ b/t/t9210-scalar.sh
> @@ -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 &&

We _could_ make this a bit more solid by adding a test that:

  1. Initializes a new repository.

  2. Saves the configuration.

  3. Performs `scalar reconfigure`.

  4. Asserts that all new non-section-header lines in the configuration
     have a trailing "#set by scalar" comment.

This would ensure that there is no callsite we forgot to add the new
annotation to, and that there are new future callsites where somebody
isn't aware of the comments.

I don't insist on such a test though, so please feel free to ignore this
suggestion.

Patrick

[PATCH v2 2/5] scalar: use index.skipHash=true for performance

[Derrick Stolee via GitGitGadget]

From: Derrick Stolee <stolee@gmail.com>

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.

Signed-off-by: Derrick Stolee <stolee@gmail.com>
---
 scalar.c          | 2 +-
 t/t9210-scalar.sh | 4 ++++
 2 files changed, 5 insertions(+), 1 deletion(-)

diff --git a/scalar.c b/scalar.c
index be3878948d..1cb8ce3723 100644
--- a/scalar.c
+++ b/scalar.c
@@ -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 },
diff --git a/t/t9210-scalar.sh b/t/t9210-scalar.sh
index 43c210a23d..923c243c13 100755
--- a/t/t9210-scalar.sh
+++ b/t/t9210-scalar.sh
@@ -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

[PATCH v2 3/5] scalar: remove stale config values

[Derrick Stolee via GitGitGadget]

From: Derrick Stolee <stolee@gmail.com>

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.

Signed-off-by: Derrick Stolee <stolee@gmail.com>
---
 scalar.c          |  5 -----
 t/t9210-scalar.sh | 20 ++++++++++----------
 2 files changed, 10 insertions(+), 15 deletions(-)

diff --git a/scalar.c b/scalar.c
index 1cb8ce3723..5fef9a4104 100644
--- a/scalar.c
+++ b/scalar.c
@@ -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 },
diff --git a/t/t9210-scalar.sh b/t/t9210-scalar.sh
index 923c243c13..009437a5f3 100755
--- a/t/t9210-scalar.sh
+++ b/t/t9210-scalar.sh
@@ -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

Re: [PATCH v2 3/5] scalar: remove stale config values

[Matthew Hughes]

On Mon, Dec 01, 2025 at 04:50:45PM +0000, Derrick Stolee via GitGitGadget wrote:
> * 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.

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?

> * 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.

Thanks for the detail on all of these, very helpful

Re: [PATCH v2 3/5] scalar: remove stale config values

[Patrick Steinhardt]

On Mon, Dec 01, 2025 at 05:46:46PM +0000, Matthew Hughes wrote:
> On Mon, Dec 01, 2025 at 04:50:45PM +0000, Derrick Stolee via GitGitGadget wrote:
> > * 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.
> 
> 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?

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

Re: [PATCH v2 3/5] scalar: remove stale config values

[Matthew Hughes]

On Tue, Dec 02, 2025 at 08:53:45AM +0100, Patrick Steinhardt wrote:
> 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?

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

> index.recordEndOfIndexEntries
> ...
> Defaults to true if index.threads has been explicitly enabled, false
> otherwise

Re: [PATCH v2 3/5] scalar: remove stale config values

[Patrick Steinhardt]

On Tue, Dec 02, 2025 at 07:04:24PM +0000, Matthew Hughes wrote:
> On Tue, Dec 02, 2025 at 08:53:45AM +0100, Patrick Steinhardt wrote:
> > 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?
> 
> That was my understanding from a cursory read of the results of searching for
> 'index.threads' in git-config:
> 
> > index.recordEndOfIndexEntries
> > ...
> > Defaults to true if index.threads has been explicitly enabled, false
> > otherwise

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

Re: [PATCH v2 3/5] scalar: remove stale config values

[Junio C Hamano]

Patrick Steinhardt <ps@pks.im> writes:

> 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.

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.

> 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. 

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.

> 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

Re: [PATCH v2 3/5] scalar: remove stale config values

[Patrick Steinhardt]

On Sun, Dec 07, 2025 at 09:34:05AM +0900, Junio C Hamano wrote:
> Patrick Steinhardt <ps@pks.im> writes:
> 
> > 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.
> 
> 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.

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

> > 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. 
> 
> 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.

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?

> 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.

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

Re: [PATCH v2 3/5] scalar: remove stale config values

[Derrick Stolee]

On 12/1/2025 12:46 PM, Matthew Hughes wrote:
> On Mon, Dec 01, 2025 at 04:50:45PM +0000, Derrick Stolee via GitGitGadget wrote:
>> * 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.
> 
> 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 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

[PATCH v2 4/5] scalar: alphabetize and simplify config

[Derrick Stolee via GitGitGadget]

From: Derrick Stolee <stolee@gmail.com>

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'.

Signed-off-by: Derrick Stolee <stolee@gmail.com>
---
 scalar.c | 60 ++++++++++++++++++++++++++++----------------------------
 1 file changed, 30 insertions(+), 30 deletions(-)

diff --git a/scalar.c b/scalar.c
index 5fef9a4104..1fbd02e03a 100644
--- a/scalar.c
+++ b/scalar.c
@@ -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

[PATCH v2 5/5] scalar: document config settings

[Derrick Stolee via GitGitGadget]

From: Derrick Stolee <stolee@gmail.com>

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

Helped-by: Junio C Hamano <gitster@pobox.com>
Helped-by: Patrick Steinhardt <ps@pks.im>
Signed-off-by: Derrick Stolee <stolee@gmail.com>
---
 Documentation/scalar.adoc | 158 ++++++++++++++++++++++++++++++++++++++
 1 file changed, 158 insertions(+)

diff --git a/Documentation/scalar.adoc b/Documentation/scalar.adoc
index f81b2832f8..f1d776ae0b 100644
--- a/Documentation/scalar.adoc
+++ b/Documentation/scalar.adoc
@@ -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

Re: [PATCH v2 5/5] scalar: document config settings

[Matthew Hughes]

On Mon, Dec 01, 2025 at 04:50:47PM +0000, Derrick Stolee via GitGitGadget wrote:
> Add user-facing documentation that justifies the values being set by
> 'scalar clone', 'scalar register', and 'scalar reconfigure'.

Thanks! This is exactly what I was hoping for.

> +REQUIRED AND RECOMMENDED CONFIG
> +-------------------------------

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.

> +gc.auto=0::
> +	This disables automatic garbage collection, since Scalar uses background
> +	maintenance to keep the repository data in good shape.

Checking my understanding: this means there will be _no_ automatic GC in a
scalar repo? Since scalar calls 'maintenance register' which means
maintenance.strategy will be set to 'incremental' which won't schedule any gc
runs

Re: [PATCH v2 5/5] scalar: document config settings

[Patrick Steinhardt]

On Mon, Dec 01, 2025 at 05:58:06PM +0000, Matthew Hughes wrote:
> On Mon, Dec 01, 2025 at 04:50:47PM +0000, Derrick Stolee via GitGitGadget wrote:
> > Add user-facing documentation that justifies the values being set by
> > 'scalar clone', 'scalar register', and 'scalar reconfigure'.
> 
> Thanks! This is exactly what I was hoping for.
> 
> > +REQUIRED AND RECOMMENDED CONFIG
> > +-------------------------------
> 
> 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.
> 
> > +gc.auto=0::
> > +	This disables automatic garbage collection, since Scalar uses background
> > +	maintenance to keep the repository data in good shape.
> 
> Checking my understanding: this means there will be _no_ automatic GC in a
> scalar repo? Since scalar calls 'maintenance register' which means
> maintenance.strategy will be set to 'incremental' which won't schedule any gc
> runs

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

Re: [PATCH v2 5/5] scalar: document config settings

[Henrique Ferreiro]

On 12/1/25 5:50 PM, Derrick Stolee via GitGitGadget wrote:
> From: Derrick Stolee <stolee@gmail.com>
> 
> Add user-facing documentation that justifies the values being set by
> 'scalar clone', 'scalar register', and 'scalar reconfigure'.

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.

> +REQUIRED AND RECOMMENDED CONFIG
> +-------------------------------

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.

> +am.keepCR=true::
 > +core.logAllRefUpdates=true::
 > +credential.https://dev.azure.com.useHttpPath=true::
 > +http.sslBackend=schannel::

These options are not related to performance. Why not keeping them out
of scalar?

> +core.autoCRLF=false::
> +core.safeCRLF=false::
 > +index.threads=true::

These options just duplicate the default settings.

> +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.

I disagree with this reasoning. This thread was actually brought to my
attention when working on setting manyFiles to true in scalar:
https://github.com/git/git/pull/2125.

Do you foresee any features that would apply to scalar but not to
manyFiles? I'd even say that some scalar options could be moved to
manyFiles instead, so that people that don't use scalar can benefit too.

Cheers,
Henrique.

Re: [PATCH v2 5/5] scalar: document config settings

[Derrick Stolee]

On 12/11/2025 9:20 AM, Henrique Ferreiro wrote:
> On 12/1/25 5:50 PM, Derrick Stolee via GitGitGadget wrote:
>> From: Derrick Stolee <stolee@gmail.com>
>>
>> Add user-facing documentation that justifies the values being set by
>> 'scalar clone', 'scalar register', and 'scalar reconfigure'.
> 
> 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.
> 
>> +REQUIRED AND RECOMMENDED CONFIG
>> +-------------------------------
> 
> 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.
 
>> +am.keepCR=true::
>> +core.logAllRefUpdates=true::
>> +credential.https://dev.azure.com.useHttpPath=true::
>> +http.sslBackend=schannel::
> 
> These options are not related to performance. Why not keeping them out
> of scalar?
> 
>> +core.autoCRLF=false::
>> +core.safeCRLF=false::
>> +index.threads=true::
> 
> These options just duplicate the default settings.

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.
 
>> +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.
> 
> I disagree with this reasoning. This thread was actually brought to my
> attention when working on setting manyFiles to true in scalar:
> https://github.com/git/git/pull/2125.
> 
> Do you foresee any features that would apply to scalar but not to
> manyFiles? I'd even say that some scalar options could be moved to
> manyFiles instead, so that people that don't use scalar can benefit too.

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

Re: [PATCH v2 5/5] scalar: document config settings

[Henrique Ferreiro]

On 12/12/25 3:06 PM, Derrick Stolee wrote:
> On 12/11/2025 9:20 AM, Henrique Ferreiro wrote:
>> On 12/1/25 5:50 PM, Derrick Stolee via GitGitGadget wrote:
>>> From: Derrick Stolee <stolee@gmail.com>
>>>
>>> Add user-facing documentation that justifies the values being set by
>>> 'scalar clone', 'scalar register', and 'scalar reconfigure'.
>>
>> 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.
>>
>>> +REQUIRED AND RECOMMENDED CONFIG
>>> +-------------------------------
>>
>> 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.
>   
>>> +am.keepCR=true::
>>> +core.logAllRefUpdates=true::
>>> +credential.https://dev.azure.com.useHttpPath=true::
>>> +http.sslBackend=schannel::
>>
>> These options are not related to performance. Why not keeping them out
>> of scalar?

It appears you missed my question above.

>>> +core.autoCRLF=false::
>>> +core.safeCRLF=false::
>>> +index.threads=true::
>>
>> These options just duplicate the default settings.
> 
> 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.

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?

>>> +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.
>>
>> I disagree with this reasoning. This thread was actually brought to my
>> attention when working on setting manyFiles to true in scalar:
>> https://github.com/git/git/pull/2125.
>>
>> Do you foresee any features that would apply to scalar but not to
>> manyFiles? I'd even say that some scalar options could be moved to
>> manyFiles instead, so that people that don't use scalar can benefit too.
> 
> 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.

OK. I'll do that, thanks!

> Thanks,
> -Stolee
> 

Re: [PATCH v2 0/5] Audit and document Scalar config

[Junio C Hamano]

"Derrick Stolee via GitGitGadget" <gitgitgadget@gmail.com> writes:

>      -@@ 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)

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.

>       +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.

Nicely explained.

Will replace (when I land).

Thanks.

[PATCH v3 0/5] Audit and document Scalar config

[Derrick Stolee via GitGitGadget]

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/ffa61066-7004-48dd-9096-85b305373bc7@gmail.com/

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

[PATCH v3 1/5] scalar: annotate config file with "set by scalar"

[Derrick Stolee via GitGitGadget]

From: Derrick Stolee <stolee@gmail.com>

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.

Co-authored-by: Patrick Steinhardt <ps@pks.im>
Signed-off-by: Patrick Steinhardt <ps@pks.im>
Signed-off-by: Derrick Stolee <stolee@gmail.com>
---
 scalar.c          | 24 +++++++++++++++++-------
 t/t9210-scalar.sh |  3 +++
 2 files changed, 20 insertions(+), 7 deletions(-)

diff --git a/scalar.c b/scalar.c
index f754311627..1c7bd1a8f8 100644
--- a/scalar.c
+++ b/scalar.c
@@ -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 {
diff --git a/t/t9210-scalar.sh b/t/t9210-scalar.sh
index bd6f0c40d2..43c210a23d 100755
--- a/t/t9210-scalar.sh
+++ b/t/t9210-scalar.sh
@@ -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

[PATCH v3 2/5] scalar: use index.skipHash=true for performance

[Derrick Stolee via GitGitGadget]

From: Derrick Stolee <stolee@gmail.com>

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.

Signed-off-by: Derrick Stolee <stolee@gmail.com>
---
 scalar.c          | 2 +-
 t/t9210-scalar.sh | 4 ++++
 2 files changed, 5 insertions(+), 1 deletion(-)

diff --git a/scalar.c b/scalar.c
index 1c7bd1a8f8..55b8542770 100644
--- a/scalar.c
+++ b/scalar.c
@@ -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 },
diff --git a/t/t9210-scalar.sh b/t/t9210-scalar.sh
index 43c210a23d..923c243c13 100755
--- a/t/t9210-scalar.sh
+++ b/t/t9210-scalar.sh
@@ -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

[PATCH v3 3/5] scalar: remove stale config values

[Derrick Stolee via GitGitGadget]

From: Derrick Stolee <stolee@gmail.com>

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.

Signed-off-by: Derrick Stolee <stolee@gmail.com>
---
 scalar.c          |  5 -----
 t/t9210-scalar.sh | 20 ++++++++++----------
 2 files changed, 10 insertions(+), 15 deletions(-)

diff --git a/scalar.c b/scalar.c
index 55b8542770..aeebea41fa 100644
--- a/scalar.c
+++ b/scalar.c
@@ -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 },
diff --git a/t/t9210-scalar.sh b/t/t9210-scalar.sh
index 923c243c13..009437a5f3 100755
--- a/t/t9210-scalar.sh
+++ b/t/t9210-scalar.sh
@@ -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

[PATCH v3 4/5] scalar: alphabetize and simplify config

[Derrick Stolee via GitGitGadget]

From: Derrick Stolee <stolee@gmail.com>

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'.

Signed-off-by: Derrick Stolee <stolee@gmail.com>
---
 scalar.c | 60 ++++++++++++++++++++++++++++----------------------------
 1 file changed, 30 insertions(+), 30 deletions(-)

diff --git a/scalar.c b/scalar.c
index aeebea41fa..3b25fd3f35 100644
--- a/scalar.c
+++ b/scalar.c
@@ -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

[PATCH v3 5/5] scalar: document config settings

[Derrick Stolee via GitGitGadget]

From: Derrick Stolee <stolee@gmail.com>

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

Helped-by: Junio C Hamano <gitster@pobox.com>
Helped-by: Patrick Steinhardt <ps@pks.im>
Signed-off-by: Derrick Stolee <stolee@gmail.com>
---
 Documentation/scalar.adoc | 164 ++++++++++++++++++++++++++++++++++++++
 scalar.c                  |   4 +
 2 files changed, 168 insertions(+)

diff --git a/Documentation/scalar.adoc b/Documentation/scalar.adoc
index f81b2832f8..ee9fc0ac0c 100644
--- a/Documentation/scalar.adoc
+++ b/Documentation/scalar.adoc
@@ -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].
diff --git a/scalar.c b/scalar.c
index 3b25fd3f35..21ab1dba89 100644
--- a/scalar.c
+++ b/scalar.c
@@ -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

Re: [PATCH v3 5/5] scalar: document config settings

[Henrique Ferreiro]

On 12/12/25 4:15 PM, Derrick Stolee via GitGitGadget wrote:
> From: Derrick Stolee <stolee@gmail.com>
> 
> Add user-facing documentation that justifies the values being set by
> 'scalar clone', 'scalar register', and 'scalar reconfigure'.
> 
> Helped-by: Junio C Hamano <gitster@pobox.com>
> Helped-by: Patrick Steinhardt <ps@pks.im>
> Signed-off-by: Derrick Stolee <stolee@gmail.com>
> ---
>   Documentation/scalar.adoc | 164 ++++++++++++++++++++++++++++++++++++++
>   scalar.c                  |   4 +
>   2 files changed, 168 insertions(+)
> 
> diff --git a/Documentation/scalar.adoc b/Documentation/scalar.adoc
> index f81b2832f8..ee9fc0ac0c 100644
> --- a/Documentation/scalar.adoc
> +++ b/Documentation/scalar.adoc
> @@ -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.

I missed this in a previous email. Given the above reasoning, wouldn't
it make more sense to check for a previous value of 1, instead of
forcing 1 also on new enlistments?

> +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.

A quick look at preload-index.c doesn't show any references to reading 
index.threads but I might have missed it. On the other hand, it looks 
like index.recordEndOfIndexEntries is also affected by index.threads, so 
it might be worth explicitly mentioning it.

> +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].
> diff --git a/scalar.c b/scalar.c
> index 3b25fd3f35..21ab1dba89 100644
> --- a/scalar.c
> +++ b/scalar.c
> @@ -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" },

Re: [PATCH v3 0/5] Audit and document Scalar config

[Junio C Hamano]

"Derrick Stolee via GitGitGadget" <gitgitgadget@gmail.com> writes:

> 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.

Looking good.

>       +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.

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.

Re: [PATCH v3 0/5] Audit and document Scalar config

[Derrick Stolee]

On 12/12/2025 6:49 PM, Junio C Hamano wrote:
> "Derrick Stolee via GitGitGadget" <gitgitgadget@gmail.com> writes:
> 
>> 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.
> 
> Looking good.
> 
>>       +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.
> 
> 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

Re: [PATCH v3 0/5] Audit and document Scalar config

[Junio C Hamano]

Derrick Stolee <stolee@gmail.com> writes:

>>>      -+	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.
>> 
>> 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".

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