Please consider making the python bindings optional

[josch]

Hi,

the second paragraph of the README.md states:

Imath also includes optional python bindings

The python bindings though, are in fact (as of the time i'm writing this) not optional. If one tries to build a project that requires imath using cmake one gets the following error output during configuration if the python bindings are not installed:

CMake Error at /usr/lib/x86_64-linux-gnu/cmake/Imath/ImathTargets.cmake:115 (message):
  The imported target "Imath::PyImath_Python3_11" references the file

     "/usr/lib/x86_64-linux-gnu/libPyImath_Python3_11-3_1.so.29.8.0"

  but this file does not exist.  Possible reasons include:

  * The file was deleted, renamed, or moved to another location.

  * An install or uninstall procedure did not complete successfully.

  * The installation package was faulty and contained

     "/usr/lib/x86_64-linux-gnu/cmake/Imath/ImathTargets.cmake"

  but not all the files it references.

Call Stack (most recent call first):
  /usr/lib/x86_64-linux-gnu/cmake/Imath/ImathConfig.cmake:40 (include)
  CMakeLists.txt:53 (find_package)


-- Configuring incomplete, errors occurred!

What makes the python bindings not optional is the EXPORT line in src/python/config/ModuleDefine.cmake. The following patch fixes the problem and makes the python bindings indeed optional as stated in the readme:

--- a/src/python/config/ModuleDefine.cmake
+++ b/src/python/config/ModuleDefine.cmake
@@ -55,7 +55,6 @@ function(PYIMATH_ADD_LIBRARY_PRIV libname)
   add_library(${PROJECT_NAME}::${libname} ALIAS ${libname})
 
   install(TARGETS ${libname}
-    EXPORT ${PROJECT_NAME}
     RUNTIME DESTINATION ${CMAKE_INSTALL_BINDIR}
     LIBRARY DESTINATION ${CMAKE_INSTALL_LIBDIR}
     ARCHIVE DESTINATION ${CMAKE_INSTALL_LIBDIR}

To make sure, that this does not break anything, I obtained a list of all source packages in Debian unstable which directly or indirectly use imath. The full list is:

actiona amberol auto-multiple-choice beads blender budgie-control-center 
calligra chafa cheese cimg converseen darknet darktable digikam dmtx-utils 
drawtiming dvdauthor dx embree enblend-enfuse eviacam exactimage ffcv field3d 
freecad freeimage frei0r gegl gem gimp gmic gnome-authenticator 
gnome-dvb-daemon gnome-metronome gnome-sound-recorder gnome-subtitles gnunet 
gnustep-gui gst-plugins-bad1.0 gstreamer-editing-services1.0 gstreamer-vaapi 
gst-rtsp-server1.0 gtk4 hugin imagemagick imview inkscape jmagick jpeg-xl 
kimageformats kio-extras krita kxstitch kylin-scanner lebiniou libopenshot 
libvigraimpex luminance-hdr megapixels mgba mia mldemos mlt monado mrgingham 
mrpt mupen64plus-core nheko nip2 nitrokey-authenticator node-opencv nomacs 
nvidia-texture-tools nvidia-vaapi-driver obs-advanced-scene-switcher odr-padenc 
olive-editor opencfu opencolorio opencv openexr openexr-viewers openimageio 
openvdb os-autoinst osm2pgsql otb otpclient performous pfstools photoqt 
php-facedetect php-imagick pink-pony pitivi povray pqiv pragha pstoedit 
pulseeffects pythonmagick pytorch pyzbar qimgv rails r-cran-magick 
ros-image-pipeline ros-image-transport-plugins ros-opencv-apps 
ros-vision-opencv rss-glx ruby-image-processing ruby-rmagick ruby-vips 
rust-gstreamer-play rust-gstreamer-play-sys rust-zbar-rust sayonara sight 
simple-whip-client siril slic3r-prusa swayimg synfig synfigstudio 
ukui-biometric-auth ukui-biometric-manager ukui-control-center ukui-greeter 
ukui-screensaver uprightdiff vdr-plugin-skinenigmang vips virtuoso-opensource

Out of those packages, the following packages fail with above error if the python imath module is not installed:

• field3d
• kimageformats
• kio-extras
• krita
• libvigraimpex
• olive-editor
• openexr
• openvdb

After removing the EXPORT line, these build successfully again. Credit goes to @jspricke for finding the EXPORT line as the culprit.

Would you like me to file a pull request with the change proposed above?

The practical problem that removing the hard python requirement solves is, that otherwise packages that directly or indirectly require imath will pull in the host architecture python which makes the cross-build dependencies unsatisfiable for the long list of packages above.

Thanks!

[cary-ilm]

In the top-level CMakeLists.txt, the src/python directory should only be included if PYTHON is set, so without that setting, the bindings should be ignored already:

if (PYTHON OR PYBIND11)
  add_subdirectory(src/python)
endif()

Could the problem be that the PYTHON variable is setting set in your build unintentionally? Admittedly, the naming is unnecessarily generic, it would be safer if named something specific to Imath, like IMATH_PYTHON.

[josch]

The Debian imath package explicitly wants to build the python binding. This is desirable (in Debian, we build source packages with all available features enabled) and not what I propose to change. For that reason, the Debian imath package sets -DPYTHON=ON when building imath:

https://sources.debian.org/src/imath/3.1.9-3/debian/rules/#L21

But I'm not talking about making the python bindings optional when building imath. I'm talking about making them optional when installing everything and then building a package that uses imath via cmake. This is why this affects all the other packages I listed above. It is about building software that requires imath not about building imath itself. Through the EXPORT statement, software that wants imath automatically also requires the python binding. That's the thing that I think should be different. None of the 134 software packages that directly or indirectly need imath to build fail to build after making the python bindings optional.

[cary-ilm]

Ah, that makes sense, I was thinking only of the build. If you're confident that removing the EXPORT solves the problem and isn't otherwise necessary, please do submit a PR. As you point out, we should also clarify the wording in the README.md to state that the bindings are optional at build time.