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

From: Junio C Hamano <gitster@pobox.com>
Date: Oct 14, 2025, 03:00 UTC
Message-ID: <xmqqecr6yypu.fsf@gitster.g>
In-Reply-To: <20251013174658.236940-1-okhuomonajayi54@gmail.com>

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.