Re: [PATCH v4 4/5] builtin/hash-object: fix uninitialized hash function

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

Junio C Hamano <gitster@pobox.com> writes:

> Junio C Hamano <gitster@pobox.com> writes:
>
>> From: Patrick Steinhardt <ps@pks.im>
>>
>> The git-hash-object(1) command allows users to hash an object even
>> without a repository. Starting with c8aed5e8da (repository: stop setting
>> SHA1 as the default object hash, 2024-05-07), this will make us hit an
>> uninitialized hash function, which subsequently leads to a segfault.
>>
>> Fix this by falling back to SHA-1 explicitly when running outside of
>> a Git repository. Users can use GIT_DEFAULT_HASH environment to
>> specify what hash algorithm they want, so arguably this code should
>> not be needed.
>
> By the way, this breaks the expectation of t1007 and t1517 when run
> with GIT_DEFAULT_HASH=sha256 (I recall that they passed with the
> earlier iteration of my "fall back to GIT_DEFAULT_HASH" in [1/5],
> but we decided abusing GIT_DEFAULT_HASH is a bad idea).

The breakage in 1517 turns out to be a thinko on my part.  Outside a
repository, we do use SHA-1 with our "if the_hash_algo is not set
yet, default to SHA-1" in patch-id and hash-object but the way I
prepared the expected output was to use whatever GIT_TEST_DEFAULT_HASH
was in use.  A fix would be to go outside a repository and force the
hash with GIT_DEFAULT_HASH to sha1 when preparing the expected output.

I haven't looked at the breakage in 1007 yet, though.

diff --git i/t/t1517-outside-repo.sh w/t/t1517-outside-repo.sh
index 6f32a40c6d..6363c8e3c4 100755
--- i/t/t1517-outside-repo.sh
+++ w/t/t1517-outside-repo.sh
@@ -21,22 +21,24 @@ test_expect_success 'set up a non-repo directory and test file' '
 	git diff >sample.patch
 '
 
-test_expect_success 'compute a patch-id outside repository' '
-	git patch-id <sample.patch >patch-id.expect &&
+test_expect_success 'compute a patch-id outside repository (default to SHA-1)' '
 	(
 		cd non-repo &&
-		git patch-id <../sample.patch >../patch-id.actual
-	) &&
-	test_cmp patch-id.expect patch-id.actual
+		GIT_DEFAULT_HASH=sha1 \
+		git patch-id <../sample.patch >patch-id.expect &&
+		git patch-id <../sample.patch >patch-id.actual &&
+		test_cmp patch-id.expect patch-id.actual
+	)
 '
 
-test_expect_success 'hash-object outside repository' '
-	git hash-object --stdin <sample.patch >hash.expect &&
+test_expect_success 'hash-object outside repository (default to SHA-1)' '
 	(
 		cd non-repo &&
-		git hash-object --stdin <../sample.patch >../hash.actual
-	) &&
-	test_cmp hash.expect hash.actual
+		GIT_DEFAULT_HASH=sha1 \
+		git hash-object --stdin <../sample.patch >hash.expect &&
+		git hash-object --stdin <../sample.patch >hash.actual &&
+		test_cmp hash.expect hash.actual
+	)
 '
 
 test_expect_success 'apply a patch outside repository' '