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

yarn.lock changes, even though it was just generated #7770

Closed
AviVahl opened this issue Dec 16, 2019 · 19 comments
Closed

yarn.lock changes, even though it was just generated #7770

AviVahl opened this issue Dec 16, 2019 · 19 comments
Assignees
Labels
fixed-in-modern This issue has been fixed / implemented in Yarn 2+.

Comments

@AviVahl
Copy link

AviVahl commented Dec 16, 2019

@arcanis this started happening fairly recently. I'm not sure if this is a regression due to 1.21.1, or the new fsevents release (or something else entirely).

Do you want to request a feature or report a bug?
Bug. Probably affecting many out there.

What is the current behavior?
yarn changes yarn.lock after a secondary run.

If the current behavior is a bug, please provide the steps to reproduce.

https://github.com/wixplosives/sample-monorepo

git clone git@github.com:wixplosives/sample-monorepo.git
cd sample-monorepo
rm yarn.lock
yarn # generates a fresh new yarn.lock file
rm -rf node_modules # important, otherwise bug doesn't happen
git add yarn.lock # to be able to see whether lock file changed
yarn
git status
# bug is here. "modified:   yarn.lock"

What is the expected behavior?
A fresh install of yarn should generate a "final" lock file. Running yarn on the generated yarn.lock should not change it unless a version request was changed (someone modified package.json since lock file generation).

Please mention your node.js, yarn and operating system version.
yarn 1.21.1
node 12.13.1
fedora 31

Observations
fsevents released a new node12 compatible version (v1.2.11). yarn picks that version up. it's an optional dependency of chokidar. older versions of fsevents had node-pre-gyp as a dependency, and that seems to be the major change in the lock file. not sure if it's related, but perhaps you'll know more. Should be noted that fsevents does not end up installed (optional dep) on non-darwin machines, such as my linux machine.

@AviVahl
Copy link
Author

AviVahl commented Dec 16, 2019

Another hypothesis:
First (without yarn.lock) run picks up (and keeps) dependencies from optional dependencies that were ignored. Second run (without node_modules) removes them from lock file.

@kambing86
Copy link

kambing86 commented Dec 18, 2019

found different behaviour, but it seems like related

fsevents is not installed in node_modules, but node-pre-gyp is added into yarn.lock, and it will stay in yarn.lock no matter how many times I run yarn

the only way is to remove node-pre-gyp from yarn.lock and run yarn again to remove all unrelated dependencies

yarn install v1.21.0
[1/4] Resolving packages...
[2/4] Fetching packages...
info fsevents@2.1.2: The platform "linux" is incompatible with this module.
info "fsevents@2.1.2" is an optional dependency and failed compatibility check. Excluding it from installation.
info fsevents@1.2.11: The platform "linux" is incompatible with this module.
info "fsevents@1.2.11" is an optional dependency and failed compatibility check. Excluding it from installation.
[3/4] Linking dependencies...
[4/4] Building fresh packages...
success Saved lockfile.
Done in 36.26s.
$ yarn why node-pre-gyp
yarn why v1.21.0
[1/4] Why do we have the module "node-pre-gyp"...?
[2/4] Initialising dependency graph...
[3/4] Finding dependency...
error We couldn't find a match!
Done in 0.72s.

@AviVahl
Copy link
Author

AviVahl commented Dec 18, 2019

That's the same issue.
I have a bit different observation to: "it will stay in yarn.lock no matter how many times I run yarn"

As noted in the steps to reproduce, if I manually remove node_modules and then execute yarn, it will remove node-pre-gyp.

@AviVahl
Copy link
Author

AviVahl commented Dec 18, 2019

@arcanis I know you guys are busy with yarn@2 (berry), but having no deterministic lock file is a really major issue.

This issue affects any project that uses webpack@4 (which brings watchpack@1, which brings chokidar@2, which brings fsevents@1). 😟

@arcanis
Copy link
Member

arcanis commented Dec 18, 2019

@arcanis I know you guys are busy with yarn@2 (berry), but having no deterministic lock file is a really major issue.

I understand - however I'm concerned investigating and fixing that will ask what will amount to a significant amount of time, and I'm pretty busy (I have another fix I need to do on the v1 trunk).

Given that we plan to land Yarn 2 in January, and given that it features a completely revamped lockfile architecture I trust much more than the current one, I'd tend to defer the fix until then unless another contributor can take ownership 🤔

@AviVahl
Copy link
Author

AviVahl commented Dec 18, 2019

appreciate the response. hopefully transitioning from yarn v1 to v2 would be smooth...

@arcanis
Copy link
Member

arcanis commented Dec 18, 2019

The main bump in the migration path at the moment is that PnP is required, which sometimes cause issues with packages that haven't been tested under it (we're typically quite fast at correcting those when we can work with their maintainers).

As an additional safeguard we're working at this very moment on an optional plugin that will generate a node_modules folder like the v1 used to - making the transition much easier if you use one of the packages described above, at the cost of being less performant and stable than the regular PnP installs (but still more than the v1 😄).

It's the main blocker for the stable release so you can expect it to be there on day one, but still - if you're interested to help us, trying out PnP and fixing potential issues in third-parties is immensely appreciated! 🙂

@rally25rs
Copy link
Contributor

I'll try to investigate this as I find some free time (assigning to myself).

@rally25rs rally25rs self-assigned this Dec 18, 2019
@rally25rs
Copy link
Contributor

At initial glance, I think this has something to do with the existence of the node_modules/.yarn-integrity file. Instead of the rm -rf node_modules step, you can just delete that file, then the lockfile will change on the next yarn-install.

You can also reproduce this by not deleting node_modules and instead just running a yarn install --force

@rally25rs
Copy link
Contributor

rally25rs commented Dec 19, 2019

Did a bit of research, and just dropping some notes:

You can reproduce this issue with the very simple non-workspace package.json

{
  "name": "sample",
  "devDependencies": {
    "fsevents": "1.2.11"
  }
}

Then

yarn install
git commit -am "initial"
yarn install --force

at this point yarn.lock will be different.

It looks like fsevents includes a bundledDependency

  "bundledDependencies": [
    "node-pre-gyp"
  ],

and on the initial clean install, node-pre-gyp is added to the lockfile.

On subsequent yarn installs, it is omitted, hence the lockfile difference.

Next step will be figuring out why...


Edit

Looks like my initial hunch was wrong about the bundledDependencies. I think the actual problem here is that the metadata from the npm registry: http://registry.npmjs.org/fsevents contains node-pre-gyp as a dependency:

image

However once downloaded, the actual fsevents package.json does not:

{
  "name": "fsevents",
  "version": "1.2.11",
  "description": "Native Access to Mac OS-X FSEvents",
  "main": "fsevents.js",
  "dependencies": {
    "bindings": "^1.5.0",
    "nan": "^2.12.1"
  },
  "os": [
    "darwin"
  ],
  "engines": {
    "node": ">=4.0"
  },
  "scripts": {
    "test": "node ./test/fsevents.js && node ./test/function.js 2> /dev/null"
  },
  "repository": {
    "type": "git",
    "url": "https://github.com/strongloop/fsevents.git"
  },
  "keywords": [
    "fsevents",
    "mac"
  ],
  "author": "Philipp Dunkel <pip@pipobscure.com>",
  "license": "MIT",
  "bugs": {
    "url": "https://github.com/strongloop/fsevents/issues"
  },
  "bundledDependencies": [
    "node-pre-gyp"
  ],
  "homepage": "https://github.com/strongloop/fsevents"
}

I suspect yarn is using the deps from the registry the first time, then the deps from the downloaded cached package.json the second.

@AviVahl
Copy link
Author

AviVahl commented Dec 19, 2019

Great catch. Is this something I should report to npmjs support itself (as a registry bug)?

@AviVahl
Copy link
Author

AviVahl commented Dec 19, 2019

Thinking about this a bit more, it appears the npm registry exposes "bundledDependencies" as "dependencies", but the json also includes both the "bundledDependencies" and "os" fields themselves.

Shouldn't yarn ignore these dependencies if "os" doesn't match and it got there from an "optionalDependencies" request? Or maybe I'm looking at it all reversed? Maybe for a truly cross-os deterministic lock file, yarn should resolve and lock "optionalDependencies" chain even if not actually installed in current environment. (it would just skip running their install/postinstall scripts)

@rally25rs
Copy link
Contributor

@AviVahl

Maybe for a truly cross-os deterministic lock file, yarn should resolve and lock "optionalDependencies" chain even if not actually installed in current environment.

This is the case. Since it could be there on other people's installs, if we don't still resolve and lock the "optional" ones, then you will get lockfile changes depending on whether the last person to run yarn install was on linux or windows (npm had this problem; their lockfile would change based on OS. Not sure if they fixed that at some point)

should report to npmjs support itself (as a registry bug)?

Maybe? It'd probably be worth getting a comment from the npm team on the difference.


To dig a little deeper into why this matters to yarn;

When resolving the dependency tree, we check to see if a lockfile already exists, and if so, then checks to see if the exact locked version is already downloaded and extracted in the yarn cache.

If it is already cached, we use the package.json to find the next level of sub dependencies, since we already have that information downloaded, and don't have to make an HTTP request to the registry.

If the version desired is not in the cache or if the lockfile doesn't exist (or is believed to be invalid or out of date) then we get the package metadata from the registry and use that to get the sub dependencies.

This unfortunately means there are 2 possible sources of truth for the sub dependencies of a package. Picking either one and using only it would cause some issues for yarn;

If you only rely on the registry metadata and ignore package.json, then you always need to check the registry; we don't currently cache that metadata, so we don't have it stored locally, so this would break "offline" installs which was one of yarn's original core features.

If you rely only on package.json, then you always have to download a package, extract the .tgz, and load and parse package.json to determine the next level of dependencies. This would probably negatively affect performance (although we usually end up downloading them eventually anyway, unless they get eliminated due to being "optional" and not matching the node version or OS)

One possible solution would be to start caching the metadata responses from the registry locally too, along with the package .tgz file. But any way you slice it; which source of truth is "correct?" and why are they reportedly different? It would also be a fairly big change to yarn to alter the way that this works, and with yarn2 on the horizon, I'm not sure if a change of this magnitude would be worth it.


I'd be interested to see if it is "normal" for the npm registry to report dependencies differently than the library's package.json, or if this is a rare oddity. If you do open an npm issue, please link it here.

@AviVahl
Copy link
Author

AviVahl commented Dec 19, 2019

The forum in https://npm.community is in read-only mode, so I believe they can only be contacted by support e-mails (via https://www.npmjs.com/support).

The thing is, even with yarn getting the responses it gets from the registry, it should still be able to see fsevents being flagged as an optional dependency (https://registry.npmjs.org/chokidar -> "versions" -> "2.1.8" -> "optionalDependencies"), so it could in theory resolve and lock, but not actually install these deps (since "os" isn't matching).

npm seemingly fixed it in v5.6: https://blog.npmjs.org/post/167963735925/v560-2017-11-27

@AviVahl
Copy link
Author

AviVahl commented Dec 19, 2019

related to #5998?

@rally25rs
Copy link
Contributor

I opened a question against npm/cli (see linked issue above), but judging from the npm client verbose log output:

...
npm sill install loadAllDepsIntoIdealTree
npm http fetch GET 200 https://registry.npmjs.org/fsevents 10ms (from cache)
npm sill pacote version manifest for fsevents@1.2.11 fetched in 15ms
npm http fetch GET 200 https://registry.npmjs.org/nan 7ms (from cache)
npm http fetch GET 200 https://registry.npmjs.org/node-pre-gyp 9ms (from cache)
npm http fetch GET 200 https://registry.npmjs.org/bindings 11ms (from cache)
npm sill pacote range manifest for nan@^2.12.1 fetched in 14ms
npm sill pacote range manifest for node-pre-gyp@* fetched in 14ms
npm sill pacote range manifest for bindings@^1.5.0 fetched in 16ms
...

I'm guessing they actually cache the registry metadata responses too (which yarn does not, we use the package.json inside the cached library instead) and use that when rebuilding the dependency tree, which would explain why npm doesn't exhibit this same lockfile change issue.


I'm thinking the "correct" solution here would be to mimic that behavior and start caching the registry metadata responses, instead of relying on the library's pacakge.json to match, but I really don't like that the npm registry seems to override what a package publisher puts in their package.json as the source of truth for dependencies. I think that could cause other issues, like if you were to clone fsevents and add it as a file:../fsevents or link:../fsevents, or to npm link / yarn link a locally cloned fsevents repo, or install direct from the package .tgz file, it would have different dependencies...

I guess let's see if there is any comment on the npm issue that I opened...

@renchap
Copy link

renchap commented Mar 4, 2020

Any news on this?

@razh
Copy link

razh commented Mar 6, 2020

For now, we've added chokidar v3 as a yarn resolution in our package.json file:

{
  "resolutions": {
    "chokidar": "^3.3.1"
  }
}

We noticed that all of our dependencies that depend on fsevents v1 are doing so because they depend on chokidar v2. chokidar v3 is mostly backward compatible with v2 and we don't need to support Node.js < 8.

CaptainAchab pushed a commit to TankerHQ/sdk-js that referenced this issue Mar 20, 2020
Reasons explained here: yarnpkg/yarn#7770 (comment)

Tl;dr:
* fresh run of `yarn` (without yarn.lock) resolves dependencies from npm registry
* following runs of `yarn` resolve dependencies from local package.json files
* the fsevents packages declares node-pre-gyp dep as optional but the fact that
  the dependency is optional is not reflected on npm's registry...
@merceyz
Copy link
Member

merceyz commented Jan 2, 2021

Most likely fixed in v2

https://yarnpkg.com/getting-started/migration

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
fixed-in-modern This issue has been fixed / implemented in Yarn 2+.
Projects
None yet
Development

No branches or pull requests

8 participants