-
Notifications
You must be signed in to change notification settings - Fork 3.4k
Add caching for the /gutenberg directory
#10971
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
Closed
Closed
Changes from 8 commits
Commits
Show all changes
11 commits
Select commit
Hold shift + click to select a range
4a8fa4f
Add caching for the `/gutenberg` directory
desrosj c63bcc9
Update the inline documentation for each job.
desrosj 83348a8
Use "sha" instead of "ref"
desrosj a74c880
Exclude the `node_modules` directory in the cache.
desrosj 4634909
Prevent globbing and word splitting.
desrosj af4e29a
Include build-specific changes in cache key.
desrosj e6dd184
Remove surrounding quotations from path patterns
desrosj 6f425be
Add trailing slash to exclude pattern.
desrosj 1a2fa13
Adjust glob pattern.
desrosj 32bc588
Remove exclusion pattern.
desrosj 40bcc92
Leave `node_modules`
desrosj File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
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
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
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
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
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
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
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
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
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
Oops, something went wrong.
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.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why is
gutenberg/node_modulesexcluded?There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I mean, wouldn't we want to cache
node_modules?There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm experimenting to see what the effect of caching this has on the overall time it takes to run the workflow. The
setup-nodeaction already caches npm dependencies in an optimized way, so I'm not certain that it's required to cache thenode_modulesdirectory. Just testing an assumption before committing to storing an 800MB cache key (the entire repository is limited to 10GB at any given time).There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I assumed
setup-nodeonly caches the rootnode_modulesdirectory, not thegutenberg/node_modulesdirectory. But apparently it doesn't even cache the root one, per the docs:There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I believe it caches the global npm download cache which has compressed tarballs. This is mainly because the resulting
node_modulesfolder varies for different platforms/operating systems.My assumption is that the global cache should contain any packages installed on the runner from any
package.jsonfile. But to confirm this, we'd have to find a specific dependency found only in the Gutenberg repository and confirm it ends up in the generated cache key (deleting the key manually and then rerunning a workflow that generates a new one with debug mode will output every file included).If that is the case, then the changes here will actually have no effect at all since the cache already includes the necessary details.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It looks like
jest-jasmine2is a dependency ingutenbergand is found nowhere in the dependency tree forwordpress-develop. Testing this now.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
With some assistance from Claude, it seems you have to take the
sha512integrity value in the lock file for the package, convert it from base64 to hexadecimal. Then the first 4 digits represent the directory path, and the rest is the name. So forjest-jasmine2it should be.npm/_cacache/content-v2/sha512/83/c2/05d0c1e40d87834adf69b1e631dc7c7c9e3cba98f9e3d1bbd7e5fd7d4172e09cbe39ba9ccb2eefc45e34bb9b89047afe7b95f9e2791427b1f1ee1e0b7f44ddde0c3c9b5e7c2fc76I don't see that in the log output, though. So seems it's not the case.
It looks like the reusable Gutenberg build process testing workflow specifies multiple lock files for
setup-node. This is probably the best option, but the problem is that the file does not exist untilnpm installruns. So passinggutenberg/package-lock.jsonwill result in an error.Any other ideas @westonruter?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Could the
package-lock.jsonfile be pulled down beforehand and written into thegutenbergdirectory before runningnpm installat the root? The raw URL for the file should be easy to construct since we have the Gutenberg hash available.