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 should be the default behavior #4147

Open
k0pernikus opened this issue Aug 11, 2017 · 36 comments
Open

yarn install --frozen-lockfile should be the default behavior #4147

k0pernikus opened this issue Aug 11, 2017 · 36 comments

Comments

@k0pernikus
Copy link

k0pernikus commented Aug 11, 2017

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

It's a feature-request due to a default behavior that leads to unexpected behavior during:

yarn install

What is the current behavior?

On yarn install the default behavior is that the yarn.lock file is mutated if package.json and yarn.lock are out of sync:

yarn init
yarn add moment
npm install --save lodash

The yarn.lock contains:

moment@^2.18.1:
  version "2.18.1"
  resolved "https://registry.yarnpkg.com/moment/-/moment-2.18.1.tgz#c36193dd3ce1c2eed2adb7c802dbbc77a81b1c0f"

Now if one runs

yarn install

the yarn.lock will be unexpectedly updated with an unknown future version of a dependency, only defined by the range of the provided semvar potentially breaking the build in the future, esp. if run on a build machine (jenkins):

$ yarn install v0.27.5
warning ../package.json: No license field
[1/4] Resolving packages...
[2/4] Fetching packages...
[3/4] Linking dependencies...
[4/4] Building fresh packages...
success Saved lockfile.
Done in 0.53s.

The yarn.lock now contains the following on the build machine, yet the version of lodash is not locked in and prone to change in the future:

lodash@^4.17.4:
  version "4.17.4"
  resolved "https://registry.yarnpkg.com/lodash/-/lodash-4.17.4.tgz#78203a4d1c328ae1d86dca6460e369b57f4055ae"

moment@^2.18.1:
  version "2.18.1"
  resolved "https://registry.yarnpkg.com/moment/-/moment-2.18.1.tgz#c36193dd3ce1c2eed2adb7c802dbbc77a81b1c0f"

What is the expected behavior?

The one that the frozen-lockfile provides, as it yields an error. This is quite helpful on build machine, i.e. Jenkins as those builds should fail. It is up to the developer on how to resolve the dependencies.

$ yarn install --frozen-lockfile
yarn install v0.27.5
warning ../package.json: No license field
[1/4] Resolving packages...
error Your lockfile needs to be updated, but yarn was run with `--frozen-lockfile`.
info Visit https://yarnpkg.com/en/docs/cli/install for documentation about this command.

As @BYK pointed out it would also be already helpful if yarn detects if it is running are in CI mode.

My in detail rationale for the change of the default behavior was posted in the already closed issue making a case for pure-lockfile.

Here's the gist of it:

Only commands like add, remove, and upgrade should mutate the yarn.lock.

yarn install should just do that, namely either install the dependencies in their locked in version or fail if it detects a mismatch between the package.json and yarn.lock. (The only exception being if there's no yarn.lock in the first place. Then, and only then, it may create one, yet it never, ever should touch it again.)


NOTE: The highest-rated comment's solution has its own problems: #4570

@BYK
Copy link
Member

BYK commented Aug 11, 2017

I tend to agree with this. First I thought this should be the default only on CI but now, I think may be we should expect the user to pass a flag like -u like in jest when running yarn install if we want it to update the lockfile.

Thoughts @yarnpkg/core ?

@BYK BYK added this to Backlog in Yarn 1.0 Aug 11, 2017
@rally25rs
Copy link
Contributor

I think there is some suspicion that a lot of people directly add dependencies to package.json without using add so a -u flag would still allow that workflow.

Though, I'm curious how common it is for someone to unintentionally npm i --save a new dependency and not want their lockfile updated. I'm not entirely sure what @k0pernikus 's workflow is here where a CI server would add a new dependency using NPM. Why would you not add the lodash dependency through yarn and commit the lockfile if you know you need lodash as a dependency?

That said, this does seem like a nice safeguard against a constantly changing lockfile, which is a problem I'm having in NPM5 (my NPM generated lockfile constantly changes depending on which dev on the team last ran it 😢 ). I'm fine either way on this one.

@k0pernikus
Copy link
Author

@rally25rs The new dependency was added by a fellow developer via their local npm by mistake. The project uses yarn and everything should go through it. Since not all our project are using yarn this is not an unlikely thing to happen, and also muscle memory can be hard to overcome.

They should have used yarn add, yet only committed a changed package.json to the repository.

The whole point is that I want to enforce that yarn.lock is always up-to-date. I want that very safeguard against a developer forgetting to use the yarn workflow and that they always have to check in an up-to-date yarn.lock.

@arcanis
Copy link
Member

arcanis commented Aug 11, 2017

You can add --frozen-lockfile true in a .yarnrc file located inside your repository, and it will have the behaviour you're looking for.

@k0pernikus
Copy link
Author

k0pernikus commented Aug 11, 2017

@arcanis There is a problem of not being able to update your yarn.lock file at all when using the setting in .yarnrc, see: #4570

@arcanis
Copy link
Member

arcanis commented Aug 11, 2017

We would have to set it up for each repository of everyone running Yarn ... 😉

@BYK
Copy link
Member

BYK commented Aug 11, 2017

We would have to set it up for each repository of everyone running Yarn ... 😉

This alone tells me that it needs to be the default.

@arcanis
Copy link
Member

arcanis commented Aug 11, 2017

So we discussed it a bit with @BYK and thought of the following option which we both agree looks sensible and should cover most cases:

  • First, we would add a warning to Yarn when the lockfile is changed by a yarn install invocation. This warning would not prevent the installation by default.
  • Then, when we roll the --strict flag (where warnings become errors), it will abort the install.
  • Still to be discussed, but --strict could then become the default when running under CI envs

This way, Yarn would stay easy to use by developers, while still providing you a way to ensure your lockfiles are always kept up to date and you never have this kind of unpredictable behavior.

How does that sound?

@k0pernikus
Copy link
Author

k0pernikus commented Aug 11, 2017

@arcanis I am a bit confused though as to why making yarn install fail on install if the package.json and yarn.lock is out of sync only is at odds with "staying easy to use by developers".

The added warning is nice, though I highly suspect it becoming nothing but logging noise on a lot of build machines nobody will notice unless the build fails due to an unexpected version bump.

I can understand that there is some restraint in wanting to change a default behavior of a command, and if it was to change, I assume the builds of quite a few people are about to break. Yet I'd consider that a good thing.

As on the yarn's main page, I find claims about yarn being ultra fast, mega secure, and super reliable:

Super Reliable.
Using a detailed, but concise, lockfile format, and a deterministic algorithm for installs, Yarn is able to guarantee that an install that worked on one system will work exactly the same way on any other system.

The current default is at odds with that claim.

Nowhere is it stated that yarn is easy to use ;)

In the end, having a strict option would be fine with me, even though I detest having to enable that setting in the .yarnrc for each of our projects using yarn.

Or what do you mean by when you "roll the --strict flag" exactly?

@CrabDude
Copy link
Contributor

My 2 cents.

In other threads, package.json is acknowledged as the canonical source from which yarn.lock is derived, and so allowing for the update of package.json directly to update a package is a very natural aspect of the workflow and disallowing it feels somewhat counter.

Our CI does use --frozen-lockfile so, I like turning --strict on in CI, but I dislike the removal of the ability to edit package.json directly b/c I think it is a natural workflow for most of the developers I support, and it's less deviating from the default workflow (npm).

In practice, setting --frozen-lockfile by default would not preclude a developer from committing an out-of-sync package.json, though it would prevent the CI gotcha. In non-CI environments, 99% of the time, the out-of-sync package.json will be obvious due to the altered yarn.lock in VCS, which is just as effective as an error IMO while being more permissive with the (new) warning message. It is worth mentioning though that we poison-pill the use of npm in our project, which in CI --frozen-lockfile helps enforce.

@bestander
Copy link
Member

In other threads, package.json is acknowledged as the canonical source from which yarn.lock is derived, and so allowing for the update of package.json directly to update a package is a very natural aspect of the workflow and disallowing it feels somewhat counter.

Yep, this is an important use case.
I would not want to lose it.

@CrabDude
Copy link
Contributor

I should also add, that humorously / ironically, someone just came to me about yarn.lock getting altered, even though they didn't touch a dependency. I think this was caused by the following:

2 unrelated commits altering yarn.lock merged via git cherry-pick > yarn then optimizes yarn.lock (when the developer runs yarn locally).

In this case, I personally believe it's less alarming to generate the altered yarn.lock than to fail, and I'm not sure how we could handle the git merge alternatively without integrating yarn itself into our actual merge pipeline.

@rally25rs
Copy link
Contributor

@k0pernikus not sure if this will help your situation, but you could also try to prevent NPM use by erroring out of a preinstall script, something like:

  "scripts": {
    "preinstall": "node -e \"if(process.env.npm_execpath.indexOf('yarn') === -1) throw new Error('You must use Yarn to install, not NPM')\""
  },

@arcanis
Copy link
Member

arcanis commented Aug 11, 2017

Our CI does use --frozen-lockfile so, I like turning --strict on in CI, but I dislike the removal of the ability to edit package.json directly b/c I think it is a natural workflow for most of the developers I support, and it's less deviating from the default workflow (npm).

Yep, totally agree! :) That's why the option we've came upon is to keep the default as it is (running yarn install and expecting it to Just Work is a behavior that we believe is firmly implanted into JS developers habits), while still decreasing the risk of confusion by adding a warning that can be turned into fatal.

@k0pernikus
Copy link
Author

k0pernikus commented Aug 11, 2017

@CrabDude

In other threads, package.json is acknowledged as the canonical source from which yarn.lock is derived, and so allowing for the update of package.json directly to update a package is a very natural aspect of the workflow and disallowing it feels somewhat counter. ...
I dislike the removal of the ability to edit package.json directly b/c I think it is a natural workflow for most of the developers I support, and it's less deviating from the default workflow (npm).

If one would change the default behavior of yarn, it doesn't necessarily mean that the current default cannot become its own flag, e.g. yarn install --generate-lockfile or yarn install --freeze-missing; whatever makes sense. (I'd dislike --update as that might be confused with yarn upgrade.)

Or have the generation of the lockfile as its own command:

yarn lock
yarn freeze
yarn tie-up // *scnr*

@kaylie-alexa
Copy link
Member

I'm also +1 for keeping current default behavior as is. If I understand your issue correctly @k0pernikus , having --frozen-lockfile as a default installation method still won't stop a developer from committing changes only in package.json, since they're totally skipping the yarn workflow. The CI server will catch it, however if we enable the --strict by default in CI environments as @BYK suggested, that commit would not be merged. I think there are other ways to catch the issue, by using the strict flag/.yarnc file, or adding a pre-commit hook.

@BYK
Copy link
Member

BYK commented Aug 14, 2017

Our CI does use --frozen-lockfile so, I like turning --strict on in CI, but I dislike the removal of the ability to edit package.json directly b/c I think it is a natural workflow for most of the developers I support, and it's less deviating from the default workflow (npm).

Let me clarify my view on this a bit. I wasn't suggesting removing the option of directly editing a package.json file but merely requiring passing an additional flag like -u (idea stolen from Jest's snapshot updating) or -f to indicate consent before making a potentially destructive change. Now I know almost everyone uses version control so any "destructive" change can be recovered from. That said, it doesn't change the fact that yarn is potentially doing something that alters your configuration where you don't expect it to. This would also help catching inconsistent yarn versions across peers, which would be unsafe since the hoisting or resolution algorithm may change between versions.

All that said, I think we have some consensus around keeping the existing behavior and moving forward with just a warning and expecting people to set their CI up responsibly via --strict or --frozen-lockfile. Is that accurate?

@ProdigySim
Copy link

We moved to npm-shrinkwrap, yarn, etc. to lock down dependency versions. yarn.lock is not much of a lock file if it can be mutated by every yarn command.

My team was surprised by the auto-merge-conflict-resolution feature. An accidental un-merged yarn.lock file was checked in, and instead of failing the build our CI happily used some algorithm to decide how to merge the file.

I can't fathom a safe automated merge algorithm for yarn.lock, and I couldn't find documentation. How does it know if I want to upgrade or downgrade the conflicting package?

Being able to disable this with .yarnrc is helpful, but it looks like there's no way to override --frozen-lockfile true if I set that in .yarnrc. So to migrate to that solution I would have to ask my users to edit their .yarnrc to make certain changes.

@ispringer
Copy link

@ProdigySim, couldn't overriding the frozen option be achieved by deleting yarn.lock before running yarn install?

@raijinsetsu
Copy link

I'm not sure where this PR is going, so I'll just add my two cents.

At first, I was going to argue that "--frozen-lockfile" should be the default for the "install" command because specifying the option in .yarnrc would apply to all commands, not just install. However, I found the doc for .yarnrc and you can specify options per command.

So, my group will be adding "--install.frozen-lockfile true" to ".yarnrc" resolves the issue for us. The concern that this has to be done for each repository is minimal as there is already a slew of work that needs to be done to setup a project.

@ProdigySim
Copy link

I totally missed that somehow. Restricting --frozen-lockfile to just the install command should still allow yarn add and yarn remove to work as expected.

--install.frozen-lockfile true

in .yarnrc should handle things well for my cases.

@k0pernikus
Copy link
Author

k0pernikus commented Sep 21, 2018

Having a look at the big sibling npm, it is interesting to to see how they handle this issue now.

Since May 2018 they they have added the npm ci command which is intended for use on CI deployments and throws an error if there is no package-lock.json.

Reverting my example in my very first post:

npm init
npm install moment
yarn add lodash

Running npm ci will produce an error:

npm ci
npm ERR! cipm can only install packages when your package.json and package-lock.json or npm-shrinkwrap.json are in sync. Please update your lock file with `npm install` before continuing.

npm ci will only be read-only. It never mutates the lock file.

Running npm install will sync the package-lock.json file.

Maybe this is also a way for yarn to move forward by providing a dedicated:

 yarn ci

@arcanis
Copy link
Member

arcanis commented Sep 21, 2018

We have started moving towards making Yarn a developer tool rather than a production tool. The end goal (early next year) is that running Yarn on CI will not be needed anymore. So no yarn ci, but no yarn install either 🙂

Check the Plug'n'Play rfc, which is tightly connected to this goal: yarnpkg/rfcs#101

@maciej-gurban
Copy link

maciej-gurban commented Mar 26, 2019

@arcanis The linked goal has been closed, but it doesn't seem that the issue discussed in this thread has been resolved. Could you share some updates?

@k0pernikus
Copy link
Author

@arcanis @BYK Can you please give an update of the status of this issue?

The only thing that I noticed recently is that in the meantime is that this weird behavior has been better documented via #5847 at the yarn cli docs:

If you need reproducible dependencies, which is usually the case with the continuous integration
systems, you should pass --frozen-lockfile flag.

and:

If you want to ensure yarn.lock is not updated, use --frozen-lockfile.

While this is helpful and awesome, it still feels like a workaround and not a solution. The default is neither "super reliable" nor "deterministic".

We have safeguarded most our projects against many of the issues, though we still sometimes run into troubles esp. in conjunction with #4570 making it impossible to use the proposed config of adding --frozen-lockfile true to the .yarnrc. (A new developer may try to add a .yarnrc file to make the to make passing the parameter redundant. After that commit, the same or another developer adds a new dependency but the the yarn.lock file is never updated. A few hours are wasted until somebody aware of this issue realizes the cause and explains them the idiosyncratic and counter-intuitive behavior.)

@arcanis
Copy link
Member

arcanis commented Nov 22, 2019

There is no plan to change this in a non-major release. The next one is currently in development, but even though we'll likely make changes to how frozen lockfiles work I'm not sure the CI mode will be part of it (next release will already be rather large as it is, so I'd prefer to slate it for 3.x).

@afdev82
Copy link

afdev82 commented Feb 5, 2020

If I could add my thoughts to this discussion...
I think the problem here is that we want to use the same command to do two different things that are not compatible:

  1. keep the lock file in sync with the package.json
  2. install the dependencies reading from the lock file

In my opinion, we should use two different commands for the two cases:

  1. run yarn sync
  2. run yarn install
    If there is no yarn lock file, both the commands could create one.

Every time yarn install runs, it should read from the lock file and change the node_modules accordingly.
If we want to sync the lock file and never change it with the same command, it's not possible.

subdavis added a commit to dandi/dandiarchive-legacy that referenced this issue Mar 9, 2020
Noticed a discussion on this in another thread.

It's best not to mix yarn and npm.  You should also use `--frozen-lockfile` or `--pure-lockfile` when installing into a docker image.  yarnpkg/yarn#4147
@AxelTheGerman
Copy link

Agreeing with @afdev82 here, I can see 2 different use cases.

  1. Add a new dependency (obviously need to update the lockfile)
  2. Install all dependencies from the lockfile

I encourage everyone to look around how other package managers work. I'm a ruby guy so here is how Bundler does it.

  1. Add a new dependency
    a. Edit your Gemfile
    b. Run bundle install - adds new dependency to Gemfile.lock
  2. Install all dependencies
    a. Run bundle install - installs exactly the specified versions from Gemfile.lock
  3. Update all dependencies
    a. Run bundle update - updates according to semver range if any in Gemfile
  4. Update specific dependencies
    a. Run bundle update dep_a dep_b - my only grief is that this one also updates downstream dependencies and might update more than you want. They have a --source flag to try only updating the dependency itself

Not saying bundler is the best BUT why even have a lockfile if installing dependencies modifies the lockfile?

@lobo-tuerto
Copy link

I think installing dependencies when there is a lockfile in place, should only install what the lockfile says it should. Nothing else.

For additional behaviours we can always pass a flag (like -u or something).

@mattvb91
Copy link

Definitely feel like this should be default... Anything else defeats the purpose of the lock file in the first place

@Brodan
Copy link

Brodan commented Feb 11, 2021

Just ran into this and wasted a bunch of hours figuring out that this was the issue. Would love to see this behavior implemented as the default.

@MrBrN197
Copy link

any updates on this? just started using yarn and was surprised no one was finding this an issue.
I was trying to view a project source code that used yarn and so installed it and run yarn install.
"Noooo don't modify the lock file... I want to see the code at it's current state."

Why introduce a lock file then. what exactly are you locking? The thing that you are going to then update.

@arcanis
Copy link
Member

arcanis commented Feb 12, 2021

Locking, as we have no intent to change the default in 1.x since it'd be a breaking change. There's however a good chance something similar will be implemented for CI exclusively in the next major release (which is developed on a separate repository).

Why introduce a lock file then. what exactly are you locking? The thing that you are going to then update.

The lockfile is only modified if your package.json references packages that aren't part of your lockfile. In other, it's only modified if you messed up the state, for example by committing a new dependency without running an install.

@yarnpkg yarnpkg locked as resolved and limited conversation to collaborators Feb 12, 2021
@BYK
Copy link
Member

BYK commented Feb 21, 2021

@arcanis would it make sense to close this issue and link people to the relevant issue in yarnpkg/berry?

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests