Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Caching Android ndk fails with permission #1337

Closed
6 tasks
daisy1754 opened this issue Jul 31, 2020 · 7 comments
Closed
6 tasks

Caching Android ndk fails with permission #1337

daisy1754 opened this issue Jul 31, 2020 · 7 comments
Assignees
Labels
Area: Android investigate Collect additional information, like space on disk, other tool incompatibilities etc. OS: Ubuntu

Comments

@daisy1754
Copy link

daisy1754 commented Jul 31, 2020

Describe the bug
Since Android NDK is not preinstalled (#643), I'm writing custom cache rule:

      - name: NDK Cache
        id: ndk-cache
        uses: actions/cache@v2
        with:
          path: |
            /usr/local/lib/android/sdk/ndk
          key: ndk-cache-21.0.6113669

When github action restores cache, I'm seeing following error:

/bin/tar: ../../../../../usr/local/lib/android/sdk/ndk: Cannot mkdir: Permission denied

Area for Triage: Android

Question, Bug, or Feature?: Feature

Virtual environments affected

  • macOS 10.15
  • Ubuntu 16.04 LTS
  • Ubuntu 18.04 LTS
  • Ubuntu 20.04 LTS
  • Windows Server 2016 R2
  • Windows Server 2019

Expected behavior
setting proper permission to /usr/local/lib/android/sdk/ndk so it can be cached

Actual behavior
cache restoring fails with permission issue

@htbkoo
Copy link

htbkoo commented Jul 31, 2020

Not sure if this would help / is a good idea, but I had tried to work around it by adding this follow step before the NDK cache step:

    - id: fix-ndk-permission
      name: Fix the NDK folder permission to allow caching
      shell: bash
      run: |
        sudo mkdir ${ANDROID_HOME}/ndk
        sudo chmod -R a+rwx ${ANDROID_HOME}/ndk

TL;DR: I had tried to create the ndk folder under ${ANDROID_HOME} manually and grant a 777 permission to all

But yea I admit this seems to be hacky and it would be great if the team can fix it in a more proper way / enable NDK caching by default

@maxim-lobanov
Copy link
Contributor

Hello everyone,
We would like to get additional discussion with actions/cache team because fix from image side doesn't look good from long term perspective.

Just to summarize context of problem:

  1. Folder /usr/local/lib/android/sdk/ndk doesn't exist on image by default
  2. Root permissions are set for whole /usr/local directory and it is available only with sudo (it is expected for Linux images)
  3. Repro steps:
  • User runs sudo /usr/local/lib/android/sdk/tools/bin/sdkmanager --install "ndk;20.0.5594570" (sudo is necessary here because of permissions)
  • User cache this directory /usr/local/lib/android/sdk/ndk that is created by first command
  • actions/cache can't restore this folder because it doesn't use sudo

Workaround:
Add before cache step:

sudo mkdir ${ANDROID_HOME}/ndk
sudo chmod -R a+rwx ${ANDROID_HOME}/ndk

Solution

As a solution, we can create /usr/local/lib/android/sdk/ndk folder on image and set proper permissions for whole /usr/local/lib/android folder. And this solution will resolve this particular issue. But the root cause is deeper and I anticipate more issues in future with other tools and same root cause.
Simple example:

runs-on: ubuntu-latest
steps:
- uses: actions/cache@v2.1.0
  with:
    path: /usr/local/my_awesome_tool 
    key: mykey
- name: Install awesome tool
  run: |
    sudo mkdir /usr/local/my_awesome_tool
    echo "binary file is here" | sudo tee -a /usr/local/my_awesome_tool/binary

This example shows that any other customer can face with the same issue with any other tool and actions/cache.
How can we help customer with this case? I don't think that setting user permissions on the whole /usr/local folder (or even whole machine) is good idea.

I understand position of actions/cache owners that action shouldn't use sudo tar everywhere because it could cause problems on self-hosted machines. But may be we could find better way to handle it.
@joshmgross, what do you think about it?

@nikita-bykov
Copy link
Contributor

We have merged the PR that sets proper permissions for whole /usr/local/lib/android folder. Images with changes will be rolled out around next week.

@nikita-bykov
Copy link
Contributor

All changes were successfully deployed.

@maxim-lobanov
Copy link
Contributor

Reopened issue since caching still doesn't work properly for some components
cc: @dmitry-shibanov to take a look

@dmitry-shibanov
Copy link
Contributor

Hello, we merged new fix for permissions. Changes will be added in next images rollout.

@dmitry-shibanov
Copy link
Contributor

Hello, changes were added. If you have any concerns feel free to reopen the issue.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Area: Android investigate Collect additional information, like space on disk, other tool incompatibilities etc. OS: Ubuntu
Projects
None yet
Development

No branches or pull requests

6 participants