Re: [PATCH] setup: default hash algo to GIT_HASH_DEFAULT

[Junio C Hamano <gitster@pobox.com>]

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

> When we have no other hash algorithm to choose, we set no hash
> algorithm, which ends up with us defaulting to SHA-1.  Instead, default
> to GIT_HASH_DEFAULT.  This means that we will choose SHA-256 in breaking
> changes mode and in Git 3.0, which is what we want.
>
> Reported-by: Adam Dinwoodie <adam@dinwoodie.org>
> Signed-off-by: brian m. carlson <sandals@crustytoothpaste.net>
> ---
>  setup.c         |  2 ++
>  t/t0001-init.sh | 16 ++++++++++++++++
>  2 files changed, 18 insertions(+)
>
> diff --git a/setup.c b/setup.c
> index 61bcdafe82..4a0dfc8908 100644
> --- a/setup.c
> +++ b/setup.c
> @@ -2830,6 +2830,8 @@ static void repository_format_configure(struct repository_format *repo_fmt,
>  			repo_fmt->hash_algo = env_algo;
>  	} else if (cfg.hash != GIT_HASH_UNKNOWN) {
>  		repo_fmt->hash_algo = cfg.hash;
> +	} else {
> +		repo_fmt->hash_algo = GIT_HASH_DEFAULT;
>  	}

Hmph, maybe I am not reading the code flow correctly, but does this
not break running "git init" to reinitialize an already initialized
repository?  I mimicked the new test in your patch and wrote the
following just immediately after it, which

 * figures out our hash and the other hash
 * create a new repository with the "other" hash
 * run "git init" without specifying any hash

It is debatable what should happen when you run "git init" from Git
3.0 without any specialization in an existing SHA-1 repository.

Possibilities include:

 1. turn an existing SHA-1 repository into SHA-256 including format
    conversion.

 2. notice that the repository is already set up to use the SHA-1
    format, and that the user did not explicitly say what format to
    use, and behave as if --object-format=sha1 were given with a
    notice or warning.

 3. notice that the repository is already set up to use the SHA-1
    format, but our default format is SHA-256.  Because the user did
    not countermand via an explicit --object-format=sha1 on the
    command line, we should fail loudly, "cannot convert this SHA-1
    repository to SHA-256".

I think #1 is impractical, #2 is probably the best, and #3 would
also be reasonable.

But the behaviour I am seeing with

    $ make WITH_BREAKING_CHANGES=YesPlease
    $ cd t && WITH_BREAKING_CHANGES=YesPlease sh t0001-init.sh -i -v

with the additional patch is different from any of the three (well,
technically, because the repository is empty, we _could_ argue that
we did #1, but ...).

I wonder what the "git init" command does wrt the reference storage
format (Patrick CC'ed for this)?  I am sure we do not convert from
reffiles to reftable on the fly at "git init" time.


diff --git i/t/t0001-init.sh w/t/t0001-init.sh
index bc30cf853b..1d48a712ae 100755
--- i/t/t0001-init.sh
+++ w/t/t0001-init.sh
@@ -532,6 +532,31 @@ test_expect_success 'git init uses default object format' '
 	)
 '
 
+test_expect_success 'git (re)init does not clobber object format' '
+	test_when_finished "rm -rf repo" &&
+	(
+		sane_unset GIT_DEFAULT_HASH &&
+		if test_have_prereq WITH_BREAKING_CHANGES
+		then
+			ours=sha256 opposite=sha1
+		else
+			ours=sha1 opposite=sha256
+		fi &&
+
+		# initialize to non-default format
+		git init --object-format=$opposite repo &&
+		git -C repo rev-parse --show-object-format >actual &&
+		echo "$opposite" >expect &&
+		test_cmp expect actual &&
+
+		# reinitialize without configuration
+		git init repo &&
+		git -C repo rev-parse --show-object-format >actual &&
+		echo "$opposite" >expect &&
+		test_cmp expect actual
+	)
+'
+
 test_expect_success 'init honors GIT_DEFAULT_HASH' '
 	test_when_finished "rm -rf sha1 sha256" &&
 	GIT_DEFAULT_HASH=sha1 git init sha1 &&