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

From: Patrick Steinhardt <ps@pks.im>
Date: Apr 3, 2025, 05:10 UTC
Message-ID: <Z-4YP5ppG8Jnrt3z@pks.im>
In-Reply-To: <46xnrbtafdg2iafownzujbpzjuygpns4g4r26wzxxv6xmi5mg7@m3dw6cfk6f7q>

On Wed, Apr 02, 2025 at 10:59:16AM -0500, Justin Tobler wrote:

Show 27 quoted lines

> On 25/04/02 09:38AM, Patrick Steinhardt wrote:
> > On Tue, Apr 01, 2025 at 03:36:30PM -0500, Justin Tobler wrote:
> > > 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?
> 
> It is suggested later to rename "non-crypto-SHA-1" to "SHA-1 without
> collision detection", which could lead to something like this:
> 
>     SHA-1: SHA1_OPENSSL (No collision detection)
>     SHA-1 without collision detection: none
> 
> which could be a bit misleading IMO. It might be best to leave the
> option omitted if it is not defined.

The problem of leaving the info away entirely is that it also makes it undiscoverable. Anyway -- I think it would be nice to always print this line and improve the format a bit to make it less awkward, but I won't resist if you decide to leave it as-is. After all we're already showing strictly more information than before, so it's a net win regardless.

Patrick