Fix Ubuntu Jammy Build. Close #232

nachovizzo commented 1 year ago

This is my attempt to solve OpenVDB build issues.

Reference

All the inspiration was taken from vdbfusion

ToDo

[ ] OpenVDB is now built as a shared library, but it's straightforward to make it static. It will depend on the final application. I will need help from people using the package to provide feedback on the build solution.
[ ] Test with crazy native installations of different combinations of TBB+OpenVDB to make sure the build also works

Build on dockers

I've created a dummy repo to test the build both for ros humble and ros iron here. Additionally, I've added a CI to the repo to make sure I didn't have any garbage locally. The CI is passing

Comments

I'm pulling the apparently only 3 dependencies that stvl needs in order to build in the CI apt-get update && apt-get install --no-install-recommends -y ros-${ROS_DISTRO}-navigation2 ros-${ROS_DISTRO}-nav2-bringup ros-${ROS_DISTRO}-pcl-conversions

agoeckner commented 9 months ago

Is this PR ready to go? @nachovizzo @SteveMacenski

SteveMacenski commented 9 months ago

This is a good question. @nachovizzo?

@agoeckner if you clone this, does it work for you? One of my asks is going to be a 3rd party tester anyway, so you're in the right spot asking the right questions :-)

agoeckner commented 9 months ago

Looks good to me! I tested in a fresh ros:humble Docker container after running rosdep install.

root@7221f9331f4d:/ws# colcon build
Starting >>> spatio_temporal_voxel_layer
[Processing: spatio_temporal_voxel_layer]
[Processing: spatio_temporal_voxel_layer]
[Processing: spatio_temporal_voxel_layer]
[Processing: spatio_temporal_voxel_layer]
[Processing: spatio_temporal_voxel_layer]
[Processing: spatio_temporal_voxel_layer]
[Processing: spatio_temporal_voxel_layer]
[Processing: spatio_temporal_voxel_layer]
[Processing: spatio_temporal_voxel_layer]
[Processing: spatio_temporal_voxel_layer]
[Processing: spatio_temporal_voxel_layer]
--- stderr: spatio_temporal_voxel_layer
CMake Warning at cmake/find_dependencies.cmake:41 (message):
  Building OpenVDB from source
Call Stack (most recent call first):
  CMakeLists.txt:34 (include)

Cloning into 'external_openvdb'...
HEAD is now at ddf922f Updated change logs for 10.0.1
CMake Deprecation Warning at openvdb/openvdb/CMakeLists.txt:128 (message):
  Support for Boost versions < 1.76 is deprecated and will be removed.

/ws/spatio_temporal_voxel_layer/src/spatio_temporal_voxel_grid.cpp: In member function ‘void volume_grid::SpatioTemporalVoxelGrid::ResetGridArea(const volume_grid::occupany_cell&, const volume_grid::occupany_cell&, bool)’:
/ws/spatio_temporal_voxel_layer/src/spatio_temporal_voxel_grid.cpp:401:8: warning: statement has no effect [-Wunused-value]
  401 |   for (cit_grid; cit_grid.test(); ++cit_grid)
      |        ^~~~~~~~
---
Finished <<< spatio_temporal_voxel_layer [5min 47s]

Summary: 1 package finished [5min 48s]
  1 package had stderr output: spatio_temporal_voxel_layer
root@7221f9331f4d:/ws#

Side note: it's crazy that the ros-humble-pcl-conversions package requires 1900MB of dependencies, including basically an entire X11 installation...

SteveMacenski commented 9 months ago

That can possibly be addressed? It might be including more modules in pcl than strictly required. The X11 is probably the visualization stuff from VKT which is almost certainly not required

agoeckner commented 9 months ago

Probably, though for this PR I think we're good to continue.

nachovizzo commented 9 months ago

Holy moly, this went off my radar for a while. @SteveMacenski , when I did it ti was building on a Docker I build (I even think I made a separate repo for that). But I have no clue how to test this on "the farm". But if @agoeckner already tried then I'd say is good to go.

agoeckner commented 9 months ago

My test was also performed in Docker (the official ros:humble image).

SteveMacenski commented 9 months ago

Great, I just checked with Intrinsic and it is possiible to do this in the build farm. @nachovizzo can this be modified to be statically linked so that it is included in this package and won't mess with the users' local openVDB install if they have their own? I believe (?) that would sufficiently isolate this so we could have it be standalone

SteveMacenski commented 9 months ago

@nachovizzo I just spoke with Intrinsic again and I think this is a good workflow:

Create a new STVL directory to hold the current package
Create a new pkg in this repo openvdb_vendor in accordance to this example using your code https://github.com/mjcarroll/ogre_next_vendor/blob/0e73c5e3cbb3098e9cef1c900bc79e918371c133/CMakeLists.txt#L7-L26
Set the dependency / find_package(openvdb_vendor) on that vendor version in STVL to use it, so it finds the opt/ros/ version instead of the system installed version

We should then be good to go. If someone wants to use their own openvdb version, then its as simple as removing the _vendor and it'll use the system instaalled version. That's the recommended way per Michael Carroll

mjcarroll commented 9 months ago

If someone wants to use their own openvdb version, then its as simple as removing the _vendor and it'll use the system installed version.

It would likely be easier to just change the version that the ament_cmake_vendor_package is looking for if you want to bump to newer version (or different branches).

There is also an opportunity to build some logic to just use the system installed package if it meets a sufficient threshold version, which could make the transition to 24.04 easier when that time comes. That is, of course, dependent on the 24.04 packaged version of openvdb being not-broken.

mjcarroll commented 9 months ago

Also note that my vendor package that Steve references is probably quicker and dirtier than many of our vendor packages that are currently built in the buildfarm.

Some additional examples:

SteveMacenski commented 8 months ago

More instructions! https://github.com/ros2/ros2_documentation/pull/3759/files

Timple commented 7 months ago

Hi all,

We're running into this issue now. Luckily the last activity wasn't too long ago. We happily volunteer to be the third party testers.

Is someone still pursuing this? Or should we do the vendor package release?

nachovizzo commented 7 months ago

@Timple I'm trying to understand what should I do so I can do it :) I'll ping any updates in this PR

Timple commented 7 months ago

From what I gathered so far:

Create a package called openvdb_vendor.
Make it look like this package, but obviously switch all ogre stuff by openvdb: https://github.com/mjcarroll/ogre_next_vendor/blob/0e73c5e3cbb3098e9cef1c900bc79e918371c133/CMakeLists.txt#L7-L26
Include openvdb_vendor in spatio_temporal_voxel_layer package.xml and CmakeLists.txt such that it compiles.
Release both packages to the buildfarm

SteveMacenski commented 7 months ago

Please take it up! Happy to merge once it’s has some third party testing with the vendor package. That vendor package can live here so I can release them together.

nachovizzo commented 7 months ago

closing in favor of https://github.com/SteveMacenski/spatio_temporal_voxel_layer/pull/281

SteveMacenski / spatio_temporal_voxel_layer