Skip to content

CMake: Small fixes for boost, assimp and cuda #8

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

Merged
merged 5 commits into from
Apr 7, 2025
Merged

Conversation

M2-TE
Copy link
Contributor

@M2-TE M2-TE commented Apr 7, 2025

  • set CUDA_ARCHITECTURES property with CMake >= 3.23 while still respecting the CMAKE_* var if set
  • find boost in CONFIG mode as preferred by CMake (available since Boost 1.70)
  • linking rmagine-core against specific assimp::assimp target, as ASSIMP_INCLUDE_DIRS and ASSIMP_LIBRARIES were not defined in my case (Assimp v5.4.3). The latter two are still used as a fallback, should the alias target be missing
  • changed cmake message from Components build: to Components being built:, as it is still the configure stage, not the build stage

@M2-TE
Copy link
Contributor Author

M2-TE commented Apr 7, 2025

CMake Error in src/rmagine_core/CMakeLists.txt:
Imported target "assimp::assimp" includes non-existent path
"/usr/lib/include"

Say what now?

@amock
Copy link
Member

amock commented Apr 7, 2025

First of all, thanks for the additions; they all seem to be valid. It seems the assimp package in the ubuntu 20 apt repositories contains a hard-coded path to the include directory. The easiest solution would be to just remove the support for Ubuntu 20. However, I have still some ROS 1 projects running under Ubuntu 20 that are not migrated to ROS 2 yet. So I would be happy if we would find a solution so that it still compiles with Ubuntu 20.

@amock
Copy link
Member

amock commented Apr 7, 2025

Currently, I also have no tests for the GPU part of rmagine. So in the meantime we'll just test your CUDA-related cmake additions internally on different machines.

@M2-TE
Copy link
Contributor Author

M2-TE commented Apr 7, 2025

I agree that Ubuntu 20 shouldn't be crossed off the list, even if it's due to a library fault. As you yourself said, it is still very widespread for ROS 1. I've therefore changed the assimp linking portion to this:

# link to assimp::assimp when its library and include dir vars are missing
if (DEFINED ASSIMP_INCLUDE_DIRS AND DEFINED ASSIMP_LIBRARIES)
  target_include_directories(rmagine-core PUBLIC ${ASSIMP_INCLUDE_DIRS})
  target_link_libraries(rmagine-core ${ASSIMP_LIBRARIES})
else()
  target_link_libraries(rmagine-core assimp::assimp)
endif()

This will essentially prioritize the linking (include + libs) via CMake vars, but should still always result in the same outcome as with the ALIAS target

@amock amock merged commit 71d010f into uos:main Apr 7, 2025
6 checks passed
@M2-TE M2-TE deleted the cmake-fixes branch April 7, 2025 13:36
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants