Re: [PATCH v4 1/5] setup: add an escape hatch for "no more default hash algorithm" change

From: Junio C Hamano <gitster@pobox.com>
Date: May 14, 2024, 15:05 UTC
Message-ID: <xmqqseyk77sl.fsf@gitster.g>
In-Reply-To: <ZkLpSW6TNcpKpg-V@tanuki>

Patrick Steinhardt <ps@pks.im> writes:

> The problem with reusing "GIT_DEFAULT_HASH" is that we unconditionally
> set it in our test suite in "test-lib.sh". This will have the effect
> that we will never hit segfaults in our tests because we always end up
> setting up the default hash, whereas our users now will.

I tried the version you posted with only SHA-1 fallback and it failed with SHA256 CI jobs, and eventually came up with using the existing environment variable.

The variable is the perfect fit in the longer term for our purpose. It is what the end-users will set, not for papering over remaining bugs from the "no longer there is a fallback default" change, but for telling Git that they want to use sha256 repositories.

With GIT_DEFAULT_HASH set to sha256, we will be testing a configuration that is very close to those end-users.

There probably are four cases we need to check:

 - In a repository, invocations of commands that should honor the
   hash function that is in use by the repository (i.e. this is true
   for most commands and their invocations).

 - In a repository, invocations of commands that should ignore the
   repository settings and always use SHA-1 (i.e. proposed fix to
   patch-id).

 - Outside a repository, invocations with GIT_DEFAULT_HASH set
   should probably parallel the above two, as if GIT_DEFAULT_HASH
   came from the repository.

 - Outside a repository, invocations without GIT_DEFAULT_HASH set
   should all default to SHA-1???

The last combination cannot be tested UNLESS we unset GIT_DEFAULT_HASH that is given by test-lib.sh, but that can easily be arranged, now we have one primary/central place that tests out-of-repository invocations of various commands in t1517.

And using the existing environment variable has an added benefit that we do not have to add an extra option to random commands.