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

CI is broken on Rolling #1970

Closed
mikeferguson opened this issue Feb 23, 2023 · 8 comments · Fixed by #1994
Closed

CI is broken on Rolling #1970

mikeferguson opened this issue Feb 23, 2023 · 8 comments · Fixed by #1994
Labels
bug Something isn't working

Comments

@mikeferguson
Copy link
Contributor

The fix is almost certainly needed to be upstream - just ticketing here for visibility (so we don't have multiple people working on it)

It looks like the failure is actually in sensor_msgs:

image

@mikeferguson mikeferguson added the bug Something isn't working label Feb 23, 2023
@mikeferguson mikeferguson self-assigned this Feb 23, 2023
@mikeferguson
Copy link
Contributor Author

  • We depend on action_msgs
  • action_msgs depends on service_msgs - so it should already be installed

@mikeferguson
Copy link
Contributor Author

mikeferguson commented Feb 23, 2023

Locally, I can build moveit_msgs against ros2-testing debs (and I uninstalled all of ros-rolling-* before tasting, so it's not a missing dependency). None of the service_msgs stuff is in the main ros2 repos (we haven't haven't had a sync yet)

@sjahr
Copy link
Contributor

sjahr commented Mar 6, 2023

This commit could be the culprit: ros2/rosidl_defaults@a3f753d. But it still does not explain why service_msgs is not installed 🤔

jspricke added a commit to jspricke/moveit2 that referenced this issue Mar 6, 2023
To install missing ros-rolling-service-msgs.

Closes: moveit#1970
@jspricke
Copy link
Contributor

jspricke commented Mar 6, 2023

The job uses an old Docker image of the ros2-testing repos where ros-rolling-service-msgs is not installed and it does not apt dist-upgrade the system: https://github.com/ros-planning/moveit2/blob/main/.docker/source/Dockerfile#L28. So service_msgs is never picked up and thus missing. The logical fix is to use dist-upgrade in the job, as proposed in a144a3c but Docker lint complains about that: https://github.com/jspricke/moveit2/actions/runs/4346334681. So you can either fix Docker lint or build a new Docker base image and run into this problem again, next time. Your call.

Edit: A third option would be to not install (old) packages in the base image.

@mikeferguson
Copy link
Contributor Author

@tylerjw - I think this may be more up your alley - since it looks like it is our CI, not some upstream break

@mikeferguson mikeferguson removed their assignment Mar 6, 2023
@jspricke
Copy link
Contributor

jspricke commented Mar 7, 2023

Discussed this in the standup, the problem is that the ci-testing Docker images are based on the ci images which the added ros2-testing apt sources: https://github.com/ros-planning/moveit2/blob/main/.docker/ci-testing/Dockerfile and the apt-get upgrade in there is not enough to install a new dependency package. @henningkayser will rework the Dockerfile to work from a base image.

@vatanaksoytezer
Copy link
Contributor

I think we only need to add a rosdep step to ci-testing and that should be sufficient. The dependency in question (service_msgs) is not being installed in apt update / apt upgrade, it should be installed and triggered through rosdep install

@jspricke
Copy link
Contributor

jspricke commented Mar 7, 2023

@vatanaksoytezer no, service_msgs is a dependency of rcl already.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants