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: 1762 bytes --]

On Mon, May 13, 2024 at 06:14:33PM -0700, Junio C Hamano wrote:
[snip]
> diff --git a/repository.c b/repository.c
> index 15c10015b0..f912ee9a7c 100644
> --- a/repository.c
> +++ b/repository.c
> @@ -1,5 +1,6 @@
>  #include "git-compat-util.h"
>  #include "abspath.h"
> +#include "environment.h"
>  #include "repository.h"
>  #include "object-store-ll.h"
>  #include "config.h"
> @@ -19,6 +20,27 @@
>  static struct repository the_repo;
>  struct repository *the_repository = &the_repo;
>  
> +static void set_default_hash_algo(struct repository *repo)
> +{
> +	const char *hash_name;
> +	int algo;
> +
> +	hash_name = getenv(GIT_DEFAULT_HASH_ENVIRONMENT);
> +	if (!hash_name)
> +		return;
> +	algo = hash_algo_by_name(hash_name);
> +
> +	/*
> +	 * NEEDSWORK: after all, falling back to SHA-1 by assigning
> +	 * GIT_HASH_SHA1 to algo here, instead of returning, may give
> +	 * us better behaviour.
> +	 */
> +	if (algo == GIT_HASH_UNKNOWN)
> +		return;
> +
> +	repo_set_hash_algo(repo, algo);
> +}

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 would propose to revert this back to the first iteration you had,
where the workaround only enables the SHA1 fallback. No users have yet
complained about the inability to pick the hash algo outside of a repo,
indicating that it's not widely used. And when they complain, there is
more motivation to fix this properly by adding a `--object-hash=` switch
to the respective commands so that a user can pick the desired object
hash.

Patrick

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