On Tue, May 9, 2017 at 1:21 PM, Johan Corveleyn <jcor...@gmail.com> wrote: > On Tue, Apr 4, 2017 at 11:33 AM, Stefan Sperling <s...@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
