On Tue, May 9, 2017 at 1:21 PM, Johan Corveleyn <jcorvel_at_gmail.com> wrote:
> On Tue, Apr 4, 2017 at 11:33 AM, Stefan Sperling <stsp_at_elego.de> wrote:
>> On Mon, Feb 20, 2017 at 09:05:25AM +0100, Bert Huijben wrote:
>>> This code is still in trunk without any of the discussed improvements, so
>>> this change is currently part of 1.10.0-alpha1.
>>>
>>> If we don't implement the improvements I think we should check if we want
>>> to revert to the 1.0-1.9 behavior before we really look at releasing 1.10.
>>>
>>> See discussion below
>>>
>>> Bert
>>
>> I think the proposed approach as implemented on trunk can no longer be
>> considered viable, unfortunately, because of this step:
>>
>>> > >>> 4. Calculate SHA-1 checksum of detranslated contents of working file
>>> > >>> and compare it with pristine's checksum stored in wc.db.
>>
>> Given that the SHA1 collision problem is real, we are now trying to stop
>> relying on hashes to compare content. So it does not make sense to add
>> new code which relies on hashes in this way, in my opinion.
>>
>> It seems that using SHA1 to compare content is key to the proposed approach.
>> If that is correct, then I don't agree with releasing 1.10 with this feature
>> and I would be in favour of reverting this change.
>>
>> Ivan, do you have any further comments on this thread? You have remained
>> silent for quite some time now :(
>
> Where are we with this? Seems the consensus is to revert r1759233 to
> not further increase our reliance on sha1? Or is there still a way to
> keep r1759233 in some way, and improve it to make the sha1 test
> "sensitive but not specific", like danielsh proposed?
>
> Ivan?
Hmmm, I'm wondering, now we decided to disallow sha1 collisions to
enter the repository, how does this reflect on this discussion?
Okay, it's another "collision-sensitive-dependency" on sha1, but there
are others in the working copy (pristines) and ra_serf, and since
we're assuming a non-sha1-collision world (by blocking them from the
repository), it makes little sense to me to simply revert this, and
not fix other sha1-dependencies. OTOH, if we want to gradually reduce
our dependence on sha1, this is an easy one to remove, since it's not
in production yet ... Dunno.
However, there were also other, performance-related, objections to the
new implementation. Ivan proposed improvements [1] but they were never
implemented.
So, what to do?
Unless someone objects in the coming couple of days, I'd say we revert r1759233.
[1] https://svn.haxx.se/dev/archive-2016-09/0016.shtml
--
Johan
Received on 2017-05-22 22:56:48 CEST