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
chore: improve Yarn 2 cache on CI #11781
Conversation
Build successful! You can test your changes in the REPL here: https://babeljs.io/repl/build/27863/ |
This pull request is automatically built and testable in CodeSandbox. To see build info of the built libraries, click here or the icon next to each commit SHA. Latest deployment of this branch, based on commit b990407:
|
06b433b
to
411800f
Compare
411800f
to
771e85b
Compare
.github/workflows/ci.yml
Outdated
@@ -3,13 +3,45 @@ name: Node CI | |||
on: [push, pull_request] | |||
|
|||
jobs: | |||
validate-yarn-cache: |
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.
This step ensures the test jobs later will always be served with proper yarn cache.
cc @arcanis Do you think yarn 2 will ever support yarn install --fetch-only
, which skips the linking stage and only fetche new packages to yarn cache folder. The idea is to run yarn install --fetch-only
only if yarn.lock
is changed, and the CI jobs afterward will be served with a proper yarn cache. In all it can save network requests and reduce carbon emissions.
Currently when yarn.lock
is updated, the validate-yarn-cache
job will introduce 20s (linking) overhead to our CI -- since we meant to update the yarn cache folder only.
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 not sure it'll be in the default distribution. One problem with the v1 was that installs could be done in various slightly different ways that made the surface for unforeseen edge cases much larger. I'd like to avoid that with one standardized path as much as possible.
That being said, a plugin to do this would be trivial, so we probably can find something eventually, once we've figured out a good way to develop/host such plugins (it's already possible, but there's no comprehensive documentation).
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.
You can sort of get this behaviour if you set this specific job to install using PnP and with build scripts disabled, setting these env variable should do the trick:
YARN_NODE_LINKER=pnp
YARN_ENABLE_SCRIPTS=false
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.
@merceyz We opt-out of PnP because Flow does not support PnP: yarnpkg/berry#634 (comment).
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.
Indeed, but for this specific job, validate-yarn-cache, its job is to generate a valid cache, not run Flow, so installing using PnP would do that without the cost of linking. The cache for PnP and node-modules is the same.
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.
Awesome: https://github.com/babel/babel/pull/11781/checks?check_run_id=991006280#step:6:27 The pnp linker costs only 4s compared to 20s of 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.
You can reduce it even more if you also set YARN_ENABLE_SCRIPTS=false
, you can do this since you don't need to run postinstalls to get a valid cache
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.
Impressive: https://github.com/babel/babel/pull/11781/checks?check_run_id=991025473#step:6:26 3s on the linker stage now. @merceyz Thank you!
34bb1a4
to
ee7912c
Compare
ee7912c
to
ff13492
Compare
e3dbdc4
to
d484dbb
Compare
.github/workflows/ci.yml
Outdated
@@ -3,13 +3,46 @@ name: Node CI | |||
on: [push, pull_request] | |||
|
|||
jobs: | |||
validate-yarn-cache: |
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.
Nit: maybe this should be called prepare-yarn-cache
, since "validate" implies that it would fail the build if, for example, the cache is old.
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 have added a "name"
section in https://github.com/babel/babel/pull/12002/files/d484dbba30163e84d5b8cc610efea652b50c6a67..303ec4a3229d17092d770e01cbe6a77bcc72a075#diff-e9f950f17198d3d5e3122a44230a09b9R7
I can port the name changes here and also rename the job id.
d484dbb
to
b990407
Compare
@actions/cache
to our GitHub coverage workflowBased on example: https://github.com/actions/cache/blob/master/examples.md#node---yarn
The cache is working as expected. We can compare the Yarn fetch time between
main
and this PR.