From: brian m. carlson Date: Tue, 14 Oct 2025 21:18:36 GMT Subject: Re: [PATCH] [PATCH] [Outreachy] builtin/patch-id.c: clarify SHA1 usage for patch IDs Message-ID: In-Reply-To: <20251013174658.236940-1-okhuomonajayi54@gmail.com> 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 > --- > 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