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 16, 2024, 15:31 UTC
Message-ID: <xmqqcyplu61f.fsf@gitster.g>
In-Reply-To: <xmqqikzg48hm.fsf@gitster.g>

Junio C Hamano <gitster@pobox.com> writes:

Show 6 quoted lines

> 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.

Not really. It does not add anything to help either real world or our tests. The current test setting is already bad enough in that, unlike in the real world settings, even tests with the SHA-1 algorithm has GIT_DEFAULT_HASH environment variable set, which means that such a "if the environment variable is not set, further fall back to SHA-1" does not do anything.

Unless we change t/test-lib.sh not to set GIT_DEFAULT_HASH tweaking the fallback default in repository.c:set_default_hash_algo() based on GIT_DEFAULT_HASH would not be a workable solution.

I wanted to arrange things so that the end-user exectuion by default has an extra fallback (perhaps to SHA-1, or GIT_DEFAULT_HASH) to avoid disrupting their real-world use, which we can disable in our tests to expose code paths that still rely on the "default" set when in-core repository struct gets initialized, but that is not possible without changing the way t/test-lib.sh uses GIT_DEFAULT_HASH, it seems. So the arrangement unfortunately has to be "we have no default, and bugs will break the real-world uses as well as tests the same way. The real-world users have to export an extra 'workaround' environment variable to force "default" to SHA-1 (or GIT_DEFAULT_HASH) --- which may be "workable" but very far from being intuitive. They can set GIT_DEFAULT_HASH but to make it effective everywhere, including the "default" given by set_default_hash_algo(), they need to set this other "workaround" thing.

> 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.

While I think a manual setting of the_hash_algo in "diff --no-index" code path should not hardcode "SHA-1" but instead use the hash specified by the GIT_DEFAULT_HASH environment to be consistent with the use of "git" by the same parent process that had that variable exported to the environment, that should not be done globally in repository.c:set_default_hash_algo().