Link rcpputils::filesystem to solve error on RHEL
#1665
Conversation
Signed-off-by: GitHub <[email protected]>
There was a problem hiding this comment.
@audrow Ups not so simple
CMake Error at CMakeLists.txt:30 (add_library):
08:00:04 Target "rosbag2_compression" links to target "rcpputils::filesystem" but
08:00:04 the target was not found. Perhaps a find_package() call is missing for an
08:00:04 IMPORTED target, or an ALIAS target is missing?
Basically need to add find_package(rcpputils) above.
rosbag2_compression/CMakeLists.txt
Outdated
| if(CMAKE_COMPILER_IS_GNUCXX) | ||
| target_link_libraries(${PROJECT_NAME} | ||
| rcpputils::filesystem | ||
| ) | ||
| endif() | ||
|
|
There was a problem hiding this comment.
@audrow @clalancette I think we need
if(CMAKE_COMPILER_IS_GNUCXX)
target_link_libraries(${PROJECT_NAME} stdc++fs)
#endif
IMO target_link_libraries(${PROJECT_NAME} rcpputils::filesystem) is not going to help here.
There was a problem hiding this comment.
This actually isn't needed, as a similar thing is done above with ament_target_dependencies.
Instead, we need a change to the code itself to switch back to using rcpputils::fs. I'll push some changes shortly that make it compile for me, though @MichaelOrlov suggests that it may not work in practice.
There was a problem hiding this comment.
@clalancette Yes, it is not going to work out of the box. I will do rework shortly. Give me a couple of hours.
Signed-off-by: Chris Lalancette <[email protected]>
|
@audrow @clalancette I created a new PR #1666 with fix in replacement to the PR. Please review #1666 |
|
Sounds good, closing this one. |
It seems #1653 created a regression on Humble on RHEL.
Here is an attempt to fix it. I'll run RHEL CI before merging.