Add option to control whether setup-node is used for cache fallback#67
Closed
mcmire wants to merge 1 commit into
Closed
Add option to control whether setup-node is used for cache fallback#67mcmire wants to merge 1 commit into
mcmire wants to merge 1 commit into
Conversation
Currently, this action uses two strategies for caching `node_modules`. It will first attempt to restore a previous cache, and if there is a cache miss, the action will fall back to `actions/setup-node` and its cache. Here's we've observed in other repos such as the extension: if `yarn.lock` is updated in a PR, the first job to run this action will correctly see a cache miss (since the cache key is based on the contents of `yarn.lock`). However, `actions/setup-node` may report a cache *hit*. That is very strange, because its cache key is supposedly _also_ based on the contents of `yarn.lock` — except even stranger, the hash of `yarn.lock` that `setup-node` computes is different from the one that this action computes. We haven't been able to nail down the exact differences between this action and `actions/setup-node` that would cause this behavior. Regardless, it may not be necessary to fall back to `actions/setup-node` for cache restoration. Therefore, this commit adds an option so that the fallback can be disabled.
Member
|
The It's not clear to me why we'd ever want to disable the |
Contributor
Author
|
Closing in favor of #66. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
Currently, this action uses two strategies for caching
node_modules. It will first attempt to restore a previous cache, and if there is a cache miss, the action will fall back toactions/setup-nodeand its cache.Here's we've observed in other repos such as the extension: if
yarn.lockis updated in a PR, the first job to run this action will correctly see a cache miss (since the cache key is based on the contents ofyarn.lock). However,actions/setup-nodemay report a cache hit. That is very strange, because its cache key is supposedly also based on the contents ofyarn.lock— except even stranger, the hash ofyarn.lockthatsetup-nodecomputes is different from the one that this action computes.We haven't been able to nail down the exact differences between this action and
actions/setup-nodethat would cause this behavior. Regardless, it may not be necessary to fall back toactions/setup-nodefor cache restoration. Therefore, this commit adds an option so that the fallback can be disabled.References
See this Slack thread for more information: https://consensys.slack.com/archives/C094GV3E7SB/p1777056238889789
Note
Low Risk
Low risk: only adds a new optional input that gates an existing cache-restore step, changing behavior only when explicitly disabled.
Overview
Adds a new
use-setup-node-for-cache-fallbackinput (defaulttrue) to control whether the action falls back toactions/setup-node’s Yarn cache restore when thenode_modulescache is missed.When set to
false, theConditionally restore action/setup-node cachestep is skipped, preventingsetup-nodefrom restoring.yarnas a secondary cache source.Reviewed by Cursor Bugbot for commit 7ddb11a. Bugbot is set up for automated code reviews on this repo. Configure here.