build-image
build-image copied to clipboard
fix: fix the builds for public projects that use yarn 2 (or above) with workspaces
Summary
Fixes #762
The problem it solves
Currently builds are failing for public projects that use yarn 2 (or above) with workspaces. The cached node modules are cached and restored to the wrong location.
The bug
A bit of context: in yarn 1.x only private repos can have workspaces, but this limitation was removed in yarn 2:
Worktrees used to be required to be private (ie list
"private": true
in their package.json). This requirement got removed with the 2.0 release in order to help standalone projects to progressively adopt workspaces (for example by listing their documentation website as a separate workspace). (source: docs)
The bug is in the run-build-functions.sh
file:
# YARN_IGNORE_PATH will ignore the presence of a local yarn executable (i.e. yarn 2) and default
# to using the global one (which, for now, is always yarn 1.x). See https://yarnpkg.com/configuration/yarnrc#ignorePath
# we can actually use this command for npm workspaces as well
workspace_output="$(YARN_IGNORE_PATH=1 yarn workspaces --json info 2>/dev/null)"
workspace_exit_code=$?
As you can see it tries to output information about the workspaces with yarn 1.x. in the command substitution and redirects STDERR to /dev/null
. After that it only checks if the process exited with 0 or an error code, but ignores the error message, which would tell us, that "workspaces can only be enabled in private projects". Here's an example for the problem from the theatre-js/theatre repo:
- when the errors are hidden:
$ YARN_IGNORE_PATH=1 yarn workspaces --json info 2>/dev/null
{"type":"info","data":"Visit \u001b[1mhttps://yarnpkg.com/en/docs/cli/workspaces\u001b[22m for documentation about this command."}
- when the errors are visible:
$ YARN_IGNORE_PATH=1 yarn workspaces --json info
{"type":"error","data":"Workspaces can only be enabled in private projects."}
{"type":"info","data":"Visit \u001b[1mhttps://yarnpkg.com/en/docs/cli/workspaces\u001b[22m for documentation about this command."}
The solution
- We can capture the error messages instead of discarding them to check if the problem is related to private workspaces.
- If it is, then we check the version of yarn that is used in the project and use the new
yarn workspaces list --json
command to get the information about the workspaces
How I tested the solution
We have a public repo whose builds are failing without clearing the cache after every build. Here's how the cached folders look like with the current build script:
The yarn workspaces are not detected in the logs:
But here's the previous pictures look like for the fixed build script:
I'm not sure if I missed anything, but feel free to tell me if there are things to change! :relaxed:
For us to review and ship your PR efficiently, please perform the following steps:
- [x] Open a bug/issue before writing your code ๐งโ๐ป. This ensures we can discuss the changes and get feedback from everyone that should be involved. If you`re fixing a typo or something that`s on fire ๐ฅ (e.g. incident related), you can skip this step.
- [x] Read the contribution guidelines ๐. This ensures your code follows our style guide and passes our tests.
- [x] Update or add tests (if any source code was changed or added) ๐งช
- [x] Update the included software doc (if you updated included software) ๐
- [x] Update or add documentation (if features were changed or added) ๐
- [ ] Make sure the status checks below are successful โ
A picture of a cute animal (not mandatory, but encouraged)
added to backend/support pairing - @audreyshub could you please raise this with the next person you meet with to ask the relevant team to review this for us?
Thank you for your contribution @fulopkovacs! ๐ And thanks @fool for raising this. I've added this to our board so we can triage it and prioritise the review next Monday during our planning session.
@JGAntunes I finally pass the PR back to review! :grin: There were a few surprises with the tests (e.g.: many of the tests in yarn.bats
executed the run_yarn
function, which removes the default yarn installation, but there was no proper cleanup after these tests, so the binary for yarn
was not available for tests in other files).
One of our biggest pain point gets addressed, if this PR gets merged, but there will still be the problem of caching, which doesn't really work with yarn berry now (35-45s for us for every deploy). Are there any (public) news about that? I saw a PR about it (https://github.com/netlify/build-image/pull/465), but it didn't receive much attention from your side (I didn't check the code of the PR though).
(Btw having worked with the code base and seeing the tests I understand that full support for yarn berry would require quite a bit of planning and effort, so I don't expect you to fix the issue overnight. I would be happy with just a bit more clarity about the place of this feature on the roadmap :relaxed: )
Support got already implemented inside https://github.com/netlify/build-image/pull/858