-
Notifications
You must be signed in to change notification settings - Fork 264
Use qt6 as the default dependency from rosdep #1635
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
Conversation
Signed-off-by: Alejandro Hernandez Cordero <[email protected]>
|
Related PR ros/rosdistro#48855 to bring svg devel package |
luca-della-vedova
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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 |
Related with #1634