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

v2.3.0 Python bindings source builds failing to load libOpenColorIO #1846

Closed
remia opened this issue Sep 4, 2023 · 4 comments · Fixed by #1849
Closed

v2.3.0 Python bindings source builds failing to load libOpenColorIO #1846

remia opened this issue Sep 4, 2023 · 4 comments · Fixed by #1849

Comments

@remia
Copy link
Collaborator

remia commented Sep 4, 2023

Filing this following report by @mjtitchener-fn during today's TSC meeting for tracking purpose.

On the latest release 2.3 and main branch, when building the Python package from source, there is an issue with RPATH not correctly installed preventing the import of PyOpenColorIO when using shared build.

The issue is most likely here, CMAKE_INSTALL_RPATH is indeed always defined at this point because it is set globally here.

This does not affect the wheels because RPATH are defined independently here.

@KevinJW
Copy link
Contributor

KevinJW commented Sep 5, 2023

My build on Linux, sets an rpath based on the top level setting e.g. cmake command

cmake -DCMAKE_INSTALL_PREFIX="$DEST" -DCMAKE_INSTALL_RPATH_USE_LINK_PATH=TRUE -DOCIO_INSTALL_EXT_PACKAGES=ALL -DPython_EXECUTABLE=$PYTHON_EXECUTABLE -DPYTHON=$PYTHON_EXECUTABLE -DOCIO_USE_OPENEXR_HALF=ON -Dyaml-cpp_STATIC_LIBRARY=ON -DHalf_STATIC_LIBRARY=ON ${BASE}/${SOURCE}

and then near the end of the build we see:

-- Set runtime path of "/<my local install path>/opencolorio.v2.3.0-python2.7/bin/ociodisplay" to "$ORIGIN/../lib64:$ORIGIN"
-- Set runtime path of "/<my local install path>/opencolorio.v2.3.0-python2.7/lib64/python2.7/site-packages/PyOpenColorIO/PyOpenColorIO.so" to "$ORIGIN/../lib64:$ORIGIN"

So the python bindings DSO is using the same rpath as the executables rather than the override, as @remia is suggesting.

This occurs for both Python 2.7 and 3.

@KevinJW
Copy link
Contributor

KevinJW commented Sep 5, 2023

So is it as simple as:

diff --git a/src/bindings/python/CMakeLists.txt b/src/bindings/python/CMakeLists.txt
index d7da50fd..94af5630 100644
--- a/src/bindings/python/CMakeLists.txt
+++ b/src/bindings/python/CMakeLists.txt
@@ -170,7 +170,7 @@ if(UNIX)
        set_property(TARGET PyOpenColorIO PROPERTY POSITION_INDEPENDENT_CODE ON)
 endif()
 
-if (UNIX AND NOT CMAKE_SKIP_RPATH AND NOT CMAKE_INSTALL_RPATH)
+if (UNIX AND NOT CMAKE_SKIP_RPATH AND CMAKE_INSTALL_RPATH)
     # Update the default RPATH so the Python binding dynamic library can find the OpenColorIO
     # dynamic library based on the default installation directory structure.
     if (APPLE)

@mjtitchener-fn
Copy link

I was chatting to @remia about this on Slack after the TSC call yesterday.
It does appear to be that simple, the following fixed it for me.

- if (UNIX AND NOT CMAKE_SKIP_RPATH AND NOT CMAKE_INSTALL_RPATH)
+ if (UNIX AND NOT CMAKE_SKIP_RPATH)

@KevinJW
Copy link
Contributor

KevinJW commented Sep 5, 2023

Indeed, I left the AND CMAKE_INSTALL_RPATH as the variable is used inside the if 'block', but I'm in no way a cmake expert, around the effects of the variable being not set/false, etc

@KevinJW KevinJW changed the title Python bindings source builds failing to load libOpenColorIO v2.3.0 Python bindings source builds failing to load libOpenColorIO Sep 5, 2023
@remia remia linked a pull request Sep 6, 2023 that will close this issue
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
3 participants