Subject: Re: [PATCH 2/2] Windows: stop supplying BLK_SHA1=YesPlease by default

From: Ævar Arnfjörð Bjarmason <[email-masked]>
Date: Thu, 28 Dec 2017 14:47:43 +0100

On Wed, Dec 27 2017, Jonathan Nieder jotted:

> +git-for-windows
> Ævar Arnfjörð Bjarmason wrote:
>
>> Using BLK_SHA1 in lieu of the OpenSSL routines was done in [1], but
>> since DC_SHA1 is now the default for git in general it makes sense for
>> Windows to use that too, this looks like something that was missed
>> back in [2].
>>
>> As noted in [3] OpenSSL has a performance benefit compared to BLK_SHA1
>> on MinGW, so perhaps that and the Windows default should be changed
>> around again, but that's a topic for another series, it seems clear
>> that this specific flag is nobody's explicit intention.
>
> I have some memory of performance issues on Windows when DC_SHA1 was
> introduced leading to interest in a mixed configuration with DC_SHA1
> only being used where it is security sensitive (e.g. for object naming
> but not for packfile trailers).
>
> Did anything come of that?

This was Johannes Schindelin (CC'd) on-list when the sha1dc discussion
first came up earlier this year. I.e. it's slower, so we could use
openssl on trusted data and sha1dc on untrusted data, but nothing came
of that.

> In any event removing this BLK_SHA1 setting looks like a good change
> to me, but I'd rather that Windows folks weigh in.

Yup, will CC them + Johannes for v2.

> Thanks,
> Jonathan
>
> [...]
>> --- a/config.mak.uname
>> +++ b/config.mak.uname
>> @@ -361,7 +361,6 @@ ifeq ($(uname_S),Windows)
>>  	NO_REGEX = YesPlease
>>  	NO_GETTEXT = YesPlease
>>  	NO_PYTHON = YesPlease
>> -	BLK_SHA1 = YesPlease
>>  	ETAGS_TARGET = ETAGS
>>  	NO_INET_PTON = YesPlease
>>  	NO_INET_NTOP = YesPlease