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/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.
Ya, this should also be expanded a bit. I think in combination with the
expanded documentation for the prior patch, something like this might be
a bit better.
"When a faster SHA-1 implementation without collision detection is used
for only non-cryptographic purposes, the algorithm is diplayed in the form
`non-collision-detecting-SHA-1: <option>`."
> > 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.
> 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.
I was trying to indicate that this SHA-1 backend was used only in
non-cryptographic scenarios, but I agree that this name is not great.
Calling it "SHA-1 used for non-cryptographic purposes" is a bit of a
mouthful, but maybe that is fine?
Another idea I had was to call it "fast-SHA-1:" since it's intended as a
performance optimization used in certain cases.
> How about we rename this to "SHA-1 without collision detection:"?
Being verbose here is probably best. I'll probably use something like
"non-collision-detecting-SHA-1:" in the next version.
-Justin
