Re: [PATCH] rerere: match the hash algorithm with its length

From: brian m. carlson <sandals@crustytoothpaste.net>
Date: Jul 23, 2023, 15:03 UTC
Message-ID: <ZL1BNxVWKGx0Gi1b@tapette.crustytoothpaste.net>
In-Reply-To: <xmqqa5vou9ar.fsf@gitster.g>

On 2023-07-21 at 23:36:12, Junio C Hamano wrote:

Show 8 quoted lines

> The "conflict ID" used by "git rerere" to identify past conflicts we
> saw has been a SHA-1 hash of the normalized text taken from the
> conflicted region.  0d7c419a (rerere: convert to use the_hash_algo,
> 2018-10-15) updated the rerere machinery to use more general "hash"
> instead of hardcoded SHA-1 by using the_hash_algo, GIT_MAX_RAWSZ and
> their friends, but the code that read from the MERGE_RR records were
> left unconverted to still use get_sha1_hex(), possibly breaking the
> operation in SHA-256 repositories.

I agree consistency here is a good idea. However, I should point out the definition of `get_sha1_hex`:

int get_sha1_hex(const char *hex, unsigned char *sha1)
{
	return get_hash_hex_algop(hex, sha1, the_hash_algo);
}

Thus `get_sha1_hex` uses `the_hash_algo`, and therefore your change is equivalent to what was there before, I believe. That's because during the SHA-256 code work, we could either send a bunch of patches to fix all of the instance of `get_sha1_hex` or we could just patch that function to use the default hash algorithm, and I, for better or for worse, made the decision to avoid the churn.

I still firmly agree that your change is better, because it is easier to read and less confusing, and that is a major improvement in itself. However, I would suggest that the commit message be updated to reflect that if possible.

-- 
brian m. carlson (he/him or they/them)
Toronto, Ontario, CA