Re: [PATCH] [PATCH] [Outreachy] builtin/patch-id.c: clarify SHA1 usage for patch IDs

["brian m. carlson" <sandals@crustytoothpaste.net>]

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

On 2025-10-13 at 17:46:58, Okhuomon Ajayi wrote:
> Patch IDs in Git must always use SHA1, regardless of the repository's
> object hash. Previously, the code relied on `the_hash_algo` which could
> vary depending on the repository, and included a NEEDSWORK comment
> suggesting this should be fixed.
> 
> This patch updates the comment to clearly state that SHA1 is required
> for patch IDs and sets the hash algorithm to SHA1 if it is not already
> set. This ensures consistent computation of patch IDs in accordance
> with git-patch-id(1).
> 
> No functional behavior is changed, but misleading comments are removed
> and the code now explicitly enforces correct SHA1 usage for patch IDs.
> 
> Signed-off-by: Okhuomon Ajayi <okhuomonajayi54@gmail.com>
> ---
>  builtin/patch-id.c | 11 +++--------
>  1 file changed, 3 insertions(+), 8 deletions(-)
> 
> diff --git a/builtin/patch-id.c b/builtin/patch-id.c
> index d26e9d0c1e..d47b6f5a3f 100644
> --- a/builtin/patch-id.c
> +++ b/builtin/patch-id.c
> @@ -246,16 +246,11 @@ int cmd_patch_id(int argc,
>  			     patch_id_usage, 0);
>  
>  	/*
> -	 * 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.
> +	 * Patch IDs must always use SHA1, regardless of the repository's
> +	 * object hash, See git-patch-id(1) for details. 
>  	 */
>  	if (!the_hash_algo)
> -		repo_set_hash_algo(the_repository, GIT_HASH_DEFAULT);
> +		repo_set_hash_algo(the_repository, GIT_HASH_SHA1);

Hmmm.  If I run git patch-id in a SHA-256 repository, then I get a
SHA-256 output here and it's worked this way since Git 2.29.

I know the comment says what it says, but I personally disagree with
this approach.  There will be a point in time where SHA-1 is so weak as
to be useless and people will want to build a Git version without it.
For instance, many government agencies around the world have a 2030
deadline for completely stopping all use of SHA-1.  If we continue to
use SHA-1 here, then this will have to change anyway in a few years, so
we'd be better off keeping the default algorithm for now and adding an
option to control which hash is used.
-- 
brian m. carlson (they/them)
Toronto, Ontario, CA

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