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

[Patrick Steinhardt <ps@pks.im>]

[-- Attachment #1: Type: text/plain, Size: 2361 bytes --]

On Tue, May 14, 2024 at 10:19:01AM -0700, Junio C Hamano wrote:
> Junio C Hamano <gitster@pobox.com> writes:
> 
> > Partially revert c8aed5e8 (repository: stop setting SHA1 as the
> > default object hash, 2024-05-07), to keep end-user systems still
> > broken when we have gap in our test coverage but yet give them an
> > escape hatch to set the GIT_DEFAULT_HASH environment variable to
> > "sha1" in order to revert to the previous behaviour.  This variable
> > has been in use for using SHA-256 hash by default, and it should be
> > a better fit than inventing a new and test-only knob.
> 
> Having done all of this, I actually am very tempted to add the
> "always default to SHA-1" back as a fallback position to the
> set_default_hash_algo() function.  We know we are going to get the
> right hash algorithm when working in the repository, so the only
> case the default matters in practice is when working outside the
> repository.
> 
> We already have such a custom code for "git diff --no-index", and we
> are adding a few more back in here, but they can disappear if we had
> code to set the fallback default when GIT_DEFAULT_HASH does not
> exist here.  The "always use SHA-1 regardless of the hash used by
> the repository" code like "patch-id" should not depend on such a
> fallback default but should have its own code to explicitly set it.
> 
> As the user can tweak what algorithm they want if the wanted to, it
> does not sound too bad to have a fallback default when the user did
> not choose.

I think that this is going into the wrong direction. The patches that we
have added surface real issues. If we now unconditionally add back the
fallback to SHA-1, then we only punt those issues further down the road
to the point in time where we drop `the_hash_algo` and/or
`the_repository`. All the issues that were surfaced until now are
technical debt, and the proposed fixes have been documented with a
"TODO" item that they need more work.

In some cases it's as simple as adding in an option to the respective
command that lets the user pick the hash algorithm, and I do not think
that GIT_DEFAULT_HASH is a proper replacement for that. In other cases
like for git-patch-id(1) the issue runs deeper and we need to refactor
the whole subsystem to stop relying on `the_hash_algo` altogether.

Patrick

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]