Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

zlib dependency requirement for 2.2.1 #1771

Closed
remia opened this issue Feb 20, 2023 · 8 comments · Fixed by #1777
Closed

zlib dependency requirement for 2.2.1 #1771

remia opened this issue Feb 20, 2023 · 8 comments · Fixed by #1777

Comments

@remia
Copy link
Collaborator

remia commented Feb 20, 2023

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
Copy link
Contributor

KevinJW commented Feb 20, 2023

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
Copy link
Collaborator

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

@remia
Copy link
Collaborator Author

remia commented Feb 20, 2023

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

@doug-walker
Copy link
Collaborator

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
Copy link
Collaborator Author

remia commented Mar 24, 2023

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?

@doug-walker doug-walker reopened this Mar 24, 2023
@cedrik-fuoco-adsk
Copy link
Contributor

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
Copy link
Collaborator Author

remia commented Apr 7, 2023

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
Copy link
Collaborator

Per agreement, the min ZLIB version is now 1.2.8.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
4 participants