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

Rebuild lanelet2-io with correct pugixml and ros-humble-ros-workspace with fastrtps fix #244

Merged
merged 6 commits into from
Jan 15, 2025

Conversation

traversaro
Copy link
Member

@wep21 noticed that after the build in #242, the build ros-humble-lanelet2-io package uses pugixml 1.15, while rviz uses pugixml 1.14 . This was a consequence of us not using the conda-forge complete pinning file due to prefix-dev/rattler-build#1285, and a recent pugixml update conda-forge/pugixml-feedstock#27 .

As only the lanelet2-io package is affected, the best solution is to just rebuild this package with the correct pugixml version, to permit these I done the modification in vinca proposed in RoboStack/vinca#67 .

@traversaro
Copy link
Member Author

The PRs are rebuilding a bit too many packages, probably there is something wrong in RoboStack/vinca#67 .

@traversaro
Copy link
Member Author

The PRs are rebuilding a bit too many packages, probably there is something wrong in RoboStack/vinca#67 .

No, the problem is present also in other PRs that do not change anything else. I guess the problem is that there were new releases (ros/rosdistro#44186 and ros/rosdistro#44185) and somehow vinca is not skipping them as the version is new, even if the build number is the same?

@traversaro
Copy link
Member Author

The PRs are rebuilding a bit too many packages, probably there is something wrong in RoboStack/vinca#67 .

No, the problem is present also in other PRs that do not change anything else. I guess the problem is that there were new releases (ros/rosdistro#44186 and ros/rosdistro#44185) and somehow vinca is not skipping them as the version is new, even if the build number is the same?

I can't reproduce the issue locally, it is like for some reason locally trigger_new_versions is false, while in CI is true , see https://github.com/RoboStack/vinca/blob/aa102b280adcad0374e4d231b25d21bf0db27d61/vinca/main.py#L987 .

@traversaro
Copy link
Member Author

The PRs are rebuilding a bit too many packages, probably there is something wrong in RoboStack/vinca#67 .

No, the problem is present also in other PRs that do not change anything else. I guess the problem is that there were new releases (ros/rosdistro#44186 and ros/rosdistro#44185) and somehow vinca is not skipping them as the version is new, even if the build number is the same?

I can't reproduce the issue locally, it is like for some reason locally trigger_new_versions is false, while in CI is true , see https://github.com/RoboStack/vinca/blob/aa102b280adcad0374e4d231b25d21bf0db27d61/vinca/main.py#L987 .

I found the problem. For some reason since oursland/ros-jazzy@f916d0e we are using the -n (that is the command line option for trigger_new_versions) only in the PR CI. I think we can safely remove it.

@Tobias-Fischer
Copy link
Contributor

LGTM, thanks - only nit pick: could you please change "Remainder" to "Reminder" in the vinca*.yaml files?

@traversaro
Copy link
Member Author

LGTM, thanks - only nit pick: could you please change "Remainder" to "Reminder" in the vinca*.yaml files?

Done in 3f01400 .

@Tobias-Fischer
Copy link
Contributor

Cool - if you mark it as ready to review, I'll merge :)

@traversaro
Copy link
Member Author

Cool - if you mark it as ready to review, I'll merge :)

The non-PR CI jobs use the master branch of vinca, so we need to first merge RoboStack/vinca#67 or change the CI to point to the PR branch (that I would avoid).

@traversaro traversaro changed the title Rebuild lanelet2-io with correct pugixml Rebuild lanelet2-io with correct pugixml and ros-humble-ros-workspace with fastrtps fix Jan 15, 2025
@traversaro
Copy link
Member Author

I also bumped the build bumber of ros_workspace as it needed to solve prefix-dev/pixi#2910, the downside is that now we also need to merge RoboStack/vinca#68 before merging this PR.

fyi @ruben-arts

@Tobias-Fischer
Copy link
Contributor

I think we can merge now?

@traversaro
Copy link
Member Author

I think we can merge now?

Sorry, I had to refresh pixi.lock with the latest vinca commit. Now it is ready to go (if CI is happy).

@traversaro traversaro marked this pull request as ready for review January 15, 2025 19:37
@Tobias-Fischer Tobias-Fischer merged commit 0651465 into main Jan 15, 2025
6 checks passed
@traversaro
Copy link
Member Author

@wep21 can you check if this fixed your problem?

@Tobias-Fischer Tobias-Fischer deleted the rebuildlanelet2withcorrectpugixml branch January 15, 2025 23:28
@wep21
Copy link
Contributor

wep21 commented Jan 16, 2025

@traversaro @Tobias-Fischer The issue seems to be solved. Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants