[turborepo] [yarn 3] pruned lockfile missing resolution entries #2791
Comments
|
This was first mentioned in my ticket in #2049. I will close my version as this ticket is more scoped. CC: @chris-olszewski, since you're assigned to that ticket, perhaps you intend to assign yourself to this ticket? |
|
Any update on this? I don't see much activity happening on this issue but I have the exact same problem in our app and this is quite blocking. |
|
Looks like this work might be in progress. #4437 @chris-olszewski do you still expect this to be fixed in the |
|
It should be released as part of a canary next week. |
### Description Built on top of #4629 Overview of changes: - ~~Change `subgraph` so we calculate workspace's closure in parallel in a similar manner to Go~~ This had to get reverted, it appears that rayon was causing [compilation issues on linux](https://github.com/vercel/turbo/actions/runs/4791974785/jobs/8522988391#step:4:648) - `npm_subgraph` -> `subgraph` makes this FFI call generic - Addition of an optional `resolutions` map to the `subgraph` and `transitive_closure`, this is only used by Berry. In the ideal world this would be all of the root `package.json`, but serializing all of that via protobuf is overkill since berry is the only lockfile that needs that info and we only use that field. - Addition of `patches` FFI function, this wasn't needed before as NPM doesn't have patch files encoded in the lockfile - Addition of `global_changes` FFI function, previously for NPM we were just doing this in Go since there were just two fields we needed to check and parsing JSON is cheap. - Switches lockfile usage of berry from the Go implementation to the Rust one introduced in #4589 Reviewer notes: - The passing of the optional resolutions map is very icky and one off, but this is the only lockfile the requires additional information to properly parse. Once the package graph is squarely in Rust land this should go back to being more generic. This will finally close out #2791 ### Testing Instructions Added a new integration test that uses resolutions. This integration test invokes the new `patches` FFI call. For a full test of the new resolution feature, clone and follow the instructions in the [repro](https://github.com/erj826/turbo-prune-bug-repro-11-21) The `lockfile_aware_caching` integration tests all hit the new `global_changes` FFI callsite and provide test coverage. --------- Co-authored-by: Alexander Lyon <arlyon@me.com>
|
Hello all, if everyone could test the latest canary (1.9.4-canary.5) I would appreciate it a ton! This issue has been one edge case after another and I think I've covered them all, but testing this against more projects would be great! |
|
Hi @chris-olszewski, thank you for the progress on this item! In my large monorepo, there is, unfortunately, still some drift. However, it's gotten much closer than it was before. I will see what I can do to share a private repo. Also, for others reading this, the release is actually |
|
One edge case: // package/a
"dependencies": {
"@project/b": "*"
}
// package/c
"dependencies": {
"@project/b": "workspace:*"
}with Results in while a regular |
|
Running into another edge case when running an install after pruning (yarn 3.5): DockerfileFROM node:16.20.0 AS base
RUN mkdir /home/node/app
FROM base AS pruner
WORKDIR /home/node/app
COPY . .
RUN yarn --quiet dlx turbo@1.9.4-canary.5 prune --scope=<my app> --docker
FROM base AS installer
WORKDIR /home/node/app
COPY --from=pruner /home/node/app/out/json/ .
COPY --from=pruner /home/node/app/out/yarn.lock ./yarn.lock
COPY --from=pruner /home/node/app/.yarnrc.yml ./.yarnrc.yml
COPY --from=pruner /home/node/app/.yarn ./.yarn
RUN CYPRESS_INSTALL_BINARY=0 PLAYWRIGHT_SKIP_BROWSER_DOWNLOAD=1 yarn --immutable
I can try to spin up a reproduction repo when I get some time. |
|
Thanks for testing this out!
Are there entire entries missing or is it primarily drift between the keys?
I see the issue here, should have a fix up soon. @erj826 |
### Description Fixes the issue found by @quinnturner in [#2891](#2791 (comment)). Crux of the issue was we were being too restrictive when tracking dependency protocols to properly infer them for later construction. We now allow for at most two descriptors in the protocol resolver: one without a protocol and one with a protocol. Another change was no longer adding all descriptors for a workspace and instead only including descriptors that come from other workspaces. ### Testing Instructions New testing fixture contains a lockfile where one workspace depends on another via `*` and the other depends on it via `workspace:*`.
### Description Addresses the issue @erj826 found in [#2791](#2791 (comment)) The issue came from us including all descriptors for patches included in the pruned lockfile. This is fine for patches created by `yarn patch` since this uses `resolutions` which results in the patch having a single descriptor. Yarn also includes [some patches](https://github.com/yarnpkg/berry/tree/master/packages/plugin-compat) by default if they appear in your lockfile. These patches don't go through `resolutions` and instead get routed through the [`reduceDependency` hook](https://yarnpkg.com/advanced/plugin-tutorial#hook-reduceDependency) which produces a new descriptor for each descriptor used for the original package. The fix was to only include patch descriptors if the embedded descriptor is present in the pruned lockfile. ### Testing Instructions Added a new test fixture where multiple workspaces depend on `resolve`, but use different ranges.
|
@quinnturner and @erj826 |
|
Hi @chris-olszewski, I just tested. It's closer; amazing job! There is still precisely one issue in my monorepo. It removed a reference to -"@babel/plugin-proposal-private-property-in-object@npm:^7.16.0, @babel/plugin-proposal-private-property-in-object@npm:^7.21.0":
+"@babel/plugin-proposal-private-property-in-object@npm:^7.21.0":
version: 7.21.0
resolution: "@babel/plugin-proposal-private-property-in-object@npm:7.21.0"
dependencies:I have updated my private repository with the latest canary. You should have access to it; I added you as a contributor last week 😄 What's confusing is that the reference to |
|
In either case, this issue, as described, seems to be resolved. If there is something to do with |
|
Sorry, I missed that invite. Thanks for testing this out, I'm so excited that we're this close. I'll try to get that fixed up ASAP, I at least know where we're doing something wrong. (That reference is injected via a default plugin that ships as part of the yarn binary |
|
@quinnturner A partial work around you can poke around with while I continue to poke at this would be to add |
|
@chris-olszewski thank you for the quick turnaround! Prune appears to be working without any issues for our monorepo using |
|
Hi, i'm not sure if the following is related to this same issue but seems to me that there are some similarities. I have to patch a package in my project and i'm using is this the same issue or a different one? happy to provide more information if needed or any help to troubleshoot this. |
|
Only downgrading from |
Same for me |
|
Not sure if this helps but I ran into this as well on When I remove it the install succeeds. |
|
I have the same problem with alex-dixon in version 1.10.12. After I ran |
What version of Turborepo are you using?
1.6.3
What package manager are you using / does the bug impact?
Yarn v2/v3 (node_modules linker only)
What operating system are you using?
Mac
Describe the Bug
To optimize our Docker builds we are attempting to use turbo prune to create a lighter weight image. After running prune we copy our pruned lockfile and .json files into a directory and then attempt to run an immutable install.
We have observed that when the root package.json includes yarn resolutions we are unable to run
yarn --immutablesuccessfully in the pruned monorepo.For example
In this reproduction
ajvis a dependency ofeslintso it is a necessary dependency in the pruned monorepo.When the resolution is not included in the root package.json the generated
out/yarn.lockis correct, and includes anajventry.When the resolution is included in the root package.json then generated
out/yarn.lockis incorrect, and does not include anyajventries.Expected Behavior
The pruned lockfile should include dependencies that have a corresponding yarn resolution.
To Reproduce
See the README.md in the reproduction repo for more details.
Reproduction Repo
https://github.com/erj826/turbo-prune-bug-repro-11-21
The text was updated successfully, but these errors were encountered: