build.ps1: Build the Android SDKs for API 24 instead#86957
Conversation
|
@swift-ci test windows |
|
Hmm, looks like neither the Windows PR test or toolchain CI build the Android SDKs, so this can't be tested until after it's merged onto the main branch and run through the main "package" toolchain CI, which both builds and tests the Android SDKs on Windows. Maybe we can extend the PR toolchain CI to at least build the Android SDKs also? Otherwise, it would be useful to add a swift-ci trigger to be able to invoke the main package toolchain CI too, just as we can for the PR toolchain CI. |
|
Please test this locally before merging. Alternatively, I believe that @justice-adams-apple is working on getting some additional testing for Android configured, and could wait for that. |
I haven't used Windows in more than a decade, so won't be me testing this. @Steelskin, any Windows BCNY CI that builds the Android SDKs that you can run this pull through? @justice-adams-apple, are you working on further testing for Android on Windows or on linux? If linux only, it would be useful to have a manual CI trigger for the main Windows toolchain CI, which appears to be the only official swift.org CI that currently builds these Android SDKs on Windows. |
I ran your changes through our CI and it passed our Android smoke test. |
I'm not sure how the CI run helps here - that doesn't use build.ps1 and so it would continue to build at the same API level. The need here is to run a manual test so that we do not break swift.org CI. |
You're right. I clearly need more coffee. Sorry @finagolfin I guess you'll have to run it yourself. I'll do another run where I change the Android version on our build. |
|
Interestingly, changing the Android API level to 24 triggered a compiler crash on our CI while building the experimental Static Overlay on all 4 android architectures. This is not happening on trunk so it is something specific to using Android 24. |
|
Huh, that is a strange error, as it seems to happen when opening the It is extremely strange that this would work for API 28 and not for API 24 in this way, would it be feasible to bisect the API levels between those two and see at what API level it starts failing? The other possibility is that this is simply a flake... |
I believe this is highlighting a pre-existing issue that is triggered by these changes, not caused by these changes. The regular SDK and the experimental dynamic SDK both build fine.
I am starting jobs for this starting with 26, and I'll keep you posted.
We do builds in parallel for every architecture and all 4 Android architectures failed in the same manner. |
Oh, great. 👍
Thank you, ❤️ I'm starting some linux CI jobs using this new base API level config today, and will let you know what that finds too.
Good point, you did mention that. 😶🌫️ Looking at the cross-compilation flags used, one key difference that stands out to me is that you guys are building with I wonder how well applying that API level in the |
|
This passed with API version 26. I am trying API version 25 now. |
|
I am seeing the same issue with 25. So this is working with API 26+. In the short term, do you have any objection to setting the default to 26 rather than 24? We can work on the 25- issue separately. |
|
Is it failing for API 25 with the exact same error output? Would help to paste that too... Any API level you guys want to build the Android SDK with on Windows is fine with me, as I haven't used Swift on Windows. We don't build the new Runtimes/ config on the official Android CI that produces the SDK bundles for linux and mac, so we haven't seen this failure. We would like to get those linux/mac SDK bundles down to API level 24 this month, but whatever API level you guys are comfortable with on Windows should be fine. I will update this pull to 26 and we can go from there. |
|
@swift-ci smoke test |
finagolfin
changed the title
|
So from the build logs my hypothesis is that this has something to do with file-paths case sensitivity on Windows in the new runtime overlays. Specifically since we are hitting Line 151 in 57dfde7 @Steelskin could you maybe try do another run on API 24 or 25 but changing set(NDK_SYSROOT_INCLUDE "${CMAKE_ANDROID_NDK}/toolchains/llvm/prebuilt/windows-x86_64/sysroot/usr/include")
# RESOLVE THE REAL PATH (Fixes casing mismatches like 'C:' vs 'c:')
get_filename_component(CANONICAL_NDK_INCLUDE "${NDK_SYSROOT_INCLUDE}" REALPATH)
# FIXME: how do we determine the sysroot? `CMAKE_SYSROOT` does not contain the sysroot.
file(CONFIGURE
OUTPUT android-ndk-overlay.yaml
CONTENT [[
---
version: 0
case-sensitive: false
use-external-names: true
roots:
- name: "@CANONICAL_NDK_INCLUDE@"
type: directory
contents:
- name: module.modulemap
type: file
external-contents: "@CMAKE_CURRENT_SOURCE_DIR@/android.modulemap"
- name: SwiftAndroidNDK.h
type: file
external-contents: "@CMAKE_CURRENT_SOURCE_DIR@/SwiftAndroidNDK.h"
- name: SwiftBionic.h
type: file
external-contents: "@CMAKE_CURRENT_SOURCE_DIR@/SwiftBionic.h"
]]
ESCAPE_QUOTES @ONLY NEWLINE_STYLE LF) |
|
@swift-ci build toolchain windows |
2 similar comments
|
@swift-ci build toolchain windows |
|
@swift-ci build toolchain windows |
|
@swift-ci build toolchain windows |
|
I tried building the Windows toolchain against Android API 24 with only two experimental Android SDKs, AArch64 and armv7, and hit an issue where Also, I'm unable to reproduce the build errors shown on the BCNY CI, for which I can think of three possible reasons:
To see if these other possibilities still cause problems, would you try the |
|
@swift-ci build toolchain windows |
|
I activated all four Android SDKs in the last run and everything built fine at Android API 24, only failing a single unrelated lldb test. Let's see if that test's a flake, re-running. @swift-ci build toolchain windows |
finagolfin
changed the title
|
Sigh, another lldb test flaked @swift-ci build toolchain windows |
|
Alright, building against Android API 24 on the PR toolchain CI worked, just waiting to hear from the BCNY people now if you're good with this change. |
Also, we've never supported older than API level 21, so changed that minumum.
|
Now that a Windows toolchain built fully with two Android SDKs on CI, earlier with all four arches, proposing that we get this in. @swift-ci smoke test |
|
Ping @compnerd or @Steelskin, any further comment on this trunk change for Android on Windows? If it still causes a problem for your BCNY CI alone, presumably you can roll it back locally there, but it causes no problem on the official Windows toolchain CI. |
The only comment I have is please keep this in sync with the other builds of the Android SDK. |
|
Sure, I have submitted this for the official trunk Android CI that runs on linux also, swiftlang/swift-docker#528, so it is ready to go there too. |
|
This just passed the Windows trunk snapshot toolchain CI, the only Windows CI that currently builds and tests the Android SDKs. @Steelskin, please let us know if you still see any problems with this pull in your local CI. |
We have the same issue as before on our CI. I will take a look as to why this is happening because this is a compiler crash. Even if our configuration is wrong, the compiler should not crash as a result. We don't use build.ps1 to build, as we have our own custom GHA actions to do so. For now, we're going to keep our build with SDK 28. Once the issue is found and fixed, we'll downgrade to 24 too. |
|
OK, let us know what you find. 👍 |
4 participants
Let's see if this passes all Windows CI. I'm putting together a pull next for the official Android CI that generates the SDK bundles, which runs on linux, but @marcprux already tested this out there months ago, swiftlang/swift-docker#509, and we've since added the
posix_spawnchanges that he was stubbing out back then.This is based on @compnerd and other workgroup members' go-ahead in the latest Android workgroup meeting and the work @madsodgaard has been doing to drive this minimum API down to 24 and below.