Repository navigation
Fix navmap_rviz_plugin build with Qt5 rviz_common and Qt6 installed - #44
Conversation
fmrico
left a comment
There was a problem hiding this comment.
Thanks, the diagnosis is right: a Qt5 rviz_common (Kilted and older) looks for Qt6 first in its extras, so with Qt6 installed Qt5 is never loaded, and the old get_target_property check came after find_package(rviz_common), too late. Reading the exported dependencies first, and disabling Qt6 when rviz uses Qt5, follows rviz's own Qt as we want.
CI only builds on Rolling (Qt6), so I checked the Qt5 side in Docker (ros:kilted-ros-base and ros:jazzy-ros-base, with qt6-base-dev installed):
- With the released
rviz_common(Kilted 15.0.15, Jazzy 14.1.24) the extras ask for Qt5 directly, so both rolling and this PR build: the PR does not break the released distros. - With the Qt6-first extras of rviz_common 15.2.x (patched into Kilted's install), rolling fails with exactly the error in the description, and this PR builds; the plugin links only
libQt5Core,libQt5GuiandlibQt5Widgets.
Besides the inline comments:
- Minor:
find_filecachesRVIZ_COMMON_DEPS_FILE, so a build directory reused with another distro keeps the old path. Unlikely with colcon (one build dir per distro), just noting it.
| # looks for Qt6 first if installed and then fails to find Qt5, so Qt6 must be disabled. | ||
| find_file(RVIZ_COMMON_DEPS_FILE ament_cmake_export_dependencies-extras.cmake | ||
| PATHS ENV AMENT_PREFIX_PATH PATH_SUFFIXES share/rviz_common/cmake NO_DEFAULT_PATH) | ||
| file(READ "${RVIZ_COMMON_DEPS_FILE}" rviz_common_deps) |
There was a problem hiding this comment.
If AMENT_PREFIX_PATH is not set or does not contain rviz_common (e.g. a build that only passes CMAKE_PREFIX_PATH), find_file gives RVIZ_COMMON_DEPS_FILE-NOTFOUND and file(READ) aborts the configure with "failed to open for reading", which does not say what is wrong. Before this change that case worked.
Could it also search CMAKE_PREFIX_PATH, and when the file is not found skip the read and keep the previous detection (get_target_property on rviz_common::rviz_common after find_package(rviz_common))? Something like:
find_file(RVIZ_COMMON_DEPS_FILE ament_cmake_export_dependencies-extras.cmake
PATHS ENV AMENT_PREFIX_PATH ${CMAKE_PREFIX_PATH}
PATH_SUFFIXES share/rviz_common/cmake NO_DEFAULT_PATH)
if(RVIZ_COMMON_DEPS_FILE)
...
endif()There was a problem hiding this comment.
See this comment for the description of the fix. I also added a NO_CACHE just in case a build directory is reused with another distro.
|
Thanks for the review and for the extra testing with the containers! I think these changes should cover everything to make the find more robust:
|
|
Thanks @Butakus Merging 🚀 |
Hi, it seems the Qt5/Qt6 issue is never resolved...
On Ubuntu 24.04,
navmap_rviz_pluginfails to configure when the Qt6 development files are installed:rviz_common15.2.2 is built against Qt5, but itsrviz_common-extras.cmakelooks for Qt6 first and only falls back to Qt5 if Qt6 is missing. With Qt6 installed, Qt5 is never loaded, and the exported-dependencies step then runs a barefind_package(Qt5), which fails.The existing Qt detection that we have in the CMakeLists.txt for the plugin (
get_target_propertyonrviz_common::rviz_common) runs afterfind_package(rviz_common), so CMake never reaches it. CMake aborts with an error when callingfind_package(rviz_common).Fix
Before
find_package(rviz_common), readrviz_common's exported dependencies to work out whether it uses Qt5 or Qt6:qt_major, which is used forfind_package(Qt${qt_major} ...)and linking, as before;CMAKE_DISABLE_FIND_PACKAGE_Qt6so rviz's extras don't pick up Qt6.