Skip to content
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

rtabmap: 0.21.5-2 in 'jazzy/distribution.yaml' [bloom] #41892

Merged
merged 1 commit into from
Jul 5, 2024

Conversation

matlabbe
Copy link
Contributor

@matlabbe matlabbe commented Jul 1, 2024

Increasing version of package(s) in repository rtabmap to 0.21.5-2:

@github-actions github-actions bot added the jazzy Issue/PR is for the ROS 2 Jazzy distribution label Jul 1, 2024
@matlabbe
Copy link
Contributor Author

matlabbe commented Jul 1, 2024

@marcoag new release without gtsam dependency

@clalancette clalancette added the held for sync Issue/PR has been held because the distribution is in a sync hold label Jul 1, 2024
@clalancette
Copy link
Contributor

Holding for Jazzy sync

@marcoag
Copy link
Contributor

marcoag commented Jul 5, 2024

Merging this in an effort to solve a RHEL regression: https://build.ros2.org/view/Jbin_rhel_el964/job/Jbin_rhel_el964__rtabmap__rhel_9_x86_64__binary/46/console

@matlabbe please take the RHEL error above into account once you bring the gtsam dependency back.

Maybe this type of changes deserves a patch bump instead of a debinc. Also, I assume this means you're moving tags around in your repo which is not a good practice.

@marcoag marcoag merged commit 86560b8 into ros:master Jul 5, 2024
4 checks passed
@matlabbe matlabbe deleted the bloom-rtabmap-72 branch July 5, 2024 04:55
@matlabbe
Copy link
Contributor Author

matlabbe commented Jul 5, 2024

The version of rtabmap library is not driven by the resulting ros package, but the standalone library itself (which is independent of ros). When there is no change to standalone library, but only for ros (because of some issues with build farm), I use the debinc instead. The main reason the version should stay the same is that I keep in sync rtabmap and all rtabmap_ros packages with same version (e.g. like opencv does with opencv_contrib), making it easier in the future to checkout compatible old versions between rtabmap and rtabmap_ros. Thus if I bump patch on rtabmap repo, I need to bump to same version for all 13 packages in rtabmap_ros repo, which can be time consuming and generate more pull requests here...

@matlabbe
Copy link
Contributor Author

matlabbe commented Jul 5, 2024

If the new source build failures https://build.ros2.org/job/Jsrc_uN__rtabmap__ubuntu_noble__source/10/ are caused by tag issues, remove or disable rtabmap from jazzy to unblock your jazzy sync. I'll deal with the versions when I come back from vacation.

@marcoag
Copy link
Contributor

marcoag commented Jul 10, 2024

Thanks @matlabbe, the sync is out. As you can see in the post the latest successful build of rtabmap was not removed and it remains on the repo for now, although rtabmap_* are still not there:

https://repo.ros2.org/status_page/ros_jazzy_default.html?q=rtabmap

The version of rtabmap library is not driven by the resulting ros package, but the standalone library itself (which is independent of ros). When there is no change to standalone library, but only for ros (because of some issues with build farm), I use the debinc instead. The main reason the version should stay the same is that I keep in sync rtabmap and all rtabmap_ros packages with same version (e.g. like opencv does with opencv_contrib), making it easier in the future to checkout compatible old versions between rtabmap and rtabmap_ros. Thus if I bump patch on rtabmap repo, I need to bump to same version for all 13 packages in rtabmap_ros repo, which can be time consuming and generate more pull requests here...

Sorry for the late reply to this (took me some time to gather more info). What you mentioned makes total sense, from the package point of view it can be considered a metadata change and then the -release (debinc) number can be the one bumped. For the record, Debian only allows this on the development version. Besides, removing/adding a dependency in your package seems to bring heavy breaking changes (missing/added parts) of the library so this could even be considered for a major bump if SemVer is followed.

Regarding the concerns of keeping package and library version the same, this is not a mandatory thing for ROS packages and in fact this is a hard thing to do when the ROS package is a wrapper over a library like yours (i.e. RobotLocomotion/ros-drake-vendor#16 (comment)).

Please note these are only suggestions I think might improve the experience of your package users but you are welcome to manage your package versioning as you see best fit.

@matlabbe
Copy link
Contributor Author

Thanks for the referred link, keeping in sync major.minor only between the repos like cartographer could be a compromise, where the patch could diverge. Not sure when that would be fixed, but I created a ticket to track that issue: introlab/rtabmap#1313

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
held for sync Issue/PR has been held because the distribution is in a sync hold jazzy Issue/PR is for the ROS 2 Jazzy distribution
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants