[build-windows-toolchain.bat] Add AndroidSDKArgs parameters to cross-compile Android SDKs#87867
[build-windows-toolchain.bat] Add AndroidSDKArgs parameters to cross-compile Android SDKs#87867finagolfin wants to merge 1 commit into
AndroidSDKArgs parameters to cross-compile Android SDKs#87867Conversation
|
@swift-ci smoke test windows |
1 similar comment
|
@swift-ci smoke test windows |
|
@swift-ci smoke test windows |
| if not "%WINDOWS_SDKS%"=="" set "WindowsSDKArgs=%WindowsSDKArgs% -WindowsSDKArchitectures %WINDOWS_SDKS%" | ||
|
|
||
| :: Build the arguments related to Android SDK cross-compilation | ||
| set "AndroidSDKArgs=-Android -AndroidSDKVersions Android -AndroidSDKArchitectures armv7" |
There was a problem hiding this comment.
| set "AndroidSDKArgs=-Android -AndroidSDKVersions Android -AndroidSDKArchitectures armv7" | |
| set "AndroidSDKArgs=-Android -AndroidSDKVersions Android -AndroidSDKArchitectures armv7 -AndroidSDKVersions AndroidExperimental -AndroidSDKLinkModes dynamic" |
This should cut the cost as we wouldn't build the static runtime.
There was a problem hiding this comment.
Currently, I am not building the "Experimental" SDKs at all in this pull, only the original Build-SDK for 32-bit Android armv7, which finishes in 16 mins as we discussed. Are you saying that you'd now like to add the dynamic "Experimental" Android runtime also to the Windows smoke test CI, despite that taking longer?
There was a problem hiding this comment.
Yes, the dynamic experimental SDK builds should be preferred. I don't think that they take longer (do you have numbers for the builds that show that? The summary can be misleading as the experimental SDK is both static and dynamic, so you need to halve the number).
There was a problem hiding this comment.
Let me paste what you wrote earlier off github:
"I was thinking that we enable just shared armv7 for pre-commit CI, leave the other builds to nightly. That minimises the cost (~15m) but gives us some Android coverage."
I didn't know what you meant by shared armv7, but figured you were talking about the Build-SDK action I configured here, since I knew that only took around 15 mins. Did you mean the dynamic experimental runtime in the above message too?
I don't think that they take longer
I simply meant that building both Build-SDK and the dynamic experimental runtime takes longer than the former alone.
However, there is a problem with just trying to build the experimental SDK alone, as Testing only builds if the Build-SDK step is enabled (see the linked CI log from there when I tried to only build Experimental Android SDKs).
There was a problem hiding this comment.
Yes, I meant the experimental SDK builds (we are trying to remove the old SDK builds). The testing dependency sounds new and a regression that we should try to fix.
|
@swift-ci smoke test windows |
| # Copy static dependencies | ||
| foreach ($Build in $AndroidSDKBuilds) { | ||
| if (-not $Build.LinkModes.Contains("static")) { continue } | ||
| if (-not $Build.LinkModes.Contains("static") -or -not $AndroidSDKVersions.Contains("AndroidExperimental")) { continue } |
There was a problem hiding this comment.
I don't understand this change, why is this needed now but wasn't previously?
There was a problem hiding this comment.
Previously I simply disabled this block entirely, whereas now I added this proper check if the Experimental SDKs are being built, and only disable it if they aren't.
There was a problem hiding this comment.
That doesn't entirely make sense - I don't think that we would be generating the static content in the old runtimes build (and if we are we should just disable that).
There was a problem hiding this comment.
Oh, if you mean why this check is needed at all, the logic below only installs static libraries into the Experimental SDK. This always happened as long as both the regular and Experimental SDKs were being built on the trunk snapshot toolchain CI, but once I tried to only invoke Build-SDK in this pull, with no Experimental Android SDK, the below logic broke.
There was a problem hiding this comment.
That doesn't entirely make sense - I don't think that we would be generating the static content in the old runtimes build (and if we are we should just disable that).
Look at the below install logic: it only copies static libraries to the Experimental SDKs. Do the original SDKs need them too?
There was a problem hiding this comment.
The point is that -AndroidSDKLinkModes being dynamic would already skip this appropriately.
There was a problem hiding this comment.
Agreed, but I'm trying to fix this generally so it is possible to not build the experimental SDK, only the original Build-SDK. If you don't care about that config because you plan to remove the original SDK config anyway on Windows soon, I can remove this fix.
There was a problem hiding this comment.
Yeah, I don't think that we care about that config - the sooner that we can remove the legacy SDK the better.
There was a problem hiding this comment.
Well, you'll have to remove the Testing dependency on the legacy Build-SDK first, as the latest smoke test here shows.
finagolfin
changed the title
AndroidSDKArgs parameter to cross-compile Android SDKsAndroidSDKArgs parameters to cross-compile Android SDKs
|
@swift-ci smoke test windows |
…s-compile Android SDKs Also, fix build.ps1 for when the "Experimental" Android SDKs with static libraries aren't built.
|
@shahmishal and @compnerd, got this new Android interface for the Windows CI working, so need review on what you think its final shape should be. Once this is in, we would like the CI team to add armv7 alone to the Windows smoke test PR CI, with This will allow us to test out Android changes on the pre-commit Windows PR CI before merging. |
| set "AndroidSDKArgs=" | ||
| if not "%ANDROID_SDKS%"=="" goto Android | ||
| if not "%ANDROID_SDK_VERSIONS%"=="" goto Android | ||
| if not "%ANDROID_LINK_MODES%"=="" goto Android |
There was a problem hiding this comment.
If any or all of these three Android parameters are passed in, build the Android SDKs with those flags. Otherwise, building the Android SDKs is disabled by default, as it always has been.
|
Let's see if adding cross-repo pulls for the Android PR CI works yet, completely unrelated to this pull. @swift-ci test android |
|
Another cross-repo testing check, again completely unrelated to this pull @swift-ci test android |
3 participants
The @swiftlang/android-workgroup has been discussing adding Android SDK builds to the pre-commit Windows PR CI, so this is a new interface to add that Windows CI config.