-
Notifications
You must be signed in to change notification settings - Fork 1.8k
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
[armadillo] Add version 12.6.4 #19907
[armadillo] Add version 12.6.4 #19907
Conversation
🤖 Beep Boop! This pull request is making changes to 'recipes/armadillo//'. 👋 @samuel-emrys you might be interested. 😉 |
I detected other pull requests that are modifying armadillo/all recipe: This message is automatically generated by https://github.com/ericLemanissier/conan-center-conflicting-prs so don't hesitate to report issues/improvements there. |
This comment has been minimized.
This comment has been minimized.
This is necessary because armadillo_buts/include_hdf5.h includes the hdf5 headers. When a consumer links armadillo with hdf5 and then includes the armadillo headers, they need to be able to discover them.
b21f586
to
0703c9e
Compare
This comment has been minimized.
This comment has been minimized.
This is necessary to avoid linker errors as of binutils 2.22, which gives errors similar to `libhdf5.so.310: error adding symbols: DSO missing from command line`. This more strict requirement in binutils requires that `-lhdf5` or `-lopenblas` be added to the consumer linker command line invocation, which passing these libraries transitively does.
Conan v1 pipeline ✔️All green in build 3 (
Conan v2 pipeline ✔️
All green in build 3 (
|
@@ -186,10 +186,10 @@ def requirements(self): | |||
# See https://gitlab.com/conradsnicta/armadillo-code/-/issues/227 for more information. | |||
if self.options.use_hdf5 and Version(self.version) < "12": | |||
# Use the conan dependency if the system lib isn't being used | |||
self.requires("hdf5/1.14.0") | |||
self.requires("hdf5/1.14.0", transitive_headers=True, transitive_libs=True) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Per conversation in Slack:
Without the traits, consumers fail during linking with home/conan/w/prod-v2/bsr/9620/afdcf/p/openb04dc4c8cf208a/p/lib/libopenblas.so.0: error adding symbols: DSO missing from command line
https://c3i.jfrog.io/c3i/misc-v2/logs/pr/19907/2-linux-gcc/armadillo/10.7.0//6057bdc3db7276f815dd06db0a25b3e779201d99-test.txt When shared = True
An explanation https://linuxpip.org/how-to-fix-dso-missing-from-command-line. This is a tightening of requirements in later versions of binutils requiring consumers to explicitly link shared libs even when the symbol can be identified in a transitive dep. Setting transitive_libs=True adds -lopenblas to the consumer link args which is the suggested fix and resolves this
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks!
Specify library name and version: armadillo/12.6.4
Add version 12.6.4 of armadillo. The patch file here makes the same changes as previous patches, but added to account for differences in the upstream CMakeLists.txt that prevents existing patch files from being applied.