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

Cache key should also include architecture #35

Closed
abbudao opened this issue Feb 26, 2024 · 1 comment · Fixed by #36
Closed

Cache key should also include architecture #35

abbudao opened this issue Feb 26, 2024 · 1 comment · Fixed by #36

Comments

@abbudao
Copy link
Contributor

abbudao commented Feb 26, 2024

Hello, I've started a POC to adopt DevBox for my company, and I am currently using this action on CI. I ran an issue in which the same cache key is being used on different architectures, which will result in the wrong output. Example run:

Cache Size: ~440 MB (460937930 B)
/home/runner/.local/bin/tar -xf /home/runner/actions-runner/_work/_temp/16a74c1d-2a75-4a3c-bb3c-19732e2dc6af/cache.tzst -P -C /home/runner/actions-runner/_work/portfolio/portfolio --use-compress-program unzstd
Received 460937930 of 460937930 (100.0%), 9.6 MBs/sec
Cache restored successfully
Cache restored from key: Linux-devbox-nix-store-42205890a2f8496977b209fc3bc849407e06ed0661ca863ad39f934a80c55928
Run devbox run --config=. -- echo "Packages installed!"
/home/runner/actions-runner/_work/_temp/727404b5-3f8b-47c6-9702-3cc0e102006c.sh: line 1: /usr/local/bin/devbox: cannot execute binary file: Exec format error
Error: Process completed with exit code 126.

From this line, I assume we should also be using runner.arch on the cache key, like:

    - name: Mount devbox cli cache
      if: inputs.refresh-cli == 'false'
      id: cache-devbox-cli
      uses: actions/cache/restore@v4
      with:
        path: /usr/local/bin/devbox
        key: ${{ runner.os }}-${{ runner.arch }}-devbox-cli-${{ env.latest_version }}

If welcome, I can open a PR with the change.

@LucilleH
Copy link
Contributor

If welcome, I can open a PR with the change.

@abbudao Please do!

github-merge-queue bot pushed a commit that referenced this issue Mar 18, 2024
Use architecture as part of the cache key to avoid bad cache hits. The
current implementation can crash pipelines using matrix strategies or
when different pipelines use different architectures but leverage the
Devbox action.

Fixes: #35

---------

Signed-off-by: Pedro Morello Abbud <abbudao@gmail.com>
Co-authored-by: Lucille Hua <lucille.hua@jetpack.io>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging a pull request may close this issue.

2 participants