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

[swift 6] cherry-pick Android NDK overlay into Swift 6 #74758

Merged
merged 6 commits into from
Jul 3, 2024

Conversation

hyp
Copy link
Contributor

@hyp hyp commented Jun 27, 2024

Explanation: This set of changes cherry-picks all the changes required to adopt the new 'Android' stdlib module that overlays the 'Android' NDK headers, instead of using 'Glibc' that was broken before, and wasn't usable with C++ interoperability enabled. These changes introduce the module map that covers a wide portion of the Android NDK, the and Android overlay platform module instead of Glibc. There was a request from the upstream community to consider bringing these changes to Swift 6 here: https://forums.swift.org/t/improving-swift-support-and-interoperability-experience-for-android/71470 , so I wanted to ask the branch managers to see if they would be willing to take these changes in.

Scope: All Android platform users have to import 'Android' now instead of 'Glibc' . The compiler changes impact Clang importer and the stdlib.
Issue: N/A
Original PR:
#73534
#72161
#72634
#74160
#74348
#74742
Risk:
Some risk for all platforms is involved specifically when it comes to the changes related to applying the overlay, and the stdlib changes needed to build for Android. The changes should not impact other platforms whatsoever, and so far there were no reports from users using the 'main'-based toolchains about regressions related to these Android overlay changes. For Android, these changes have been tested quite thoroughly on the 'main'-based toolchains by building downstream adopter projects using the new overlays, including swift-corelibs-foundation and other packages like swift-firebase (github.com/thebrowsercompany/swift-firebase).
Testing: Manual testing of Android SDK build on Windows platform.
Reviewer: @compnerd, @finagolfin, @ian-twilightcoder

hyp and others added 6 commits June 26, 2024 18:18
[android] add a module map for Android NDK
[android] add an Android NDK Swift overlay
[android] Fix Synchronization Android build
[android] do not import stdatomic from android's libc++
This module was left out from the modulariation pass. The declarations
here are required for building swift-foundation for Android.
@hyp hyp requested a review from a team as a code owner June 27, 2024 01:21
@hyp
Copy link
Contributor Author

hyp commented Jun 27, 2024

@swift-ci please test

@finagolfin
Copy link
Contributor

I just built the latest June 13 trunk source snapshot toolchain natively on Android with this pull, no problems and fixed several C++ Interop tests in the compiler validation suite. I also set up my daily Android CI a couple weeks ago to test this new trunk Android overlay with several Swift packages, finagolfin/swift-android-sdk#151, haven't seen a problem since patching those packages to import Android instead.

Finally, I just reviewed this pull and other than the small change to getLibcFileMapping() in the ClangImporter and the header inclusion for linux-based platforms in stdlib/public/runtime/Paths.cpp, the remaining changes are all tightly scoped to Android and present no risk for the officially-supported OS platforms.

I think this is safe enough to merge and improves Android support for the upcoming Swift 6 release.

@finagolfin
Copy link
Contributor

@compnerd, would you merge? I'd like this to be in the next 6.0 snapshot tag, so I can start running it through my daily Android CI.

@DougGregor
Copy link
Member

Eh, I'll go ahead and merge.

@finagolfin
Copy link
Contributor

I tweaked my Android overlay patches to apply to this 6.0 branch, finagolfin/swift-android-sdk#156, and those Swift packages' tests passed on the Android x86_64 emulator after being built with this 6.0 toolchain also: now to start upstreaming my patches.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants