-
-
Notifications
You must be signed in to change notification settings - Fork 652
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
update to core22, remove ros1, enable humble instead of foxy #849
update to core22, remove ros1, enable humble instead of foxy #849
Conversation
why should I remove ROS1? |
We cannot remove ROS1, nor Foxy. I am still learning this part of the snap system with multiple channels :) |
962b39c
to
fed1140
Compare
@facontidavide can you take a look now? 2 pipelines, no ros1 removal. could you also run ci? |
Interesting, it fails to build even core20/foxy snaps due to astuff/astuff_sensor_msgs@583e90b removing a message package. it's also breaking non-snap related builds such as #842 for the same reason. |
fed1140
to
bb9ce5b
Compare
snap_core22/snapcraft.yaml
Outdated
snapcraftctl set-version "$version" | ||
snapcraftctl set-grade "$grade" |
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.
craftctl set version
could be used here from what this guide says.
Sme for the grade.
snap_core22/snapcraft.yaml
Outdated
- ros-humble-geometry-msgs | ||
- ros-humble-ackermann-msgs | ||
- ros-humble-action-msgs | ||
- ros-humble-actionlib-msgs |
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.
Would be easier to read if this was sorted alphabetically.
snap_core22/snapcraft.yaml
Outdated
- ros-humble-webots-ros2-msgs | ||
- ros-humble-wiimote-msgs | ||
- ros-humble-wireless-msgs | ||
# - ros-humble-astuff-sensor-msgs # new ppa, should we add it? https://github.com/astuff/pacmod3/commit/ff0f8f1bb1a1ba88f495a3294725207309c739ae |
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.
I would say to remove since it's not in the official ROS 2 repo.
snap_core22/snapcraft.yaml
Outdated
# - ros-humble-pacmod3-msgs # new ppa, should we add it? https://github.com/astuff/pacmod3/commit/ff0f8f1bb1a1ba88f495a3294725207309c739ae | ||
# - ros-humble-raptor-dbw-msgs # https://github.com/NewEagleRaptor/raptor-dbw-ros/commit/7feaf35d996addac469a0816ab0d7ab9037ca6dd | ||
# - ros-humble-raptor-pdu-msgs # https://github.com/NewEagleRaptor/raptor-dbw-ros/commit/7feaf35d996addac469a0816ab0d7ab9037ca6dd |
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.
Same here, if they are not part of the official ROS 2 release it shouldn't be added otherwise everyone is going to add their custom msgs.
snap_core22/snapcraft.yaml
Outdated
source: . | ||
cmake-parameters: | ||
- -DCMAKE_BUILD_TYPE=Release | ||
- "-DCMAKE_PREFIX_PATH=$(echo $SNAPCRAFT_CMAKE_ARGS | awk -F= '{printf(\"%s/usr/lib/$SNAPCRAFT_ARCH_TRIPLET/cmake/Qt5\", $2)}')" |
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.
- "-DCMAKE_PREFIX_PATH=$(echo $SNAPCRAFT_CMAKE_ARGS | awk -F= '{printf(\"%s/usr/lib/$SNAPCRAFT_ARCH_TRIPLET/cmake/Qt5\", $2)}')" | |
- "-DCMAKE_PREFIX_PATH=$(echo $SNAPCRAFT_CMAKE_ARGS | awk -F= '{printf(\"%s/usr/lib/$CRAFT_ARCH_TRIPLET/cmake/Qt5\", $2)}')" |
snap_core22/snapcraft.yaml
Outdated
- -DBUILD_TESTING=OFF | ||
- -DBUILD_DOCS=OFF | ||
# point to previously build plotjuggler | ||
- -Dplotjuggler_DIR:PATH=$SNAPCRAFT_STAGE/usr/local/lib/cmake/plotjuggler |
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.
- -Dplotjuggler_DIR:PATH=$SNAPCRAFT_STAGE/usr/local/lib/cmake/plotjuggler | |
- -Dplotjuggler_DIR:PATH=$CRAFT_STAGE/usr/local/lib/cmake/plotjuggler |
@Guillaumebeuzeboc I applied all the changes you suggested, please have a look and resolve if you are ok with them, here is the relevant commit e599e88 |
LGTM 👍 |
b742cef
to
83cbe14
Compare
everything looks green. Do you think it can be merged? |
Yes @facontidavide , the only missing part is the publish stage which will trigger after the merge, I will follow the CI to see what happens. And if we merge this we can close #867 |
Given that the newer ROS versions require ubuntu22 as the base OS, an update to the snapcraft files is needed and this PR brings the snap to core22.
Since ROS1 is no longer supported in 22, I had to duplicate the pipelines in the old ros1/ros2/core20 and the new ros2/core22.
The existing snap is still the default, if a user needs the new snap with core22 they can install by manually changing the snap track to humble in their machine.
Eventually the new default should become core22 and at that point users still on ros1 will have to change their snap track.
Requires #853 before CI can pass.