git@vger.kernel.org mailing list mirror (one of many)
 help / color / mirror / code / Atom feed
From: "Derrick Stolee via GitGitGadget" <gitgitgadget@gmail.com>
To: git@vger.kernel.org
Cc: Johannes.Schindelin@gmx.de, peff@peff.net, jnareb@gmail.com,
	pclouds@gmail.com, carenas@gmail.com, avarab@gmail.com,
	Junio C Hamano <gitster@pobox.com>
Subject: [PATCH v3 0/5] Create 'feature.*' config area and some centralized config parsing
Date: Tue, 30 Jul 2019 12:35:25 -0700 (PDT)	[thread overview]
Message-ID: <pull.292.v3.git.gitgitgadget@gmail.com> (raw)
In-Reply-To: <pull.292.v2.git.gitgitgadget@gmail.com>

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

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

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

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

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

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

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

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

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

Thanks, -Stolee

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

 * Rearranged how we are checking for the feature.* settings, so if one
   layer turns the setting on and a later layer turns it off, we do not
   adjust the defaults.
   
   
 * Switched to using enums for the non-boolean config options.
   
   
 * Placed the repo_settings struct directly in the repository struct.
   
   
 * All struct members are initialized to -1 using memset.
   
   
 * The config changes around directory rename detection is greatly
   simplified.
   
   

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

 * Remove repo-settings.h by placing the struct and function definitions in
   repository.h.
   
   
 * Move the initialization int into "struct repo_settings".
   
   

[1] https://public-inbox.org/git/pull.254.git.gitgitgadget@gmail.com/

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

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


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

Range-diff vs v2:

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

-- 
gitgitgadget

  parent reply	other threads:[~2019-07-30 19:35 UTC|newest]

Thread overview: 61+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-07-22 17:54 [PATCH 0/5] Create 'feature.*' config area and some centralized config parsing Derrick Stolee via GitGitGadget
2019-07-22 17:54 ` [PATCH 1/5] repo-settings: consolidate some config settings Derrick Stolee via GitGitGadget
2019-07-23 13:12   ` Johannes Schindelin
2019-07-22 17:54 ` [PATCH 2/5] repo-settings: add feature.manyCommits setting Derrick Stolee via GitGitGadget
2019-07-23 14:53   ` Johannes Schindelin
2019-07-24 10:41     ` Derrick Stolee
2019-07-22 17:54 ` [PATCH 3/5] repo-settings: parse core.untrackedCache Derrick Stolee via GitGitGadget
2019-07-23 15:04   ` Johannes Schindelin
2019-07-24 19:27     ` Derrick Stolee
2019-07-22 17:54 ` [PATCH 4/5] repo-settings: create feature.manyFiles setting Derrick Stolee via GitGitGadget
2019-07-22 17:54 ` [PATCH 5/5] repo-settings: create feature.experimental setting Derrick Stolee via GitGitGadget
2019-07-23 15:20   ` Johannes Schindelin
2019-07-25  1:47     ` Derrick Stolee
2019-07-25  2:23 ` [PATCH v2 0/5] Create 'feature.*' config area and some centralized config parsing Derrick Stolee via GitGitGadget
2019-07-25  2:23   ` [PATCH v2 2/5] repo-settings: add feature.manyCommits setting Derrick Stolee via GitGitGadget
2019-07-25  2:23   ` [PATCH v2 1/5] repo-settings: consolidate some config settings Derrick Stolee via GitGitGadget
2019-07-25  9:30     ` Johannes Schindelin
2019-07-25  2:23   ` [PATCH v2 3/5] repo-settings: parse core.untrackedCache Derrick Stolee via GitGitGadget
2019-07-25  9:36     ` Johannes Schindelin
2019-07-25  2:23   ` [PATCH v2 4/5] repo-settings: create feature.manyFiles setting Derrick Stolee via GitGitGadget
2019-07-25  2:23   ` [PATCH v2 5/5] repo-settings: create feature.experimental setting Derrick Stolee via GitGitGadget
2019-07-25  9:40   ` [PATCH v2 0/5] Create 'feature.*' config area and some centralized config parsing Johannes Schindelin
2019-07-30 19:35   ` Derrick Stolee via GitGitGadget [this message]
2019-07-30 19:35     ` [PATCH v3 1/5] repo-settings: consolidate some config settings Derrick Stolee via GitGitGadget
2019-07-30 20:47       ` Junio C Hamano
2019-07-30 19:35     ` [PATCH v3 2/5] repo-settings: add feature.manyCommits setting Derrick Stolee via GitGitGadget
2019-07-30 20:57       ` Junio C Hamano
2019-07-31 13:17         ` Johannes Schindelin
2019-07-31 15:48           ` Junio C Hamano
2019-07-31 15:01       ` Ævar Arnfjörð Bjarmason
2019-08-01 18:27         ` Derrick Stolee
2019-07-30 19:35     ` [PATCH v3 3/5] repo-settings: parse core.untrackedCache Derrick Stolee via GitGitGadget
2019-07-30 19:35     ` [PATCH v3 4/5] repo-settings: create feature.manyFiles setting Derrick Stolee via GitGitGadget
2019-07-30 19:35     ` [PATCH v3 5/5] repo-settings: create feature.experimental setting Derrick Stolee via GitGitGadget
2019-08-08 18:34       ` Elijah Newren
2019-08-08 18:48         ` Derrick Stolee
2019-08-08 18:59         ` Junio C Hamano
2019-08-08 19:12           ` Derrick Stolee
2019-08-08 20:31             ` Elijah Newren
2019-08-08 20:49               ` Derrick Stolee
2019-08-08 19:19           ` Elijah Newren
2019-08-08 20:07             ` Junio C Hamano
2019-08-08 20:46               ` Derrick Stolee
2019-08-13 18:37     ` [PATCH v4 0/6] Create 'feature.*' config area and some centralized config parsing Derrick Stolee via GitGitGadget
2019-08-13 18:37       ` [PATCH v4 1/6] repo-settings: consolidate some config settings Derrick Stolee via GitGitGadget
2019-08-13 18:37       ` [PATCH v4 3/6] commit-graph: turn on commit-graph by default Derrick Stolee via GitGitGadget
2021-09-20  0:42         ` Junio C Hamano
2021-09-20  1:22           ` Ævar Arnfjörð Bjarmason
2021-09-20  1:25           ` brian m. carlson
2021-09-20 12:53             ` Phillip Wood
2021-09-20 13:30               ` Ævar Arnfjörð Bjarmason
2021-09-20 18:00                 ` Phillip Wood
2021-09-20 19:18                 ` Jeff King
2019-08-13 18:37       ` [PATCH v4 2/6] t6501: use 'git gc' in quiet mode Derrick Stolee via GitGitGadget
2019-08-13 18:37       ` [PATCH v4 4/6] repo-settings: parse core.untrackedCache Derrick Stolee via GitGitGadget
2019-08-13 18:37       ` [PATCH v4 5/6] repo-settings: create feature.manyFiles setting Derrick Stolee via GitGitGadget
2019-08-13 18:37       ` [PATCH v4 6/6] repo-settings: create feature.experimental setting Derrick Stolee via GitGitGadget
2019-08-13 21:04       ` [PATCH v4 0/6] Create 'feature.*' config area and some centralized config parsing Junio C Hamano
2019-08-13 21:08         ` Junio C Hamano
2019-08-14 10:32           ` Derrick Stolee
2019-08-14 10:38             ` Derrick Stolee

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

  List information: http://vger.kernel.org/majordomo-info.html

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=pull.292.v3.git.gitgitgadget@gmail.com \
    --to=gitgitgadget@gmail.com \
    --cc=Johannes.Schindelin@gmx.de \
    --cc=avarab@gmail.com \
    --cc=carenas@gmail.com \
    --cc=git@vger.kernel.org \
    --cc=gitster@pobox.com \
    --cc=jnareb@gmail.com \
    --cc=pclouds@gmail.com \
    --cc=peff@peff.net \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
Code repositories for project(s) associated with this public inbox

	https://80x24.org/mirrors/git.git

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).