From: Okhuomon Ajayi Date: Tue, 14 Oct 2025 08:04:53 GMT Subject: Re: [PATCH] [PATCH] [Outreachy] builtin/patch-id.c: clarify SHA1 usage for patch IDs Message-ID: In-Reply-To: Thanks for the explanation! I’ll update the patch so it doesn’t touch the_hash_algo globally, and instead uses SHA1 only for the patch-ID computation itself. I’ll also tweak the comment to make it clear that this is just part of the bigger work to standardize patch-ID handling across Git On Tue, Oct 14, 2025 at 4:00 AM Junio C Hamano wrote: > > Okhuomon Ajayi writes: > > > 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. > > I do not think that is what the comment suggests to do. > > Read it again: > > ... should be removed in favor of converting the code that > computes patch IDs to always use SHA1. > > There are code paths that compute patch IDs elsewhere, and they are > not immediately below the NEEDSWORK comment. The code relies on > the_hash_algo and that is why the "hack" makes repo_set_hash_algo() > call. The suggestion is to convert that code that hashes patch to > compute patch IDs not to use the_hash_algo that is repository > dependeant. I think get_one_pathcid() function in the same file is > one of them. > > > 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). > > And if it is already set? I think what your first paragraph claims > to be problematic is that case, and the patch does not touch that > case at all. > > Blindly setting the_hash_algo to SHA-1 may not be the end of the > "solution", so whoever wants to work on this needs to be extra > careful. If the code after this point, starting from the call to > generate_id_list() we see in the post context, need to touch any Git > objects in the current repository (e.g., to obtain patch text or > some configuration data), such accesses need to use the hash that > the repository uses. Only the final "now we have this patch, and we > learned what the configuration says how we should compute the > patch-id. Let's hash the patch text following the specified > algorithm" step should use SHA-1 as the hash algorithm. > > Perhaps we are lucky that this program has *no* need to access > objects in the repository (my quick scan says this seems to work on > an external text file and does not generate diffs locally out of > objects), and it may not depend on configuration data coming from > any objects in the repository (there are some configuration variables > whose values are blob object names that instructs Git to read such > an object). In such a case, then the solution may be to always > make the code ignore the_hash_algo and unconditionally using SHA1.