On Wed, Oct 19 2022, Junio C Hamano wrote:
> Eric Sunshine <sunshine@xxxxxxxxxxxxxx> writes:
>
>>> diff --git a/Makefile b/Makefile
>>> +ifdef DC_SHA1
>>> +$(error the DC_SHA1 flag is no longer used, and has become the default. Adjust your build scripts accordingly)
>>> +endif
>>
>> bikeshedding: Do we really need to penalize (abuse) people merely for
>> asking us to do what we're already doing anyhow?
>
> A valid question.
>
> I can understand and very much appreciate [1/4] as a very focused
> fix to the problem. Very small part of this step, namely, make the
> DC_SHA1 the default everywhere, is also very much welcome.
>
> Everything else I see in these patches are extra "while we are at
> it" that should not exist. These "while at it" changes tend to
> somehow implement more subjective choices that will cause more
> discussion and take more review resources. Not all "white at it"
> may be more subjective, but at least in this series, they appear
> to be.
>
> They distract us from the core changes and slows us down. It is OK
> to do them as totally unrelated clean-up changes long after the dust
> settles, but not entangled with the other more important changes
> like these patches.
There's things I can eject from this series, but I don't really find it
to be "while at it" changes, I suspect what you're thinking of is
one/some of:
- Re-arranging the Makefile commentary into sections: I did that
because now the structure is very much one-paragraph-per-flag.
So before 2/4 there's no good place to put that "Alternate
implementations" in a way that clearly refers to the surrounding
SHA{1,256} discussion. But yeah, I could try harder to keep that diff
size down, or just not update the docs.
- We're still claiming that we use OpenSSL by default, so I fixed the
docs in general (not just the Makefile). Would you like this to be
just a "we forgot OSX?" series?
- Ditto asking to provide NO_DC_SHA1=Y now in addition to
e.g. BLK_SHA1=Y if you *really* want to use that
non-collision-detecting SHA-1 implementation.
E.g. BLK_SHA1=Y was added in 2009. It's a small one-time bother to
add NO_DC_SHA1=Y to your scripts if you *really* mean "I want the
less safe SHA-1".
But I think it's more likely that someone running into that error
will be happy that the default has changed in a strong way.
We've been *strongly* recommending sha1collisiondetection for a while
now, but the structure of the build system has been the exact
opposite, if you asked for anything else we'd avoid using it, and
only default to it if nothing else was picked.
- In particular the current flag on OSX is "APPLE_COMMON_CRYPTO", which
is now split up into that & "APPLE_SHA1".
Maybe changing our default flags is sufficient, and we don't need to
worry about 3rd party build scripts having the previous "Yeah, I have
that OS library" effectively meaning "...and use it for everything
possible, including SHA-1".
Or maybe you're OK with topic(s) at large, i.e. "switch the default,
update the docs, make sure noone's left behind", but would just like it
done in more incremental steps?
