Re: [PATCH 3/7] pack-objects: add GIT_TEST_FULL_NAME_HASH

From: Taylor Blau <me@ttaylorr.com>
Date: Nov 21, 2024, 20:15 UTC
Message-ID: <Zz+U8IyHqBNRIn6m@nand.local>
In-Reply-To: <259734e0bcea952c2c09b0fb3a017e139922b975.1730775908.git.gitgitgadget@gmail.com>

On Tue, Nov 05, 2024 at 03:05:03AM +0000, Derrick Stolee via GitGitGadget wrote:

Show 12 quoted lines

> diff --git a/ci/run-build-and-tests.sh b/ci/run-build-and-tests.sh
> index 2e28d02b20f..75b40f07bbd 100755
> --- a/ci/run-build-and-tests.sh
> +++ b/ci/run-build-and-tests.sh
> @@ -30,6 +30,7 @@ linux-TEST-vars)
>  	export GIT_TEST_NO_WRITE_REV_INDEX=1
>  	export GIT_TEST_CHECKOUT_WORKERS=2
>  	export GIT_TEST_PACK_USE_BITMAP_BOUNDARY_TRAVERSAL=1
> +	export GIT_TEST_FULL_NAME_HASH=1
>  	;;
>  linux-clang)
>  	export GIT_TEST_DEFAULT_HASH=sha1

Hmm. I appreciate what this new GIT_TEST_ variable is trying to do, but I am somewhat saddened to see this list in linux-TEST-vars growing rather than shrinking.

I'm most definitely part of the problem here, but I think too often we add new entries to this list and let them languish without ever removing them after they have served their intended purpose.

So I think the question is: what do we hope to get out of running the test suite in a mode where we use the full-name hash all of the time? I can't imagine any interesting breakage (other than individual tests' sensitivity to specific delta/base pairs) that would be caught by merely changing the hash function here.

I dunno. Maybe there is some exotic behavior that this shook out for you during development which I'm not aware of. If that were the case, I think that keeping this variable around makes sense, since the appearance of that exotic behavior proves that the variable is useful at shaking out bugs.

But assuming not, I think that I would just as soon avoid this test variable entirely, which I think in this case amounts to dropping this patch from the series.

Thanks, Taylor