ci: Remove obsolete NDK/Java setup and run once on Windows #92
Conversation
|
Turns out the problem is in the NDK's compiler wrapper scripts, on both Windows and Linux: neither #!/bin/bash
if [ "$1" != "-cc1" ]; then
`dirname $0`/clang --target=aarch64-linux-android30 "$@"
else
# Target is already an argument.
`dirname $0`/clang "$@"
fiThe NDK is apparently not built to support this case anyway, and while it appears github is aware of this issue they bailed out of a potential fix at the last moment. Perhaps we should seek to revive that PR or ping the GH environment maintainers about it again, but for now the workaround is to create a symlink ourselves. |
| @@ -17,7 +17,7 @@ impl Ndk { | |||
| let mut sdk_path = std::env::var("ANDROID_HOME").ok(); | |||
| if sdk_path.is_some() { | |||
| println!( | |||
| "Warning: You use environment variable ANDROID_HOME that is deprecated.\ | |||
| "Warning: You use environment variable ANDROID_HOME that is deprecated. \ | |||
| Please, remove it and use ANDROID_SDK_ROOT instead. Now ANDROID_HOME is used" | |||
There was a problem hiding this comment.
I'm not a fan of this warning. Any objections to flipping it around and using non-deprecated ANDROID_SDK_ROOT (just like ANDROID_NDK_ROOT below) before resorting to legacy ANDROID_HOME?
We might still print a warning if legacy variables are set, and mention they are not used.
Besides, turns out ANDROID_NDK_HOME and ANDROID_NDK_PATH are a thing as well, though I've never seen NDK_HOME.
There was a problem hiding this comment.
I think it's derived from here https://developer.android.com/studio/command-line/variables#envar
There was a problem hiding this comment.
Right, despite its deprecation it still has precedence over ANDROID_SDK_ROOT. If we want to adhere to these rules perhaps we should implement number 3 as well:
If
ANDROID_HOMEis defined but does not exist or does not contain a valid SDK installation, the value inANDROID_SDK_ROOTis used instead.
There was a problem hiding this comment.
Even though I'm also more in favor of dropping support for ANDROID_HOME or reordering priorities with ANDROID_SDK_ROOT being higher on the list
In [1] this hack was introduced because GH Actions virtual-environments installed the SDK in `Program Files` which includes a space, but this is not supported by the NDK. Now that the issue has been addressed [2] and the images deployed this workaround can be removed again. [1]: rust-mobile#92 [2]: actions/runner-images#2343
In [1] this hack was introduced because GH Actions virtual-environments installed the SDK in `Program Files` which includes a space, but this is not supported by the NDK. Now that the issue has been addressed [2] and the images deployed this workaround can be removed again. [1]: rust-mobile#92 [2]: actions/runner-images#2343
In [1] this hack was introduced because GH Actions virtual-environments installed the SDK in `Program Files` which includes a space, but this is not supported by the NDK. Now that the issue has been addressed [2] and the images deployed this workaround can be removed again. [1]: rust-mobile#92 [2]: actions/runner-images#2343
#118) * ci: Separate apk check and build steps An obscure one-line `apk build` failure got shadowed by a much larger deprecation warning shown by `cargo check` right above, deemed irrelevant. Separating these steps will make it clear when the issue is code-related (ndk, ndk-glue) or build-related (ndk-build, cargo-apk, cargo-subcommand). * ci: Remove Windows NDK symlink; GH images now include space-less NDK In [1] this hack was introduced because GH Actions virtual-environments installed the SDK in `Program Files` which includes a space, but this is not supported by the NDK. Now that the issue has been addressed [2] and the images deployed this workaround can be removed again. [1]: #92 [2]: actions/runner-images#2343
…s (#118) * ci: Separate apk check and build steps An obscure one-line `apk build` failure got shadowed by a much larger deprecation warning shown by `cargo check` right above, deemed irrelevant. Separating these steps will make it clear when the issue is code-related (ndk, ndk-glue) or build-related (ndk-build, cargo-apk, cargo-subcommand). * ci: Remove Windows NDK symlink; GH images now include space-less NDK In [1] this hack was introduced because GH Actions virtual-environments installed the SDK in `Program Files` which includes a space, but this is not supported by the NDK. Now that the issue has been addressed [2] and the images deployed this workaround can be removed again. [1]: rust-mobile/ndk#92 [2]: actions/runner-images#2343
…s (#118) * ci: Separate apk check and build steps An obscure one-line `apk build` failure got shadowed by a much larger deprecation warning shown by `cargo check` right above, deemed irrelevant. Separating these steps will make it clear when the issue is code-related (ndk, ndk-glue) or build-related (ndk-build, cargo-apk, cargo-subcommand). * ci: Remove Windows NDK symlink; GH images now include space-less NDK In [1] this hack was introduced because GH Actions virtual-environments installed the SDK in `Program Files` which includes a space, but this is not supported by the NDK. Now that the issue has been addressed [2] and the images deployed this workaround can be removed again. [1]: rust-mobile/ndk#92 [2]: actions/runner-images#2343
GitHub already includes the Android SDK/NDK and Java in their image, no need to download and install it again slowing down the build process.
Also run at least one build job on Windows to make sure it remains sane. It didn't seem necessary to run both nightly and stable for every target though.
Note that this is in part to bring to attention an issue with spaces in CC/LINKER paths:
It looks like the code using
CARGO_TARGET_aarch64_linux_android_LINKER(though the same happens forCC_/CCX_<triple>) properly understands that that's one string (seeing the quotes in the log) yet somehow invokes it in the wrong way. Anyone have an idea where to look and resolve this?(I'm not a fan of the
if: runner.os != 'Windows'around most tests but we can't really run "Android-native" unix code on Windows)EDIT: On the note of CC, how about including an example/test that validates native compilation?