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.

Sign up for GitHub

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

Jump to bottom

OpenColorIO: Fix dynamic linking for OpenColorIO >= 2.3.1 & fix package_info #23112

Closed
wants to merge 4 commits into from
Closed

OpenColorIO: Fix dynamic linking for OpenColorIO >= 2.3.1 & fix package_info #23112

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
606e6be
Make the linking inside the OpenColorIO CMakeFile PUBLIC to ensure th…
irieger Mar 16, 2024
38d395c
Announce minizip only for newer versions as the v1.1.1 test package d…
irieger Mar 16, 2024
27ad6e2
Fix version for minizip requires
irieger Mar 16, 2024
5f51411
Merge remote-tracking branch 'cci-origin/master' into opencolorio/fix…
irieger Apr 2, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
18 changes: 17 additions & 1 deletion recipes/opencolorio/all/conanfile.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -94,7 +94,10 @@ def validate(self):

if Version(self.version) == "1.1.1" and self.options.shared and self.dependencies["yaml-cpp"].options.shared:
raise ConanInvalidConfiguration(f"{self.ref} requires static build yaml-cpp")
if Version(self.version) >= "2.2.1" and self.options.shared and self.dependencies["minizip-ng"].options.shared:
if Version(self.version) >= "2.2.1" and \
Version(self.version) < "2.3.1" and \
self.options.shared and \
self.dependencies["minizip-ng"].options.shared:
raise ConanInvalidConfiguration(f"{self.ref} requires static build minizip-ng")

def build_requirements(self):
Expand Down Expand Up @@ -188,6 +191,19 @@ def package_info(self):

self.cpp_info.libs = ["OpenColorIO"]

self.cpp_info.requires.append("expat::expat")
self.cpp_info.requires.append("yaml-cpp::yaml-cpp")
self.cpp_info.requires.append("openexr::openexr")
self.cpp_info.requires.append("lcms::lcms")
if Version(self.version) >= "2.0.0":
self.cpp_info.requires.append("pystring::pystring")
else:
self.cpp_info.requires.append("tinyxml::tinyxml")
if Version(self.version) >= "2.2.0":
self.cpp_info.requires.append("imath::imath")
if Version(self.version) >= "2.2.0":
self.cpp_info.requires.append("minizip-ng::minizip")

if Version(self.version) < "2.1.0":
if not self.options.shared:
self.cpp_info.defines.append("OpenColorIO_STATIC")
Expand Down
11 changes: 10 additions & 1 deletion recipes/opencolorio/all/patches/2.3.1-0001-fix-cmake-source-dir-and-targets.patch
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -59,9 +59,18 @@ index 175d89c2..ac93b87a 100644
include(CheckCXXCompilerFlag)

diff --git src/OpenColorIO/CMakeLists.txt src/OpenColorIO/CMakeLists.txt
index 26b4bb4c..d54cff3d 100755
index 26b4bb4c..f366bdd1 100755
--- src/OpenColorIO/CMakeLists.txt
+++ src/OpenColorIO/CMakeLists.txt
@@ -301,7 +301,7 @@ target_include_directories(OpenColorIO
)

target_link_libraries(OpenColorIO
- PRIVATE
+ PUBLIC # Change to public so that the tools properly link
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Has this been submitted upstream? This looks like something that would better live as an official change upstream, where it can be properly maintained

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm going to put up a pull request, but I'm currently working on getting into the CLA for the Linux Foundation which the upstream is part of. But I can also start the pull request later without the CLA standing and retrigger the check later. (And then reference the PR in the conandata)

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@irieger If you have a blocker with CLA because your job (for instance), don't worry, we can send the patch too. Thank you for checking it!

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No, the job is not a problem. It is more the other way round. Trying to get the CLA through my company (we have a company CLA there) as I don't like the Linux foundation requireing a physical address, which is why I refuse to sign it with my personal details. (An open source focused organisation like that having such a bad privacy statement is a red flag for me when asked for personal details.)

Me doing this is no problem for my company and although this contributions here are for my personal interest, I guess it is good to be active in open source projects from a company perspective anyway and I have some colleagues contributing to the Academy Software Foundation projects (ASFW is hosted - or whatever the right english word is here - by the Linux Foundation).

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Created a PR: AcademySoftwareFoundation/OpenColorIO#1958

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As this is anyway only a small part of the overall compatibility conan patching, I guess it doesn't make sense to add the upstream URL maybe?

expat::expat
Imath::Imath
pystring::pystring
@@ -309,8 +309,8 @@ target_link_libraries(OpenColorIO
"$<BUILD_INTERFACE:utils::from_chars>"
"$<BUILD_INTERFACE:utils::strings>"
Expand Down
Loading