Use qt6 as the default dependency from rosdep#1635
Conversation
Signed-off-by: Alejandro Hernandez Cordero <ahcorde@gmail.com>
|
Related PR ros/rosdistro#48855 to bring svg devel package |
luca-della-vedova
left a comment
There was a problem hiding this comment.
Thanks you for the fix!
For reference, I believe the issue was that we export Qt5 as a dependency and call find_package on it:
$ cat /opt/ros/rolling/share/rviz_common/cmake/ament_cmake_export_dependencies-extras.cmake
# generated from ament_cmake_export_dependencies/cmake/ament_cmake_export_dependencies-extras.cmake.in
set(_exported_dependencies "geometry_msgs;message_filters;pluginlib;Qt5;rclcpp;rviz_ogre_vendor;rviz_rendering;sensor_msgs;std_msgs;std_srvs;tf2;tf2_ros;yaml_cpp_vendor;yaml-cpp")
But on the other hand we find_package for either Qt6 or Qt5, whichever is found first:
$ cat /opt/ros/rolling/share/rviz_common/cmake/rviz_common-extras.cmake
# find package Qt5 because otherwise using the rviz_common::rviz_common
# exported target will complain that the Qt5::Widgets target does not exist
find_package(QT NAMES Qt6 Qt5 QUIET COMPONENTS Widgets)
find_package(Qt${QT_VERSION_MAJOR} QUIET COMPONENTS Widgets)
So if Qt6 is installed we end up with a clash where we find both Qt6 and Qt5 and that doesn't really work.
On the other hand, with this PR, the ament_cmake_export_dependencies-extras.cmake becomes the following:
$ cat install/rviz_common/share/rviz_common/cmake/ament_cmake_export_dependencies-extras.cmake
# generated from ament_cmake_export_dependencies/cmake/ament_cmake_export_dependencies-extras.cmake.in
set(_exported_dependencies "geometry_msgs;message_filters;pluginlib;Qt6;rclcpp;rviz_ogre_vendor;rviz_rendering;sensor_msgs;std_msgs;std_srvs;tf2;tf2_ros;yaml_cpp_vendor;yaml-cpp")
So we will correctly only look for Qt6 (and possibly fallback to Qt5, but I haven't tested that code path).
I wonder if it is an explicit aim to keep supporting Qt5 or not, I haven't tested that path
|
Pulls: #1635
|
|
@ros-pull-request-builder retest this please |
1 similar comment
|
@ros-pull-request-builder retest this please |
|
Hi @ahcorde, I'm not sure but apparently this change broke rolling jobs, as they can't find Reference Builds
Log Output |
|
I also had to do this: # rviz switched to Qt6: https://github.com/ros2/rviz/pull/1635
if(rviz_common_VERSION_MAJOR GREATER_EQUAL 15 AND rviz_common_VERSION_MINOR GREATER_EQUAL 1)
find_package(Qt6 COMPONENTS Network REQUIRED)
set(QT_VERSION_MAJOR 6)
else()
find_package(Qt5 COMPONENTS Network REQUIRED)
set(QT_VERSION_MAJOR 5)
endif()
...
target_link_libraries(${PROJECT_NAME} PUBLIC
proj Qt${QT_VERSION_MAJOR}::Network
etcI could not rely on finding Qt6 because a users laptop might have both available. |
…case for Jazzy/Kilted with both Qt6 and Qt5, where Qt5 should be picked.
* Adopt QT_VERSION_MAJOR approach used in rvzi2 * Start using <depend> check on ROS_DISTRO to conditionally require qt5 or qt6 depending on ROS_DISTRO * Explicitly prefer Qt6 when Qt6 and Qt5 are both present to work around the default CMake ascending ordering resolving to Qt5. - For Rolling, Qt6 should be used if both are present - For Jazzy and Kilted, Qt5 should be used if both are present, but this will not cover that edge case * Adopt Timple's approach ros2/rviz#1635 (comment) for supporting edge case for Jazzy/Kilted with both Qt6 and Qt5, where Qt5 should be picked. * Set QT_DIR to fix transitive dependencies that should resolve to Qt6 but incorrectly resolve to Qt5 due to default cmake order resolution
Related with #1634