Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Eigen Plugins not compiled. #382

Open
mariamhegazy opened this issue Jan 13, 2023 · 12 comments
Open

Eigen Plugins not compiled. #382

mariamhegazy opened this issue Jan 13, 2023 · 12 comments
Labels
Milestone

Comments

@mariamhegazy
Copy link

mariamhegazy commented Jan 13, 2023

I want to use grid_map_core so I built my package using colcon but I keep getting this error;
fatal error: grid_map_core/eigen_plugins/FunctorsPlugin.hpp: No such file or directory

I am using the repo rolling on ubuntu 22.04 and my cmake file looks like this:

cmake_minimum_required(VERSION 3.8)
 project(shaft-mapping)

if(CMAKE_COMPILER_IS_GNUCXX OR CMAKE_CXX_COMPILER_ID MATCHES "Clang")
  add_compile_options(-Wall -Wextra -Wpedantic)
endif() 
find_package(ament_cmake REQUIRED)
find_package(rclcpp REQUIRED)
find_package(sensor_msgs REQUIRED)
find_package(std_msgs REQUIRED)
find_package(grid_map_core REQUIRED)
find_package(grid_map_cmake_helpers REQUIRED)

if(BUILD_TESTING)
  find_package(ament_lint_auto REQUIRED)
  set(ament_cmake_copyright_FOUND TRUE)
  set(ament_cmake_cpplint_FOUND TRUE)
  ament_lint_auto_find_test_dependencies()
endif()


add_executable(reading_laser src/reading_laser.cpp)
ament_target_dependencies(reading_laser rclcpp std_msgs sensor_msgs)

add_executable(grid src/Grid.cpp)
ament_target_dependencies(grid  grid_map_core)


install(TARGETS
  DESTINATION lib/${PROJECT_NAME}/
  ARCHIVE DESTINATION lib
  LIBRARY DESTINATION lib
  RUNTIME DESTINATION lib/${PROJECT_NAME}
)

ament_package()
@samprt-heracles
Copy link

Hello, how did you fix this error ?

1 similar comment
@GradyLiugy
Copy link

Hello, how did you fix this error ?

@Ryanf55 Ryanf55 reopened this Mar 20, 2024
@Ryanf55 Ryanf55 added ros2 Affects ROS 2 bug labels Mar 20, 2024
@Ryanf55
Copy link
Collaborator

Ryanf55 commented Mar 20, 2024

Apparently this still affects humble. Re-opened.

@Ryanf55 Ryanf55 added this to the 2.2.0 milestone Mar 20, 2024
@StefanFabian
Copy link
Contributor

I've traced it to this cmake file which pollutes the definitions of any project that find_packages grid_map_core:
https://github.com/ANYbotics/grid_map/blob/master/grid_map_core/cmake/grid_map_core-extras.cmake
here https://github.com/ANYbotics/grid_map/blob/ros2/grid_map_core/CMakeLists.txt#L130

and your reading_laser target does not link against grid_map_core so it doesn't have the include path for grid_map_core.

@Ryanf55
Copy link
Collaborator

Ryanf55 commented Jun 10, 2024

Nice find. Would you be interested to issue a PR to fix it?

@StefanFabian
Copy link
Contributor

Unfortunately, I do not have the time to properly fix this and check that this doesn't have side effects.
I assume it would be enough to add these definitions to the target with a public interface instead of a separate cmake.

@Ryanf55
Copy link
Collaborator

Ryanf55 commented Jul 28, 2024

Can you supply a minimum reproducible example to demonstrate the problem? CI is passing the build, so it's not a use case that's being tested.

@StefanFabian
Copy link
Contributor

The problem will only appear in projects consuming this library since it's leaking global cmake configurations.
Simple example:

  • New project with two executables.
  • find_package(grid_map_core REQUIRED)
  • target_link_libraries for only one of the executables
  • In the code for both executables, include Eigen/Core and do some basic matrix addition or something

Ryanf55 added a commit to Ryanf55/grid_map that referenced this issue Jul 31, 2024
Signed-off-by: Ryan Friedman <ryanfriedman5410+github@gmail.com>
@Ryanf55
Copy link
Collaborator

Ryanf55 commented Jul 31, 2024

Thanks. I've pushed to a branch to reproduce this issue.
https://github.com/Ryanf55/grid_map/tree/382-bug

Here's the problem I see.

ryan@B650-970:~/Dev/ros2_ws/src/grid_map$ colcon build --packages-up-to issue382
[0.185s] WARNING:colcon.colcon_core.package_selection:Some selected packages are already built in one or more underlay workspaces:
        'grid_map_core' is in: /opt/ros/humble
If a package in a merged underlay workspace is overridden and it installs headers, then all packages in the overlay must sort their include directories by workspace order. Failure to do so may result in build failures or undefined behavior at run time.
If the overridden package is used by another package in any underlay, then the overriding package in the overlay must be API and ABI compatible or undefined behavior at run time may occur.

If you understand the risks and want to override a package anyways, add the following to the command line:
        --allow-overriding grid_map_core

This may be promoted to an error in a future release of colcon-override-check.
Starting >>> grid_map_cmake_helpers
Finished <<< grid_map_cmake_helpers [0.05s]
Starting >>> grid_map_core
Finished <<< grid_map_core [0.18s]                     
Starting >>> issue382
--- stderr: issue382                             
<command-line>: fatal error: grid_map_core/eigen_plugins/FunctorsPlugin.hpp: No such file or directory
compilation terminated.
gmake[2]: *** [CMakeFiles/main2.dir/build.make:76: CMakeFiles/main2.dir/main2.cpp.o] Error 1
gmake[1]: *** [CMakeFiles/Makefile2:165: CMakeFiles/main2.dir/all] Error 2
gmake[1]: *** Waiting for unfinished jobs....
gmake: *** [Makefile:91: all] Error 2
---
Failed   <<< issue382 [0.50s, exited with code 2]

Summary: 2 packages finished [0.88s]
  1 package failed: issue382
  1 package had stderr output: issue382

I likely don't have time to resolve this in the next two weeks (vacation and busy at my day-job), so help is appreciated.
Even just finding the docs on how these plugins are supposed to work.

An example of them in use is here: https://github.com/facebookincubator/Eigen-FBPlugins/blob/1ad1fe115462178eb549e97c2e97861ef0da76fd/README.md?plain=1#L20

Whatever was done in grid_map seems like quite the leap from what's recommended here:
https://eigen.tuxfamily.org/dox/TopicCustomizing_Plugins.html

@Ryanf55
Copy link
Collaborator

Ryanf55 commented Jul 31, 2024

I tried applying patch faa4ae0, but found that applying these definitions to the target is more complicated than initially proposed by a public target property because this file is exported.

As my branch stands now, this is the configuration error I get trying to use grid_map_core:

CMake Error at /home/ryan/Dev/ros2_ws/src/grid_map/install/grid_map_core/share/grid_map_core/cmake/grid_map_core-extras.cmake:7 (target_compile_definitions):
  Cannot specify compile definitions for target "grid_map_core" which is not
  built by this project.
Call Stack (most recent call first):
  /home/ryan/Dev/ros2_ws/src/grid_map/install/grid_map_core/share/grid_map_core/cmake/grid_map_coreConfig.cmake:41 (include)
  CMakeLists.txt:4 (find_package)


CMake Error at /home/ryan/Dev/ros2_ws/src/grid_map/install/grid_map_core/share/grid_map_core/cmake/grid_map_core-extras.cmake:16 (target_compile_definitions):
  Cannot specify compile definitions for target "grid_map_core" which is not
  built by this project.
Call Stack (most recent call first):
  /home/ryan/Dev/ros2_ws/src/grid_map/install/grid_map_core/share/grid_map_core/cmake/grid_map_coreConfig.cmake:41 (include)
  CMakeLists.txt:4 (find_package)

Once exported, the library name is grid_map_core::grid_map_core. Perhaps this doesn't need to be added to the CONFIG_EXTRAS in ament_package? Once I remove that, it can compile.

I have no idea how well this will work, so for now, I'll propose a draft PR that anyone using plugins can test out. I'd like at least a few users of these to give it a try.

Ryanf55 added a commit to Ryanf55/grid_map that referenced this issue Jul 31, 2024
Signed-off-by: Ryan Friedman <ryanfriedman5410+github@gmail.com>
@StefanFabian
Copy link
Contributor

StefanFabian commented Jul 31, 2024

I don't think you need that config extras file at all.
Adding it to your CMakeLists should be sufficient.
The problem is that if you add such a config extras file, every project using the library will also have it added and they won't have the target for which you are adding the definitions.
Hence, the error.
You only want this to be applied to your target in your library so there is no need to export anything manually.
If set to public the compile definitions should be exported to consuming targets automatically.

@Ryanf55
Copy link
Collaborator

Ryanf55 commented Nov 28, 2024

I don't think you need that config extras file at all. Adding it to your CMakeLists should be sufficient. The problem is that if you add such a config extras file, every project using the library will also have it added and they won't have the target for which you are adding the definitions. Hence, the error. You only want this to be applied to your target in your library so there is no need to export anything manually. If set to public the compile definitions should be exported to consuming targets automatically.

If you have availability, can you please submit a PR. Are you saying I change this to PUBLIC
https://github.com/ANYbotics/grid_map/pull/475/files#diff-be85320bce6ea1f2b8f51cdf6b1a569db10dd2452203ee28d096cd46fd20780fR7

AND move that call into the main CMakeLists instead of a confix-extras file?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

5 participants