search

AcademySoftwareFoundation / OpenColorIO

A color management framework for visual effects and animation.
https://opencolorio.org
BSD 3-Clause "New" or "Revised" License
1.76k stars 437 forks source link

zlib dependency requirement for 2.2.1 #1771

Closed remia closed 1 year ago

remia commented 1 year ago

Starting from version 2.2.1 OpenColorIO will no longer compile with zlib version < 1.2.13. I understand this was done following a CVE report on older versions of the library. I think we should accept earlier versions as well and keep 1.2.13 as the default version only when zlib is built in source, eg. with OCIO_INSTALL_EXT_PACKAGES=ALL.

Any opinion or things I may have missed?

KevinJW commented 1 year ago

This does follow what I thought we discussed in the TSC meetings:

If we are compiling without user supplied dependencies we use the recommended one, but if the user supplies their own and it is compatible then we should allow it to compile.

This was so that users on a particular VFX platform can update OCIO without having to update everything else.

doug-walker commented 1 year ago

@remia , yes totally agree. Cedrik has a PR to allow this which is almost finished.

remia commented 1 year ago

Thanks for the feedback Doug and Kevin, glad to know it's taking care of!

doug-walker commented 1 year ago

We've now merged #1777, which allows us to have both a min version and recommended version. Our min zlib version is now at 1.2.10. We could probably lower this but it would require some additional testing work. If anyone wants us to lower the min version further for zlib, please let us know what version you'd like and we will re-open the issue.

remia commented 1 year ago

Having a quick check at CentOS 7, which is still in use in some places, it comes with zlib 1.2.7, that could be reasonable minimum version?

cedrik-fuoco-adsk commented 1 year ago

I've tested zlib 1.2.7 on the different platforms, and there is an issue with their main CMakeLists.txt (line 169) for MacOS. The --version-script linker option does not exist for MacOS's version of ld.

Zlib 1.2.7 probably works fine on MacOS, but OCIO can not built it without a patch.

It was fixed for the release of zlib 1.2.8 (see line 206).

I would suggest to use 1.2.8 as I didn't see any errors with that version. If we really need to support CentOS 7 and the zlib version that comes with it, we will need to do a manually patch when OCIO build zlib itself.

remia commented 1 year ago

Thanks for the investigation @cedrik-fuoco-adsk, considering the build issue I agree 1.2.8 is fine to use and if someone wants to build in an older environment, a patch to change the minimal version in OCIO would be more appropriate to apply by the user directly.

doug-walker commented 1 year ago

Per agreement, the min ZLIB version is now 1.2.8.