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

Work around rviz_common import error #274

Merged
merged 1 commit into from
Jun 6, 2019
Merged

Conversation

ruffsl
Copy link
Member

@ruffsl ruffsl commented Jun 5, 2019

@ruffsl ruffsl requested a review from mikaelarguedas June 5, 2019 23:45
@ghost
Copy link

ghost commented Jun 6, 2019

Thanks. I'll hold off on merging my workaround since you got this done so fast.

@ruffsl
Copy link
Member Author

ruffsl commented Jun 6, 2019

@osrf/docker-image-maintainers , feel free to merge if approved.

@mikaelarguedas
Copy link
Contributor

Yeah that's one way to work around it.

I don't see a working case before that would be broken by this change and as it seems to fix navigation2 ci that's fine by me as a temporary workaround 👍 .

From the issue description it sounds like the issue is assumed to be in the nightly image, but at first glance it looks like any user using the extracted archive as an underlay will be facing the same problem (docker involved or not), is that correct?

@crdelsey Will the end issue and long term fix be located on the ros2/rviz repository ?

@ghost
Copy link

ghost commented Jun 6, 2019

any user using the extracted archive as an underlay will be facing the same problem (docker involved or not), is that correct

Yes that is correct.

Will the end issue and long term fix be located on the ros2/rviz repository ?

I don't know. I don't think so. I don't understand the correct solution yet. There are 3 possibilities I can see:

  1. Fix ament. Since CMake doesn't seem to support what we need here, it probably means making a custom equivalent of what CMake does internally.
  2. Change the rviz build to not use interfaces. I'm not sure they are really necessary in this case, but there are other packages that use them as well, so they'd still have the problem
  3. Change the way the build farm does the nightly build, so that the packages end up in the /opt/ros/... directory tree.

@ruffsl ruffsl merged commit c05b9a9 into master Jun 6, 2019
@mikaelarguedas
Copy link
Contributor

thanks @crdelsey for the clarification 👍

@mikaelarguedas mikaelarguedas deleted the ament_export_interfaces branch June 6, 2019 09:38
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.

2 participants