Skip to content

[release/6.0][android] pick fixes for building Android stdlib and swift-foundation to swift 6 #77626

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

Conversation

hyp
Copy link
Contributor

@hyp hyp commented Nov 14, 2024

picks:

  • 7a5a0aa
  • 7815f84
  • 5905dc9
  • 47f9d78
  • c036270
  • Risk:
    Relatively low. Impacts mostly on Android bits, and introduces a cmake flag for hiding the build of builtin float overlay. Also fixes NOSWIFTRT flag used when building the stdlib's swift-reflection-test.
  • Testing:
    Local android build testing, and thebrowsercompany/swift-build build testing.
    Validated when building the windows stdlib as well.
  • Reviewers:
    @compnerd

hyp and others added 4 commits November 14, 2024 13:17
(cherry-picked from commit 7815f84)
(cherry-picked from commit 7a5a0aa)
(cherry-picked from commit 5905dc9)
Introduce the first APINotes injection for the Android platform. This
follows the VCRuntime pattern of permitting the SDK to provide API Notes
that augment the system SDK. This adds a workaround for incorrect
nullability on the `fts_open` function in bionic. The system library
itself is fixed at:
https://android-review.googlesource.com/c/platform/bionic/+/3151616

(cherry-picked from 47f9d78)
…lesystem.apinotes

This apinote file needs to be accessible in the locally built Android SDK as it's being built with build.ps1, so that swift-foundation can be built with that file present. This change ensures that the file is copied over into the local build directory for that Android SDK, in addition to being included in the installed component

This change also places the component into lib/swift/apinotes, as that's where the clang importer already looks for API notes

(cherry-picked from c036270)
@hyp hyp requested a review from a team as a code owner November 14, 2024 21:25
@hyp
Copy link
Contributor Author

hyp commented Nov 14, 2024

@swift-ci please test

set(NOSWIFTRT_KEYWORD "NOSWIFTRT")
else()
set(NOSWIFTRT_KEYWORD "")
endif()
Copy link
Member

Choose a reason for hiding this comment

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

Why does this get handled differently from the other targets?

Copy link
Member

@finagolfin finagolfin left a comment

Choose a reason for hiding this comment

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

The API notes changes would be useful for my Android SDK bundle too.

The remaining CMake and concurrency changes are in trunk already and have not caused a problem for the trunk SDK bundle produced by my Android CI, so should be fine.

@hyp
Copy link
Contributor Author

hyp commented Nov 22, 2024

@swift-ci please test

@hyp
Copy link
Contributor Author

hyp commented Nov 22, 2024

CC @DougGregor

Copy link
Member

@DougGregor DougGregor left a comment

Choose a reason for hiding this comment

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

Thanks for cherry-picking these

@DougGregor
Copy link
Member

@swift-ci please test Windows

@DougGregor DougGregor merged commit fe0ba3b into swiftlang:release/6.0 Dec 3, 2024
5 checks passed
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