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

From: Patrick Steinhardt <ps@pks.im>
Date: May 14, 2024, 04:32 UTC
Message-ID: <ZkLpSW6TNcpKpg-V@tanuki>
In-Reply-To: <20240514011437.3779151-2-gitster@pobox.com>

On Mon, May 13, 2024 at 06:14:33PM -0700, Junio C Hamano wrote: [snip]

Show 35 quoted lines

> 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