Thread (2 messages) 2 messages, 2 authors, 2025-10-14

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

From: brian m. carlson <hidden>
Date: 2025-10-14 21:18:44

On 2025-10-13 at 17:46:58, Okhuomon Ajayi wrote:
quoted hunk ↗ jump to hunk
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 <redacted>
---
 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

Attachments

Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help