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

Add note about uninstalling debians before installing MoveIt2 from source #763

Merged
merged 2 commits into from
Nov 23, 2022

Conversation

AndyZe
Copy link
Member

@AndyZe AndyZe commented Nov 13, 2022

This simple change should prevent some issues like this one: moveit/moveit2_tutorials#546

@rhaschke
Copy link
Contributor

Wouldn't it be better if you fix MoveIt2's build config? Obviously, the include order is screwed.

@AndyZe
Copy link
Member Author

AndyZe commented Nov 15, 2022

@rhaschke the include order might be screwed but that's another, separate issue. The problem here (as I understand it) is colcon doesn't support overlays and underlays. @tylerjw prob knows more.

Here are some links where we've had to suggest debian removal or users figured it out on their own. I know there are more occurrences but these are the ones I can find easily:

moveit/moveit2#826 (comment)
moveit/moveit2#1205 (comment)
moveit/moveit2#1465 (comment)

@tylerjw
Copy link
Member

tylerjw commented Nov 16, 2022

@rhaschke here is some threads about it:

colcon/colcon-core#465
ros2/ros2#1150 (comment)

The short story is that overlays do not work in ROS 2.

@rhaschke
Copy link
Contributor

The short story is that overlays do not work in ROS 2.

I'm sorry, but this statement is wrong. workspace overlays are a fundamental building block of ROS and they work both in ROS1 and ROS2. Yes, there was a bug related to include-folder ordering in Galactic and before (see ament/ament_cmake#402).
However, for Humble, they decided to use unique include folders (see ament/ament_cmake#402 (comment)), which would solve that issue.
As far as I can see, MoveIt2 doesn't yet comply to this rule and I was asking to change that.

The override warning introduced in colcon/colcon-core#449 as an error and later downgraded to a warning (colcon/colcon-core#466) pops up any time you attempt to override a package. So, yes, if you want to avoid the warning completely, you need to uninstall debian packages. But they don't break the build if you follow best-practice rules.

@tylerjw
Copy link
Member

tylerjw commented Nov 16, 2022

As far as I can see, MoveIt2 doesn't yet comply to this rule and I was asking to change that.

Do you know of any documentation on how to fix this? I'd be happy to clean this up if I can. My understanding (probably mis-understanding) from reading those threads is that all they did was add a warning because they decided they couldn't fix it.

@rhaschke
Copy link
Contributor

Did you read this doc linked from one of the comments?

@tylerjw
Copy link
Member

tylerjw commented Nov 16, 2022

Did you read this doc linked from one of the comments?

This makes me so frustrated. I've asked people from OR several times if there are docs for this somewhere. Why did they hide these docs here if everyone who publishes a library needs to know about this? Shouldn't this be in the docs for ament in the main ROS docs?

This smells so much like something technical is broken about the tooling (cmake, ament, colcon) and they decided to "solve" it by requiring users to do yet more things they will easily make mistakes doing. Anyway, I'll go try to make this update to MoveIt2 and all the libraries I publish.

@AndyZe
Copy link
Member Author

AndyZe commented Nov 23, 2022

Merging since it's useful information, for now. We can revert it later if MoveIt installation gets cleaned up.

@AndyZe AndyZe merged commit 5bdfabb into moveit:main Nov 23, 2022
@AndyZe AndyZe deleted the andyz/uninstall_debs branch November 23, 2022 18:29
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