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

["Derrick Stolee via GitGitGadget" <gitgitgadget@gmail.com>]

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