dropbox / lepton

Lepton is a tool and file format for losslessly compressing JPEGs by an average of 22%.
https://blogs.dropbox.com/tech/2016/07/lepton-image-compression-saving-22-losslessly-from-images-at-15mbs/
Apache License 2.0
5.01k stars 355 forks source link

Existing zlib should be used whenever possible #18

Closed UnitedMarsupials-zz closed 8 years ago

UnitedMarsupials-zz commented 8 years ago

All modern Unix-like systems (BSDs, MacOS, Linux, Solaris) already come with zlib. Same is usually true about OpenSSL.

When building lepton on such systems, the dependencies/ folder should be completely ignored. In fact, it should be possible to not have it at all. The fix will be three-fold:

danielrh commented 8 years ago

when running this at scale we want to be super meticulous about running it with a pinned version of zlib. We don't want to have any surprises if some package gets an unexpected patch and suddenly the md5sum of a file might change when run through the same compression pass because zlib decided to repack things differently.

We could, however, add a configure flag to use the system zlib, I suppose.

UnitedMarsupials-zz commented 8 years ago

We don't want to have any surprises if some package gets an unexpected patch

I understand, but this is dangerous thinking -- where do you stop? Should every zlib-using software package bundle its own copy "just in case"? The approach escalates very quickly -- how about bundling a specific compiler?

We could, however, add a configure flag to use the system zlib, I suppose.

As a minimum, yes. I still think, it ought to be the default setting. If you are worried about regression-tests failing because of zlib-differences, make zlib's version (obtained by calling zlibVersion() at run time) part of the test-log -- the way you already output git's commit-tag.

danielrh commented 8 years ago

There's now a configure option to use the system crypto libs and libz fd84210340dcd64ee2738a9159034d6fb36bcdd3 enjoy!