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

Export threading library via extras and not ament_export_libraries to avoid warnings when cross-compiling #53

Merged
merged 1 commit into from
Feb 26, 2019

Conversation

esteve
Copy link
Member

@esteve esteve commented Feb 25, 2019

This PR is based on the work that @anup-pem did internally at Apex. He found that cross compiling ROS2 caused the warning below, which doesn't show up when compiling natively because pthread is found on the host but not on the sysroot.

The fix just removes the ament_export_libraries(pthread) call and delegates the logic to the extras file.

/cc @filiperinaldi @lmayencourt

--- stderr: velodyne_driver                                  
CMake Warning at /home/anup.pemmaiah/2394/apex_ws/install/share/rmw_implementation/cmake/ament_cmake_export_libraries-extras.cmake:116 (message):
  Package 'rmw_implementation' exports library 'pthread' which couldn't be
  found
:
:
---

@esteve esteve requested a review from wjwwood February 25, 2019 08:40
@esteve esteve added in progress Actively being worked on (Kanban column) in review Waiting for review (Kanban column) and removed in progress Actively being worked on (Kanban column) labels Feb 25, 2019
Copy link
Member

@wjwwood wjwwood left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Makes sense to me, based on the idea that ament_export_libraries should just be used with libraries you create yourself (which may or may not be correct).

I think it would be good to get a 👍 from @dirk-thomas too.

@wjwwood
Copy link
Member

wjwwood commented Feb 25, 2019

CI:

  • Linux Build Status
  • Linux-aarch64 Build Status
  • macOS Build Status
  • Windows Build Status

@wjwwood
Copy link
Member

wjwwood commented Feb 26, 2019

The test failures seem to be unrelated. The test_cli one's I'm not sure about, but the sros2 one is one that I see on our nightly repeated jobs.

@esteve
Copy link
Member Author

esteve commented Feb 26, 2019

@dirk-thomas I've addressed your feedback, are you ok with the changes?

@dirk-thomas dirk-thomas merged commit c5f5324 into ros2:master Feb 26, 2019
@dirk-thomas dirk-thomas removed the in review Waiting for review (Kanban column) label Feb 26, 2019
@esteve esteve deleted the cross_compile branch February 26, 2019 16:16
@esteve
Copy link
Member Author

esteve commented Feb 26, 2019

Thanks @wjwwood and @dirk-thomas for the review.

@dirk-thomas
Copy link
Member

Thank you for the patch.

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

Successfully merging this pull request may close these issues.

None yet

3 participants