MRPT / mrpt

:zap: The Mobile Robot Programming Toolkit (MRPT)
https://docs.mrpt.org/reference/latest/
BSD 3-Clause "New" or "Revised" License
1.97k stars 632 forks source link

mrpt1 builds failing on ARM platforms #767

Closed mikaelarguedas closed 6 years ago

mikaelarguedas commented 6 years ago

The version currently released in ROS fails to build on ARM for both kinetic and melodic (link to the failing jobs below). Is mrpt1 expected to work on ARM ? if so we'll look forward the next release. Otherwise please let us know so that we can disable the arm jobs for this package. Thanks!

http://build.ros.org/job/Mbin_dsv8_dSv8__mrpt1__debian_stretch_arm64__binary/ http://build.ros.org/view/Mbin_ubhf_uBhf/job/Mbin_ubhf_uBhf__mrpt1__ubuntu_bionic_armhf__binary/ http://build.ros.org/view/Mbin_ubv8_uBv8/job/Mbin_ubv8_uBv8__mrpt1__ubuntu_bionic_arm64__binary/

http://build.ros.org/view/Kbin_uxhf_uXhf/job/Kbin_uxhf_uXhf__mrpt1__ubuntu_xenial_armhf__binary/ http://build.ros.org/view/Kbin_uxv8_uXv8/job/Kbin_uxv8_uXv8__mrpt1__ubuntu_xenial_arm64__binary/ http://build.ros.org/view/Kbin_djv8_dJv8/job/Kbin_djv8_dJv8__mrpt1__debian_jessie_arm64__binary/

/cc @clalancette

jolting commented 6 years ago

I believe this was recently addressed: https://github.com/MRPT/mrpt/commit/219020317d503bfac08a44ee10b2415ff7ae7150

jlblancoc commented 6 years ago

@mikaelarguedas Exactly, as @jolting says, this was fixed just yesteday in that commit. However, we have a more serious issue (https://github.com/ros-infrastructure/ros_buildfarm/issues/543) trying to release for kinetic. I could make a new release with the patch for melodic, though. Just me know how "urgent" do you think it is, just in case we first attempt to solve the Kinetic build farm problem.

mikaelarguedas commented 6 years ago

Thanks for the quick response. Anytime before the melodic release (May 23rd) is fine

jlblancoc commented 6 years ago

Should be done after https://github.com/ros/rosdistro/pull/17869

We'll keep an eye on the build farm anyway... Cheers!

jolting commented 6 years ago

Should use the packaged octomap version instead of downloading it.

jlblancoc commented 6 years ago

Right! Also, it still fails. Will check the CMake logic, probably I made some stupid mistake. After fixing those two points, will try it again.

jlblancoc commented 6 years ago

Should be fixed after:

Rosdistro PR:

jolting commented 6 years ago

That certainly takes a long time to build. Almost 8 hours and still no where near complete.

mikaelarguedas commented 6 years ago

This is a very heavy package. It does timeout sometimes on amd64 machines so it will likely be close to the 10hour limit we set for the ARM jobs. On the bright side that shows that the compilation goes much further than the previous version when it was failing after 20 minutes :).

As a general policy, we recommend making ROS packages as small and modular as possible. This package is at the opposite side of the spectrum. I encourage you to consider splitting the features into several ROS packages like other frameworks and libraries do. This allows more liberty for users wanting to use only a subset of features without pulling many dependencies or spending a significant amount of time building from source.

jlblancoc commented 6 years ago

Building the apps takes 50 minutes, so a solution would be just build (and install) the libraries. I think it can be done via a patch to the CMake script, probably as a bloom patch.

Will give it a try...

jlblancoc commented 6 years ago

Finally found this which seems a better idea. Just made this change. Let's repeat the release and wait for the build farm...

mikaelarguedas commented 6 years ago

Looks like it did the job :+1: Thanks for the quick fix!