search

AcademySoftwareFoundation / Imath

Imath is a C++ and python library of 2D and 3D vector, matrix, and math operations for computer graphics
https://imath.readthedocs.io
BSD 3-Clause "New" or "Revised" License
356 stars 104 forks source link

Alembic Windows compilation Python3.9.10 `called with non-existent target "Imath::PyImath_Python3_9".` #395

Open OlivierArgentieri opened 2 months ago

OlivierArgentieri commented 2 months ago

Compilation issue on windows

TLDR:

CMake Error at python/PyAlembic/Tests/CMakeLists.txt:49 (GET_TARGET_PROPERTY): get_target_property() called with non-existent target "Imath::PyImath_Python3_9".

How to reproduce

Requierements:

cmake --build .\build\ --config Release --target install 


### Compile IMath
In `<IMath_SOURCE_ROOT>/`

```sh
cmake -S . -B ./build -DPYTHON=ON -DCMAKE_INSTALL_PREFIX=E:/compile_alembic/Imath-3.1.11/install -DCMAKE_BUILD_TYPE=Release -DBoost_ROOT=E:/compile_alembic/boost-1.85.0/install_prefix -DPython_EXECUTABLE=C:/Python39/python.exe -DPython3_EXECUTABLE=C:/Python39/python.exe -DBoost_NO_BOOST_CMAKE=OFF

cmake --build .\build\ --config Release --target install

Compile Alembic

cmake -S . -B ./build/ -DImath_ROOT=E:/compile_alembic/Imath-3.1.11/install -DUSE_PYALEMBIC=ON -DCMAKE_INSTALL_PREFIX=E:/compile_alembic/alembic-1.8.5/install -DBoost_INCLUDE_DIR=E:/compile_alembic/boost-1.85.0/install_prefix/includes/boost-1.85.0  -DCMAKE_BUILD_TYPE=Release

And now you should see this error:

CMake Error at python/PyAlembic/Tests/CMakeLists.txt:49 (GET_TARGET_PROPERTY):
  get_target_property() called with non-existent target
  "Imath::PyImath_Python3_9".

Things tried

set_property(TARGET Imath::PyImath_Python3_9 APPEND PROPERTY IMPORTED_CONFIGURATIONS RELEASE)
set_target_properties(Imath::PyImath_Python3_9 PROPERTIES
  IMPORTED_IMPLIB_RELEASE "${_IMPORT_PREFIX}/lib/PyImath_Python3_9-3_1.lib"
  IMPORTED_LOCATION_RELEASE "${_IMPORT_PREFIX}/bin/PyImath_Python3_9-3_1.dll"
  )

Then i also add this in ImathTargets.cmake 

...
add_library(Imath::PyImath_Python3_9 INTERFACE IMPORTED)
set_target_properties(Imath::PyImath_Python3_9 PROPERTIES
  INTERFACE_COMPILE_DEFINITIONS "IMATH_DLL"
  INTERFACE_COMPILE_FEATURES "cxx_std_11"
  INTERFACE_COMPILE_OPTIONS "/EHsc"
  INTERFACE_INCLUDE_DIRECTORIES "E:/compile_alembic/boost-1.85.0/install_prefix/includes/boost-1.85.0"
  INTERFACE_LINK_LIBRARIES "Imath::ImathConfig"
)
...

Everything i tried above, didn't work and when i try to compile without python binding it's works...

I don't know if it's normal or not, but i have searched for this Imath::PyImath_Python3_9 target, and i didn't find it in any of targets .cmake file under <PREFIX_IMATH>/lib/cmake/Imath/, that's why i'm posting this issue here instead of on alembic google group.

Maybe i made something wrong, i'm a beginner with all this compilation pipeline stuff, using Imath and alembic, don't hesistate to share with me some feedbacks or idea.

Thank you

lamiller0 commented 2 months ago

We noticed this problem on Linux as well.

We suspect its related to this: https://github.com/AcademySoftwareFoundation/Imath/issues/360

Making it optional ended up totally removing Imath::PyImath_Python3_9 from ImathTargets-release.cmake.

meshula commented 2 months ago

@lamiller0 Do you think the problem is that the PyAlembic links against PyImath and therefore the EXPORT is in fact required when you are linking against PyImath? And in fact the fix does not make the python bindings optional but merely makes the linking of the python bindings impossible?

I do notice that alembic is not listed in the list of tested packages on Debian on the original report.

I wonder if the original bug report

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.

Indicates a more direct problem, like /usr/lib/x86_64-linux-gnu/libPyImath_Python3_11-3_1.so.29.8.0 actually does not exist, as the error says; perhaps it is not named correctly in ImathTargets.cmake, or perhaps it wasn't copied to the target destination.

I notice that the original bug report does not post the contents of ImathTargets.cmake, so I think we are fishing in the dark here. I assume that if you restore the line, then PyAlembic links again?

lamiller0 commented 2 months ago

I assume that if you restore the line, then PyAlembic links again?

I ran out of time to be able to confirm this, but hope to early next week. I suspect the solution will be to EXPORT it if building PyImath.

OlivierArgentieri commented 2 months ago

Thank you all for your answers.

So I retried to compile alembic using an old version of imath, the built of alembic was a success using Imath v3.1.8 !

lgritz commented 2 months ago

I think the idea of the original PR was that you didn't want that export to occur if you were turning off the python bindings. But instead of making it conditional on that, it just removed the export entirely, which breaks other projects that want the exported config to contain the target for the python module.

But admittedly, I'm a little hazy on the whole thing, so I may be misunderstanding.

lgritz commented 2 months ago

I guess maybe it's more subtle than that? If I understand the original PR correctly, they were ok building the python bindings, but didn't want to install them, and in that case, having the exported target caused problems downstream. But now removing the exported target entirely causes problems for other downstream projects that need it (such as Alembic). So I stand by my assertion that the right fix should have been to selectively export the target so that somehow it still is present when you do want to both build and install the python bindings. (I'm still a little unclear on what the use case is for wanting to build, but not install, the python bindings.)

meshula commented 2 months ago

You're exactly right, Larry

The motivation to build the python bindings but not install them is this: Whether or not you EXPORT them, they are made available to Python by virtue of being in site-packages or whatever. The usual Python thing. EXPORT makes the library available for linking in a C++ linking area AND ALSO tells anything linking via the c/c++ toolchain that it needs to link to that library.

If you are linking against Imath, that's independent of linking against PyImath. So why do all those projects in debian think they need to link against pyimath in the first place? I think that's the root cause bad news right there, and eliding EXPORT is a red herring. Does our cmake put a dependency on pyimath on everything that links imath? That would be a root cause but I don't see where that would be happening.

The EXPORT is in the MODULE_DEFINE for PyImath, not the library define for Imath. Do how does the python module impinge on the C++ build IN ANY WAY unless the export line itself is bad. That could be it.

EXPORT ${PROJECT_NAME}

PROJECT_NAME is likely Imath and this corrupts things, and probably we should exporting PYMODULE_NAME or whatever mystery sauce cmake has for the purpose.

lamiller0 commented 2 months ago

I just did a few experiments with bringng back the EXPORT line back and diffing what changed when building WITH python:

In ImathTargets.cmake:

foreach(_cmake_expected_target IN ITEMS Imath::ImathConfig Imath::Imath Imath::PyImathConfig Imath::PyImath_Python3_9)

The Imath::PyImath_Python3_9 is added.

Also this block is added:

# Create imported target Imath::PyImath_Python3_9
add_library(Imath::PyImath_Python3_9 SHARED IMPORTED)

set_target_properties(Imath::PyImath_Python3_9 PROPERTIES
  INTERFACE_COMPILE_FEATURES "cxx_std_14"
  INTERFACE_INCLUDE_DIRECTORIES "/spfs/include"
  INTERFACE_LINK_LIBRARIES "Imath::Imath"
)

In ImathTargets-release.cmake this block is added:

# Import target "Imath::PyImath_Python3_9" for configuration "Release"
set_property(TARGET Imath::PyImath_Python3_9 APPEND PROPERTY IMPORTED_CONFIGURATIONS RELEASE)
set_target_properties(Imath::PyImath_Python3_9 PROPERTIES
  IMPORTED_LINK_DEPENDENT_LIBRARIES_RELEASE "Python3::Python"
  IMPORTED_LOCATION_RELEASE "${_IMPORT_PREFIX}/lib64/libPyImath_Python3_9-3_1.so.29.10.0"
  IMPORTED_SONAME_RELEASE "libPyImath_Python3_9-3_1.so.29"
  )

list(APPEND _cmake_import_check_targets Imath::PyImath_Python3_9 )
list(APPEND _cmake_import_check_files_for_Imath::PyImath_Python3_9 "${_IMPORT_PREFIX}/lib64/libPyImath_Python3_9-3_1.so.29.10.0" )
meshula commented 2 months ago

Thanks for that. None of that introduces an erroneous new dependency to non-python oriented targets.