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

[Taylor Blau <me@ttaylorr.com>]

On Tue, Nov 05, 2024 at 03:05:03AM +0000, Derrick Stolee via GitGitGadget wrote:
> 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