On Tue, May 14, 2024 at 10:19:01AM -0700, Junio C Hamano wrote: > Junio C Hamano <gitster@xxxxxxxxx> 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:
signature.asc
Description: PGP signature
