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 install --frozen-lockfile is not failing as expected #5840

Closed
DavideDaniel opened this issue May 18, 2018 · 25 comments · Fixed by VincentBailly/yarn#38 · May be fixed by #6554
Closed

yarn install --frozen-lockfile is not failing as expected #5840

DavideDaniel opened this issue May 18, 2018 · 25 comments · Fixed by VincentBailly/yarn#38 · May be fixed by #6554
Assignees
Labels
fixed-in-modern This issue has been fixed / implemented in Yarn 2+. triaged

Comments

@DavideDaniel
Copy link

DavideDaniel commented May 18, 2018

Do you want to request a feature or report a bug?

Bug

What is the current behavior?
yarn install --frozen-lockfile does not generate a new yarn.lock but it does NOT fail as expected.

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

  1. Have a yarn.lock with a package.json with some dependencies and no diffs.
  2. Remove a dependency in your package.json (npm uninstall --save something)
  3. yarn install --frozen-lockfile proceeds with install instead of failing as docs say, and does not generate a new yarn.lock to reflect the change in package.json.

What is the expected behavior?
Per docs: Don’t generate a yarn.lock lockfile and fail if an update is needed.
It should fail as expected as CI builds need to break earlier rather than wait for build and then fail on a missing or mismatched dependency.

Please mention your node.js, yarn and operating system version.
Node 8.9.1 | 9.6.1
npm 3.10.10 | 5.6.0
MacOS 10.12.6

@ghost ghost assigned torifat May 18, 2018
@ghost ghost added the triaged label May 18, 2018
@DavideDaniel
Copy link
Author

Closing this ticket as it was due to modifications in config files related to repos and not yarn

@DavideDaniel
Copy link
Author

Actually I need someone else to validate this as the issue is persisting despite us trying to fix things on our end.

@DavideDaniel DavideDaniel changed the title yarn install --frozen-lockfile generates a new yarn.lock instead of failing yarn install --frozen-lockfile is not failing as expected May 18, 2018
@DavideDaniel
Copy link
Author

DavideDaniel commented May 18, 2018

Turns out there were some things wrong on our end but there is still an issue with yarn install --frozen-lockfile - I've edited the title and the original post to reflect that.

yarn install --frozen-lockfile does not generate a new yarn.lock but it does NOT fail as expected.

I realize in the reproduce steps I said to use npm uninstall rather than yarn remove. It's on purpose to reflect realities of working on a shared codebase where someone might either manually manipulate the package.json or use npm client out of habit.

@DavideDaniel
Copy link
Author

I haven't had a chance to validate but a starting point might be #5572. In trying to address #4778, code was deleted that would cause frozen-lockfile check to fail.

@devrelm
Copy link

devrelm commented Jun 22, 2018

@DavideDaniel I'm seeing the same issue with --frozen-lockfile not throwing an error.

@torifat Have you had an opportunity to look into this?

@devrelm
Copy link

devrelm commented Jul 6, 2018

@arcanis Is there any movement on this? There hasn't been any response from the assignee or anyone else within the Yarn org.

@arcanis
Copy link
Member

arcanis commented Jul 7, 2018

yarn install --frozen-lockfile proceeds with install instead of failing as docs say, and does not generate a new yarn.lock to reflect the change in package.json.

I don't understand. What makes you think it should fail? If you remove dependencies from your package.json, your lockfiles might now contain extraneous useless entries that could be optimized away, but it'll still install things in a repeatable way without having to be modified (so no error needed).

Does that make sense?

@devrelm
Copy link

devrelm commented Jul 7, 2018

@arcanis From the docs:

yarn install --frozen-lockfile

Don’t generate a yarn.lock lockfile and fail if an update is needed.

It should fail because the docs says it will. It is what separates it from --pure-lockfile.

We need this functionality to catch cases on our ci where a developer updates a dependency without updating the lockfile. Otherwise every other developer that pulls down the commit ends up with a modified yarn.lock when they run yarn, forcing one of them to open a one-off PR just to update the lock.file.

@devrelm
Copy link

devrelm commented Jul 7, 2018

@arcanis It looks like --frozen-lockfile might only be broken for workspaces. I've set up a minimal repo here that reproduces the issue:

https://github.com/devrelm/frozen-lockfile

@arcanis
Copy link
Member

arcanis commented Jul 7, 2018

It should fail because the docs says it will. It is what separates it from --pure-lockfile.

No, pure-lockfile simply doesn't write the changes.

  • default = Update the lockfile if we want/need
  • pure = Update the lockfile if we want/need, but doesn't write it
  • frozen = Throws if the lockfile need to be modified

I can see why you'd think frozen-lockfile should throw on this occasion tho. If you're willing to open a PR, I can review it.

@Gudahtt
Copy link
Member

Gudahtt commented Jul 16, 2018

@arcanis So --frozen-lockfile will only throw if the updates are required, and will not complain if there are potential optimizations to be made to the lock file (e.g. deleting a package). Is that correct? That might explain the OP's problem. Also this might be something to keep in mind for #5847

Nonetheless, the problem demonstrated by the repo @devrelm created doesn't meet that description. In that repo, a workspace dependency is updated to require a different version than is present in the lockfile. That means that an update to the lockfile is strictly required, yet --frozen-lockfile still doesn't throw.

I've experienced the same problem, and I also suspect that it's only broken for workspaces. Even adding an entirely new dependency in a workspace doesn't seem to make --frozen-lockfile fail.

Perhaps we should create a separate ticket, as it does appear that it might be a different problem than described in the OP.

@Gudahtt
Copy link
Member

Gudahtt commented Jul 16, 2018

I've created a failing test here: https://github.com/Gudahtt/yarn/tree/workspace-frozen-lockfile

@edmorley
Copy link
Contributor

edmorley commented Aug 21, 2018

I just ran into this (package removed from package.json and the PR author had forgotten to update yarn.lock, but yet CI succeeded) - and it was very surprising to see that it's expected for --frozen-lockfile to succeed in this case.

I totally understand that some optimizations (eg general yarn.lock overlapping versions consolidation) shouldn't result in a --frozen-lockfile failure, but forgetting to update the lockfile after a package was removed seems much more significant than that.

@Gudahtt
Copy link
Member

Gudahtt commented Aug 21, 2018

I have created #6291 for the workspace-specific problem discussed here.

@DavideDaniel can you reproduce this problem in a project that isn't using workspaces? If not, perhaps this ticket can be closed.

@edmorley
Copy link
Contributor

Mine occurred when not using a workspace.

@Gudahtt
Copy link
Member

Gudahtt commented Aug 22, 2018

@edmorley Ah, right. I think there are two distinct issues here, and yours is the other one. Sorry for the confusion.

The first issue is the situation described by you , where --frozen-lockfile succeeds even if there are optimizations to be made (i.e. packages to remove). This behavior is intentional, though there are good arguments to be made that this behavior should be changed. The documentation should definitely be improved here (#5847), but I don't think it's a bug.

The second issue is when updates are absolutely required, but --frozen-lockfile still succeeds. This is definitely a bug, and we have only seen it occur with workspaces. It was brought up here because the situation was similar, but it's a separate issue.

It looks like the OP was in the same situation as you, where the command was behaving as designed but in confusing manner not well explained by the documentation. But I'm not completely certain of that. I wanted to know from the OP whether they only encountered this problem when removing packages (just as you), or whether they encountered it when adding packages as well (but just chose to use removal as an example in the issue description).

@kachkaev
Copy link

kachkaev commented Feb 26, 2019

Our current workaround in a Linux-based CI environment:

yarn install

git diff yarn.lock
if ! git diff --exit-code yarn.lock; then
  echo "Changes were detected in yarn.lock file after running 'yarn install', which is not expected. Please run 'yarn install' locally and commit the changes.";
  exit 1;
fi

The problem remained unnoticed for months 😅

@jbinto
Copy link
Contributor

jbinto commented Mar 1, 2019

We have a similar workaround to @kachkaev

#!/bin/bash

# This file exists because as of [email protected], --frozen-lockfile is completely
# broken when combined with Yarn workspaces. See https://github.com/yarnpkg/yarn/issues/6291

CKSUM_BEFORE=$(cksum yarn.lock)
yarn install
CKSUM_AFTER=$(cksum yarn.lock)


if [[ $CKSUM_BEFORE != $CKSUM_AFTER ]]; then
  echo "yarn_frozen_lockfile.sh: yarn.lock was modified unexpectedly - terminating"
  exit 1
fi

the-spyke added a commit to the-spyke/undercut that referenced this issue Jul 21, 2020
wincent added a commit to liferay/liferay-frontend-projects that referenced this issue Nov 16, 2020
This should ensure that we don't run into the kind of issue mentioned
here:

    #233 (comment)

Note that `--frozen-lockfile` is supposed to take care of this for us,
but it is a known issue that it doesn't work properly in monorepos:

    yarnpkg/yarn#5840

There is a fix for it:

    yarnpkg/yarn#6554

But the issue is nearly two years old and the PR is almost as old. Maintainer replies here:

    yarnpkg/yarn#6554 (comment)

that:

> The change appears sound, but we're currently working on the v2
> (scheduled before the end of the year) and I'm somewhat worried about
> introducing subtle bugs right before releasing the next major (the
> way I see it, the current v1 is, despite its shortcomings, relatively
> stable)."

We face a similar concern in liferay-portal and have a ticket for that:

    https://issues.liferay.com/browse/LPS-110313
wincent added a commit to liferay/liferay-frontend-projects that referenced this issue Nov 16, 2020
This should ensure that we don't run into the kind of issue mentioned
here:

    #233 (comment)

Note that `--frozen-lockfile` is supposed to take care of this for us,
but it is a known issue that it doesn't work properly in monorepos:

    yarnpkg/yarn#5840

There is a fix for it:

    yarnpkg/yarn#6554

But the issue is nearly two years old and the PR is almost as old. Maintainer replies here:

    yarnpkg/yarn#6554 (comment)

that:

> The change appears sound, but we're currently working on the v2
> (scheduled before the end of the year) and I'm somewhat worried about
> introducing subtle bugs right before releasing the next major (the
> way I see it, the current v1 is, despite its shortcomings, relatively
> stable)."

We face a similar concern in liferay-portal and have a ticket for that:

    https://issues.liferay.com/browse/LPS-110313
@paul-soporan
Copy link
Member

Fixed in the v2.

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

@paul-soporan paul-soporan added the fixed-in-modern This issue has been fixed / implemented in Yarn 2+. label Jan 2, 2021
@Aryaman73
Copy link

@paul-soporan Just for clarity - was this ever fixed for Classic Yarn (1.x)? Facing this issue in 1.22.5 and I'd rather not have to migrate to v2. 🙃

@paul-soporan
Copy link
Member

1.x is frozen and will only receive security fixes, so the answer is no.

@AlonMiz
Copy link

AlonMiz commented Jun 28, 2021

that's bad, we can't migrate to v2. at least not in the near year. we'll probably go with pnpm.

if there's an open PR for something that is hanging for a long time, why not merge it?
#6554

@Aryaman73
Copy link

Aryaman73 commented Jun 28, 2021

@AlonMiz v1 is frozen, and they've moved most of their developer resources to v2 instead. Merging this PR would mean merging any open PR that is approved or close to it, which ruins the entire point of freezing this version. It's definitely annoying, though 😕

I might be wrong but I do think there are forks of Yarn v1 that have merged this fix-in, you can see that above by looking at repos where this issue was mentioned. This isn't a great solution since using unofficial spin-offs of Yarn can carry massive security issues, but still.

mrtnzlml added a commit to adeira/universe that referenced this issue Jul 6, 2021
Seems like `yarn install --frozen-lockfile` doesn't always fail when `yarn.lock` changes (we put there the option believing it does). See: yarnpkg/yarn#5840

Instead, we should simply try to install the dependencies as usual and fail if something in the repo changes unexpectedly.

Inspiration: facebook/docusaurus@4d93c89
mrtnzlml added a commit to adeira/universe that referenced this issue Jul 6, 2021
Seems like `yarn install --frozen-lockfile` doesn't always fail when `yarn.lock` changes (we put there the option believing it does). See: yarnpkg/yarn#5840

Instead, we should simply try to install the dependencies as usual and fail if something in the repo changes unexpectedly.

Inspiration: facebook/docusaurus@4d93c89
kodiakhq bot pushed a commit to adeira/universe that referenced this issue Jul 6, 2021
Seems like `yarn install --frozen-lockfile` doesn't always fail when `yarn.lock` changes (we put there the option believing it does). See: yarnpkg/yarn#5840

Instead, we should simply try to install the dependencies as usual and fail if something in the repo changes unexpectedly.

Inspiration: facebook/docusaurus@4d93c89
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+. triaged
Projects
None yet