-
Notifications
You must be signed in to change notification settings - Fork 12.9k
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
Upgrade Android SDK to API level 17 #71123
Conversation
r? @kennytm (rust_highfive has picked a reviewer for you, use r? to override) |
cc @joshtriplett for tiering policy |
☔ The latest upstream changes (presumably #71180) made this pull request unmergeable. Please resolve the merge conflicts. |
Happy to update the PR, once I get information if this approach is indeed the right one. |
cc @rust-lang/compiler @rust-lang/release -- do we know of any reason not to do this? It looks reasonable to me given the usage figures and such. @badboy -- could you enable to dist-android and arm-android on the "try" builder and then we can run that here to check if your changes work? Something like this should work: diff --git a/src/ci/azure-pipelines/try.yml b/src/ci/azure-pipelines/try.yml
index 38a0685e0f7..6244e1413d0 100644
--- a/src/ci/azure-pipelines/try.yml
+++ b/src/ci/azure-pipelines/try.yml
@@ -28,6 +28,8 @@ jobs:
dist-x86_64-linux: {}
dist-x86_64-linux-alt:
IMAGE: dist-x86_64-linux
+ dist-android: {}
+ arm-android: {}
# The macOS and Windows builds here are currently disabled due to them not being
# overly necessary on `try` builds. We also don't actually have anything that |
390e178
to
603b7d0
Compare
Done! |
@bors try |
⌛ Trying commit 603b7d0bbfa0897808274cc5b8b62d1e25cccb82 with merge d370f240f65cb60d4c29900f6ab94bf49e1533dd... |
RE: Upgrading Android; I don't see any reason to not upgrade, but we should try to define a band of what API level to support. Someone who has more experience with the Android ecosystem would have to chime in here. It's my understanding that Google's supported versions doesn't necessarily line up with what being used by vendors. |
The job Click to expand the log.
I'm a bot! I can only do what humans tell me to, so if this was not helpful or you have suggestions for improvements, please ping or otherwise contact |
Gnaaah, formatting error, because now that it's one item less it should be on a single line. |
I think it would be also be worth updating https://forge.rust-lang.org/release/platform-support.html to include the minimum supported API level. The workarounds for missing |
Happy to tackle that, once this actually sticks. |
💔 Test failed - checks-azure |
I don't see any reason not to do it, but I'm not that familiar with android. It'd be great if we had some go-to folks to ping on these matters. |
I missed an |
603b7d0
to
408da63
Compare
@bors try (I assume I can't actually do that) |
@badboy: 🔑 Insufficient privileges: not in try users |
@bors try |
@badboy any updates? |
Unfortunately not. I have not yet had the time to re-run the tests to reproduce the issue first. |
☔ The latest upstream changes (presumably #73276) made this pull request unmergeable. Please resolve the merge conflicts. |
@bors rollup=never |
I'm closing this due to inactivity. |
This is a followup to the call for help.
It should upgrade the Android API level Rust is compiled against from 14 to 17.
For now all I did was replace
14
with17
in the Dockerfiles, fix some comments, remove special-cased functions and fixed some imports.Tested with the
arm-android
Docker image. Full log in a Gist.I don't actually know if the upgrades work just like that or if it requires someone from infra to help out.If this all does work out we could consider re-enabling the tests from #70958 to see how it goes.Revisions:
posix_memalign
issue, so I opted to go one higher.