Re: [PATCH v3 3/5] builtin/patch-id: fix uninitialized hash function

[Patrick Steinhardt <ps@pks.im>]

[-- Attachment #1: Type: text/plain, Size: 1411 bytes --]

On Mon, May 13, 2024 at 04:11:01PM -0700, Junio C Hamano wrote:
> Junio C Hamano <gitster@pobox.com> writes:
[snip]
> > +	/*
> > +	 * We rely on `the_hash_algo` to compute patch IDs. This is dubious as
> > +	 * it means that the hash algorithm now depends on the object hash of
> > +	 * the repository, even though git-patch-id(1) clearly defines that
> > +	 * patch IDs always use SHA1.
> > +	 *
> > +	 * NEEDSWORK: This hack should be removed in favor of converting
> > +	 * the code that computes patch IDs to always use SHA1.
> > +	 */
> > +	if (!startup_info->have_repository)
> > +		repo_set_hash_algo(the_repository, GIT_HASH_SHA1);
> 
> Hmph, in other places I did
> 
> 	if (!the_hash_algo)
> 		repo_set_hash_algo(the_repository, GIT_HASH_SHA1);
> 
> to find the case where we need a reasonable default.
> 
> Is there a practical difference?  If there isn't we should
> standardise one and use the same test consistently everywhere.
> 
> Not that it matters for this particular case, where we in the longer
> term should be hardcoding the use of SHA-1 even in SHA-256 repository
> for the pupose of computing the patch-id.

To the best of my knowledge there isn't. What I prefer about my approach
is that it explicitly points out that this is conditional on whether or
not we have a repository. But in the end I don't mind much which of both
versions we use.

Patrick

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