tesseract-robotics / tesseract

Motion Planning Environment
http://tesseract-docs.rtfd.io
Other
278 stars 88 forks source link

Fixes for building on Ubuntu Noble #1016

Closed rjoomen closed 4 months ago

rjoomen commented 5 months ago

Pending a rebase after #1006 and #1015 have been merged.

codecov[bot] commented 5 months ago

Codecov Report

All modified and coverable lines are covered by tests :white_check_mark:

Project coverage is 89.84%. Comparing base (2a92fa8) to head (18a89da).

Additional details and impacted files [![Impacted file tree graph](https://app.codecov.io/gh/tesseract-robotics/tesseract/pull/1016/graphs/tree.svg?width=650&height=150&src=pr&token=nh4aHZzgpR&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=tesseract-robotics)](https://app.codecov.io/gh/tesseract-robotics/tesseract/pull/1016?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=tesseract-robotics) ```diff @@ Coverage Diff @@ ## master #1016 +/- ## ========================================== - Coverage 89.86% 89.84% -0.02% ========================================== Files 280 280 Lines 15908 15908 ========================================== - Hits 14296 14293 -3 - Misses 1612 1615 +3 ``` [see 2 files with indirect coverage changes](https://app.codecov.io/gh/tesseract-robotics/tesseract/pull/1016/indirect-changes?src=pr&el=tree-more&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=tesseract-robotics)
Levi-Armstrong commented 5 months ago

@rjoomen Ping me when this ready. I assume you are working on changes to get the CI to pass?

rjoomen commented 5 months ago

Yes, there's a test failing and I didn't have time to look into it. Will have time tomorrow, I hope.

cboostjvisser commented 4 months ago

I have been investigating the failing tests. I have not been able to pinpoint the exact problem (yet). But here are my findings:

The failing tests are:

There are already comments about failing on Ubuntu 18.04 and a #if BOOST_VERSION > 106800 && !__APPLE__ surrounding the tests.

Boost investigation:

dlopen() seems to be called with exact the same arguments. But does not return handle in 1.83. See here: https://github.com/boostorg/dll/blob/6c60dde50bf67138c90cc84938111866813feaff/include/boost/dll/detail/posix/shared_library_impl.hpp#L118

Tests might not be correct?

I'm also questioning if the tests are correct. If I read this post: https://stackoverflow.com/a/17716576 It seems like we need to load a local library with "./" leading the libname, or else DT_RPATH or DT_RUNPATH needs to be set.

The current tesseract_common/class_loader.hpp always sets boost::dll::load_mode::search_system_folders if there is no library_path specified. For example: https://github.com/tesseract-robotics/tesseract/blob/2a92fa8db2953591976fc91062aa3a6e31847838/tesseract_common/include/tesseract_common/class_loader.hpp#L92 That will trigger the "./" preprending to be disabled in the Boost.DLL library: https://github.com/boostorg/dll/blob/6c60dde50bf67138c90cc84938111866813feaff/include/boost/dll/detail/posix/shared_library_impl.hpp#L97

Conclusion

Removing all the boost::dll::load_mode::search_system_folders in tesseract_common/class_loader.hpp seem to fix the tests. But not sure what other problems that might cause. I was not able to find what causes the tests to work with Boost 1.74 but not with Boost 1.83.

rjoomen commented 4 months ago

Additional information, adding the current directory explicitly also fixes the tests.

Change lines 85 and 86 to:

EXPECT_TRUE(ClassLoader::isClassAvailable(symbol_name, lib_name, ".")); 
auto plugin = ClassLoader::createSharedInstance<TestPluginBase>(symbol_name, lib_name, ".");

And add this before line 155:

plugin_loader.search_paths.insert(".");
rjoomen commented 4 months ago

The current tesseract_common/class_loader.hpp always sets boost::dll::load_mode::search_system_folders if there is no library_path specified. For example:

https://github.com/tesseract-robotics/tesseract/blob/2a92fa8db2953591976fc91062aa3a6e31847838/tesseract_common/include/tesseract_common/class_loader.hpp#L92

That will trigger the "./" preprending to be disabled in the Boost.DLL library: https://github.com/boostorg/dll/blob/6c60dde50bf67138c90cc84938111866813feaff/include/boost/dll/detail/posix/shared_library_impl.hpp#L97

Conclusion

Because class_loader sets mode search_system_folders, the Tesseract class loaders by design do not use the local directory if no search path is given.

@Levi-Armstrong, should we modify the class loader or the tests to deal with this?

Levi-Armstrong commented 4 months ago

I would update the unit test for now.