test(ci): use community NDK install action #626
Conversation
mikehardy
force-pushed
the
use-community-ndk-action
branch
3 times, most recently
from
202c1f6 to
415dcf2
Compare
mikehardy
force-pushed
the
use-community-ndk-action
branch
2 times, most recently
from
4e694ae to
4633469
Compare
|
🤔 quick build on all 3 worked (except an emulator boot issue on ubuntu?) and codeql worked, but release build failed with something that looks unrelated. This is close but may be something still to tweak, we'll see after re-runs
...on my self-hosted macOS runner. Hmmm, will think on that |
|
Not quite ready yet - this isn't installing things the way I thought it was. It is downloading correctly now, but it leaves the original download zip, and an expanded tree from the download zip in the worker temp area Then the NDK itself seems to be in ANDROID_HOME but has timestamps that make me think it came down a different way - so I'm not sure about this at the moment... and it leads to out-of-storage-space errors with the zip + 2 copies |
|
emulator boot issue may need something like I needed on react-native-firebase to pin the version to a slightly older one, for unknown reasons |
note that CodeQL runs on ubuntu which runs out of space, and our purge script doesn't work there, so liberate space the way we normally do on ubuntu also the way we should use the action is to not add it to local cache (which is creates a copy for no reason, in my opinion) but we should "link to sdk" which copies it into android home, which is where our workflows expect to find it
mikehardy
force-pushed
the
use-community-ndk-action
branch
from
4633469 to
d7bd110
Compare
| run: | | ||
| cargo install toml-cli | ||
| ANDROID_NDK_VERSION=$(toml get gradle/libs.versions.toml versions.ndk --raw) | ||
| ANDROID_NDK_MAJOR_VERSION=$(echo $ANDROID_NDK_VERSION | cut -f1 -d.) |
There was a problem hiding this comment.
Optional: Is it possible to extract this block to a script. I held myself back from saying when I saw it twice, but 4 times feels like a maintenance burden
There was a problem hiding this comment.
I've gone back and forth on "workflows that use scripts" and "workflows that have everything contained in the workflow"
The scripts way has the obvious benefit of DRY, which is nice, but on balance I'm so used to copy-paste in workflows at this point (almost the whole workflow is copy paste...) that I'm not bothered by DRY in workflows much anymore, so having it all in one spot is currently winning since I end up looking at it more frequently and grooming it more often than I would if it were tucked away separate
it's a slight preference so if you feel strongly, sure as it is obviously repetition, but if you don't feel strongly I'll leave it personally.
Where I definitely use scripts is if I can abstract something needed for devs and ci, but if it's ci-specific....meh
There was a problem hiding this comment.
No strong feelings, let's get this in
|
Okay - going to treat this as an approve but I want to see the release workflow pass on self-hosted runner first, so I'm going to rerun that until it hits one and I see it pass. Was having a storage issue before that I may have fixed, but may not have. |
|
worked self-hosted. the action isn't perfect and I may PR some better temp space mgmt to it if the maintainer is open to it, but it appears serviceable. And it's code we don't own, so +1. Going in |
Attempt to use the community NDK install action vs our bespoke scripts
Note that a deep analysis of this earlier indicated it used the github actions/toolkit which automatically handles transient network failures during download with some retries - so if it works at all, it should be as robust or better than our existing
The only difficulty here is we need to transform our definition of the NDK version - which is the full / real version, to what I would consider the "marketing" version
e.g. 29.0.23124 --> r29
or 28.3.5432 --> r28d
the pattern is
r<MAJOR><MINORth letter of alphabet>so not too badref: https://github.com/android/ndk/wiki/Unsupported-Downloads