Skip to content

ClangImporter: add support for Android API Notes #74829

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 1, 2024

Conversation

compnerd
Copy link
Member

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

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
@compnerd
Copy link
Member Author

CC: @finagolfin @etcwilde @drodriguez

@compnerd
Copy link
Member Author

@swift-ci please test

@compnerd
Copy link
Member Author

Please test with following PRs:
swiftlang/swift-installer-scripts#310

@swift-ci please build toolchain Windows platform

@finagolfin
Copy link
Member

Oh nice, does this mean we can avoid the workarounds in our swift-corelibs-foundation pulls now? I will try this natively on Android and see.

@compnerd
Copy link
Member Author

Depending on the workaround needed, yes. We should be able to fix any of the signatures but cannot fix the implementation of the library or add new functions without shims.

@compnerd
Copy link
Member Author

@swift-ci please test

@finagolfin
Copy link
Member

Depending on the workaround needed, yes.

I was talking about my workaround for this very Bionic fts_open() type signature and your alternate workaround for the same code here.

I will try this pull out and see, but presumably the goal is to no longer require those changes.

@compnerd
Copy link
Member Author

Yes, I think that we should be able to remove that workaround with this change.

@compnerd
Copy link
Member Author

Please test with following PRs:
swiftlang/swift-installer-scripts#310

@swift-ci please build toolchain Windows platform


swift_install_in_component(FILES
posix_filesystem.apinotes
DESTINATION "share"
Copy link
Member

Choose a reason for hiding this comment

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

Do you install the compiler and this file on Windows before building swift-corelibs-foundation with that new installed location? I just tried using this with #5010 on Android and this file wasn't available, because build-script uses the freshly-built compiler in build/ to compile swift-corelibs-foundation, not the installed one from --install-prefix.

I will submit a change to place this file in build/ also, so we can use it when cross-compiling the corelibs for Android with build-script also.

Copy link
Member Author

Choose a reason for hiding this comment

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

Yes, we install the compiler first to use it for the subsequent steps. I think that it makes more sense to change build-script to alter the behaviour and to stage the compiler first.

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.

2 participants