这是indexloc提供的服务，不要输入任何密码
Skip to content

fix: modern yarn lockfile parsing error #1597

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 3 commits into from
Aug 1, 2022
Merged

fix: modern yarn lockfile parsing error #1597

merged 3 commits into from
Aug 1, 2022

Conversation

erj826
Copy link
Contributor

@erj826 erj826 commented Jul 29, 2022 •
edited
Loading

Description

The header string written to the pruned lockfile by the turbo prune command starts each of the metadata fields at the beginning of a line. This causes the __metadata, version, and cache_key fields to be parsed as separate fields, when really the version and cache_key should be nested within __metadata. Turbo throws an error here when running a yarn workspaces focus my-app install afterwards due to the un-nested metadata:

ERROR  yarn.lock: could not unmarshal lockfile: yaml: unmarshal errors:
--
  |   | line 5: cannot unmarshal !!int `5` into fs.LockfileEntry
  |   | line 6: cannot unmarshal !!int `8` into fs.LockfileEntry

Solution

Because the yamlEncoder sets the indent to two, adding two spaces before each of the fields should make this valid syntax.

Before:

__metadata:
version: 5
cacheKey: 8

After:

__metadata:
  version: 5
  cacheKey: 8

Env

Yarn v3.2.2
Turborepo v1.3.4

@vercel
Copy link

vercel bot commented Jul 29, 2022

Someone is attempting to deploy a commit to the Vercel Team on Vercel.

A member of the Team first needs to authorize it.

@erj826 erj826 changed the title Fix Yarn 3 Lockfile Parsing Error Fix Modern Yarn Lockfile Parsing Error Jul 29, 2022
@erj826 erj826 marked this pull request as ready for review July 29, 2022 17:03
@tknickman
Copy link
Member

@erj826 thanks for this! Out of curiosity what version of yarn are you using?

@erj826
Copy link
Contributor Author

erj826 commented Jul 29, 2022

@erj826 thanks for this! Out of curiosity what version of yarn are you using?

@tknickman yarn 3.2.2

@erj826 erj826 changed the title Fix Modern Yarn Lockfile Parsing Error fix: modern yarn lockfile parsing error Jul 30, 2022
@tknickman
Copy link
Member

Thanks for the additional info, this looks good.

I was playing around with it locally, and with the same version yarn auto corrects the formatting in my pruned yarn.lock (from an install in /out post prune). I didn't try with focus however.

That being said the current behavior is definitely not correct so this fix looks great.

@vercel
Copy link

vercel bot commented Aug 1, 2022 •
edited
Loading

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Updated
turbo-site ✅ Ready (Inspect) Visit Preview Sep 1, 2022 at 3:14PM (UTC)

@tknickman tknickman added kind: bug Something isn't working pr: automerge Kodiak will merge these automatically after checks pass labels Aug 1, 2022
@tknickman
Copy link
Member

@erj826 FYI ya'll might run into some issues with prune on yarn v3.

There was some discussion (and a subset or your changes) here:
#643

@kodiakhq kodiakhq bot merged commit 59f7105 into vercel:main Aug 1, 2022
@quinnturner
Copy link

quinnturner commented Aug 2, 2022 •
edited
Loading

Somewhat humorous, despite this improving the format of the lockfile, this breaks turbo prune on our project for Yarn v3. Of course, prune is not expected to work, but it kind of did.

Now, I get Internal Error: Assertion failed: Expected the lockfile entry to have a resolution field (@ampproject/remapping@npm:^2.1.0)

@erj826
Copy link
Contributor Author

erj826 commented Aug 2, 2022

@quinnturner now it seems like yarn is able to parse the lockfile and catch that the prune-generated lockfile doesn't include the resolution field -- which your comment on #643 looks like it'll fix?

@quinnturner
Copy link

quinnturner commented Aug 2, 2022 •
edited
Loading

Exactly, yep! @erj826 I have a WIP that may or may not work out where I expand on that change by fully supporting Yarn Berry as a backend. My goal is to split up the Lockfile definition into separate definitions, one per backend. No promises though, as I am newer to go and have other commitments 😄

EDIT: here is my WIP quinnturner#2, for those who are interested. It breaks up the Yarn Berry and Yarn Classic parsing. There is a bunch of code duplication which needs to be cleaned up eventually. I have it set the correct version and cache key.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@tknickman tknickman tknickman approved these changes

@jaredpalmer jaredpalmer Awaiting requested review from jaredpalmer

@gsoltis gsoltis Awaiting requested review from gsoltis

@gaspar09 gaspar09 Awaiting requested review from gaspar09

@nathanhammond nathanhammond Awaiting requested review from nathanhammond

Assignees
No one assigned
Labels
kind: bug Something isn't working pr: automerge Kodiak will merge these automatically after checks pass
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@erj826 @tknickman @quinnturner