Fix rhel and windows builds #164
Conversation
…hould depend on them (they are meant only to be loaded as python extensions) Signed-off-by: Ivan Santiago Paunovic <ivanpauno@ekumenlabs.com>
|
|
Using the NumPy component of FindPython3 introduced an issue in our Windows debug build, I will fix that one separately https://ci.ros2.org/view/nightly/job/nightly_win_deb/2357/console. |
| ARCHIVE DESTINATION lib | ||
| LIBRARY DESTINATION lib | ||
| RUNTIME DESTINATION bin) | ||
|
|
||
| # Export this target so downstream interface packages can depend on it | ||
| rosidl_export_typesupport_targets("${rosidl_generator_py_suffix}" "${_target_name_lib}") |
There was a problem hiding this comment.
Downstream interface packages depend on them. I'm not 100% sure they have to depend on them though.
That's using the special rosidl suffix'd targets variable. How about removing only the ament_export_targets()? That way downstream targets won't accidentally link against them if using pkg_name_TARGETS or ament_target_dependencies().
There was a problem hiding this comment.
Downstream interface packages depend on them. I'm not 100% sure they have to depend on them though.
Ah, I see.
I'm pretty sure that's not needed, but let me double check.
There was a problem hiding this comment.
Downstream interface packages depend on them. I'm not 100% sure they have to depend on them though.
I checked the generated code, and yes a downstream interface package depends on the generated library.
How about removing only the ament_export_targets()? That way downstream targets won't accidentally link against them if using pkg_name_TARGETS or ament_target_dependencies().
Agreed.
I will run CI again to see if it actually fixes the problem, I think it should.
There was a problem hiding this comment.
I checked the generated code, and yes a downstream interface package depends on the generated library.
Based on this, it seems that a bunch of stuff that I moved to PRIVATE in #160 is not correct.
Why I did that is because ament_export_dependencies() doesn't allow you to specify components, so I worked that arround by not having to export anything at all.
But in retrospective that seems incorrect.
I have now exported the dependency of FindPython3 manually, creating a config file.
That's using the special rosidl suffix'd targets variable. How about removing only the ament_export_targets()? That way downstream targets won't accidentally link against them if using pkg_name_TARGETS or ament_target_dependencies().
Unfortunately rosidl_export_typesupport_targets() requires you to also ament_export_targets().
It would be nice if ament_export_targets() had an option to not append the target to pkg_name_TARGETS.
But considering that I have undone most of what I did in #160, I think we don't need to worry about that now.
|
Just as an FYI, before this PR asan builds are also broken (that is, |
Signed-off-by: Ivan Santiago Paunovic <ivanpauno@ekumenlabs.com>
…s that's needed when messages depend on each other Signed-off-by: Ivan Santiago Paunovic <ivanpauno@ekumenlabs.com>
|
Signed-off-by: Ivan Santiago Paunovic <ivanpauno@ekumenlabs.com>
Signed-off-by: Ivan Santiago Paunovic <ivanpauno@ekumenlabs.com>
|
|
|
This would be a very useful change for our cross-compiled builds for macOS ARM64 in RoboStack (currently facing an issue with absolute paths to numpy include dir that are exported from here): https://github.com/RoboStack/ros-humble/runs/7052219327?check_suite_focus=true |
It's on hiatus at the moment. At the very least it needs to be rebased and retested with the latest, and then it would also need to be reviewed.
Can you please confirm that it actually does fix it? |
|
I'm working with This does not work. It fails with
For context (not directly relevant for this PR), |
Fixes:
It undoes most of #160.
Instead of making libraries and headers private, it exports FindPython3 dependency correctly.
Unfortunately, ament_export_dependencies() doesn't support configuring components, so I have exported it manually using an extra config file.
Old post:
Nothing should depend on them (they are meant only to be loaded as python extensions).Issue was introduced in #160.This was causing build issues in rhel https://ci.ros2.org/view/nightly/job/nightly_linux-rhel_release/1139/console#console-section-12.It didn't cause issues in other platforms as they realize the library they're linking agains isn't being used for anything, so they don't look at the missing symbols.But rhel linker seems to be more picky.