From 5cb17e4232d14d4a8340f8d320066a75bc7ad9ff Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?R=C3=A9mi=20Achard?= Date: Tue, 30 May 2023 19:32:32 +0100 Subject: [PATCH 1/8] Add command line apps and remove tests on Python wheel MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Rémi Achard --- pyproject.toml | 2 +- setup.py | 37 +++++++++++++++++---- share/cmake/macros/StripUtils.cmake | 23 +++++++++++++ src/apps/ocioarchive/CMakeLists.txt | 3 ++ src/apps/ociobakelut/CMakeLists.txt | 3 ++ src/apps/ociocheck/CMakeLists.txt | 3 ++ src/apps/ociochecklut/CMakeLists.txt | 3 ++ src/apps/ocioconvert/CMakeLists.txt | 3 ++ src/apps/ociodisplay/CMakeLists.txt | 3 ++ src/apps/ociolutimage/CMakeLists.txt | 3 ++ src/apps/ociomakeclf/CMakeLists.txt | 3 ++ src/apps/ocioperf/CMakeLists.txt | 3 ++ src/apps/ociowrite/CMakeLists.txt | 3 ++ src/apps/pyocioamf/pyocioamf.py | 9 +++-- src/apps/pyociodisplay/pyociodisplay.py | 6 +++- src/bindings/python/CMakeLists.txt | 3 ++ src/bindings/python/package/command_line.py | 18 ++++++++++ tests/python/OpenColorIOTestSuite.py | 2 +- 18 files changed, 119 insertions(+), 11 deletions(-) create mode 100644 share/cmake/macros/StripUtils.cmake create mode 100644 src/bindings/python/package/command_line.py diff --git a/pyproject.toml b/pyproject.toml index e19afc34bf..ae690df988 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -17,7 +17,7 @@ build-backend = "setuptools.build_meta" [tool.cibuildwheel] build-verbosity = "1" -test-command = "python -m PyOpenColorIO.tests.OpenColorIOTestSuite" +test-command = "python {project}/tests/python/OpenColorIOTestSuite.py" test-requires = ["numpy"] manylinux-x86_64-image = "manylinux2014" diff --git a/setup.py b/setup.py index 614752a527..23e5bc1f1e 100644 --- a/setup.py +++ b/setup.py @@ -59,6 +59,7 @@ def __init__(self, name, sourcedir=""): class CMakeBuild(build_ext): def build_extension(self, ext): extdir = os.path.abspath(os.path.dirname(self.get_ext_fullpath(ext.name))) + bindir = os.path.join(extdir, "bin") # required for auto-detection & inclusion of auxiliary "native" libs if not extdir.endswith(os.path.sep): @@ -73,12 +74,13 @@ def build_extension(self, ext): cmake_args = [ "-DCMAKE_LIBRARY_OUTPUT_DIRECTORY={}".format(extdir), + "-DCMAKE_RUNTIME_OUTPUT_DIRECTORY={}".format(bindir), "-DPython_EXECUTABLE={}".format(sys.executable), # Not used on MSVC, but no harm "-DCMAKE_BUILD_TYPE={}".format(cfg), "-DBUILD_SHARED_LIBS=OFF", "-DOCIO_BUILD_DOCS=ON", - "-DOCIO_BUILD_APPS=OFF", + "-DOCIO_BUILD_APPS=ON", "-DOCIO_BUILD_TESTS=OFF", "-DOCIO_BUILD_GPU_TESTS=OFF", # Make sure we build everything for the requested architecture(s) @@ -121,7 +123,8 @@ def build_extension(self, ext): # Multi-config generators have a different way to specify configs if not single_config: cmake_args += [ - "-DCMAKE_LIBRARY_OUTPUT_DIRECTORY_{}={}".format(cfg.upper(), extdir) + "-DCMAKE_LIBRARY_OUTPUT_DIRECTORY_{}={}".format(cfg.upper(), extdir), + "-DCMAKE_RUNTIME_OUTPUT_DIRECTORY_{}={}".format(cfg.upper(), bindir), ] build_args += ["--config", cfg] @@ -156,11 +159,33 @@ def build_extension(self, ext): version=get_version(), package_dir={ 'PyOpenColorIO': 'src/bindings/python/package', - 'PyOpenColorIO.tests': 'tests/python', - 'PyOpenColorIO.data': 'tests/data', + 'PyOpenColorIO.bin.pyocioamf': 'src/apps/pyocioamf', + 'PyOpenColorIO.bin.pyociodisplay': 'src/apps/pyociodisplay', }, - packages=['PyOpenColorIO', 'PyOpenColorIO.tests', 'PyOpenColorIO.data'], + packages=[ + 'PyOpenColorIO', + 'PyOpenColorIO.bin.pyocioamf', + 'PyOpenColorIO.bin.pyociodisplay', + ], ext_modules=[CMakeExtension("PyOpenColorIO.PyOpenColorIO")], cmdclass={"build_ext": CMakeBuild}, - include_package_data=True + include_package_data=True, + entry_points={ + 'console_scripts': [ + # Native applications + 'ocioarchive=PyOpenColorIO.command_line:main', + 'ociobakelut=PyOpenColorIO.command_line:main', + 'ociocheck=PyOpenColorIO.command_line:main', + 'ociochecklut=PyOpenColorIO.command_line:main', + 'ocioconvert=PyOpenColorIO.command_line:main', + 'ociodisplay=PyOpenColorIO.command_line:main', + 'ociolutimage=PyOpenColorIO.command_line:main', + 'ociomakeclf=PyOpenColorIO.command_line:main', + 'ocioperf=PyOpenColorIO.command_line:main', + 'ociowrite=PyOpenColorIO.command_line:main', + # Python applications + 'pyocioamf=PyOpenColorIO.bin.pyocioamf.pyocioamf:main', + 'pyociodisplay=PyOpenColorIO.bin.pyociodisplay.pyociodisplay:main', + ] + }, ) diff --git a/share/cmake/macros/StripUtils.cmake b/share/cmake/macros/StripUtils.cmake new file mode 100644 index 0000000000..3652f0dc9b --- /dev/null +++ b/share/cmake/macros/StripUtils.cmake @@ -0,0 +1,23 @@ +# SPDX-License-Identifier: BSD-3-Clause +# Copyright Contributors to the OpenColorIO Project. + +# This function is based on Pybind11's pybind11_strip. + +macro(ocio_strip_binary target_name) + string(TOUPPER "${CMAKE_BUILD_TYPE}" _uppercase_CMAKE_BUILD_TYPE) + if(NOT MSVC AND NOT "${_uppercase_CMAKE_BUILD_TYPE}" MATCHES DEBUG|RELWITHDEBINFO) + if(CMAKE_STRIP) + if(APPLE) + set(_strip_opt -x) + endif() + + add_custom_command( + TARGET ${target_name} + POST_BUILD + COMMAND ${CMAKE_STRIP} ${_strip_opt} $ + ) + unset(_strip_opt) + endif() + endif() + unset(_uppercase_CMAKE_BUILD_TYPE) +endmacro() diff --git a/src/apps/ocioarchive/CMakeLists.txt b/src/apps/ocioarchive/CMakeLists.txt index f39e428349..599d706f09 100644 --- a/src/apps/ocioarchive/CMakeLists.txt +++ b/src/apps/ocioarchive/CMakeLists.txt @@ -24,6 +24,9 @@ target_link_libraries(ocioarchive MINIZIP::minizip-ng ) +include(StripUtils) +ocio_strip_binary(ocioarchive) + install(TARGETS ocioarchive RUNTIME DESTINATION ${CMAKE_INSTALL_BINDIR} ) diff --git a/src/apps/ociobakelut/CMakeLists.txt b/src/apps/ociobakelut/CMakeLists.txt index 70cc43937e..3d6e586b96 100755 --- a/src/apps/ociobakelut/CMakeLists.txt +++ b/src/apps/ociobakelut/CMakeLists.txt @@ -33,6 +33,9 @@ target_link_libraries(ociobakelut OpenColorIO ) +include(StripUtils) +ocio_strip_binary(ociobakelut) + install(TARGETS ociobakelut RUNTIME DESTINATION ${CMAKE_INSTALL_BINDIR} ) diff --git a/src/apps/ociocheck/CMakeLists.txt b/src/apps/ociocheck/CMakeLists.txt index cd6cf787e3..024139546d 100755 --- a/src/apps/ociocheck/CMakeLists.txt +++ b/src/apps/ociocheck/CMakeLists.txt @@ -22,6 +22,9 @@ target_link_libraries(ociocheck OpenColorIO ) +include(StripUtils) +ocio_strip_binary(ociocheck) + install(TARGETS ociocheck RUNTIME DESTINATION ${CMAKE_INSTALL_BINDIR} ) diff --git a/src/apps/ociochecklut/CMakeLists.txt b/src/apps/ociochecklut/CMakeLists.txt index 59dc5daeac..431b1b79fb 100644 --- a/src/apps/ociochecklut/CMakeLists.txt +++ b/src/apps/ociochecklut/CMakeLists.txt @@ -30,6 +30,9 @@ target_link_libraries(ociochecklut OpenColorIO ) +include(StripUtils) +ocio_strip_binary(ociochecklut) + install(TARGETS ociochecklut RUNTIME DESTINATION ${CMAKE_INSTALL_BINDIR} ) diff --git a/src/apps/ocioconvert/CMakeLists.txt b/src/apps/ocioconvert/CMakeLists.txt index 5748450775..7b7abddcf2 100755 --- a/src/apps/ocioconvert/CMakeLists.txt +++ b/src/apps/ocioconvert/CMakeLists.txt @@ -34,6 +34,9 @@ target_link_libraries(ocioconvert OpenColorIO ) +include(StripUtils) +ocio_strip_binary(ocioconvert) + install(TARGETS ocioconvert RUNTIME DESTINATION ${CMAKE_INSTALL_BINDIR} ) diff --git a/src/apps/ociodisplay/CMakeLists.txt b/src/apps/ociodisplay/CMakeLists.txt index 8cd63d893c..14b53cfda3 100755 --- a/src/apps/ociodisplay/CMakeLists.txt +++ b/src/apps/ociodisplay/CMakeLists.txt @@ -47,6 +47,9 @@ target_link_libraries(ociodisplay OpenColorIO ) +include(StripUtils) +ocio_strip_binary(ociodisplay) + install(TARGETS ociodisplay RUNTIME DESTINATION ${CMAKE_INSTALL_BINDIR} ) diff --git a/src/apps/ociolutimage/CMakeLists.txt b/src/apps/ociolutimage/CMakeLists.txt index 9ee234603e..a470d2f6eb 100755 --- a/src/apps/ociolutimage/CMakeLists.txt +++ b/src/apps/ociolutimage/CMakeLists.txt @@ -27,6 +27,9 @@ target_link_libraries(ociolutimage utils::strings ) +include(StripUtils) +ocio_strip_binary(ociolutimage) + install(TARGETS ociolutimage RUNTIME DESTINATION ${CMAKE_INSTALL_BINDIR} ) diff --git a/src/apps/ociomakeclf/CMakeLists.txt b/src/apps/ociomakeclf/CMakeLists.txt index 0179d20558..fc3bd8e9f1 100644 --- a/src/apps/ociomakeclf/CMakeLists.txt +++ b/src/apps/ociomakeclf/CMakeLists.txt @@ -23,6 +23,9 @@ target_link_libraries(ociomakeclf utils::strings ) +include(StripUtils) +ocio_strip_binary(ociomakeclf) + install(TARGETS ociomakeclf RUNTIME DESTINATION ${CMAKE_INSTALL_BINDIR} ) diff --git a/src/apps/ocioperf/CMakeLists.txt b/src/apps/ocioperf/CMakeLists.txt index efe1f0555b..c056080bc5 100644 --- a/src/apps/ocioperf/CMakeLists.txt +++ b/src/apps/ocioperf/CMakeLists.txt @@ -19,6 +19,9 @@ target_link_libraries(ocioperf utils::strings ) +include(StripUtils) +ocio_strip_binary(ocioperf) + install(TARGETS ocioperf RUNTIME DESTINATION ${CMAKE_INSTALL_BINDIR} ) diff --git a/src/apps/ociowrite/CMakeLists.txt b/src/apps/ociowrite/CMakeLists.txt index b17bee62c2..bd77acfea6 100644 --- a/src/apps/ociowrite/CMakeLists.txt +++ b/src/apps/ociowrite/CMakeLists.txt @@ -18,6 +18,9 @@ target_link_libraries(ociowrite OpenColorIO ) +include(StripUtils) +ocio_strip_binary(ociowrite) + install(TARGETS ociowrite RUNTIME DESTINATION ${CMAKE_INSTALL_BINDIR} ) diff --git a/src/apps/pyocioamf/pyocioamf.py b/src/apps/pyocioamf/pyocioamf.py index 119738ddb6..853bf8683c 100644 --- a/src/apps/pyocioamf/pyocioamf.py +++ b/src/apps/pyocioamf/pyocioamf.py @@ -470,9 +470,14 @@ def build_ctf(amf_path): ctf_path = name_ctf_output_file(amf_path) write_ctf(gt, config, amf_path, ctf_path, 'none') -if __name__=='__main__': - """ Run the script from the command-line. """ + +def main(): import sys if len( sys.argv ) != 2: raise ValueError( "USAGE: python3 amf_to_ocio.py " ) build_ctf( sys.argv[1] ) + + +if __name__=='__main__': + """ Run the script from the command-line. """ + main() diff --git a/src/apps/pyociodisplay/pyociodisplay.py b/src/apps/pyociodisplay/pyociodisplay.py index eead7b9a60..1588001a74 100644 --- a/src/apps/pyociodisplay/pyociodisplay.py +++ b/src/apps/pyociodisplay/pyociodisplay.py @@ -1337,7 +1337,7 @@ def _on_gamma_changed(self, value): self.image_plane.update_gamma(value) -if __name__ == "__main__": +def main(): # OpenGL core profile needed on macOS to access programmatic pipeline gl_format = QtOpenGL.QGLFormat() gl_format.setProfile(QtOpenGL.QGLFormat.CoreProfile) @@ -1350,3 +1350,7 @@ def _on_gamma_changed(self, value): viewer = ImageView() viewer.show() app.exec_() + + +if __name__ == "__main__": + main() diff --git a/src/bindings/python/CMakeLists.txt b/src/bindings/python/CMakeLists.txt index 7d1cbd6d26..5c97fa2c9b 100644 --- a/src/bindings/python/CMakeLists.txt +++ b/src/bindings/python/CMakeLists.txt @@ -251,6 +251,9 @@ set(PYTHON_VARIANT_PATH ${_Python_VARIANT_PATH} CACHE INTERNAL "") # Set to PyOpenColorIO site-package location. set(_PyOpenColorIO_SITE_PACKAGE_DIR "${PYTHON_VARIANT_PATH}/PyOpenColorIO") +include(StripUtils) +ocio_strip_binary(PyOpenColorIO) + install(TARGETS PyOpenColorIO LIBRARY DESTINATION ${_PyOpenColorIO_SITE_PACKAGE_DIR} ) diff --git a/src/bindings/python/package/command_line.py b/src/bindings/python/package/command_line.py new file mode 100644 index 0000000000..b167a1f989 --- /dev/null +++ b/src/bindings/python/package/command_line.py @@ -0,0 +1,18 @@ +# SPDX-License-Identifier: BSD-3-Clause +# Copyright Contributors to the OpenColorIO Project. + +import os +import subprocess +import sys + + +BIN_DIR = os.path.join(os.path.dirname(__file__), 'bin') + + +def call_program(name, args): + return subprocess.call([os.path.join(BIN_DIR, name)] + args, close_fds=False) + + +def main(): + name = os.path.basename(sys.argv[0]) + raise SystemExit(call_program(name, sys.argv[1:])) diff --git a/tests/python/OpenColorIOTestSuite.py b/tests/python/OpenColorIOTestSuite.py index 159d5eca81..d312ba7c58 100755 --- a/tests/python/OpenColorIOTestSuite.py +++ b/tests/python/OpenColorIOTestSuite.py @@ -32,7 +32,7 @@ opencolorio_dir, os.getenv('DYLD_LIBRARY_PATH', '')) sys.path.insert(0, pyopencolorio_dir) -# Else it probably means direct invocation from installed package +# Else it probably means direct invocation from cibuildwheel else: here = os.path.dirname(__file__) os.environ["TEST_DATAFILES_DIR"] = os.path.join(os.path.dirname(here), 'data', 'files') From 398a7c7c002635714860f239c9c23d80cfe2138c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?R=C3=A9mi=20Achard?= Date: Tue, 30 May 2023 19:33:46 +0100 Subject: [PATCH 2/8] Fix warnings on Clang 14 MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Rémi Achard --- src/OpenColorIO/ops/lut3d/Lut3DOpCPU.cpp | 3 --- src/libutils/oglapphelpers/msl.mm | 4 ---- 2 files changed, 7 deletions(-) diff --git a/src/OpenColorIO/ops/lut3d/Lut3DOpCPU.cpp b/src/OpenColorIO/ops/lut3d/Lut3DOpCPU.cpp index e4bb6f4715..eed2761e20 100644 --- a/src/OpenColorIO/ops/lut3d/Lut3DOpCPU.cpp +++ b/src/OpenColorIO/ops/lut3d/Lut3DOpCPU.cpp @@ -1664,7 +1664,6 @@ void InvLut3DRenderer::apply(const void * inImg, void * outImg, long numPixels) // For now, if no result is found, return 0. float result[3] = { 0.f, 0.f, 0.f }; - unsigned long cnt = 0; long level = 0; while (level >= 0) { @@ -1680,7 +1679,6 @@ void InvLut3DRenderer::apply(const void * inImg, void * outImg, long numPixels) B <= levels[level].maxVals[node * chans + 2]; currentChild[level]++; currentChildInd[level]++; - cnt++; if (inRange) { @@ -1757,4 +1755,3 @@ ConstOpCPURcPtr GetLut3DRenderer(ConstLut3DOpDataRcPtr & lut) } } // namespace OCIO_NAMESPACE - diff --git a/src/libutils/oglapphelpers/msl.mm b/src/libutils/oglapphelpers/msl.mm index 0a8453d804..170062968e 100644 --- a/src/libutils/oglapphelpers/msl.mm +++ b/src/libutils/oglapphelpers/msl.mm @@ -199,7 +199,6 @@ void RGB_to_RGBA(const float* lutValues, int valueCount, std::vector& flo // This is the first available index for the textures. m_startIndex = startIndex; - unsigned currIndex = m_startIndex; // Process the 3D LUT first. @@ -236,8 +235,6 @@ void RGB_to_RGBA(const float* lutValues, int valueCount, std::vector& flo // 3. Keep the texture id & name for the later enabling. m_textureIds.push_back(TextureId(texture, textureName, samplerState, samplerName, MTLTextureType3D)); - - currIndex++; } // Process the 1D LUTs. @@ -278,7 +275,6 @@ void RGB_to_RGBA(const float* lutValues, int valueCount, std::vector& flo MTLTextureType type = (dimensions == GpuShaderDesc::TEXTURE_2D) ? MTLTextureType2D : MTLTextureType1D; m_textureIds.push_back(TextureId(texture, textureName, samplerState, samplerName, type)); - currIndex++; } } From 1827999b18b8d1be8cb68338c223d1b291381106 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?R=C3=A9mi=20Achard?= Date: Sat, 1 Jul 2023 10:33:09 +0100 Subject: [PATCH 3/8] Python wheel using dynamic linking MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Rémi Achard --- CMakeLists.txt | 5 +++++ setup.py | 15 ++++++++++++++- share/cmake/utils/CompilerFlags.cmake | 2 +- src/OpenColorIO/CMakeLists.txt | 1 + src/bindings/python/CMakeLists.txt | 2 +- src/bindings/python/package/__init__.py | 13 +++++++++---- src/bindings/python/package/command_line.py | 2 +- 7 files changed, 32 insertions(+), 8 deletions(-) diff --git a/CMakeLists.txt b/CMakeLists.txt index a0d650bbe8..82a5655091 100755 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -159,6 +159,7 @@ endif() ############################################################################### # Other preferences + option(OCIO_VERBOSE "Display more information when searching or installing dependencies" OFF) ############################################################################### @@ -176,6 +177,9 @@ endif() option(OCIO_USE_SSE "Specify whether to enable SSE CPU performance optimizations" ON) option(OCIO_USE_OIIO_FOR_APPS "Request OIIO to build apps (ociolutimage, ocioconvert and ociodisplay), the default uses OpenEXR." OFF) +option(OCIO_NO_SONAME "Disable version symlink for OpenColorIO library" OFF) +set (OCIO_PYTHON_INSTALL_RPATH "" CACHE STRING + "Specify the RPATHs to install for the Python module (darwin and linux only)") if ("${CMAKE_SYSTEM_PROCESSOR}" MATCHES "(AMD64|IA64|EM64T|X86|x86_64|i386|i686)") option(OCIO_USE_SSE2 "Specify whether to enable SSE2 CPU performance optimizations" ON) @@ -190,6 +194,7 @@ if ("${CMAKE_SYSTEM_PROCESSOR}" MATCHES "(AMD64|IA64|EM64T|X86|x86_64|i386|i686) set(OCIO_ARCH_X86 1) endif() + ############################################################################### # GPU configuration message(STATUS "") diff --git a/setup.py b/setup.py index 23e5bc1f1e..2015cfde99 100644 --- a/setup.py +++ b/setup.py @@ -78,7 +78,9 @@ def build_extension(self, ext): "-DPython_EXECUTABLE={}".format(sys.executable), # Not used on MSVC, but no harm "-DCMAKE_BUILD_TYPE={}".format(cfg), - "-DBUILD_SHARED_LIBS=OFF", + "-DBUILD_SHARED_LIBS=ON", + # Wheels do not support symlinks resulting in OCIO lib duplicated 3 times otherwise + "-DOCIO_NO_SONAME=ON", "-DOCIO_BUILD_DOCS=ON", "-DOCIO_BUILD_APPS=ON", "-DOCIO_BUILD_TESTS=OFF", @@ -134,6 +136,17 @@ def build_extension(self, ext): if archs: cmake_args += ["-DCMAKE_OSX_ARCHITECTURES={}".format(";".join(archs))] + # When building the wheel, the install step is not executed so we need + # to have the correct RPATH directly from the build tree output. + cmake_args += ["-DCMAKE_BUILD_WITH_INSTALL_RPATH=ON"] + # Install custom RPATH matching the wheel layout + if sys.platform.startswith("linux"): + cmake_args += ["-DCMAKE_INSTALL_RPATH={}".format("$ORIGIN")] + cmake_args += ["-DOCIO_PYTHON_INSTALL_RPATH={}".format("$ORIGIN/..")] + elif sys.platform.startswith("darwin"): + cmake_args += ["-DCMAKE_INSTALL_RPATH={}".format("@loader_path")] + cmake_args += ["-DOCIO_PYTHON_INSTALL_RPATH={}".format("@loader_path/..")] + # Set CMAKE_BUILD_PARALLEL_LEVEL to control the parallel build level # across all generators. if "CMAKE_BUILD_PARALLEL_LEVEL" not in os.environ: diff --git a/share/cmake/utils/CompilerFlags.cmake b/share/cmake/utils/CompilerFlags.cmake index d4c24b6436..f8055d0f02 100644 --- a/share/cmake/utils/CompilerFlags.cmake +++ b/share/cmake/utils/CompilerFlags.cmake @@ -120,7 +120,7 @@ endif() ############################################################################### # Define RPATH. -if (UNIX AND NOT CMAKE_SKIP_RPATH) +if (UNIX AND NOT CMAKE_SKIP_RPATH AND NOT CMAKE_INSTALL_RPATH) # With the 'usual' install path structure in mind, search from the bin directory # (i.e. a binary loading a dynamic library) and then from the current directory # (i.e. dynamic library loading another dynamic library). diff --git a/src/OpenColorIO/CMakeLists.txt b/src/OpenColorIO/CMakeLists.txt index 8ec954afe3..e4cd975e35 100755 --- a/src/OpenColorIO/CMakeLists.txt +++ b/src/OpenColorIO/CMakeLists.txt @@ -417,6 +417,7 @@ set_target_properties(OpenColorIO PROPERTIES LINK_OPTIONS "${CUSTOM_LINK_FLAGS}" VERSION ${OpenColorIO_VERSION} SOVERSION ${SOVERSION} + NO_SONAME ${OCIO_NO_SONAME} PUBLIC_HEADER "${INSTALL_HEADERS}" ) diff --git a/src/bindings/python/CMakeLists.txt b/src/bindings/python/CMakeLists.txt index 5c97fa2c9b..f4116af33d 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) +if (UNIX AND NOT CMAKE_SKIP_RPATH AND NOT OCIO_PYTHON_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) diff --git a/src/bindings/python/package/__init__.py b/src/bindings/python/package/__init__.py index 3ac093f5c3..28eb7c431c 100644 --- a/src/bindings/python/package/__init__.py +++ b/src/bindings/python/package/__init__.py @@ -16,10 +16,15 @@ # environment variable to 0. # -if sys.version_info >= (3, 8) and platform.system() == "Windows" and os.getenv("OCIO_PYTHON_LOAD_DLLS_FROM_PATH", "1") == "1": - for path in os.getenv("PATH", "").split(os.pathsep): - if os.path.exists(path) and path != ".": - os.add_dll_directory(path) +if sys.version_info >= (3, 8) and platform.system() == "Windows": + # Python wheel module is dynamically linked to the OCIO lib present in the same folder. + here = os.path.abspath(os.path.dirname(__file__)) + os.add_dll_directory(here) + + if os.getenv("OCIO_PYTHON_LOAD_DLLS_FROM_PATH", "1") == "1": + for path in os.getenv("PATH", "").split(os.pathsep): + if os.path.exists(path) and path != ".": + os.add_dll_directory(path) del os, sys, platform diff --git a/src/bindings/python/package/command_line.py b/src/bindings/python/package/command_line.py index b167a1f989..e8deb9e841 100644 --- a/src/bindings/python/package/command_line.py +++ b/src/bindings/python/package/command_line.py @@ -10,7 +10,7 @@ def call_program(name, args): - return subprocess.call([os.path.join(BIN_DIR, name)] + args, close_fds=False) + return subprocess.call([os.path.join(BIN_DIR, name)] + args) def main(): From ab7933481c3925ffca6d1b439bd7c65c24357e51 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?R=C3=A9mi=20Achard?= Date: Sat, 1 Jul 2023 21:30:47 +0100 Subject: [PATCH 4/8] Fix DLL directory MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Rémi Achard --- src/bindings/python/package/__init__.py | 2 +- tests/python/ColorSpaceTest.py | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/src/bindings/python/package/__init__.py b/src/bindings/python/package/__init__.py index 28eb7c431c..23f60cd0c9 100644 --- a/src/bindings/python/package/__init__.py +++ b/src/bindings/python/package/__init__.py @@ -19,7 +19,7 @@ if sys.version_info >= (3, 8) and platform.system() == "Windows": # Python wheel module is dynamically linked to the OCIO lib present in the same folder. here = os.path.abspath(os.path.dirname(__file__)) - os.add_dll_directory(here) + os.add_dll_directory(os.path.join(here, "bin")) if os.getenv("OCIO_PYTHON_LOAD_DLLS_FROM_PATH", "1") == "1": for path in os.getenv("PATH", "").split(os.pathsep): diff --git a/tests/python/ColorSpaceTest.py b/tests/python/ColorSpaceTest.py index 7dadee0616..0c04f04b34 100644 --- a/tests/python/ColorSpaceTest.py +++ b/tests/python/ColorSpaceTest.py @@ -749,7 +749,7 @@ def check_processor_inv(self, p): cfg = OCIO.Config.CreateFromStream(CONFIG) - cfg.setSearchPath(TEST_DATAFILES_DIR) + cfg.addSearchPath(TEST_DATAFILES_DIR) # Make all color spaces suitable for the heuristics inactive. # The heuristics don't look at inactive color spaces. From db4acabc413824db8152f7459b2734745724486d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?R=C3=A9mi=20Achard?= Date: Sat, 1 Jul 2023 21:56:49 +0100 Subject: [PATCH 5/8] Check path exists MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Rémi Achard --- src/bindings/python/package/__init__.py | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/src/bindings/python/package/__init__.py b/src/bindings/python/package/__init__.py index 23f60cd0c9..ad20dad973 100644 --- a/src/bindings/python/package/__init__.py +++ b/src/bindings/python/package/__init__.py @@ -19,7 +19,9 @@ if sys.version_info >= (3, 8) and platform.system() == "Windows": # Python wheel module is dynamically linked to the OCIO lib present in the same folder. here = os.path.abspath(os.path.dirname(__file__)) - os.add_dll_directory(os.path.join(here, "bin")) + bin_dir = os.path.join(here, "bin") + if os.path.exists(bin_dir): + os.add_dll_directory(bin_dir) if os.getenv("OCIO_PYTHON_LOAD_DLLS_FROM_PATH", "1") == "1": for path in os.getenv("PATH", "").split(os.pathsep): From f44cb3e5b046d7715137ca1098e4101afc06a22c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?R=C3=A9mi=20Achard?= Date: Sun, 2 Jul 2023 12:01:32 +0100 Subject: [PATCH 6/8] Fix DLL lookup on Windows MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Rémi Achard --- pyproject.toml | 5 ++++- src/bindings/python/package/__init__.py | 19 +++++++++++++------ 2 files changed, 17 insertions(+), 7 deletions(-) diff --git a/pyproject.toml b/pyproject.toml index ae690df988..3d4842d2b6 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -17,8 +17,11 @@ build-backend = "setuptools.build_meta" [tool.cibuildwheel] build-verbosity = "1" -test-command = "python {project}/tests/python/OpenColorIOTestSuite.py" test-requires = ["numpy"] +test-command = [ + "python {project}/tests/python/OpenColorIOTestSuite.py", + "ociocheck" +] manylinux-x86_64-image = "manylinux2014" manylinux-i686-image = "manylinux2014" diff --git a/src/bindings/python/package/__init__.py b/src/bindings/python/package/__init__.py index ad20dad973..800c7968ae 100644 --- a/src/bindings/python/package/__init__.py +++ b/src/bindings/python/package/__init__.py @@ -3,6 +3,19 @@ import os, sys, platform +# +# Python wheel module is dynamically linked to the OCIO DLL present in the bin folder. +# + +if platform.system() == "Windows": + here = os.path.abspath(os.path.dirname(__file__)) + bin_dir = os.path.join(here, "bin") + if os.path.exists(bin_dir): + if sys.version_info >= (3, 8): + os.add_dll_directory(bin_dir) + else: + os.environ['PATH'] = '{0};{1}'.format(bin_dir, os.getenv('PATH', '')) + # # Python 3.8+ has stopped loading DLLs from PATH environment variable on Windows. # @@ -17,12 +30,6 @@ # if sys.version_info >= (3, 8) and platform.system() == "Windows": - # Python wheel module is dynamically linked to the OCIO lib present in the same folder. - here = os.path.abspath(os.path.dirname(__file__)) - bin_dir = os.path.join(here, "bin") - if os.path.exists(bin_dir): - os.add_dll_directory(bin_dir) - if os.getenv("OCIO_PYTHON_LOAD_DLLS_FROM_PATH", "1") == "1": for path in os.getenv("PATH", "").split(os.pathsep): if os.path.exists(path) and path != ".": From 4af3fc0d713a9caf79fab5b94964b8b36fd329db Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?R=C3=A9mi=20Achard?= Date: Wed, 5 Jul 2023 20:58:58 +0100 Subject: [PATCH 7/8] Fix linux / mac wheels MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Rémi Achard --- CMakeLists.txt | 3 -- setup.py | 46 ++++++++++++++++++++++++------ src/OpenColorIO/CMakeLists.txt | 1 - src/bindings/python/CMakeLists.txt | 2 +- 4 files changed, 39 insertions(+), 13 deletions(-) diff --git a/CMakeLists.txt b/CMakeLists.txt index 82a5655091..2dd7c1372b 100755 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -177,9 +177,6 @@ endif() option(OCIO_USE_SSE "Specify whether to enable SSE CPU performance optimizations" ON) option(OCIO_USE_OIIO_FOR_APPS "Request OIIO to build apps (ociolutimage, ocioconvert and ociodisplay), the default uses OpenEXR." OFF) -option(OCIO_NO_SONAME "Disable version symlink for OpenColorIO library" OFF) -set (OCIO_PYTHON_INSTALL_RPATH "" CACHE STRING - "Specify the RPATHs to install for the Python module (darwin and linux only)") if ("${CMAKE_SYSTEM_PROCESSOR}" MATCHES "(AMD64|IA64|EM64T|X86|x86_64|i386|i686)") option(OCIO_USE_SSE2 "Specify whether to enable SSE2 CPU performance optimizations" ON) diff --git a/setup.py b/setup.py index 2015cfde99..e584c0a1a6 100644 --- a/setup.py +++ b/setup.py @@ -39,6 +39,39 @@ def get_version(): shutil.rmtree(dirpath) +# Remove symlinks as Python wheels do not support them at the moment. +# Rename the remaining dylib to the expected install name (major.minor). +def patch_symlink(folder): + if sys.platform.startswith("darwin"): + VERSION_REGEX = re.compile( + r"^libOpenColorIO.(?P\d+).(?P\d+).\d+.dylib$") + NAME_PATTERN = "libOpenColorIO.{major}.{minor}.dylib" + elif sys.platform.startswith("linux"): + VERSION_REGEX = re.compile( + r"^libOpenColorIO.so.(?P\d+).(?P\d+).\d+$") + NAME_PATTERN = "libOpenColorIO.so.{major}.{minor}" + else: + return + + # First remove all symlinks in the folder + for f in os.listdir(folder): + filepath = os.path.join(folder, f) + if os.path.islink(filepath): + os.remove(filepath) + + # Then match the remaining OCIO dynamic lib and rename it + for f in os.listdir(folder): + filepath = os.path.join(folder, f) + match = VERSION_REGEX.search(f) + if match: + res = match.groupdict() + new_filename = NAME_PATTERN.format( + major=res["major"], minor=res["minor"]) + new_filepath = os.path.join(folder, new_filename) + os.rename(filepath, new_filepath) + break + + # Convert distutils Windows platform specifiers to CMake -A arguments PLAT_TO_CMAKE = { "win32": "Win32", @@ -79,8 +112,6 @@ def build_extension(self, ext): # Not used on MSVC, but no harm "-DCMAKE_BUILD_TYPE={}".format(cfg), "-DBUILD_SHARED_LIBS=ON", - # Wheels do not support symlinks resulting in OCIO lib duplicated 3 times otherwise - "-DOCIO_NO_SONAME=ON", "-DOCIO_BUILD_DOCS=ON", "-DOCIO_BUILD_APPS=ON", "-DOCIO_BUILD_TESTS=OFF", @@ -139,13 +170,10 @@ def build_extension(self, ext): # When building the wheel, the install step is not executed so we need # to have the correct RPATH directly from the build tree output. cmake_args += ["-DCMAKE_BUILD_WITH_INSTALL_RPATH=ON"] - # Install custom RPATH matching the wheel layout if sys.platform.startswith("linux"): - cmake_args += ["-DCMAKE_INSTALL_RPATH={}".format("$ORIGIN")] - cmake_args += ["-DOCIO_PYTHON_INSTALL_RPATH={}".format("$ORIGIN/..")] - elif sys.platform.startswith("darwin"): - cmake_args += ["-DCMAKE_INSTALL_RPATH={}".format("@loader_path")] - cmake_args += ["-DOCIO_PYTHON_INSTALL_RPATH={}".format("@loader_path/..")] + cmake_args += ["-DCMAKE_INSTALL_RPATH={}".format("$ORIGIN;$ORIGIN/..")] + if sys.platform.startswith("darwin"): + cmake_args += ["-DCMAKE_INSTALL_RPATH={}".format("@loader_path;@loader_path/..")] # Set CMAKE_BUILD_PARALLEL_LEVEL to control the parallel build level # across all generators. @@ -166,6 +194,8 @@ def build_extension(self, ext): ["cmake", "--build", "."] + build_args, cwd=self.build_temp ) + patch_symlink(extdir) + # For historical reason, we use PyOpenColorIO as the import name setup( diff --git a/src/OpenColorIO/CMakeLists.txt b/src/OpenColorIO/CMakeLists.txt index e4cd975e35..8ec954afe3 100755 --- a/src/OpenColorIO/CMakeLists.txt +++ b/src/OpenColorIO/CMakeLists.txt @@ -417,7 +417,6 @@ set_target_properties(OpenColorIO PROPERTIES LINK_OPTIONS "${CUSTOM_LINK_FLAGS}" VERSION ${OpenColorIO_VERSION} SOVERSION ${SOVERSION} - NO_SONAME ${OCIO_NO_SONAME} PUBLIC_HEADER "${INSTALL_HEADERS}" ) diff --git a/src/bindings/python/CMakeLists.txt b/src/bindings/python/CMakeLists.txt index f4116af33d..d7da50fd79 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 OCIO_PYTHON_INSTALL_RPATH) +if (UNIX AND NOT CMAKE_SKIP_RPATH AND NOT 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) From 10c3ecb2a7f6dac5f1d4bc68ccee65a69a17a4b2 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?R=C3=A9mi=20Achard?= Date: Sat, 8 Jul 2023 10:19:00 +0200 Subject: [PATCH 8/8] Fix symlinks and make doc optional MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Rémi Achard --- .github/workflows/wheel_workflow.yml | 6 +- CMakeLists.txt | 1 + setup.py | 97 +++++++++++++--------------- src/OpenColorIO/CMakeLists.txt | 9 ++- tests/data/__init__.py | 0 5 files changed, 55 insertions(+), 58 deletions(-) delete mode 100644 tests/data/__init__.py diff --git a/.github/workflows/wheel_workflow.yml b/.github/workflows/wheel_workflow.yml index 187ecb330b..702243b789 100644 --- a/.github/workflows/wheel_workflow.yml +++ b/.github/workflows/wheel_workflow.yml @@ -128,7 +128,7 @@ jobs: platforms: all - name: Build wheels - uses: pypa/cibuildwheel@v2.12.0 + uses: pypa/cibuildwheel@v2.13.1 env: CIBW_BUILD: ${{ matrix.python }} CIBW_ARCHS: ${{ matrix.arch }} @@ -194,7 +194,7 @@ jobs: python-version: '3.8' - name: Build wheels - uses: pypa/cibuildwheel@v2.12.0 + uses: pypa/cibuildwheel@v2.13.1 env: CIBW_BUILD: ${{ matrix.python }} CIBW_ARCHS: ${{ matrix.arch }} @@ -245,7 +245,7 @@ jobs: python-version: '3.8' - name: Build wheels - uses: pypa/cibuildwheel@v2.12.0 + uses: pypa/cibuildwheel@v2.13.1 env: CIBW_BUILD: ${{ matrix.python }} CIBW_ARCHS: ${{ matrix.arch }} diff --git a/CMakeLists.txt b/CMakeLists.txt index 2dd7c1372b..d5844f752b 100755 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -161,6 +161,7 @@ endif() # Other preferences option(OCIO_VERBOSE "Display more information when searching or installing dependencies" OFF) +option(OCIO_USE_SOVERSION "Enable versioning in OCIO shared library name" ON) ############################################################################### # Warnings / debugging settings diff --git a/setup.py b/setup.py index e584c0a1a6..1d67a70f24 100644 --- a/setup.py +++ b/setup.py @@ -6,7 +6,6 @@ import os import re -import shutil import subprocess import sys import tempfile @@ -16,60 +15,49 @@ # Extract the project version from CMake generated ABI header. -# NOTE: When droping support for Python2 we can use -# tempfile.TemporaryDirectory() context manager instead of try...finally. def get_version(): VERSION_REGEX = re.compile( r"^\s*#\s*define\s+OCIO_VERSION_FULL_STR\s+\"(.*)\"\s*$", re.MULTILINE) here = os.path.abspath(os.path.dirname(__file__)) - dirpath = tempfile.mkdtemp() - - try: - stdout = subprocess.check_output(["cmake", here], cwd=dirpath) - path = os.path.join(dirpath, "include", "OpenColorIO", "OpenColorABI.h") - with open(path) as f: - match = VERSION_REGEX.search(f.read()) - return match.group(1) - except Exception as e: - raise RuntimeError( - "Unable to find OpenColorIO version: {}".format(str(e)) - ) - finally: - shutil.rmtree(dirpath) - - -# Remove symlinks as Python wheels do not support them at the moment. -# Rename the remaining dylib to the expected install name (major.minor). -def patch_symlink(folder): - if sys.platform.startswith("darwin"): - VERSION_REGEX = re.compile( - r"^libOpenColorIO.(?P\d+).(?P\d+).\d+.dylib$") - NAME_PATTERN = "libOpenColorIO.{major}.{minor}.dylib" - elif sys.platform.startswith("linux"): - VERSION_REGEX = re.compile( - r"^libOpenColorIO.so.(?P\d+).(?P\d+).\d+$") - NAME_PATTERN = "libOpenColorIO.so.{major}.{minor}" - else: - return - - # First remove all symlinks in the folder - for f in os.listdir(folder): - filepath = os.path.join(folder, f) - if os.path.islink(filepath): - os.remove(filepath) - - # Then match the remaining OCIO dynamic lib and rename it - for f in os.listdir(folder): - filepath = os.path.join(folder, f) - match = VERSION_REGEX.search(f) - if match: - res = match.groupdict() - new_filename = NAME_PATTERN.format( - major=res["major"], minor=res["minor"]) - new_filepath = os.path.join(folder, new_filename) - os.rename(filepath, new_filepath) - break + + with tempfile.TemporaryDirectory() as tmpdir: + try: + subprocess.check_call(["cmake", here], cwd=tmpdir) + path = os.path.join(tmpdir, "include", "OpenColorIO", "OpenColorABI.h") + with open(path) as f: + match = VERSION_REGEX.search(f.read()) + return match.group(1) + except Exception as e: + raise RuntimeError( + "Unable to find OpenColorIO version: {}".format(str(e)) + ) + + +# Call CMake find_package from a dummy script and return whether the package +# has been found or not. +def cmake_find_package(package_name): + with tempfile.TemporaryDirectory() as tmpdir: + try: + cmakelist_path = os.path.join(tmpdir, "CMakeLists.txt") + with open(cmakelist_path, "w") as f: + f.write(""" +cmake_minimum_required(VERSION 3.13) +project(test LANGUAGES CXX) + +find_package({} REQUIRED) +""".format(package_name) + ) + + subprocess.check_call( + ["cmake", tmpdir], + cwd=tmpdir, + stdout=subprocess.DEVNULL, + stderr=subprocess.DEVNULL + ) + return True + except Exception as e: + return False # Convert distutils Windows platform specifiers to CMake -A arguments @@ -112,7 +100,7 @@ def build_extension(self, ext): # Not used on MSVC, but no harm "-DCMAKE_BUILD_TYPE={}".format(cfg), "-DBUILD_SHARED_LIBS=ON", - "-DOCIO_BUILD_DOCS=ON", + "-DOCIO_USE_SOVERSION=OFF", "-DOCIO_BUILD_APPS=ON", "-DOCIO_BUILD_TESTS=OFF", "-DOCIO_BUILD_GPU_TESTS=OFF", @@ -175,6 +163,11 @@ def build_extension(self, ext): if sys.platform.startswith("darwin"): cmake_args += ["-DCMAKE_INSTALL_RPATH={}".format("@loader_path;@loader_path/..")] + # Documentation is used for Python docstrings but we allow to build + # the wheel without docs to remove a hard dependency on doxygen. + if cmake_find_package("Doxygen"): + cmake_args += ["-DOCIO_BUILD_DOCS=ON"] + # Set CMAKE_BUILD_PARALLEL_LEVEL to control the parallel build level # across all generators. if "CMAKE_BUILD_PARALLEL_LEVEL" not in os.environ: @@ -194,8 +187,6 @@ def build_extension(self, ext): ["cmake", "--build", "."] + build_args, cwd=self.build_temp ) - patch_symlink(extdir) - # For historical reason, we use PyOpenColorIO as the import name setup( diff --git a/src/OpenColorIO/CMakeLists.txt b/src/OpenColorIO/CMakeLists.txt index 8ec954afe3..842202316c 100755 --- a/src/OpenColorIO/CMakeLists.txt +++ b/src/OpenColorIO/CMakeLists.txt @@ -411,12 +411,17 @@ elseif(APPLE) endif() endif() +if (OCIO_USE_SOVERSION) + set_target_properties(OpenColorIO PROPERTIES + VERSION ${OpenColorIO_VERSION} + SOVERSION ${SOVERSION} + ) +endif() + set_target_properties(OpenColorIO PROPERTIES OUTPUT_NAME ${PROJECT_NAME}${OCIO_LIBNAME_SUFFIX} COMPILE_OPTIONS "${PLATFORM_COMPILE_OPTIONS}" LINK_OPTIONS "${CUSTOM_LINK_FLAGS}" - VERSION ${OpenColorIO_VERSION} - SOVERSION ${SOVERSION} PUBLIC_HEADER "${INSTALL_HEADERS}" ) diff --git a/tests/data/__init__.py b/tests/data/__init__.py deleted file mode 100644 index e69de29bb2..0000000000