Skip to content

A spurious dependency on eigen_msgs is causing OMPL to fail to build.#1174

Merged
rhaschke merged 1 commit intomoveit:melodic-develfrom
clalancette:remove-ompl-eigen-dep
Oct 30, 2018
Merged

A spurious dependency on eigen_msgs is causing OMPL to fail to build.#1174
rhaschke merged 1 commit intomoveit:melodic-develfrom
clalancette:remove-ompl-eigen-dep

Conversation

@clalancette
Copy link
Copy Markdown
Contributor

@clalancette clalancette commented Oct 30, 2018

Description

Remove a spurious dependency on eigen_conversions/eigen_msgs.h in model_based_planning_context.cpp; as far as I can tell, it is not necessary, and is causing the build to fail on the Melodic build farm: http://build.ros.org/job/Mbin_uB64__moveit_planners_ompl__ubuntu_bionic_amd64__binary/17/console

Checklist

  • Required by CI: Code is auto formatted using clang-format
  • Extended the tutorials / documentation, if necessary reference
  • Include a screenshot if changing a GUI
  • Document API changes relevant to the user in the moveit/MIGRATION.md notes
  • Created tests, which fail without this PR reference
  • Decide if this should be cherry-picked to other current ROS branches
  • While waiting for someone to review your request, please consider reviewing another open pull request to support the maintainers

@clalancette
A spurious dependency on eigen_msgs is causing OMPL to fail to build.
91ab6d0 
Just remove it.

Signed-off-by: Chris Lalancette <clalancette@openrobotics.org>
rhaschke
rhaschke approved these changes Oct 30, 2018
View reviewed changes
Copy link
Copy Markdown
Contributor

@rhaschke rhaschke left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

+1 If Travis passes.

@rhaschke rhaschke mentioned this pull request Oct 30, 2018
Merged
@rhaschke rhaschke merged commit e98453a into moveit:melodic-devel Oct 30, 2018
@rhaschke
Copy link
Copy Markdown
Contributor

rhaschke commented Oct 30, 2018

Thanks @clalancette!

@rhaschke rhaschke mentioned this pull request Oct 30, 2018
Closed
11 tasks
@IanTheEngineer
Copy link
Copy Markdown
Contributor

IanTheEngineer commented Oct 30, 2018

Regression introduced in #1160 & #1099

It's not clear to me why our Docker build missed this regression.

@clalancette clalancette deleted the remove-ompl-eigen-dep branch October 30, 2018 14:36
@clalancette
Copy link
Copy Markdown
Contributor Author

clalancette commented Oct 30, 2018

Great, thanks. A new release with this change should get the build back to normal on Melodic.

@rhaschke
Copy link
Copy Markdown
Contributor

rhaschke commented Oct 30, 2018

It's not clear to me why our Docker build missed this regression.

For the same reason as last time: The binary builds on ros.buildfarm, build each package on its own, i.e. in a clean environment. Our Travis build (as well as ros.buildfarm's devel build) build all packages together, first installing the unified set of all package's dependencies. Hence another package pulling in eigen_conversions, will hide the issue.

@IanTheEngineer
Copy link
Copy Markdown
Contributor

IanTheEngineer commented Oct 30, 2018

Ah, that makes complete sense. Thanks @rhaschke

@130s
Copy link
Copy Markdown
Contributor

130s commented Oct 30, 2018

@rhaschke @clalancette Are you aware of any way to capture the issues that happen on release on ROS buildfarm?

I even went ahead tested on a Docker image ubuntu:bionic, which is very clean in ROS-sense, then built moveit by catkin_make_isolated --install (checked out 0.10.4 which is still failing to build), but it built.

sjahr pushed a commit to sjahr/moveit that referenced this pull request Jun 21, 2024
@DLu
Fix Moveit Configs Utils Bug (moveit#1174)
0f3f882
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@rhaschke rhaschke rhaschke approved these changes

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@clalancette @rhaschke @IanTheEngineer @130s