Skip to content
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

Support yarn 3 lockfile entries #643

Closed
wants to merge 4 commits into from

Conversation

theJoeBiz
Copy link

The yarn berry support is looking good, but I can't truly use turbo prune with yarn 3 because the checksum, resolution and linkType properties aren't being carried over. I updated the LockfileEntry struct locally and tested it out - here is the side-by-side comparison of root yarn.lock and out/yarn.lock:

Screen Shot 2022-01-30 at 12 19 01 AM

Btw, not sure how important the __metadata key is in the lockfile, but I think it'd be ideal if that was copied directly

@vercel
Copy link

vercel bot commented Jan 30, 2022

This pull request is being automatically deployed with Vercel (learn more).
To see the status of your deployment, click below or on the icon next to each commit.

🔍 Inspect: https://vercel.com/vercel/turbo-site/3kvjVXhqufuDDJLWGQnipykbyKL4
✅ Preview: https://turbo-site-git-fork-thejoebiz-fix-yarn-3-prune-lockfile.vercel.sh

@Xenfo
Copy link
Contributor

Xenfo commented Jan 30, 2022

It only partly works for me. Been working on this for a while, trying to get yarn --immutable to work
lEcER0br
.

@theJoeBiz
Copy link
Author

It only partly works for me. Been working on this for a while, trying to get yarn --immutable to work !

Ah, you're right. It looks like my "soft" links (the monorepo packages) are missing from the pruned lockfile. Perhaps something to do with version parsing? Example:

"@***/admin@workspace:apps/admin":
  version: 0.0.0-use.local
  resolution: "@***/admin@workspace:apps/admin"
  dependencies:
    ...

@Xenfo
Copy link
Contributor

Xenfo commented Jan 30, 2022

Probably, when I implemented Yarn 3 I didn't realize how much there was, I probably missed a bunch of stuff.

@theJoeBiz
Copy link
Author

theJoeBiz commented Jan 30, 2022

I am seeing the workspace packages listed in the node_modules/.cache/turbo/{}-turbo-lock.yaml file, so perhaps they are being filtered out before the write:

Screen Shot 2022-01-30 at 11 21 24 AM
Screen Shot 2022-01-30 at 11 23 43 AM

@theJoeBiz
Copy link
Author

@jaredpalmer I haven't had a chance to go back and look at this again since I left it, but IIRC the local workspaces weren't listed in yarn.lock for v1. I'm wondering if workspace packages are specifically being filtered out of the final list.

@Xenfo
Copy link
Contributor

Xenfo commented Feb 1, 2022

I'm not entirely sure if that's the case, I was just running yarn install --immutable and fixing one thing at a time eg: double quote missing here, add it.

@gsoltis gsoltis added the area: ergonomics Issues and features impacting the developer experience of using turbo label Feb 3, 2022
@jaredpalmer
Copy link
Contributor

We'd need to go through and fix --immutable line by line

@jaredpalmer jaredpalmer added the pr: on hold Pull requests that are "on hold" and should not be merged label Feb 25, 2022
@quinnturner
Copy link

quinnturner commented May 10, 2022

This change, as is, makes my yarn install work on a pruned project. While it doesn't support --immutable, without this change, I cannot perform a yarn install on a pruned project at all due to a 501 error from the Yarn registry. Please consider releasing this change despite it not working with --immutable as it is an incremental improvement over the existing behaviour and unblocks our implementation 😄.

EDIT 2: I have a WIP here quinnturner#2

EDIT: I do think there are some places for improvement that get us closer to --immutable, looking into it

This is more correct than what is currently there:

type LockfileEntry struct {
	// resolved version for the particular entry based on the provided semver revision
	Version    string `yaml:"version"`
	Resolved   string `yaml:"resolved,omitempty"`
	Integrity  string `yaml:"integrity,omitempty"`
	Resolution string `yaml:"resolution,omitempty"`
	// the list of unresolved modules and revisions (e.g. type-detect : ^4.0.0)
	Dependencies         map[string]string `yaml:"dependencies,omitempty"`
	PeerDependencies     map[string]string          `yaml:"peerDependencies,omitempty"`
	PeerDependenciesMeta map[string]map[string]bool `yaml:"peerDependenciesMeta,omitempty"`
	// the list of unresolved modules and revisions (e.g. type-detect : ^4.0.0)
	Bin                  map[string]string          `yaml:"bin,omitempty"`
	OptionalDependencies map[string]string          `yaml:"optionalDependencies,omitempty"`
	Checksum             string                     `yaml:"checksum,omitempty"`
	Conditions           string                     `yaml:"conditions,omitempty"`
	LanguageName         string                     `yaml:"languageName,omitempty"`
	LinkType             string                     `yaml:"linkType,omitempty"`
}

I went ahead and published turbo-yarn-berry@1.28.0-rc.1 which includes these changes and makes turbo prune work for Berry when not using --immutable. I do not necessarily recommend using the fork, it was primarily for investigation purposes.

Here are the blockers that I identified for --immutable (with checkmarks for whether support is added in my WIP PR):

  • The __metadata.version key got updated to 6; so it's best if the header is copied directly from yarn.lock instead of just writing a predefined string
  • Yarn performs a dedupe when there are entries with identical versions. Turbo must perform a similar deduping while maintaining the correct order of the versioned modules in the entry.

    Deduping is solved except for resolving the local workspace's dependencies

  • Add the local workspace packages to the lock file
  • The keys in the entry need to be re-sorted correctly

    Some sorting is fixed, others not.

  • A bunch of quoting changes

@chris-olszewski
Copy link
Contributor

Hi all, with #1789 we should be able to better support different types of lockfiles. Once that merges, I'll begin looking at updating this PR to use the new abstraction.

@theJoeBiz
Copy link
Author

@chris-olszewski That's great! I haven't had much time to dedicate to this, but I'm looking forward to checking out the new abstraction.

@quinnturner
Copy link

Similarly, I haven't been able to dedicate as much time to my WIP PR. However, I have implemented a few changes in my draft quinnturner#2 that are required for full support. My checklist above is in an accurate state.

Things that are solved:

  • yarn.lock's __metadata header are maintained from the source yarn.lock and formatted correctly (the metadata content is not static across all Berry versions)
  • Separating the lockfile definition between Classic/Berry (as they are very different)
  • Full lockfile definition implementation (peerDependencies, peerDependenciesMeta, etc.) with correct sorting of those keys
  • Post-prune deduping and quoting fixes (tricky, but seemingly solved; required a lot of manual indexing/substringing/parsing -> adding quotes)

Things there are not solved:

  • Sorting is difficult because the YAML parser that we use decides to sort characters like ^ differently compared to Berry's natural sorting algorithm
  • I hadn't figured out how to add workspace package references to the destination lockfile while also maintaining the correct formatting. i.e., keys in the destination lockfile like: `"@internal/shared-package@, @internal/shared-package@workspace:".

@toniopelo
Copy link

toniopelo commented Sep 5, 2022

This would be really great to have this! The prune command is the reason why I would adopt turbo and this is the only thing preventing me from switching.
I would gladly help but I don't dev in Go and I don't understand any of the implementation details whatsoever 🙈
There is a bunch of issues on the yarn repo that would greatly benefit from this (assuming they would be ok with using turbo of course):

yarnpkg/yarn#5428
yarnpkg/yarn#4521
yarnpkg/berry#1223

Just for context, my situation is:

  • I want to have internal workspace packages resolution like yarn workspace does to avoid the use of a private registry
  • I need to run every one of my monorepo packages inside k8s locally
  • I need the yarn install to run in the container build step (Dockerfile) to have the dependencies built based on the actual runtime it will run on. (That's why bundling is not possible here, I need to have this intermediate "regroup all my package and lockfile pruning" before beeing able to correctly build the docker image with appropriate layer caching, bundling can take place only once the node_module is formed)

I know all this is not perfectly relevant for this PR but if it can add some more motivation to see an additional use case and several issues linked to it, well, why not 😛.
If by any chance somebody knows of a standalone tool that would provide only this prune behavior, I'm all ears 👂
Thanks for your work guys!

@oceandrama
Copy link

FYI: yarn has yarn workspaces focus command to install dependencies of exact workspace

@weyert
Copy link
Contributor

weyert commented Sep 8, 2022

FYI: yarn has yarn workspaces focus command to install dependencies of exact workspace

Yeah pnpm has pnpm deploy but don't think they want to depend on such commands

@nathanhammond
Copy link
Contributor

@theJoeBiz we suspect that this has been superseded by #2019, with support released in 1.5.

Thank you for the early pass which helped us understand the shape of the problem!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area: ergonomics Issues and features impacting the developer experience of using turbo pr: on hold Pull requests that are "on hold" and should not be merged
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

10 participants