meson: always use libatomic if found #25069
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
keszybz
force-pushed
the
atomic-clang-16
branch
from
3e151c0
to
c5e1e72
Compare
bluca
approved these changes
Oct 19, 2022
bluca
added
good-to-merge/waiting-for-ci 👍
|
As suggested in #25057: maybe just drop this test and simply always link against it and rely on |
Let's do that instead, please. It's a good option, especially given my latest comment over there. But note that |
keszybz
force-pushed
the
atomic-clang-16
branch
from
c5e1e72
to
3397c0e
Compare
keszybz
removed
the
good-to-merge/waiting-for-ci 👍
|
Updated. |
thesamesam
reviewed
Oct 20, 2022
keszybz
force-pushed
the
atomic-clang-16
branch
from
3397c0e
to
ec22b2a
Compare
medhefgo
changed the title
yuwata
approved these changes
Oct 20, 2022
This comment was marked as resolved.
This comment was marked as resolved.
bluca
added
the
ci-fails/needs-rework 🔥
keszybz
force-pushed
the
atomic-clang-16
branch
from
ec22b2a
to
827c361
Compare
keszybz
removed
the
ci-fails/needs-rework 🔥
|
Pfff, no nice approach works. After trying a few different approaches, a manual check seems to be the most reliable. |
bluca
reviewed
Oct 20, 2022
meson.build
Outdated
| threads = dependency('threads') | ||
| librt = cc.find_library('rt') | ||
| libm = cc.find_library('m') | ||
| libdl = cc.find_library('dl') | ||
| libcrypt = cc.find_library('crypt') | ||
|
|
||
| # On some architectures, libatomic is required. But on some installations, | ||
| # it is found, but actual linking fails. So let's try to use it opportunstically. |
There was a problem hiding this comment.
/opportunstically/opportunistically/
bluca
approved these changes
Oct 20, 2022
I found some explanation of Red Hat / Fedora situation in scylladb/seastar#533. |
Semi-quoting systemd#25057: clang-16 has made the choice to turn on -Werror=implicit-function-declaration,implicit-int. (See Gentoo's tracker bug https://bugs.gentoo.org/870412). Added in commit 132c73b, systemd now does a check to see if libatomic is needed with some compile/link tests with e.g. __atomic_exchange_1, but the tests don't provide a prototype for __atomic_exchange_1 so with clang-16 the test fails, breaking the build. Let's simplify things by linking to libatomic unconditionally if it is found and seems to work. If actually unneeded, it might be dropped via --as-needed. This seems to work with gcc and clang. declare_dependency() is used instead of cc.find_library(), because the latter picks up a symlink in gcc private directory (e.g. /usr/lib/gcc/x86_64-redhat-linux/12/libatomic.so), and we don't want that. Fixes systemd#25057.
bluca
force-pushed
the
atomic-clang-16
branch
from
827c361
to
83fffe2
Compare
|
Fixed the typo since it was trivial, no other changes so merging |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Semi-quoting #25057:
clang-16 has made the choice to turn on -Werror=implicit-function-declaration,implicit-int. (See Gentoo's tracker bug https://bugs.gentoo.org/870412). Added in commit 132c73b, systemd now does a check to see if libatomic is needed with some compile/link tests with e.g. __atomic_exchange_1, but the tests don't provide a prototype for __atomic_exchange_1 so with clang-16 the test fails, breaking the build.
Add -Wno-implicit-function-declaration (which is supported by both gcc and clang) to should get rid of the warning or error.
(I don't have clang-16 at hand, but with clang-15.0.0-2.fc37.x86_64 the warnings are gone and the link fails, and then when -latomic is added, succeeds.)
Fixes #25057.