Skip to content

Delete manifest.xml#26

Merged
ivanpauno merged 1 commit intoros2:ros2from
giusebar:bug/remove_manifest_xml
Sep 21, 2022
Merged

Delete manifest.xml#26
ivanpauno merged 1 commit intoros2:ros2from
giusebar:bug/remove_manifest_xml

Conversation

@giusebar
Copy link
Copy Markdown

@giusebar giusebar commented Sep 8, 2022

Remove manifest.xml.

I've been trying to compile these packages in Ubuntu 20.04, ROS 2 Foxy.

After cloning the package, I'm installing the rosdep with rosdep install -i -y --from-paths . .

However, not all the dependencies defined in package.xml are installed. E.g. eigen3_cmake_module.

Because of this, when compiling with colcon build --executor=sequential --event-handlers console_direct+, I get the following errors complaining it can't find Eigen libraries (fatal error: Eigen/Core: No such file or directory).

After some investigation I've discovered that, for some reason, when trying to install dependencies via rosdep, the dependencies defined in manifest.xml are used first. And dependencies like eigen3_cmake_modules are not installed. This causes compilation via colcon to fail. The failure is due to the fact that the eigen3_cmake_module takes care of setting the correct Eigen3_INCLUDE_DIRS here.
Without that command EIGEN3_INCLUDE_DIRS will be set instead, and ignored in the CMakeLists.txt.

This should also be backported, as the issue happens also in foxy (in which i've tested this).

Remove manifest.xml. For some reason when trying to install dependencies via rosdep, the dependencies defined in manifest.xml are used first. Because of that dependencies such as eigen3_cmake_module are not installed. This causes compilation via colcon to fail.
@giusebar
Copy link
Copy Markdown
Author

@audrow @ivanpauno could you please have a look at this?
Should I propose the change upstream in the Orocos repository as well?

Thanks in advance for the help!

@ivanpauno
Copy link
Copy Markdown
Member

Hi @giusebar,

FYI, this repo is only used for ROS 2 distros up to Galactic, Humble and newer are using https://github.com/ros2/orocos_kdl_vendor (which is a cmake shim to download and build the package if not already installed).

You should be able to install this package from debians as well e.g. for Foxy apt install ros-foxy-orocos-kinematics-dynamics, i.e. don't need to build from source.

After some investigation I've discovered that, for some reason, when trying to install dependencies via rosdep, the dependencies defined in manifest.xml are used first. And dependencies like eigen3_cmake_modules are not installed. This causes compilation via colcon to fail. The failure is due to the fact that the eigen3_cmake_module takes care of setting the correct Eigen3_INCLUDE_DIRS here.

did you have the ROS_DISTRO environment variable set correctly?
If that's the case, this sounds like a bug in rosdep, though I'm not sure.

Should I propose the change upstream in the Orocos repository as well?

Removing the old manifest in this repo is fine, as it's not used at all for ROS 2 builds.
You will need to ask upstream if it's okay to remove it there, I'm not sure.

Copy link
Copy Markdown
Member

@ivanpauno ivanpauno left a comment

Choose a reason for hiding this comment

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

LGTM!

Please, double check this question:

did you have the ROS_DISTRO environment variable set correctly?

so we know if this change is really needed or not

@giusebar
Copy link
Copy Markdown
Author

Hello @ivanpauno,

Thanks for your review and reply! Yes, I tried a few times to make sure I was setting up the ROS_DISTRO environment variable. Also run the command with explicit rosdistro set: rosdep install -i -y --from-paths . --rosdistro foxy. The dependencies in package.xml get installed only when removing manifest.xml file. It seems like a bug in rosdep indeed.

@ivanpauno ivanpauno merged commit f48062a into ros2:ros2 Sep 21, 2022
@ivanpauno
Copy link
Copy Markdown
Member

@Mergifyio backport galactic foxy

mergify Bot pushed a commit that referenced this pull request Sep 21, 2022
Remove manifest.xml. For some reason when trying to install dependencies via rosdep, the dependencies defined in manifest.xml are used first. Because of that dependencies such as eigen3_cmake_module are not installed. This causes compilation via colcon to fail.

(cherry picked from commit f48062a)
@mergify
Copy link
Copy Markdown

mergify Bot commented Sep 21, 2022

backport galactic foxy

✅ Backports have been created

Details

mergify Bot pushed a commit that referenced this pull request Sep 21, 2022
Remove manifest.xml. For some reason when trying to install dependencies via rosdep, the dependencies defined in manifest.xml are used first. Because of that dependencies such as eigen3_cmake_module are not installed. This causes compilation via colcon to fail.

(cherry picked from commit f48062a)
@ivanpauno
Copy link
Copy Markdown
Member

@giusebar see orocos#222 (comment).
Based on that, I don't think this is going to be accepted upstream.
Maybe there's a way to update the manifest.xml so it works well...

Anyway, the change is okay in this repo.

ivanpauno pushed a commit that referenced this pull request Sep 21, 2022
Remove manifest.xml. For some reason when trying to install dependencies via rosdep, the dependencies defined in manifest.xml are used first. Because of that dependencies such as eigen3_cmake_module are not installed. This causes compilation via colcon to fail.

(cherry picked from commit f48062a)

Co-authored-by: giusebar <giuse.barbieri@gmail.com>
ivanpauno pushed a commit that referenced this pull request Sep 21, 2022
Remove manifest.xml. For some reason when trying to install dependencies via rosdep, the dependencies defined in manifest.xml are used first. Because of that dependencies such as eigen3_cmake_module are not installed. This causes compilation via colcon to fail.

(cherry picked from commit f48062a)

Co-authored-by: giusebar <giuse.barbieri@gmail.com>
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