[6.3] Add Android shims for spawnattr#5398
Conversation
fix existing polyfills defer
|
You'll need to fill out the release branch template linked here, shown as an example here. |
|
@finagolfin Thanks! I updated the PR. |
|
@swift-ci test |
|
@drodriguez, your review would be worthwhile here too, since you wrote most of these polyfills. |
|
Isn't this a cherry-pick of #5301? Did something change since then? |
|
Yep, just a cherry-pick, nothing changed, just wanted your approval if you think we can get this into 6.3, @drodriguez. |
|
Windows CI failed with unrelated sourcekit-lsp failure, probably a flake, so running again. @swift-ci test windows |
|
@parkera, we'd like to get this in, then try moving the Android SDK to support back to API 24 in the official CI, by using these polyfills again: please let us know what the Foundation team thinks about this mostly Android-specific 6.3 pull. |
|
@jmschonfeld or @itingliu, just checking to see if anyone on the Foundation team had any comment on this pull, which only makes minor modifications for non-Android platforms, with most of the changes here confined to @drodriguez's posix_spawn polyfills for older Android, ie before posix_spawn was added to Bionic in Android API 28. This is needed because we plan to push the official Android SDKs back to supporting Android API 24 this month, and possibly Android API 23 also, if we can iron out the remaining issues there. I don't expect any of you to review the Android changes, just the few lines we changed for all non-Windows platforms in |
4fe4d71 to
3f3f8ea
Compare
|
@swift-ci test |
|
@swift-ci test |
|
@swift-ci test windows |
|
@itingliu just wanted to check in to see if there was anything else missing from getting this merged? |
|
@swift-ci test |
|
Thanks for the fix! |
|
@itingliu, should we rebase-merge this pull to keep all commits? I don't know if you want to bring the last three commits back to trunk or if the unified path in the first commit alone is fine in that branch, since it will undergo much more testing. This GitHub UI only gives me the squash merge option, so not using that, in case you want to bring the other three commits back to trunk and it is tougher to merge those that way. Just let me know what the Foundation team prefers here. |
|
@swift-ci test linux |
|
Merged. We have an automerger set up that should be able to pick it up tomorrow. if not we will know what's wrong |
Explanation:
Adds the missing Android shims for
spawnattrto 6.3, such that we can make the first official SDK release API 24 instead 28.Scope:
Only related to Android devices under API 28. Also, only affects developers using the Process APIs.
Issues:
It is related to this forums thread https://forums.swift.org/t/android-api-minimum-for-the-swift-sdk-for-android/82874
Original PRs:
add Android shims for spawnattr #5301
Risk:
The risk of this is low, it only affects Android and devices below API 28. Also, it actually fixes the current shims, which did not work before.
Testing:
The PR passed Android CI, both the official one and the Browser Company.
@finagolfin also built this natively on Android and confirmed no regressions.
I have also tested the Process APIs using this API on both API 24 and API 28 devices, and confirmed both work as expected.
Reviewers:
@finagolfin approved it from an Android perspective.
I did not hear back from the requested Foundation reviewer, but a few people on the PR said it "probably looked fine". So would be great to get an actual approval.