Re: [PATCH v2 2/2] help: include unsafe SHA-1 build info in version

[Patrick Steinhardt <ps@pks.im>]

On Tue, Apr 01, 2025 at 03:36:30PM -0500, Justin Tobler wrote:
> diff --git a/Documentation/git-version.adoc b/Documentation/git-version.adoc
> index f06758a7cf..753794988c 100644
> --- a/Documentation/git-version.adoc
> +++ b/Documentation/git-version.adoc
> @@ -25,6 +25,9 @@ OPTIONS
>  +
>  Note that the SHA1 options `SHA1_APPLE`, `SHA1_OPENSSL`, and `SHA1_BLK` do not
>  have collision detection.
> ++
> +If built to use a faster SHA-1 implementation for non-cryptographic purposes,
> +that implementation is denoted as "non-crypto-SHA-1".
>  
>  GIT
>  ---

I got basically the same comment for this new paragraph as for the first
one. I'd either drop it or expand it so that readers know what's going
on.

> diff --git a/help.c b/help.c
> index 3aebfb3681..1238a962b0 100644
> --- a/help.c
> +++ b/help.c
> @@ -772,6 +772,11 @@ char *help_unknown_cmd(const char *cmd)
>  static void get_sha_impl(struct strbuf *buf)
>  {
>  	strbuf_addf(buf, "SHA-1: %s\n", SHA1_BACKEND);
> +
> +#if defined(SHA1_UNSAFE_BACKEND)
> +	strbuf_addf(buf, "non-crypto-SHA-1: %s\n", SHA1_UNSAFE_BACKEND);
> +#endif
> +

Should we maybe print the equivalent of "none" in case no unsafe backend
was selected?

I also think we shouldn't name this "non-crypto". The backend still is
SHA1, which is a proper cryptogtaphic hash function. It may be somewhat
broken nowadays, but that doesn't change the fact that it's a
cryptographic primitive.

How about we rename this to "SHA-1 without collision detection:"?

Patrick