Skip to content

Fix -latomic linking condition, include android #38508

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

Merged
merged 1 commit into from
Jul 20, 2021
Merged
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 4 additions & 5 deletions stdlib/public/Concurrency/CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -33,11 +33,10 @@ endif()
# Frustratingly, in many cases this isn't necessary because the
# sequence is inlined, but we have some code that's just subtle
# enough to turn into runtime calls.
if(SWIFT_HOST_VARIANT STREQUAL "Linux")
if(SWIFT_HOST_VARIANT_ARCH MATCHES "armv6|armv7|i686")
list(APPEND swift_concurrency_link_libraries
atomic)
endif()
if(SWIFT_HOST_VARIANT STREQUAL "linux" OR
Copy link
Contributor

Choose a reason for hiding this comment

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

Was the change from Linux to linux on purpose?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, SWIFT_HOST_VARIANT seems to generally be lowercase. That's the primary thing this fixes.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

As far as I can tell, the code I copy-and-pasted from is inoperative.

Copy link
Member

Choose a reason for hiding this comment

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

A shorter way to write this would be SWIFT_HOST_VARIANT MATCHES "linux|android", thanks for adding Android.

Copy link
Contributor

Choose a reason for hiding this comment

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

Are the legal values for SWIFT_HOST_VARIANT documented somewhere?

Copy link
Member

Choose a reason for hiding this comment

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

They are defined here, I count eight possible hosts.

SWIFT_HOST_VARIANT STREQUAL "android")
list(APPEND SWIFT_RUNTIME_CONCURRENCY_SWIFT_LINK_FLAGS
-latomic)
endif()

add_swift_target_library(swift_Concurrency ${SWIFT_STDLIB_LIBRARY_BUILD_TYPES} IS_STDLIB
Expand Down