From: Patrick Steinhardt <ps@pks.im>
Date: Wed, 24 Sep 2025 07:56:01 GMT
Subject: Re: [PATCH 8/9] Allow specifying compatibility hash
Message-ID: <aNOkEVUag91bv2RP@pks.im>
In-Reply-To: <20250919010911.649831-9-sandals@crustytoothpaste.net>

On Fri, Sep 19, 2025 at 01:09:10AM +0000, brian m. carlson wrote:

Tiny nit: it would help if the patch subject was prefixed with "t:".

> diff --git a/t/test-lib-functions.sh b/t/test-lib-functions.sh
> index a28de7b19b..52d7759bf5 100644
> --- a/t/test-lib-functions.sh
> +++ b/t/test-lib-functions.sh
> @@ -1708,11 +1708,16 @@ test_set_hash () {
>  # Detect the hash algorithm in use.
>  test_detect_hash () {
>  	case "${GIT_TEST_DEFAULT_HASH:-$GIT_TEST_BUILTIN_HASH}" in
> -	"sha256")
> +	*:*)
> +	    test_hash_algo="${GIT_TEST_DEFAULT_HASH%%:*}"
> +	    test_compat_hash_algo="${GIT_TEST_DEFAULT_HASH##*:}"
> +	    test_repo_compat_hash_algo="$test_compat_hash_algo"
> +	    ;;
> +	sha256)
>  	    test_hash_algo=sha256
>  	    test_compat_hash_algo=sha1
>  	    ;;
> -	*)
> +	sha1)
>  	    test_hash_algo=sha1
>  	    test_compat_hash_algo=sha256
>  	    ;;

Makes sense, I guess. It's a bit hard to judge without seeing any actual
tests, but I don't think that should hold off adding the infrastructure.
Worst case we can still adjust it at later point in time.

> diff --git a/t/test-lib.sh b/t/test-lib.sh
> index 621cd31ae1..14c777e4e2 100644
> --- a/t/test-lib.sh
> +++ b/t/test-lib.sh
> @@ -1917,6 +1917,13 @@ test_lazy_prereq DEFAULT_HASH_ALGORITHM '
>  test_lazy_prereq DEFAULT_REPO_FORMAT '
>  	test_have_prereq SHA1,REFFILES
>  '
> +# BROKEN_OBJECTS is a test if we can write deliberately broken objects and

s/if/whether/

> +# expect them to work.  When running using SHA-256 mode with SHA-1
> +# compatibility, we cannot write such objects because there's no SHA-1
> +# compatibility value for a nonexistent object.
> +test_lazy_prereq BROKEN_OBJECTS '
> +	test -z "$test_repo_compat_hash_algo"
> +'
>  
>  # Ensure that no test accidentally triggers a Git command
>  # that runs the actual maintenance scheduler, affecting a user's

Patrick