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

From: Junio C Hamano <[email-masked]>
Date: Thu, 16 May 2024 08:31:56 -0700

Junio C Hamano <[email-masked]> writes:

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