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

Use restore-key to find cache, when exact match is not available #627

Closed
3 of 5 tasks
MatthijsBurgh opened this issue Nov 17, 2022 · 9 comments
Closed
3 of 5 tasks
Assignees
Labels
feature request New feature or request to improve the current logic

Comments

@MatthijsBurgh
Copy link

MatthijsBurgh commented Nov 17, 2022

Description:
Similar to #410 and #441, but these are closed without any result.
Cache is correctly stored in the Post Setup Node, but not hit in the next run.

Run actions/setup-node@v3
  with:
    node-version: 16
    cache: yarn
    always-auth: false
    check-latest: false
    token: ***
Found in cache @ /opt/hostedtoolcache/node/16.18.0/x64
Environment details
  node: v16.18.0
  yarn: 1.22.19
  npm: 8.19.2
  
/usr/local/bin/yarn --version
1.22.19
/usr/local/bin/yarn cache dir
/home/runner/.cache/yarn/v6
yarn cache is not found

I think this is a combined bug report and feature request. As I checked the code in this action and it just checks cache with one primary key. Maybe additional restore-keys can be provided. This is available in the JS API.

Maybe node-cache-${platform}-${packageManager}- is a good restore-key, which would solve the missing hit with an updated lockfile. This would match the example from the actions/cache docs

Action version:
v3

Platform:

  • Ubuntu
  • macOS
  • Windows

Runner type:

  • Hosted
  • Self-hosted

Tools version:
node 14 & 16, yarn

Repro steps:
public repo: https://github.com/nklayman/vue-cli-plugin-electron-builder

Expected behavior:
When lockfile is updated, it should hit the most recent cache in that branch scope.

Actual behavior:
When lockfile is updated, it doesn't hit any cache.
Updated lockfile, no cache hit: https://github.com/nklayman/vue-cli-plugin-electron-builder/actions/runs/3481884289
Same lockfile, cache hit: https://github.com/nklayman/vue-cli-plugin-electron-builder/actions/runs/3487432460

@MatthijsBurgh MatthijsBurgh added bug Something isn't working needs triage labels Nov 17, 2022
@dmitry-shibanov
Copy link
Contributor

Hello @MatthijsBurgh. Could you please rename the issue because it is not a bug but a feature request.

@dmitry-shibanov dmitry-shibanov added feature request New feature or request to improve the current logic and removed bug Something isn't working needs triage labels Nov 17, 2022
@MatthijsBurgh MatthijsBurgh changed the title Yarn cache not found Use restore-key to find cache, when exact match is not available Nov 17, 2022
@MatthijsBurgh
Copy link
Author

Hello @MatthijsBurgh. Could you please rename the issue because it is not a bug but a feature request.

Done

@dsame
Copy link
Contributor

dsame commented Nov 28, 2022

Hello @MatthijsBurgh,
uncontrolled use of the previous cache causes endless grow of the cache when the new versions of the packages are added to the cached npm directory.

The experience with setup-java showed the use of restore-keys was not appropriated. As a result i had either to deny the PR or request an optional input to explicitly enable the restore-keys in order make sure the user of action knows that he is doing.

@MatthijsBurgh
Copy link
Author

@dsame Thanks. I will take this into account. I will keep you updated about I will do next.

@ramblingenzyme
Copy link

It might be worth changing the cache key to include an indication of which files are being hashed.

We have internal repos where we use setup-node with different paths passed to "cache-dependency-path" and adding the restore key as implemented in #628 might load a previous cache for a different set of dependencies, i.e. as useful as not having restore-keys at all.

@MatthijsBurgh
Copy link
Author

@ramblingenzyme When these are different actions, it uses different caches. I don't see a very straight forward solution to your problem. As the action supports more than just a single file path:

description: 'Used to specify the path to a dependency file: package-lock.json, yarn.lock, etc. Supports wildcards or a list of file names for caching multiple dependencies.'

So if you have any idea to solve this robustly. Please let me know. Otherwise going for the solution of manual enabling this as mentioned by @dsame would not affect your situation for worse.

@dsame
Copy link
Contributor

dsame commented Dec 13, 2022

@ramblingenzyme i advise you to solve your problem with the https://github.com/actions/cache action that can be turned for your specific case.

Will this suggestion work for you?

@dsame
Copy link
Contributor

dsame commented Dec 20, 2022

@ramblingenzyme @MatthijsBurgh i am going to close this issue due to inactivity, but please feel free to reopen it on create new one in case if the problem still exists

@dsame dsame closed this as completed Dec 20, 2022
@MatthijsBurgh
Copy link
Author

MatthijsBurgh commented Dec 20, 2022

@dsame I still have the PR open. I will probably work on it during the Christmas break.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature request New feature or request to improve the current logic
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants