actions / runner-images

GitHub Actions runner images
MIT License
10.05k stars 3.03k forks source link

Pre-install Android NDK versions on Actions images #643

Closed samtstern closed 4 years ago

samtstern commented 4 years ago

Tool information

Area for Triage: Android

Question, Bug, or Feature?: Feature

Virtual environments affected

Can this tool be installed during the build? Yes, it takes about 1.5 minutes

Are you willing to submit a PR? Yes!

samtstern commented 4 years ago

Submitted at the recommendation of @maxim-lobanov in this comment: https://github.com/actions/virtual-environments/issues/578#issuecomment-605886387

Installing a few common versions of the Android NDK would make builds faster, reduce network traffic, and reduce build complexity.

My Android build used to be super simple:

  build:
    runs-on: ubuntu-latest
    steps:
    - uses: actions/checkout@v2
    - name: set up JDK 1.8
      uses: actions/setup-java@v1
      with:
        java-version: 1.8
    - name: Build with Gradle
      run: ./gradlew clean ktlint build

Then an Android update forced me to add this:

  build:
    runs-on: ubuntu-latest
    steps:
    # ...
    - name: Install NDK
      run: echo "y" | sudo ${ANDROID_HOME}/tools/bin/sdkmanager --install "ndk;20.0.5594570" --sdk_root=${ANDROID_SDK_ROOT}

That took my build from 9m40s to 11m20s so now I am considering adding NDK caching but that's even more complexity to my very simple workflow.

nikita-bykov commented 4 years ago

We have added Android NDK 20.0.5594570 installation to Linux. Images with it will be rolled out around next week.

prempalsingh commented 4 years ago

I was about to remove the ndk installation step from my workflow but then realized that this got reverted in #756 . :(

maxim-lobanov commented 4 years ago

Yep, sorry for inconvenience, @prempalsingh. We have faced with disk space on Ubuntu images recently and had to revert the images and a few latest changes as a quick solution to unblock customers. As soon as issue with disk space is resolved, we will return it back. We will keep this issue opened

saurik commented 4 years ago

As someone who does a lot of work with the NDK--I seriously use it to compile for all platforms, using the Android NDK clang to target even iOS and Windows and I have a cloud deployment of the NDK on AWS Lambda for massively-parallel C/C++ builds--and who has been requesting that the macOS virtual environment ship a newer version of the NDK for a while now (as issue #120), I was really curious why there is such strong interest here in having GitHub ship a slightly older version of the NDK, particularly given that r21 (which is what is already being shipped) both continues to support the "legacy toolchain" install paths / layout and is what Google now considers to be the "Long Term Support" version of the NDK.

At first, I assumed it would have to be some legitimate-yet-subtle concern like "we do deterministic builds, and thereby need the same version of the NDK everywhere and sadly a lot of projects standardized on using r20 as r21 took a while to finalize" or "a lot of downstream enterprise developers are still on older versions of the NDK, so we are doing regression tests to compile our projects against all versions of the NDK released in the past two years"; but, I did some digging (searching on GitHub for projects that have workflow tasks that manually install r20), and as far as I can tell the core issue here is a bug in the Android Gradle Plugin that was reported as https://github.com/gradle/gradle/issues/12440, wherein it simply refuses to accept r21 because it seems to be hardcoded to insist on r20 (supposedly even if the project isn't even using the NDK...) :/.

You don't actually have to install r20, though: another option is to just tell the plugin to use r21 by adding ndkVersion "21.0.6113669" to your android block in build.gradle. To verify this would work, I took two projects with the r20 workaround, forked them to my GitHub account, verified that their workflows didn't compile if I didn't install r20, and then fixed them to support r21. One of them was a smaller project that makes extensive usage of native code: an Android wrapper for SRT; the fixed commit is https://github.com/saurik/SRTWrapper/commit/491034b0ad68860f48bf4fef0b8d385b009cf55d. For the second I went with an official Google Firebase "quick start" repository with a number of demo applications; that fixed commit is https://github.com/saurik/quickstart-android/commit/644717173640049e78ef40ad9e6517d28bcbc9b6. (@prempalsingh I wanted to use your project also, to maybe even send a pull request, but I couldn't find any repositories on your account with the r20 workaround; it is possible I just missed it, though.)

@samtstern Instead of having GitHub ship an old version of the NDK, burning an additional 3.8GB of the scant remaining 8-14GB of disk space available on the root partition (the core issue a bunch of us are running into in #709 and the reason I ended up finding this issue, linked from a recent commit that I just knew would burn a lot of precious precious disk ;P), maybe you can flex some will internally at Google to get the bug in the Android Gradle Plugin fixed? FWIW, it isn't just GitHub Actions where people are running into this, as the same problem has been noted on at least Travis CI... and like, this is even going to happen locally, so I'd think it better to not make people think the newer r21 LTS NDK is somehow discouraged; solving the root cause of this problem would likely help a lot more people ;P.

samtstern commented 4 years ago

@saurik thank you for the very deep and well reasoned investigation. I'm not on the Android team and Google is a big company but I'll see what I can do.

In the meantime I'll use your patch to fix my own builds.

I agree r20 is not a good use of GitHub's resources.

saurik commented 4 years ago

@samtstern Oh, I'm sorry, then: you GitHub bio says "Developer Relations at Google. All Things Android or Firebase.", so I thought/hoped you would have pull with the Android team :(.

samtstern commented 4 years ago

@saurik got some more information here from someone on the tools team in an older thread about this issue:

Each AGP version has a default NDK version hard-coded like the build tools. You can override this by the time AGP ships, it's entirely possible a newer NDK has been released android.ndkVersion =20.1.5948944 will fix this, or you can download version 20.0.5594570

The workaround is what you already figured out, but the hard dependency from latest AGP to not-latest NDK is something the team knows about so I don't think we will be able to change it very quickly.

I will try to see what else can be done.

samtstern commented 4 years ago

@saurik actually it looks like there's a change going in to make things less strict: https://issuetracker.google.com/144111441#comment10

Basically it would warn (rather than error) and then try to proceed with the NDK version that does exist. Which in most cases would be fine.

JavierSegoviaCordoba commented 4 years ago

I changed the android block to include a determined version of NDK, but if every time GitHub updates the OS I will get:

No version of NDK matched the requested version 21.0.6113669. Versions available locally: 21.1.6352462

I think GitHub should add 20.0.5594570 because AGP 4.1 can take at least 6 months to be released.

JavierSegoviaCordoba commented 4 years ago

Any way to cache the NDK at least?

maxim-lobanov commented 4 years ago

@JavierSegoviaCordoba , I think you can use actions/cache to cache folder /usr/local/lib/android/sdk/ndk and it will speed up your build

maxim-lobanov commented 4 years ago

Hello everyone, As I mentioned above, recently we had an incident related to disk space on Ubuntu images. Currently, this incident is resolved and Ubuntu images have enough free disk space. But we have to be very careful to choose the software that is pre-installed on the image because we are pretty close to edge (disk space issue) and we don't have a chance to increase disk space right now. Considering point above and the fact that Android NDK requires ~2-3 GB of free space, we can't pre-install it on image right now. In near future, we will keep only latest NDK on the image. Sorry for inconvenience and patience.

If your use-case requires older versions of NDK, please consider installing them in runtime using the command sudo ${ANDROID_HOME}/tools/bin/sdkmanager --install "ndk;20.0.5594570" If the time is critical for you, actions/cache can give some win with installation time.

We will happy to revisit this request and our strategy in future when we have enough resources on images but for now I am closing this issue.

JavierSegoviaCordoba commented 4 years ago

@maxim-lobanov I am having this warning:

Run actions/cache@v1
  with:
    path: /usr/local/lib/android/sdk/ndk
    key: Linux-ndkcache
    restore-keys: Linux-ndkcache

  env:
    JAVA_HOME: /opt/hostedtoolcache/jdk/8.0.252/x64
    JAVA_HOME_8.0.252_x64: /opt/hostedtoolcache/jdk/8.0.252/x64
Cache Size: ~818 MB (857762895 B)
[warning]EACCES: permission denied, mkdir '/usr/local/lib/android/sdk/ndk'
maxim-lobanov commented 4 years ago

I see, I can reproduce it via mkdir /usr/local/lib/android/sdk/ndk. Looks like actions/cache doesn't use sudo and this folder is not available without sudo by default. Quick workaround - invoke sudo mkdir /usr/local/lib/android/sdk/ndk before cache step. Long term solution: 1) Ideally, improve actions/cache to make sure that it uses sudo if needs. I like this solution because it will fix not only Android but other tools too (I believe you can reproduce the same issue if you cache any folder that is owned by root. However, owners of actions/cache may have some concerns about this solution. 2) We can set full permissions for /usr/local/lib/android/sdk folder. If you prefer this way, please log the separate issue in this repository.

JavierSegoviaCordoba commented 4 years ago

Yeah, I fixed creating the file, I think the solution 1 is better to avoid boilerplate.

daisy1754 commented 4 years ago

@maxim-lobanov FYI sudo idea was rejected here https://github.com/actions/cache/issues/133

I just filed an issue for ndk folder permission https://github.com/actions/virtual-environments/issues/1337