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

cmake: fix libatomic check with GCC 8 #1243

Closed
wants to merge 1 commit into from
Closed

cmake: fix libatomic check with GCC 8 #1243

wants to merge 1 commit into from

Conversation

lopsided98
Copy link

@lopsided98 lopsided98 commented Sep 22, 2019

Issue #, if available:
#1199

Description of changes:

Fixes detection of libatomic with GCC 8. This solution is based on https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=907277#25, which describes why the error occurs (in that case with autotools).

Some of the possible solutions are:

  1. Unconditionally call enable_language(C) so that the check runs in C mode (untested)
  2. Run a compile test that calls __atomic_load_8 with the right number of arguments
  3. Drop the explicit libatomic test altogether and just check whether C++ atomics work when linked to libatomic

I chose option 2 because it maintained the existing semantics and seemed cleaner than using the C compiler to work around this issue.

Check all that applies:

  • Did a review by yourself.
  • Added proper tests to cover this PR. (If tests are not applicable, explain.)
  • Checked if this PR is a breaking (APIs have been changed) change.
  • Checked if this PR will not introduce cross-platform inconsistent behavior.
  • Checked if this PR would require a ReadMe/Wiki update.

Check which platforms you have built SDK on to verify the correctness of this PR.

  • Linux
  • Windows
  • Android
  • MacOS
  • IOS
  • Other Platforms

By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.

@emersonknapp
Copy link

Hi! Will this be merged soon? This problem is affecting us on the latest Raspbian

@KaibaLopez
Copy link
Contributor

Sorry for hte lack of update here but the PR was merged, closing 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.

4 participants