Re: [PATCH] [PATCH] [Outreachy] builtin/patch-id.c: clarify SHA1 usage for patch IDs
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 <gitster@pobox.com> wrote:
Show 51 quoted lines
>
> Okhuomon Ajayi <okhuomonajayi54@gmail.com> 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.