[PATCH 0/5] Introduce configs for default repo format

[PATCH 0/5] Introduce configs for default repo format

[Patrick Steinhardt]

Hi,

to set up the default object and ref storage formats, users have to set
up some environment variables. This is somewhat unwieldy and not really
in line with how a user typically expects to configure Git, namely by
using the config system. It makes it harder than necessary to globally
default to the different formats and requires the user to munge with
files like `.profile` to persist that setting. Needless to say, this is
a bit of an awkward user experience.

This patch series thus introduces two new configs to set the default
object hash and ref storage format for newly created repositories. Like
this, folks can simply use the global- or system-level config to adapt
to their needs. This also has the advantage of giving them the ability
to adapt the default formats via guarded includes, such that e.g. repos
in some filesystem hierarchy use format A, whereas others use format B.

This comes from a discussion with Sebastian (Cc'd) at the Git User Group
in Berlin yesterday.

Thanks!

Patrick

Patrick Steinhardt (5):
  t0001: exercise initialization with ref formats more thoroughly
  t0001: delete repositories when object format tests finish
  setup: merge configuration of repository formats
  setup: make object format configurable via config
  setup: make ref storage format configurable via config

 Documentation/config/init.txt |  10 +++
 setup.c                       | 101 ++++++++++++++++-------
 t/t0001-init.sh               | 145 +++++++++++++++++++++++++++++++---
 3 files changed, 216 insertions(+), 40 deletions(-)

-- 
2.46.0.46.g406f326d27.dirty

[PATCH 1/5] t0001: exercise initialization with ref formats more thoroughly

[Patrick Steinhardt]

While our object format tests for git-init(1) exercise tests with all
known formats in t0001, the tests for the ref format don't. This leads
to some missing test coverage for interesting cases, like whether or not
a non-default ref storage format causes us to bump the repository format
version. We also don't test for the precedence of the `--ref-format=`
and the `GIT_DEFAULT_REF_FORMAT=` environment variable.

Extend the test suite to cover more scenarios related to the ref format.

Signed-off-by: Patrick Steinhardt <ps@pks.im>
---
 t/t0001-init.sh | 48 +++++++++++++++++++++++++++++++++++-------------
 1 file changed, 35 insertions(+), 13 deletions(-)

diff --git a/t/t0001-init.sh b/t/t0001-init.sh
index 49e9bf77c6..2093f5c1ee 100755
--- a/t/t0001-init.sh
+++ b/t/t0001-init.sh
@@ -558,15 +558,6 @@ test_expect_success DEFAULT_REPO_FORMAT 'extensions.refStorage with unknown back
 	grep "invalid value for ${SQ}extensions.refstorage${SQ}: ${SQ}garbage${SQ}" err
 '
 
-test_expect_success DEFAULT_REPO_FORMAT 'init with GIT_DEFAULT_REF_FORMAT=files' '
-	test_when_finished "rm -rf refformat" &&
-	GIT_DEFAULT_REF_FORMAT=files git init refformat &&
-	echo 0 >expect &&
-	git -C refformat config core.repositoryformatversion >actual &&
-	test_cmp expect actual &&
-	test_must_fail git -C refformat config extensions.refstorage
-'
-
 test_expect_success 'init with GIT_DEFAULT_REF_FORMAT=garbage' '
 	test_when_finished "rm -rf refformat" &&
 	cat >expect <<-EOF &&
@@ -576,15 +567,46 @@ test_expect_success 'init with GIT_DEFAULT_REF_FORMAT=garbage' '
 	test_cmp expect err
 '
 
-test_expect_success 'init with --ref-format=files' '
+backends="files reftable"
+for format in $backends
+do
+	test_expect_success DEFAULT_REPO_FORMAT "init with GIT_DEFAULT_REF_FORMAT=$format" '
+		test_when_finished "rm -rf refformat" &&
+		GIT_DEFAULT_REF_FORMAT=$format git init refformat &&
+
+		if test $format = files
+		then
+			test_must_fail git -C refformat config extensions.refstorage &&
+			echo 0 >expect
+		else
+			git -C refformat config extensions.refstorage &&
+			echo 1 >expect
+		fi &&
+		git -C refformat config core.repositoryformatversion >actual &&
+		test_cmp expect actual &&
+
+		echo $format >expect &&
+		git -C refformat rev-parse --show-ref-format >actual &&
+		test_cmp expect actual
+	'
+
+	test_expect_success "init with --ref-format=$format" '
+		test_when_finished "rm -rf refformat" &&
+		git init --ref-format=$format refformat &&
+		echo $format >expect &&
+		git -C refformat rev-parse --show-ref-format >actual &&
+		test_cmp expect actual
+	'
+done
+
+test_expect_success "--ref-format= overrides GIT_DEFAULT_REF_FORMAT" '
 	test_when_finished "rm -rf refformat" &&
-	git init --ref-format=files refformat &&
-	echo files >expect &&
+	GIT_DEFAULT_REF_FORMAT=files git init --ref-format=reftable refformat &&
+	echo reftable >expect &&
 	git -C refformat rev-parse --show-ref-format >actual &&
 	test_cmp expect actual
 '
 
-backends="files reftable"
 for from_format in $backends
 do
 	test_expect_success "re-init with same format ($from_format)" '
-- 
2.46.0.46.g406f326d27.dirty

[PATCH 2/5] t0001: delete repositories when object format tests finish

[Patrick Steinhardt]

The object format tests create one-shot repositories that are only used
by the respective test, but never delete them. This makes it hard to
pick a proper repository name in subsequent tests, as more and more
names are taken already.

Delete these repositories via `test_when_finished`.

Signed-off-by: Patrick Steinhardt <ps@pks.im>
---
 t/t0001-init.sh | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/t/t0001-init.sh b/t/t0001-init.sh
index 2093f5c1ee..795408e16c 100755
--- a/t/t0001-init.sh
+++ b/t/t0001-init.sh
@@ -500,6 +500,7 @@ test_expect_success 're-init from a linked worktree' '
 '
 
 test_expect_success 'init honors GIT_DEFAULT_HASH' '
+	test_when_finished "rm -rf sha1 sha256" &&
 	GIT_DEFAULT_HASH=sha1 git init sha1 &&
 	git -C sha1 rev-parse --show-object-format >actual &&
 	echo sha1 >expected &&
@@ -511,6 +512,7 @@ test_expect_success 'init honors GIT_DEFAULT_HASH' '
 '
 
 test_expect_success 'init honors --object-format' '
+	test_when_finished "rm -rf explicit-sha1 explicit-sha256" &&
 	git init --object-format=sha1 explicit-sha1 &&
 	git -C explicit-sha1 rev-parse --show-object-format >actual &&
 	echo sha1 >expected &&
@@ -522,6 +524,7 @@ test_expect_success 'init honors --object-format' '
 '
 
 test_expect_success 'extensions.objectFormat is not allowed with repo version 0' '
+	test_when_finished "rm -rf explicit-v0" &&
 	git init --object-format=sha256 explicit-v0 &&
 	git -C explicit-v0 config core.repositoryformatversion 0 &&
 	test_must_fail git -C explicit-v0 rev-parse --show-object-format
-- 
2.46.0.46.g406f326d27.dirty

[PATCH 3/5] setup: merge configuration of repository formats

[Patrick Steinhardt]

The configuration of repository formats is split up across two functions
`validate_hash_algorithm()` and `validate_ref_storage_format()`. This is
fine as-is, but we are about to extend the logic to also read default
values from the config. With the logic split across two functions, we
would either have to pass in additional parameters read from the config,
or read the config multiple times. Both of these options feel a bit
unwieldy.

Merge the code into a new a new function `repository_format_configure()`
that is responsible for configuring the whole repository's format. Like
this, we can easily read the config in a single place, only.

Signed-off-by: Patrick Steinhardt <ps@pks.im>
---
 setup.c | 47 ++++++++++++++++++++---------------------------
 1 file changed, 20 insertions(+), 27 deletions(-)

diff --git a/setup.c b/setup.c
index d458edcc02..5dfcdc99dd 100644
--- a/setup.c
+++ b/setup.c
@@ -2284,14 +2284,17 @@ static void separate_git_dir(const char *git_dir, const char *git_link)
 	write_file(git_link, "gitdir: %s", git_dir);
 }
 
-static void validate_hash_algorithm(struct repository_format *repo_fmt, int hash)
+static void repository_format_configure(struct repository_format *repo_fmt,
+					int hash, enum ref_storage_format ref_format)
 {
-	const char *env = getenv(GIT_DEFAULT_HASH_ENVIRONMENT);
+	const char *env;
+
 	/*
 	 * If we already have an initialized repo, don't allow the user to
 	 * specify a different algorithm, as that could cause corruption.
 	 * Otherwise, if the user has specified one on the command line, use it.
 	 */
+	env = getenv(GIT_DEFAULT_HASH_ENVIRONMENT);
 	if (repo_fmt->version >= 0 && hash != GIT_HASH_UNKNOWN && hash != repo_fmt->hash_algo)
 		die(_("attempt to reinitialize repository with different hash"));
 	else if (hash != GIT_HASH_UNKNOWN)
@@ -2302,25 +2305,22 @@ static void validate_hash_algorithm(struct repository_format *repo_fmt, int hash
 			die(_("unknown hash algorithm '%s'"), env);
 		repo_fmt->hash_algo = env_algo;
 	}
-}
-
-static void validate_ref_storage_format(struct repository_format *repo_fmt,
-					enum ref_storage_format format)
-{
-	const char *name = getenv("GIT_DEFAULT_REF_FORMAT");
+	repo_set_hash_algo(the_repository, repo_fmt->hash_algo);
 
+	env = getenv("GIT_DEFAULT_REF_FORMAT");
 	if (repo_fmt->version >= 0 &&
-	    format != REF_STORAGE_FORMAT_UNKNOWN &&
-	    format != repo_fmt->ref_storage_format) {
+	    ref_format != REF_STORAGE_FORMAT_UNKNOWN &&
+	    ref_format != repo_fmt->ref_storage_format) {
 		die(_("attempt to reinitialize repository with different reference storage format"));
-	} else if (format != REF_STORAGE_FORMAT_UNKNOWN) {
-		repo_fmt->ref_storage_format = format;
-	} else if (name) {
-		format = ref_storage_format_by_name(name);
-		if (format == REF_STORAGE_FORMAT_UNKNOWN)
-			die(_("unknown ref storage format '%s'"), name);
-		repo_fmt->ref_storage_format = format;
+	} else if (ref_format != REF_STORAGE_FORMAT_UNKNOWN) {
+		repo_fmt->ref_storage_format = ref_format;
+	} else if (env) {
+		ref_format = ref_storage_format_by_name(env);
+		if (ref_format == REF_STORAGE_FORMAT_UNKNOWN)
+			die(_("unknown ref storage format '%s'"), env);
+		repo_fmt->ref_storage_format = ref_format;
 	}
+	repo_set_ref_storage_format(the_repository, repo_fmt->ref_storage_format);
 }
 
 int init_db(const char *git_dir, const char *real_git_dir,
@@ -2353,22 +2353,15 @@ int init_db(const char *git_dir, const char *real_git_dir,
 	}
 	startup_info->have_repository = 1;
 
-	/* Check to see if the repository version is right.
+	/*
+	 * Check to see if the repository version is right.
 	 * Note that a newly created repository does not have
 	 * config file, so this will not fail.  What we are catching
 	 * is an attempt to reinitialize new repository with an old tool.
 	 */
 	check_repository_format(&repo_fmt);
 
-	validate_hash_algorithm(&repo_fmt, hash);
-	validate_ref_storage_format(&repo_fmt, ref_storage_format);
-
-	/*
-	 * Now that we have set up both the hash algorithm and the ref storage
-	 * format we can update the repository's settings accordingly.
-	 */
-	repo_set_hash_algo(the_repository, repo_fmt.hash_algo);
-	repo_set_ref_storage_format(the_repository, repo_fmt.ref_storage_format);
+	repository_format_configure(&repo_fmt, hash, ref_storage_format);
 
 	/*
 	 * Ensure `core.hidedotfiles` is processed. This must happen after we
-- 
2.46.0.46.g406f326d27.dirty

[PATCH 4/5] setup: make object format configurable via config

[Patrick Steinhardt]

The object format for repositories can either be configured explicitly
by passing the `--object-format=` option to git-init(1) or git-clone(1),
or globally by setting the `GIT_DEFAULT_HASH` environment variable.
While the former makes sense, setting random environment variables is
not really a good user experience in case someone decides to only use
SHA256 repositories.

It is only natural to expect for a user that things like this can also
be configured via their config. As such, introduce a new config
"init.defaultObjectFormat", similar to "init.defaultBranch", that allows
the user to configure the default object format when creating new repos.

Signed-off-by: Patrick Steinhardt <ps@pks.im>
---
 Documentation/config/init.txt |  5 ++++
 setup.c                       | 40 ++++++++++++++++++++++++++++
 t/t0001-init.sh               | 50 +++++++++++++++++++++++++++++++++++
 3 files changed, 95 insertions(+)

diff --git a/Documentation/config/init.txt b/Documentation/config/init.txt
index af03acdbcb..d6f8b6e61b 100644
--- a/Documentation/config/init.txt
+++ b/Documentation/config/init.txt
@@ -8,3 +8,8 @@ endif::[]
 `init.defaultBranch`::
 	Allows overriding the default branch name e.g. when initializing
 	a new repository.
+`init.defaultObjectFormat`::
+	Allows overriding the default object format for new repositories. See
+	`--object-format=` in linkgit:git-init[1]. Both the command line option
+	and the `GIT_DEFAULT_HASH` environment variable take precedence over
+	this config.
diff --git a/setup.c b/setup.c
index 5dfcdc99dd..770ad1393f 100644
--- a/setup.c
+++ b/setup.c
@@ -2284,11 +2284,49 @@ static void separate_git_dir(const char *git_dir, const char *git_link)
 	write_file(git_link, "gitdir: %s", git_dir);
 }
 
+struct default_format_config {
+	int hash;
+};
+
+static int read_default_format_config(const char *key, const char *value,
+				      const struct config_context *ctx UNUSED,
+				      void *payload)
+{
+	struct default_format_config *cfg = payload;
+	char *str = NULL;
+	int ret;
+
+	if (!strcmp(key, "init.defaultobjectformat")) {
+		ret = git_config_string(&str, key, value);
+		if (ret)
+			goto out;
+		cfg->hash = hash_algo_by_name(str);
+		if (cfg->hash == GIT_HASH_UNKNOWN)
+			warning(_("unknown hash algorithm '%s'"), str);
+		goto out;
+	}
+
+	ret = 0;
+out:
+	free(str);
+	return ret;
+}
+
 static void repository_format_configure(struct repository_format *repo_fmt,
 					int hash, enum ref_storage_format ref_format)
 {
+	struct default_format_config cfg = {
+		.hash = GIT_HASH_UNKNOWN,
+	};
+	struct config_options opts = {
+		.respect_includes = 1,
+		.ignore_repo = 1,
+		.ignore_worktree = 1,
+	};
 	const char *env;
 
+	config_with_options(read_default_format_config, &cfg, NULL, NULL, &opts);
+
 	/*
 	 * If we already have an initialized repo, don't allow the user to
 	 * specify a different algorithm, as that could cause corruption.
@@ -2304,6 +2342,8 @@ static void repository_format_configure(struct repository_format *repo_fmt,
 		if (env_algo == GIT_HASH_UNKNOWN)
 			die(_("unknown hash algorithm '%s'"), env);
 		repo_fmt->hash_algo = env_algo;
+	} else if (cfg.hash != GIT_HASH_UNKNOWN) {
+		repo_fmt->hash_algo = cfg.hash;
 	}
 	repo_set_hash_algo(the_repository, repo_fmt->hash_algo);
 
diff --git a/t/t0001-init.sh b/t/t0001-init.sh
index 795408e16c..cd34710f32 100755
--- a/t/t0001-init.sh
+++ b/t/t0001-init.sh
@@ -523,6 +523,56 @@ test_expect_success 'init honors --object-format' '
 	test_cmp expected actual
 '
 
+test_expect_success 'init honors init.defaultObjectFormat' '
+	test_when_finished "rm -rf sha1 sha256" &&
+
+	test_config_global init.defaultObjectFormat sha1 &&
+	(
+		sane_unset GIT_DEFAULT_HASH &&
+		git init sha1 &&
+		git -C sha1 rev-parse --show-object-format >actual &&
+		echo sha1 >expected &&
+		test_cmp expected actual
+	) &&
+
+	test_config_global init.defaultObjectFormat sha256 &&
+	(
+		sane_unset GIT_DEFAULT_HASH &&
+		git init sha256 &&
+		git -C sha256 rev-parse --show-object-format >actual &&
+		echo sha256 >expected &&
+		test_cmp expected actual
+	)
+'
+
+test_expect_success 'init warns about invalid init.defaultObjectFormat' '
+	test_when_finished "rm -rf repo" &&
+	test_config_global init.defaultObjectFormat garbage &&
+
+	echo "warning: unknown hash algorithm ${SQ}garbage${SQ}" >expect &&
+	git init repo 2>err &&
+	test_cmp expect err &&
+
+	git -C repo rev-parse --show-object-format >actual &&
+	echo $GIT_DEFAULT_HASH >expected &&
+	test_cmp expected actual
+'
+
+test_expect_success '--object-format overrides GIT_DEFAULT_HASH' '
+	test_when_finished "rm -rf repo" &&
+	GIT_DEFAULT_HASH=sha1 git init --object-format=sha256 repo &&
+	git -C repo rev-parse --show-object-format >actual &&
+	echo sha256 >expected
+'
+
+test_expect_success 'GIT_DEFAULT_HASH overrides init.defaultObjectFormat' '
+	test_when_finished "rm -rf repo" &&
+	test_config_global init.defaultObjectFormat sha1 &&
+	GIT_DEFAULT_HASH=sha256 git init repo &&
+	git -C repo rev-parse --show-object-format >actual &&
+	echo sha256 >expected
+'
+
 test_expect_success 'extensions.objectFormat is not allowed with repo version 0' '
 	test_when_finished "rm -rf explicit-v0" &&
 	git init --object-format=sha256 explicit-v0 &&
-- 
2.46.0.46.g406f326d27.dirty

[PATCH 5/5] setup: make ref storage format configurable via config

[Patrick Steinhardt]

Similar as the preceding commit, introduce a new "init.defaultRefFormat"
config that allows the user to globally set the ref storage format used
by newly created repositories.

Signed-off-by: Patrick Steinhardt <ps@pks.im>
---
 Documentation/config/init.txt |  5 ++++
 setup.c                       | 14 +++++++++++
 t/t0001-init.sh               | 44 +++++++++++++++++++++++++++++++++++
 3 files changed, 63 insertions(+)

diff --git a/Documentation/config/init.txt b/Documentation/config/init.txt
index d6f8b6e61b..9be97bcac8 100644
--- a/Documentation/config/init.txt
+++ b/Documentation/config/init.txt
@@ -13,3 +13,8 @@ endif::[]
 	`--object-format=` in linkgit:git-init[1]. Both the command line option
 	and the `GIT_DEFAULT_HASH` environment variable take precedence over
 	this config.
+`init.defaultRefFormat`::
+	Allows overriding the default object format for new repositories. See
+	`--ref-format=` in linkgit:git-init[1]. Both the command line option
+	and the `GIT_DEFAULT_REF_FORMAT` environment variable take precedence
+	over this config.
diff --git a/setup.c b/setup.c
index 770ad1393f..dd2251f655 100644
--- a/setup.c
+++ b/setup.c
@@ -2286,6 +2286,7 @@ static void separate_git_dir(const char *git_dir, const char *git_link)
 
 struct default_format_config {
 	int hash;
+	enum ref_storage_format ref_format;
 };
 
 static int read_default_format_config(const char *key, const char *value,
@@ -2306,6 +2307,16 @@ static int read_default_format_config(const char *key, const char *value,
 		goto out;
 	}
 
+	if (!strcmp(key, "init.defaultrefformat")) {
+		ret = git_config_string(&str, key, value);
+		if (ret)
+			goto out;
+		cfg->ref_format = ref_storage_format_by_name(str);
+		if (cfg->ref_format == REF_STORAGE_FORMAT_UNKNOWN)
+			warning(_("unknown ref storage format '%s'"), str);
+		goto out;
+	}
+
 	ret = 0;
 out:
 	free(str);
@@ -2317,6 +2328,7 @@ static void repository_format_configure(struct repository_format *repo_fmt,
 {
 	struct default_format_config cfg = {
 		.hash = GIT_HASH_UNKNOWN,
+		.ref_format = REF_STORAGE_FORMAT_UNKNOWN,
 	};
 	struct config_options opts = {
 		.respect_includes = 1,
@@ -2359,6 +2371,8 @@ static void repository_format_configure(struct repository_format *repo_fmt,
 		if (ref_format == REF_STORAGE_FORMAT_UNKNOWN)
 			die(_("unknown ref storage format '%s'"), env);
 		repo_fmt->ref_storage_format = ref_format;
+	} else if (cfg.ref_format != REF_STORAGE_FORMAT_UNKNOWN) {
+		repo_fmt->ref_storage_format = cfg.ref_format;
 	}
 	repo_set_ref_storage_format(the_repository, repo_fmt->ref_storage_format);
 }
diff --git a/t/t0001-init.sh b/t/t0001-init.sh
index cd34710f32..0178aa62a4 100755
--- a/t/t0001-init.sh
+++ b/t/t0001-init.sh
@@ -620,6 +620,19 @@ test_expect_success 'init with GIT_DEFAULT_REF_FORMAT=garbage' '
 	test_cmp expect err
 '
 
+test_expect_success 'init warns about invalid init.defaultRefFormat' '
+	test_when_finished "rm -rf repo" &&
+	test_config_global init.defaultRefFormat garbage &&
+
+	echo "warning: unknown ref storage format ${SQ}garbage${SQ}" >expect &&
+	git init repo 2>err &&
+	test_cmp expect err &&
+
+	git -C repo rev-parse --show-ref-format >actual &&
+	echo $GIT_DEFAULT_REF_FORMAT >expected &&
+	test_cmp expected actual
+'
+
 backends="files reftable"
 for format in $backends
 do
@@ -650,6 +663,27 @@ do
 		git -C refformat rev-parse --show-ref-format >actual &&
 		test_cmp expect actual
 	'
+
+	test_expect_success "init with init.defaultRefFormat=$format" '
+		test_when_finished "rm -rf refformat" &&
+		test_config_global init.defaultRefFormat $format &&
+		(
+			sane_unset GIT_DEFAULT_REF_FORMAT &&
+			git init refformat
+		) &&
+
+		echo $format >expect &&
+		git -C refformat rev-parse --show-ref-format >actual &&
+		test_cmp expect actual
+	'
+
+	test_expect_success "--ref-format=$format overrides GIT_DEFAULT_REF_FORMAT" '
+		test_when_finished "rm -rf refformat" &&
+		GIT_DEFAULT_REF_FORMAT=garbage git init --ref-format=$format refformat &&
+		echo $format >expect &&
+		git -C refformat rev-parse --show-ref-format >actual &&
+		test_cmp expect actual
+	'
 done
 
 test_expect_success "--ref-format= overrides GIT_DEFAULT_REF_FORMAT" '
@@ -660,6 +694,16 @@ test_expect_success "--ref-format= overrides GIT_DEFAULT_REF_FORMAT" '
 	test_cmp expect actual
 '
 
+test_expect_success "GIT_DEFAULT_REF_FORMAT= overrides init.defaultRefFormat" '
+	test_when_finished "rm -rf refformat" &&
+	test_config_global init.defaultRefFormat files &&
+
+	GIT_DEFAULT_REF_FORMAT=reftable git init refformat &&
+	echo reftable >expect &&
+	git -C refformat rev-parse --show-ref-format >actual &&
+	test_cmp expect actual
+'
+
 for from_format in $backends
 do
 	test_expect_success "re-init with same format ($from_format)" '
-- 
2.46.0.46.g406f326d27.dirty

Re: [PATCH 0/5] Introduce configs for default repo format

[shejialuo]

On Thu, Aug 15, 2024 at 09:59:54AM +0200, Patrick Steinhardt wrote:
> Hi,
> 
> to set up the default object and ref storage formats, users have to set
> up some environment variables. This is somewhat unwieldy and not really
> in line with how a user typically expects to configure Git, namely by
> using the config system. It makes it harder than necessary to globally
> default to the different formats and requires the user to munge with
> files like `.profile` to persist that setting. Needless to say, this is
> a bit of an awkward user experience.
> 
> This patch series thus introduces two new configs to set the default
> object hash and ref storage format for newly created repositories. Like
> this, folks can simply use the global- or system-level config to adapt
> to their needs. This also has the advantage of giving them the ability
> to adapt the default formats via guarded includes, such that e.g. repos
> in some filesystem hierarchy use format A, whereas others use format B.
> 
> This comes from a discussion with Sebastian (Cc'd) at the Git User Group
> in Berlin yesterday.
> 
> Thanks!
> 
> Patrick
> 

It's a good idea to make the user could set up the default object and
ref storage formats by "git-config(1)". I have read all the patches,
from my perspective, there is no major problems.

But I wanna ask a question here about the following code snippet:

  if (repo_fmt->version >= 0 && hash !=  GIT_HASH_UNKNOWN && hash != repo_fmt->hash_algo)
          die(_("..."));
  else if (hash != GIT_HASH_UNKNOWN)
          repo_fmt->hash_algo = hash;
  else if (env) {
          ...
          repo_fmt->hash_algo = env_algo;
  } else if (cfg.hash != GIT_HASH_UNKNOWN) {
          repo_fmt->hash_algo = cfg.hash;
  }

It's obvious that the precedence of "hash" is the top. We need to make
sure when users execute "git init --object-format=sha256" command, this
explicit info should be considered at the top. However, in the current
design, the precedence of the environment variable is higher than the
"git-config(1)".

If the user uses the following command:

  $ export GIT_DEFAULT_HASH_ENVIRONMENT=sha1
  $ git -c init.defaultObjectFormat=sha256 repo

The repo would be initialized with the sha1 algorithm. I think we should
think carefully which precedence should be higher. I cannot give an
answer here. I am not familiar with the whole database and do not the
concern. But from my own perspective, I think the precedence of the
config should be higher than the environment variable. This is a new
feature, the people who would like to use it, they will never use
environment variable and we should ignore the functionality of the
environment variable. But for people who do not know this feature, they
will continue to use the environment variable and they will never be
influenced by the configs.

Thanks,
Jialuo

Re: [PATCH 1/5] t0001: exercise initialization with ref formats more thoroughly

[Justin Tobler]

On 24/08/15 09:59AM, Patrick Steinhardt wrote:
> While our object format tests for git-init(1) exercise tests with all
> known formats in t0001, the tests for the ref format don't. This leads
> to some missing test coverage for interesting cases, like whether or not
> a non-default ref storage format causes us to bump the repository format
> version. We also don't test for the precedence of the `--ref-format=`
> and the `GIT_DEFAULT_REF_FORMAT=` environment variable.

Makes sense to plug these test gaps.

> Extend the test suite to cover more scenarios related to the ref format.
> 
> Signed-off-by: Patrick Steinhardt <ps@pks.im>
> ---
>  t/t0001-init.sh | 48 +++++++++++++++++++++++++++++++++++-------------
>  1 file changed, 35 insertions(+), 13 deletions(-)
> 
> diff --git a/t/t0001-init.sh b/t/t0001-init.sh
> index 49e9bf77c6..2093f5c1ee 100755
> --- a/t/t0001-init.sh
> +++ b/t/t0001-init.sh
> @@ -558,15 +558,6 @@ test_expect_success DEFAULT_REPO_FORMAT 'extensions.refStorage with unknown back
>  	grep "invalid value for ${SQ}extensions.refstorage${SQ}: ${SQ}garbage${SQ}" err
>  '
>  
> -test_expect_success DEFAULT_REPO_FORMAT 'init with GIT_DEFAULT_REF_FORMAT=files' '
> -	test_when_finished "rm -rf refformat" &&
> -	GIT_DEFAULT_REF_FORMAT=files git init refformat &&
> -	echo 0 >expect &&
> -	git -C refformat config core.repositoryformatversion >actual &&
> -	test_cmp expect actual &&
> -	test_must_fail git -C refformat config extensions.refstorage
> -'
> -
>  test_expect_success 'init with GIT_DEFAULT_REF_FORMAT=garbage' '
>  	test_when_finished "rm -rf refformat" &&
>  	cat >expect <<-EOF &&
> @@ -576,15 +567,46 @@ test_expect_success 'init with GIT_DEFAULT_REF_FORMAT=garbage' '
>  	test_cmp expect err
>  '
>  
> -test_expect_success 'init with --ref-format=files' '
> +backends="files reftable"
> +for format in $backends
> +do
> +	test_expect_success DEFAULT_REPO_FORMAT "init with GIT_DEFAULT_REF_FORMAT=$format" '

The `DEFAULT_REPO_FORMAT` prereq is needed here because a non-default
repo format also changes the repository format version.

> +		test_when_finished "rm -rf refformat" &&
> +		GIT_DEFAULT_REF_FORMAT=$format git init refformat &&
> +
> +		if test $format = files
> +		then
> +			test_must_fail git -C refformat config extensions.refstorage &&
> +			echo 0 >expect
> +		else
> +			git -C refformat config extensions.refstorage &&
> +			echo 1 >expect
> +		fi &&
> +		git -C refformat config core.repositoryformatversion >actual &&
> +		test_cmp expect actual &&
> +
> +		echo $format >expect &&
> +		git -C refformat rev-parse --show-ref-format >actual &&
> +		test_cmp expect actual
> +	'
> +
> +	test_expect_success "init with --ref-format=$format" '
> +		test_when_finished "rm -rf refformat" &&
> +		git init --ref-format=$format refformat &&
> +		echo $format >expect &&
> +		git -C refformat rev-parse --show-ref-format >actual &&
> +		test_cmp expect actual
> +	'
> +done
> +
> +test_expect_success "--ref-format= overrides GIT_DEFAULT_REF_FORMAT" '
>  	test_when_finished "rm -rf refformat" &&
> -	git init --ref-format=files refformat &&
> -	echo files >expect &&
> +	GIT_DEFAULT_REF_FORMAT=files git init --ref-format=reftable refformat &&
> +	echo reftable >expect &&
>  	git -C refformat rev-parse --show-ref-format >actual &&
>  	test_cmp expect actual
>  '

Nice that we now validate that the `--ref-format` option takes
precedence over the `GIT_DEFAULT_REF_FORMAT` environment variable.

-Justin

Re: [PATCH 0/5] Introduce configs for default repo format

[brian m. carlson]

[-- Attachment #1: Type: text/plain, Size: 1907 bytes --]

On 2024-08-15 at 15:24:47, shejialuo wrote:
> If the user uses the following command:
> 
>   $ export GIT_DEFAULT_HASH_ENVIRONMENT=sha1
>   $ git -c init.defaultObjectFormat=sha256 repo
> 
> The repo would be initialized with the sha1 algorithm. I think we should
> think carefully which precedence should be higher. I cannot give an
> answer here. I am not familiar with the whole database and do not the
> concern. But from my own perspective, I think the precedence of the
> config should be higher than the environment variable. This is a new
> feature, the people who would like to use it, they will never use
> environment variable and we should ignore the functionality of the
> environment variable. But for people who do not know this feature, they
> will continue to use the environment variable and they will never be
> influenced by the configs.

The standard behaviour we have with other environment variables is that
they override the config, such as with `GIT_SSH_COMMAND` and
`GIT_SSH_VARIANT`.  The reason is that the config in this case is
usually per-user or per-system, but it's very common to override
settings on an ephemeral basis with the environment.

Think of the case where you have a language package manager that's
creating a repository or fetching from one.  You can override on the
command line with `GIT_DEFAULT_HASH` or `GIT_SSH_VARIANT`, but you can't
control the actual `git` invocation, since it's baked into the package
manager.  Or if you're writing a test and need all of the Git commands
to use a particular setting, then it's easy to simply set the
environment once and forget about it.  (This is what we do at $DAYJOB to
implement SHA-256 test modes for our repos: the test script sets
`GIT_DEFAULT_HASH` and all of our test repos magically use the right
setting.)
-- 
brian m. carlson (they/them or he/him)
Toronto, Ontario, CA

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 262 bytes --]

Re: [PATCH 0/5] Introduce configs for default repo format

[brian m. carlson]

[-- Attachment #1: Type: text/plain, Size: 1469 bytes --]

On 2024-08-15 at 07:59:54, Patrick Steinhardt wrote:
> Hi,
> 
> to set up the default object and ref storage formats, users have to set
> up some environment variables. This is somewhat unwieldy and not really
> in line with how a user typically expects to configure Git, namely by
> using the config system. It makes it harder than necessary to globally
> default to the different formats and requires the user to munge with
> files like `.profile` to persist that setting. Needless to say, this is
> a bit of an awkward user experience.
> 
> This patch series thus introduces two new configs to set the default
> object hash and ref storage format for newly created repositories. Like
> this, folks can simply use the global- or system-level config to adapt
> to their needs. This also has the advantage of giving them the ability
> to adapt the default formats via guarded includes, such that e.g. repos
> in some filesystem hierarchy use format A, whereas others use format B.

I like the idea of this series, which I think is a much nicer way to set
these defaults and can also correctly apply to existing terminal
windows.  As you mentioned, this also allows different configurations
per directory, and it makes it easier if you need to have a system-wide
policy for whatever reason.

I've taken a look at the patches and they seem fine to me.  Thanks for
sending them.
-- 
brian m. carlson (they/them or he/him)
Toronto, Ontario, CA

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 262 bytes --]

Re: [PATCH 3/5] setup: merge configuration of repository formats

[Justin Tobler]

On 24/08/15 10:00AM, Patrick Steinhardt wrote:
> The configuration of repository formats is split up across two functions
> `validate_hash_algorithm()` and `validate_ref_storage_format()`. This is
> fine as-is, but we are about to extend the logic to also read default
> values from the config. With the logic split across two functions, we
> would either have to pass in additional parameters read from the config,
> or read the config multiple times. Both of these options feel a bit
> unwieldy.

Combining the repository format configuration logic into a single
function seems like a good option. It looks like go we even further
though since we also include setting the hash and ref format for
`the_repository`. Might be nice to also mention this.

> Merge the code into a new a new function `repository_format_configure()`

s/new a new/new/

> that is responsible for configuring the whole repository's format. Like
> this, we can easily read the config in a single place, only.
> 
> Signed-off-by: Patrick Steinhardt <ps@pks.im>
> ---
[snip]
> -	/*
> -	 * Now that we have set up both the hash algorithm and the ref storage
> -	 * format we can update the repository's settings accordingly.
> -	 */

The above comment somewhat made it sound like the repository format had
to be configured for both the hash algo and ref storage before updating
`the_repository`, but that does not seem to be a requirement in
actuality.

> -	repo_set_hash_algo(the_repository, repo_fmt.hash_algo);
> -	repo_set_ref_storage_format(the_repository, repo_fmt.ref_storage_format);
> +	repository_format_configure(&repo_fmt, hash, ref_storage_format);

Overall, I like that this repostory format configuration is under
`repository_format_configure()`. The `validate_*()` functions names
confused me slightly initially because I assumed they were only
validating, but they also configure the repo format. Looking good :)

-Justin

Re: [PATCH 0/5] Introduce configs for default repo format

[Junio C Hamano]

"brian m. carlson" <sandals@crustytoothpaste.net> writes:

> On 2024-08-15 at 15:24:47, shejialuo wrote:
>> If the user uses the following command:
>> 
>>   $ export GIT_DEFAULT_HASH_ENVIRONMENT=sha1
>>   $ git -c init.defaultObjectFormat=sha256 repo
>> 
>> The repo would be initialized with the sha1 algorithm. I think we should
>> think carefully which precedence should be higher. I cannot give an
>> answer here. I am not familiar with the whole database and do not the
>> concern. But from my own perspective, I think the precedence of the
>> config should be higher than the environment variable. This is a new
>> feature, the people who would like to use it, they will never use
>> environment variable and we should ignore the functionality of the
>> environment variable. But for people who do not know this feature, they
>> will continue to use the environment variable and they will never be
>> influenced by the configs.
>
> The standard behaviour we have with other environment variables is that
> they override the config, such as with `GIT_SSH_COMMAND` and
> `GIT_SSH_VARIANT`.  The reason is that the config in this case is
> usually per-user or per-system, but it's very common to override
> settings on an ephemeral basis with the environment.

Right.  It is good that somebody can give a clear answer when a new
person says they cannot and then give an answer that contradicts
with an established practice ;-).

Re: [PATCH 4/5] setup: make object format configurable via config

[Justin Tobler]

On 24/08/15 10:00AM, Patrick Steinhardt wrote:
> The object format for repositories can either be configured explicitly
> by passing the `--object-format=` option to git-init(1) or git-clone(1),
> or globally by setting the `GIT_DEFAULT_HASH` environment variable.
> While the former makes sense, setting random environment variables is
> not really a good user experience in case someone decides to only use
> SHA256 repositories.
> 
> It is only natural to expect for a user that things like this can also
> be configured via their config. As such, introduce a new config
> "init.defaultObjectFormat", similar to "init.defaultBranch", that allows
> the user to configure the default object format when creating new repos.

Adding an option to the Git config to allow the default hash algorithm
and reference format to be configured makes sense. In my opinion it is
much more ergonomic.

> 
> Signed-off-by: Patrick Steinhardt <ps@pks.im>
> ---
>  Documentation/config/init.txt |  5 ++++
>  setup.c                       | 40 ++++++++++++++++++++++++++++
>  t/t0001-init.sh               | 50 +++++++++++++++++++++++++++++++++++
>  3 files changed, 95 insertions(+)
> 
> diff --git a/Documentation/config/init.txt b/Documentation/config/init.txt
> index af03acdbcb..d6f8b6e61b 100644
> --- a/Documentation/config/init.txt
> +++ b/Documentation/config/init.txt
> @@ -8,3 +8,8 @@ endif::[]
>  `init.defaultBranch`::
>  	Allows overriding the default branch name e.g. when initializing
>  	a new repository.
> +`init.defaultObjectFormat`::
> +	Allows overriding the default object format for new repositories. See
> +	`--object-format=` in linkgit:git-init[1]. Both the command line option
> +	and the `GIT_DEFAULT_HASH` environment variable take precedence over
> +	this config.
> diff --git a/setup.c b/setup.c
> index 5dfcdc99dd..770ad1393f 100644
> --- a/setup.c
> +++ b/setup.c
> @@ -2284,11 +2284,49 @@ static void separate_git_dir(const char *git_dir, const char *git_link)
>  	write_file(git_link, "gitdir: %s", git_dir);
>  }
>  
> +struct default_format_config {
> +	int hash;
> +};
> +
> +static int read_default_format_config(const char *key, const char *value,
> +				      const struct config_context *ctx UNUSED,
> +				      void *payload)
> +{
> +	struct default_format_config *cfg = payload;
> +	char *str = NULL;
> +	int ret;
> +
> +	if (!strcmp(key, "init.defaultobjectformat")) {
> +		ret = git_config_string(&str, key, value);
> +		if (ret)
> +			goto out;
> +		cfg->hash = hash_algo_by_name(str);
> +		if (cfg->hash == GIT_HASH_UNKNOWN)
> +			warning(_("unknown hash algorithm '%s'"), str);
> +		goto out;
> +	}
> +
> +	ret = 0;
> +out:
> +	free(str);
> +	return ret;
> +}
> +
>  static void repository_format_configure(struct repository_format *repo_fmt,
>  					int hash, enum ref_storage_format ref_format)
>  {
> +	struct default_format_config cfg = {
> +		.hash = GIT_HASH_UNKNOWN,
> +	};
> +	struct config_options opts = {
> +		.respect_includes = 1,
> +		.ignore_repo = 1,
> +		.ignore_worktree = 1,
> +	};

As these format configurations only make sense to be included in the
global config, the search radius is limited accordingly. 

>  	const char *env;
>  
> +	config_with_options(read_default_format_config, &cfg, NULL, NULL, &opts);
> +
>  	/*
>  	 * If we already have an initialized repo, don't allow the user to
>  	 * specify a different algorithm, as that could cause corruption.
> @@ -2304,6 +2342,8 @@ static void repository_format_configure(struct repository_format *repo_fmt,
>  		if (env_algo == GIT_HASH_UNKNOWN)
>  			die(_("unknown hash algorithm '%s'"), env);
>  		repo_fmt->hash_algo = env_algo;
> +	} else if (cfg.hash != GIT_HASH_UNKNOWN) {
> +		repo_fmt->hash_algo = cfg.hash;

The environment variable takes higher precedence than the Git config.
Makes sense to allow the env to override Git cofiguration.

The way this patch is setup should make it easy to extend for a default
reference format config. Very nice.

-Justin

Re: [PATCH 5/5] setup: make ref storage format configurable via config

[Justin Tobler]

On 24/08/15 10:00AM, Patrick Steinhardt wrote:
> Similar as the preceding commit, introduce a new "init.defaultRefFormat"

s/as/to/

> config that allows the user to globally set the ref storage format used
> by newly created repositories.
> 
> Signed-off-by: Patrick Steinhardt <ps@pks.im>
> ---
>  Documentation/config/init.txt |  5 ++++
>  setup.c                       | 14 +++++++++++
>  t/t0001-init.sh               | 44 +++++++++++++++++++++++++++++++++++
>  3 files changed, 63 insertions(+)
> 
> diff --git a/Documentation/config/init.txt b/Documentation/config/init.txt
> index d6f8b6e61b..9be97bcac8 100644
> --- a/Documentation/config/init.txt
> +++ b/Documentation/config/init.txt
> @@ -13,3 +13,8 @@ endif::[]
>  	`--object-format=` in linkgit:git-init[1]. Both the command line option
>  	and the `GIT_DEFAULT_HASH` environment variable take precedence over
>  	this config.
> +`init.defaultRefFormat`::
> +	Allows overriding the default object format for new repositories. See

I think you meant to say "default reference format" here. :)

> +	`--ref-format=` in linkgit:git-init[1]. Both the command line option
> +	and the `GIT_DEFAULT_REF_FORMAT` environment variable take precedence
> +	over this config.
[snip]

The remainder of this patch extends from the previous patch to add
support for a configurable default reference format set in a Git config.
Nicely done and looks good to me.

-Justin

Re: [PATCH 3/5] setup: merge configuration of repository formats

[Patrick Steinhardt]

On Thu, Aug 15, 2024 at 04:37:26PM -0500, Justin Tobler wrote:
> On 24/08/15 10:00AM, Patrick Steinhardt wrote:
> > -	repo_set_hash_algo(the_repository, repo_fmt.hash_algo);
> > -	repo_set_ref_storage_format(the_repository, repo_fmt.ref_storage_format);
> > +	repository_format_configure(&repo_fmt, hash, ref_storage_format);
> 
> Overall, I like that this repostory format configuration is under
> `repository_format_configure()`. The `validate_*()` functions names
> confused me slightly initially because I assumed they were only
> validating, but they also configure the repo format. Looking good :)

Yeah, agreed. We _did_ validate whether the new configuration makes
sense, but that wasn't really the main thing those functions did.

Patrick

Re: [PATCH 0/5] Introduce configs for default repo format

[Patrick Steinhardt]

On Thu, Aug 15, 2024 at 02:52:21PM -0700, Junio C Hamano wrote:
> "brian m. carlson" <sandals@crustytoothpaste.net> writes:
> 
> > On 2024-08-15 at 15:24:47, shejialuo wrote:
> >> If the user uses the following command:
> >> 
> >>   $ export GIT_DEFAULT_HASH_ENVIRONMENT=sha1
> >>   $ git -c init.defaultObjectFormat=sha256 repo
> >> 
> >> The repo would be initialized with the sha1 algorithm. I think we should
> >> think carefully which precedence should be higher. I cannot give an
> >> answer here. I am not familiar with the whole database and do not the
> >> concern. But from my own perspective, I think the precedence of the
> >> config should be higher than the environment variable. This is a new
> >> feature, the people who would like to use it, they will never use
> >> environment variable and we should ignore the functionality of the
> >> environment variable. But for people who do not know this feature, they
> >> will continue to use the environment variable and they will never be
> >> influenced by the configs.
> >
> > The standard behaviour we have with other environment variables is that
> > they override the config, such as with `GIT_SSH_COMMAND` and
> > `GIT_SSH_VARIANT`.  The reason is that the config in this case is
> > usually per-user or per-system, but it's very common to override
> > settings on an ephemeral basis with the environment.
> 
> Right.  It is good that somebody can give a clear answer when a new
> person says they cannot and then give an answer that contradicts
> with an established practice ;-).

Yeah, thanks for putting it so clearly. I'll also put a variant of this
into the commit message to clarify.

Patrick

[PATCH v2 0/5] Introduce configs for default repo format

[Patrick Steinhardt]

Hi,

this is the second version of my patch series that introduces two new
config settings `init.defaultRefFormat` and `init.defaultObjectFormat`
to make the default formats configurable without environment variables.

Changes compared to v1:

  - Extend commit message to mention that we also move
    `repo_set_ref_storage_format()` and `repo_set_hash_algo()` into
    `repository_format_configure()`.

  - Extend commit message to explain precedence.

  - Fix a grammar issue.

  - Fix a copy/paste error in the documentation of
    `init.defaultRefFormat`.

Thanks!

Patrick

Patrick Steinhardt (5):
  t0001: exercise initialization with ref formats more thoroughly
  t0001: delete repositories when object format tests finish
  setup: merge configuration of repository formats
  setup: make object format configurable via config
  setup: make ref storage format configurable via config

 Documentation/config/init.txt |  10 +++
 setup.c                       | 101 ++++++++++++++++-------
 t/t0001-init.sh               | 145 +++++++++++++++++++++++++++++++---
 3 files changed, 216 insertions(+), 40 deletions(-)

Range-diff against v1:
1:  0d3844db32 = 1:  0d3844db32 t0001: exercise initialization with ref formats more thoroughly
2:  6b0cefef6a = 2:  6b0cefef6a t0001: delete repositories when object format tests finish
3:  16f52b75d8 ! 3:  ce0fad88bb setup: merge configuration of repository formats
    @@ Commit message
         or read the config multiple times. Both of these options feel a bit
         unwieldy.
     
    -    Merge the code into a new a new function `repository_format_configure()`
    -    that is responsible for configuring the whole repository's format. Like
    -    this, we can easily read the config in a single place, only.
    +    Merge the code into a new function `repository_format_configure()` that
    +    is responsible for configuring the whole repository's format. Like this,
    +    we can easily read the config in a single place, only.
    +
    +    Furthermore, move the calls to `repo_set_ref_storage_format()` and
    +    `repo_set_hash_algo()` into this new function as well, such that all the
    +    logic to configure the repository format is self-contained here.
     
         Signed-off-by: Patrick Steinhardt <ps@pks.im>
     
4:  e1cdaf0f0e ! 4:  8508662062 setup: make object format configurable via config
    @@ Commit message
         "init.defaultObjectFormat", similar to "init.defaultBranch", that allows
         the user to configure the default object format when creating new repos.
     
    +    The precedence order now is the following, where the first one wins:
    +
    +      1. The `--object-format=` switch.
    +
    +      2. The `GIT_DEFAULT_HASH` environment variable.
    +
    +      3. The `init.defaultObjectFormat` config variable.
    +
    +    This matches the typical precedence order we use in Git. We typically
    +    let the environment override the config such that the latter can easily
    +    be overridden on an ephemeral basis, for example by scripts.
    +
         Signed-off-by: Patrick Steinhardt <ps@pks.im>
     
      ## Documentation/config/init.txt ##
5:  a0417b7d1a ! 5:  b68a841450 setup: make ref storage format configurable via config
    @@ Metadata
      ## Commit message ##
         setup: make ref storage format configurable via config
     
    -    Similar as the preceding commit, introduce a new "init.defaultRefFormat"
    +    Similar to the preceding commit, introduce a new "init.defaultRefFormat"
         config that allows the user to globally set the ref storage format used
         by newly created repositories.
     
    @@ Documentation/config/init.txt: endif::[]
      	and the `GIT_DEFAULT_HASH` environment variable take precedence over
      	this config.
     +`init.defaultRefFormat`::
    -+	Allows overriding the default object format for new repositories. See
    -+	`--ref-format=` in linkgit:git-init[1]. Both the command line option
    -+	and the `GIT_DEFAULT_REF_FORMAT` environment variable take precedence
    -+	over this config.
    ++	Allows overriding the default ref storage format for new repositories.
    ++	See `--ref-format=` in linkgit:git-init[1]. Both the command line
    ++	option and the `GIT_DEFAULT_REF_FORMAT` environment variable take
    ++	precedence over this config.
     
      ## setup.c ##
     @@ setup.c: static void separate_git_dir(const char *git_dir, const char *git_link)
-- 
2.46.0.46.g406f326d27.dirty

[PATCH v2 1/5] t0001: exercise initialization with ref formats more thoroughly

[Patrick Steinhardt]

While our object format tests for git-init(1) exercise tests with all
known formats in t0001, the tests for the ref format don't. This leads
to some missing test coverage for interesting cases, like whether or not
a non-default ref storage format causes us to bump the repository format
version. We also don't test for the precedence of the `--ref-format=`
and the `GIT_DEFAULT_REF_FORMAT=` environment variable.

Extend the test suite to cover more scenarios related to the ref format.

Signed-off-by: Patrick Steinhardt <ps@pks.im>
---
 t/t0001-init.sh | 48 +++++++++++++++++++++++++++++++++++-------------
 1 file changed, 35 insertions(+), 13 deletions(-)

diff --git a/t/t0001-init.sh b/t/t0001-init.sh
index 49e9bf77c6..2093f5c1ee 100755
--- a/t/t0001-init.sh
+++ b/t/t0001-init.sh
@@ -558,15 +558,6 @@ test_expect_success DEFAULT_REPO_FORMAT 'extensions.refStorage with unknown back
 	grep "invalid value for ${SQ}extensions.refstorage${SQ}: ${SQ}garbage${SQ}" err
 '
 
-test_expect_success DEFAULT_REPO_FORMAT 'init with GIT_DEFAULT_REF_FORMAT=files' '
-	test_when_finished "rm -rf refformat" &&
-	GIT_DEFAULT_REF_FORMAT=files git init refformat &&
-	echo 0 >expect &&
-	git -C refformat config core.repositoryformatversion >actual &&
-	test_cmp expect actual &&
-	test_must_fail git -C refformat config extensions.refstorage
-'
-
 test_expect_success 'init with GIT_DEFAULT_REF_FORMAT=garbage' '
 	test_when_finished "rm -rf refformat" &&
 	cat >expect <<-EOF &&
@@ -576,15 +567,46 @@ test_expect_success 'init with GIT_DEFAULT_REF_FORMAT=garbage' '
 	test_cmp expect err
 '
 
-test_expect_success 'init with --ref-format=files' '
+backends="files reftable"
+for format in $backends
+do
+	test_expect_success DEFAULT_REPO_FORMAT "init with GIT_DEFAULT_REF_FORMAT=$format" '
+		test_when_finished "rm -rf refformat" &&
+		GIT_DEFAULT_REF_FORMAT=$format git init refformat &&
+
+		if test $format = files
+		then
+			test_must_fail git -C refformat config extensions.refstorage &&
+			echo 0 >expect
+		else
+			git -C refformat config extensions.refstorage &&
+			echo 1 >expect
+		fi &&
+		git -C refformat config core.repositoryformatversion >actual &&
+		test_cmp expect actual &&
+
+		echo $format >expect &&
+		git -C refformat rev-parse --show-ref-format >actual &&
+		test_cmp expect actual
+	'
+
+	test_expect_success "init with --ref-format=$format" '
+		test_when_finished "rm -rf refformat" &&
+		git init --ref-format=$format refformat &&
+		echo $format >expect &&
+		git -C refformat rev-parse --show-ref-format >actual &&
+		test_cmp expect actual
+	'
+done
+
+test_expect_success "--ref-format= overrides GIT_DEFAULT_REF_FORMAT" '
 	test_when_finished "rm -rf refformat" &&
-	git init --ref-format=files refformat &&
-	echo files >expect &&
+	GIT_DEFAULT_REF_FORMAT=files git init --ref-format=reftable refformat &&
+	echo reftable >expect &&
 	git -C refformat rev-parse --show-ref-format >actual &&
 	test_cmp expect actual
 '
 
-backends="files reftable"
 for from_format in $backends
 do
 	test_expect_success "re-init with same format ($from_format)" '
-- 
2.46.0.46.g406f326d27.dirty

[PATCH v2 2/5] t0001: delete repositories when object format tests finish

[Patrick Steinhardt]

The object format tests create one-shot repositories that are only used
by the respective test, but never delete them. This makes it hard to
pick a proper repository name in subsequent tests, as more and more
names are taken already.

Delete these repositories via `test_when_finished`.

Signed-off-by: Patrick Steinhardt <ps@pks.im>
---
 t/t0001-init.sh | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/t/t0001-init.sh b/t/t0001-init.sh
index 2093f5c1ee..795408e16c 100755
--- a/t/t0001-init.sh
+++ b/t/t0001-init.sh
@@ -500,6 +500,7 @@ test_expect_success 're-init from a linked worktree' '
 '
 
 test_expect_success 'init honors GIT_DEFAULT_HASH' '
+	test_when_finished "rm -rf sha1 sha256" &&
 	GIT_DEFAULT_HASH=sha1 git init sha1 &&
 	git -C sha1 rev-parse --show-object-format >actual &&
 	echo sha1 >expected &&
@@ -511,6 +512,7 @@ test_expect_success 'init honors GIT_DEFAULT_HASH' '
 '
 
 test_expect_success 'init honors --object-format' '
+	test_when_finished "rm -rf explicit-sha1 explicit-sha256" &&
 	git init --object-format=sha1 explicit-sha1 &&
 	git -C explicit-sha1 rev-parse --show-object-format >actual &&
 	echo sha1 >expected &&
@@ -522,6 +524,7 @@ test_expect_success 'init honors --object-format' '
 '
 
 test_expect_success 'extensions.objectFormat is not allowed with repo version 0' '
+	test_when_finished "rm -rf explicit-v0" &&
 	git init --object-format=sha256 explicit-v0 &&
 	git -C explicit-v0 config core.repositoryformatversion 0 &&
 	test_must_fail git -C explicit-v0 rev-parse --show-object-format
-- 
2.46.0.46.g406f326d27.dirty

[PATCH v2 3/5] setup: merge configuration of repository formats

[Patrick Steinhardt]

The configuration of repository formats is split up across two functions
`validate_hash_algorithm()` and `validate_ref_storage_format()`. This is
fine as-is, but we are about to extend the logic to also read default
values from the config. With the logic split across two functions, we
would either have to pass in additional parameters read from the config,
or read the config multiple times. Both of these options feel a bit
unwieldy.

Merge the code into a new function `repository_format_configure()` that
is responsible for configuring the whole repository's format. Like this,
we can easily read the config in a single place, only.

Furthermore, move the calls to `repo_set_ref_storage_format()` and
`repo_set_hash_algo()` into this new function as well, such that all the
logic to configure the repository format is self-contained here.

Signed-off-by: Patrick Steinhardt <ps@pks.im>
---
 setup.c | 47 ++++++++++++++++++++---------------------------
 1 file changed, 20 insertions(+), 27 deletions(-)

diff --git a/setup.c b/setup.c
index d458edcc02..5dfcdc99dd 100644
--- a/setup.c
+++ b/setup.c
@@ -2284,14 +2284,17 @@ static void separate_git_dir(const char *git_dir, const char *git_link)
 	write_file(git_link, "gitdir: %s", git_dir);
 }
 
-static void validate_hash_algorithm(struct repository_format *repo_fmt, int hash)
+static void repository_format_configure(struct repository_format *repo_fmt,
+					int hash, enum ref_storage_format ref_format)
 {
-	const char *env = getenv(GIT_DEFAULT_HASH_ENVIRONMENT);
+	const char *env;
+
 	/*
 	 * If we already have an initialized repo, don't allow the user to
 	 * specify a different algorithm, as that could cause corruption.
 	 * Otherwise, if the user has specified one on the command line, use it.
 	 */
+	env = getenv(GIT_DEFAULT_HASH_ENVIRONMENT);
 	if (repo_fmt->version >= 0 && hash != GIT_HASH_UNKNOWN && hash != repo_fmt->hash_algo)
 		die(_("attempt to reinitialize repository with different hash"));
 	else if (hash != GIT_HASH_UNKNOWN)
@@ -2302,25 +2305,22 @@ static void validate_hash_algorithm(struct repository_format *repo_fmt, int hash
 			die(_("unknown hash algorithm '%s'"), env);
 		repo_fmt->hash_algo = env_algo;
 	}
-}
-
-static void validate_ref_storage_format(struct repository_format *repo_fmt,
-					enum ref_storage_format format)
-{
-	const char *name = getenv("GIT_DEFAULT_REF_FORMAT");
+	repo_set_hash_algo(the_repository, repo_fmt->hash_algo);
 
+	env = getenv("GIT_DEFAULT_REF_FORMAT");
 	if (repo_fmt->version >= 0 &&
-	    format != REF_STORAGE_FORMAT_UNKNOWN &&
-	    format != repo_fmt->ref_storage_format) {
+	    ref_format != REF_STORAGE_FORMAT_UNKNOWN &&
+	    ref_format != repo_fmt->ref_storage_format) {
 		die(_("attempt to reinitialize repository with different reference storage format"));
-	} else if (format != REF_STORAGE_FORMAT_UNKNOWN) {
-		repo_fmt->ref_storage_format = format;
-	} else if (name) {
-		format = ref_storage_format_by_name(name);
-		if (format == REF_STORAGE_FORMAT_UNKNOWN)
-			die(_("unknown ref storage format '%s'"), name);
-		repo_fmt->ref_storage_format = format;
+	} else if (ref_format != REF_STORAGE_FORMAT_UNKNOWN) {
+		repo_fmt->ref_storage_format = ref_format;
+	} else if (env) {
+		ref_format = ref_storage_format_by_name(env);
+		if (ref_format == REF_STORAGE_FORMAT_UNKNOWN)
+			die(_("unknown ref storage format '%s'"), env);
+		repo_fmt->ref_storage_format = ref_format;
 	}
+	repo_set_ref_storage_format(the_repository, repo_fmt->ref_storage_format);
 }
 
 int init_db(const char *git_dir, const char *real_git_dir,
@@ -2353,22 +2353,15 @@ int init_db(const char *git_dir, const char *real_git_dir,
 	}
 	startup_info->have_repository = 1;
 
-	/* Check to see if the repository version is right.
+	/*
+	 * Check to see if the repository version is right.
 	 * Note that a newly created repository does not have
 	 * config file, so this will not fail.  What we are catching
 	 * is an attempt to reinitialize new repository with an old tool.
 	 */
 	check_repository_format(&repo_fmt);
 
-	validate_hash_algorithm(&repo_fmt, hash);
-	validate_ref_storage_format(&repo_fmt, ref_storage_format);
-
-	/*
-	 * Now that we have set up both the hash algorithm and the ref storage
-	 * format we can update the repository's settings accordingly.
-	 */
-	repo_set_hash_algo(the_repository, repo_fmt.hash_algo);
-	repo_set_ref_storage_format(the_repository, repo_fmt.ref_storage_format);
+	repository_format_configure(&repo_fmt, hash, ref_storage_format);
 
 	/*
 	 * Ensure `core.hidedotfiles` is processed. This must happen after we
-- 
2.46.0.46.g406f326d27.dirty

[PATCH v2 4/5] setup: make object format configurable via config

[Patrick Steinhardt]

The object format for repositories can either be configured explicitly
by passing the `--object-format=` option to git-init(1) or git-clone(1),
or globally by setting the `GIT_DEFAULT_HASH` environment variable.
While the former makes sense, setting random environment variables is
not really a good user experience in case someone decides to only use
SHA256 repositories.

It is only natural to expect for a user that things like this can also
be configured via their config. As such, introduce a new config
"init.defaultObjectFormat", similar to "init.defaultBranch", that allows
the user to configure the default object format when creating new repos.

The precedence order now is the following, where the first one wins:

  1. The `--object-format=` switch.

  2. The `GIT_DEFAULT_HASH` environment variable.

  3. The `init.defaultObjectFormat` config variable.

This matches the typical precedence order we use in Git. We typically
let the environment override the config such that the latter can easily
be overridden on an ephemeral basis, for example by scripts.

Signed-off-by: Patrick Steinhardt <ps@pks.im>
---
 Documentation/config/init.txt |  5 ++++
 setup.c                       | 40 ++++++++++++++++++++++++++++
 t/t0001-init.sh               | 50 +++++++++++++++++++++++++++++++++++
 3 files changed, 95 insertions(+)

diff --git a/Documentation/config/init.txt b/Documentation/config/init.txt
index af03acdbcb..d6f8b6e61b 100644
--- a/Documentation/config/init.txt
+++ b/Documentation/config/init.txt
@@ -8,3 +8,8 @@ endif::[]
 `init.defaultBranch`::
 	Allows overriding the default branch name e.g. when initializing
 	a new repository.
+`init.defaultObjectFormat`::
+	Allows overriding the default object format for new repositories. See
+	`--object-format=` in linkgit:git-init[1]. Both the command line option
+	and the `GIT_DEFAULT_HASH` environment variable take precedence over
+	this config.
diff --git a/setup.c b/setup.c
index 5dfcdc99dd..770ad1393f 100644
--- a/setup.c
+++ b/setup.c
@@ -2284,11 +2284,49 @@ static void separate_git_dir(const char *git_dir, const char *git_link)
 	write_file(git_link, "gitdir: %s", git_dir);
 }
 
+struct default_format_config {
+	int hash;
+};
+
+static int read_default_format_config(const char *key, const char *value,
+				      const struct config_context *ctx UNUSED,
+				      void *payload)
+{
+	struct default_format_config *cfg = payload;
+	char *str = NULL;
+	int ret;
+
+	if (!strcmp(key, "init.defaultobjectformat")) {
+		ret = git_config_string(&str, key, value);
+		if (ret)
+			goto out;
+		cfg->hash = hash_algo_by_name(str);
+		if (cfg->hash == GIT_HASH_UNKNOWN)
+			warning(_("unknown hash algorithm '%s'"), str);
+		goto out;
+	}
+
+	ret = 0;
+out:
+	free(str);
+	return ret;
+}
+
 static void repository_format_configure(struct repository_format *repo_fmt,
 					int hash, enum ref_storage_format ref_format)
 {
+	struct default_format_config cfg = {
+		.hash = GIT_HASH_UNKNOWN,
+	};
+	struct config_options opts = {
+		.respect_includes = 1,
+		.ignore_repo = 1,
+		.ignore_worktree = 1,
+	};
 	const char *env;
 
+	config_with_options(read_default_format_config, &cfg, NULL, NULL, &opts);
+
 	/*
 	 * If we already have an initialized repo, don't allow the user to
 	 * specify a different algorithm, as that could cause corruption.
@@ -2304,6 +2342,8 @@ static void repository_format_configure(struct repository_format *repo_fmt,
 		if (env_algo == GIT_HASH_UNKNOWN)
 			die(_("unknown hash algorithm '%s'"), env);
 		repo_fmt->hash_algo = env_algo;
+	} else if (cfg.hash != GIT_HASH_UNKNOWN) {
+		repo_fmt->hash_algo = cfg.hash;
 	}
 	repo_set_hash_algo(the_repository, repo_fmt->hash_algo);
 
diff --git a/t/t0001-init.sh b/t/t0001-init.sh
index 795408e16c..cd34710f32 100755
--- a/t/t0001-init.sh
+++ b/t/t0001-init.sh
@@ -523,6 +523,56 @@ test_expect_success 'init honors --object-format' '
 	test_cmp expected actual
 '
 
+test_expect_success 'init honors init.defaultObjectFormat' '
+	test_when_finished "rm -rf sha1 sha256" &&
+
+	test_config_global init.defaultObjectFormat sha1 &&
+	(
+		sane_unset GIT_DEFAULT_HASH &&
+		git init sha1 &&
+		git -C sha1 rev-parse --show-object-format >actual &&
+		echo sha1 >expected &&
+		test_cmp expected actual
+	) &&
+
+	test_config_global init.defaultObjectFormat sha256 &&
+	(
+		sane_unset GIT_DEFAULT_HASH &&
+		git init sha256 &&
+		git -C sha256 rev-parse --show-object-format >actual &&
+		echo sha256 >expected &&
+		test_cmp expected actual
+	)
+'
+
+test_expect_success 'init warns about invalid init.defaultObjectFormat' '
+	test_when_finished "rm -rf repo" &&
+	test_config_global init.defaultObjectFormat garbage &&
+
+	echo "warning: unknown hash algorithm ${SQ}garbage${SQ}" >expect &&
+	git init repo 2>err &&
+	test_cmp expect err &&
+
+	git -C repo rev-parse --show-object-format >actual &&
+	echo $GIT_DEFAULT_HASH >expected &&
+	test_cmp expected actual
+'
+
+test_expect_success '--object-format overrides GIT_DEFAULT_HASH' '
+	test_when_finished "rm -rf repo" &&
+	GIT_DEFAULT_HASH=sha1 git init --object-format=sha256 repo &&
+	git -C repo rev-parse --show-object-format >actual &&
+	echo sha256 >expected
+'
+
+test_expect_success 'GIT_DEFAULT_HASH overrides init.defaultObjectFormat' '
+	test_when_finished "rm -rf repo" &&
+	test_config_global init.defaultObjectFormat sha1 &&
+	GIT_DEFAULT_HASH=sha256 git init repo &&
+	git -C repo rev-parse --show-object-format >actual &&
+	echo sha256 >expected
+'
+
 test_expect_success 'extensions.objectFormat is not allowed with repo version 0' '
 	test_when_finished "rm -rf explicit-v0" &&
 	git init --object-format=sha256 explicit-v0 &&
-- 
2.46.0.46.g406f326d27.dirty

[PATCH v2 5/5] setup: make ref storage format configurable via config

[Patrick Steinhardt]

Similar to the preceding commit, introduce a new "init.defaultRefFormat"
config that allows the user to globally set the ref storage format used
by newly created repositories.

Signed-off-by: Patrick Steinhardt <ps@pks.im>
---
 Documentation/config/init.txt |  5 ++++
 setup.c                       | 14 +++++++++++
 t/t0001-init.sh               | 44 +++++++++++++++++++++++++++++++++++
 3 files changed, 63 insertions(+)

diff --git a/Documentation/config/init.txt b/Documentation/config/init.txt
index d6f8b6e61b..e45b2a8121 100644
--- a/Documentation/config/init.txt
+++ b/Documentation/config/init.txt
@@ -13,3 +13,8 @@ endif::[]
 	`--object-format=` in linkgit:git-init[1]. Both the command line option
 	and the `GIT_DEFAULT_HASH` environment variable take precedence over
 	this config.
+`init.defaultRefFormat`::
+	Allows overriding the default ref storage format for new repositories.
+	See `--ref-format=` in linkgit:git-init[1]. Both the command line
+	option and the `GIT_DEFAULT_REF_FORMAT` environment variable take
+	precedence over this config.
diff --git a/setup.c b/setup.c
index 770ad1393f..dd2251f655 100644
--- a/setup.c
+++ b/setup.c
@@ -2286,6 +2286,7 @@ static void separate_git_dir(const char *git_dir, const char *git_link)
 
 struct default_format_config {
 	int hash;
+	enum ref_storage_format ref_format;
 };
 
 static int read_default_format_config(const char *key, const char *value,
@@ -2306,6 +2307,16 @@ static int read_default_format_config(const char *key, const char *value,
 		goto out;
 	}
 
+	if (!strcmp(key, "init.defaultrefformat")) {
+		ret = git_config_string(&str, key, value);
+		if (ret)
+			goto out;
+		cfg->ref_format = ref_storage_format_by_name(str);
+		if (cfg->ref_format == REF_STORAGE_FORMAT_UNKNOWN)
+			warning(_("unknown ref storage format '%s'"), str);
+		goto out;
+	}
+
 	ret = 0;
 out:
 	free(str);
@@ -2317,6 +2328,7 @@ static void repository_format_configure(struct repository_format *repo_fmt,
 {
 	struct default_format_config cfg = {
 		.hash = GIT_HASH_UNKNOWN,
+		.ref_format = REF_STORAGE_FORMAT_UNKNOWN,
 	};
 	struct config_options opts = {
 		.respect_includes = 1,
@@ -2359,6 +2371,8 @@ static void repository_format_configure(struct repository_format *repo_fmt,
 		if (ref_format == REF_STORAGE_FORMAT_UNKNOWN)
 			die(_("unknown ref storage format '%s'"), env);
 		repo_fmt->ref_storage_format = ref_format;
+	} else if (cfg.ref_format != REF_STORAGE_FORMAT_UNKNOWN) {
+		repo_fmt->ref_storage_format = cfg.ref_format;
 	}
 	repo_set_ref_storage_format(the_repository, repo_fmt->ref_storage_format);
 }
diff --git a/t/t0001-init.sh b/t/t0001-init.sh
index cd34710f32..0178aa62a4 100755
--- a/t/t0001-init.sh
+++ b/t/t0001-init.sh
@@ -620,6 +620,19 @@ test_expect_success 'init with GIT_DEFAULT_REF_FORMAT=garbage' '
 	test_cmp expect err
 '
 
+test_expect_success 'init warns about invalid init.defaultRefFormat' '
+	test_when_finished "rm -rf repo" &&
+	test_config_global init.defaultRefFormat garbage &&
+
+	echo "warning: unknown ref storage format ${SQ}garbage${SQ}" >expect &&
+	git init repo 2>err &&
+	test_cmp expect err &&
+
+	git -C repo rev-parse --show-ref-format >actual &&
+	echo $GIT_DEFAULT_REF_FORMAT >expected &&
+	test_cmp expected actual
+'
+
 backends="files reftable"
 for format in $backends
 do
@@ -650,6 +663,27 @@ do
 		git -C refformat rev-parse --show-ref-format >actual &&
 		test_cmp expect actual
 	'
+
+	test_expect_success "init with init.defaultRefFormat=$format" '
+		test_when_finished "rm -rf refformat" &&
+		test_config_global init.defaultRefFormat $format &&
+		(
+			sane_unset GIT_DEFAULT_REF_FORMAT &&
+			git init refformat
+		) &&
+
+		echo $format >expect &&
+		git -C refformat rev-parse --show-ref-format >actual &&
+		test_cmp expect actual
+	'
+
+	test_expect_success "--ref-format=$format overrides GIT_DEFAULT_REF_FORMAT" '
+		test_when_finished "rm -rf refformat" &&
+		GIT_DEFAULT_REF_FORMAT=garbage git init --ref-format=$format refformat &&
+		echo $format >expect &&
+		git -C refformat rev-parse --show-ref-format >actual &&
+		test_cmp expect actual
+	'
 done
 
 test_expect_success "--ref-format= overrides GIT_DEFAULT_REF_FORMAT" '
@@ -660,6 +694,16 @@ test_expect_success "--ref-format= overrides GIT_DEFAULT_REF_FORMAT" '
 	test_cmp expect actual
 '
 
+test_expect_success "GIT_DEFAULT_REF_FORMAT= overrides init.defaultRefFormat" '
+	test_when_finished "rm -rf refformat" &&
+	test_config_global init.defaultRefFormat files &&
+
+	GIT_DEFAULT_REF_FORMAT=reftable git init refformat &&
+	echo reftable >expect &&
+	git -C refformat rev-parse --show-ref-format >actual &&
+	test_cmp expect actual
+'
+
 for from_format in $backends
 do
 	test_expect_success "re-init with same format ($from_format)" '
-- 
2.46.0.46.g406f326d27.dirty

Re: [PATCH v2 0/5] Introduce configs for default repo format

[Justin Tobler]

On 24/08/16 10:56AM, Patrick Steinhardt wrote:
> Hi,
> 
> this is the second version of my patch series that introduces two new
> config settings `init.defaultRefFormat` and `init.defaultObjectFormat`
> to make the default formats configurable without environment variables.
> 
> Changes compared to v1:
> 
>   - Extend commit message to mention that we also move
>     `repo_set_ref_storage_format()` and `repo_set_hash_algo()` into
>     `repository_format_configure()`.
> 
>   - Extend commit message to explain precedence.
> 
>   - Fix a grammar issue.
> 
>   - Fix a copy/paste error in the documentation of
>     `init.defaultRefFormat`.
> 
> Thanks!
> 
> Patrick

All of my comments have been addressed with this version of the patch
series. This version looks good to me. Thanks

-Justin

Re: [PATCH v2 4/5] setup: make object format configurable via config

[Junio C Hamano]

Patrick Steinhardt <ps@pks.im> writes:

> The precedence order now is the following, where the first one wins:
>
>   1. The `--object-format=` switch.
>
>   2. The `GIT_DEFAULT_HASH` environment variable.
>
>   3. The `init.defaultObjectFormat` config variable.
>
> This matches the typical precedence order we use in Git. We typically
> let the environment override the config such that the latter can easily
> be overridden on an ephemeral basis, for example by scripts.

Thanks for documenting this, as the reason for deciding the
precedence between environment and configuration is no longer as
strong as it used to be.

Before the "git -c var=val" happened, the only sensible preference
order was to allow environment to override a configured default by
doing "VAR=val git cmd ...".  In other words, the order used to be
something you could safely forget and still infer the right order
with logic alone.

But with "git -c var=val", even if a hypothetical Git allowed
configuration to override environment, a user could still override
them with "-c var=val", so "environment trumps configuration" is no
longer "because there is no sensible workaround if it were the other
way around", but merely "because that used to be the only logical
way".