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

Make spdlog an exec_depend. #27

Merged
merged 2 commits into from
Mar 12, 2020
Merged

Make spdlog an exec_depend. #27

merged 2 commits into from
Mar 12, 2020

Conversation

clalancette
Copy link
Contributor

Newer spdlog (>= 1.5.0) builds a shared library, so we
need to add an exec_depend now.

Signed-off-by: Chris Lalancette clalancette@openrobotics.org

Part of solving #26

Newer spdlog (>= 1.5.0) builds a shared library, so we
need to add an exec_depend now.

Signed-off-by: Chris Lalancette <clalancette@openrobotics.org>
@dirk-thomas
Copy link
Member

Doesn't it also export the dependency in CMake?

@clalancette
Copy link
Contributor Author

Doesn't it also export the dependency in CMake?

So I did a full build of the ros2 workspace without this, and it succeeded. I then tested out some of demo_nodes_cpp, which also succeeded. I also did an overlay build which succeeded. So I'm not entirely sure if it is needed.

That being said, it is more correct so I'll add it. Thanks for the heads up.

Signed-off-by: Chris Lalancette <clalancette@openrobotics.org>
@clalancette clalancette merged commit 2679a21 into master Mar 12, 2020
@clalancette clalancette deleted the spdlog-vendor-update branch March 12, 2020 18:27
@cottsay
Copy link
Member

cottsay commented Jul 24, 2020

@clalancette - Can this be backported to Eloquent? I'm getting RPM build failures on Fedora because libspdlog.so.1 isn't installed because the spdlog dependency is missing.

@clalancette
Copy link
Contributor Author

@clalancette - Can this be backported to Eloquent? I'm getting RPM build failures on Fedora because libspdlog.so.1 isn't installed because the spdlog dependency is missing.

I don't see why not. It will never be strictly needed on our Eloquent supported platforms, but it shouldn't hurt either. @cottsay Would you mind opening up the backport PR? I'll review it.

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