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 · 26 comments

Comments

Projects
None yet
@k0pernikus
Copy link

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.)

@BYK

This comment has been minimized.

Copy link
Member

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 the cat-discussion label Aug 11, 2017

@BYK BYK added this to Backlog in Yarn 1.0 Aug 11, 2017

@rally25rs

This comment has been minimized.

Copy link
Contributor

commented Aug 11, 2017

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

This comment has been minimized.

Copy link
Author

commented Aug 11, 2017

@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

This comment has been minimized.

Copy link
Member

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

This comment has been minimized.

Copy link
Author

commented Aug 11, 2017

@arcanis Thank you, yet I would have to set it up for each repository. So I still argue that the default behavior is off.

@arcanis

This comment has been minimized.

Copy link
Member

commented Aug 11, 2017

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

@BYK

This comment has been minimized.

Copy link
Member

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

This comment has been minimized.

Copy link
Member

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

This comment has been minimized.

Copy link
Author

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

This comment has been minimized.

Copy link
Contributor

commented Aug 11, 2017

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

This comment has been minimized.

Copy link
Member

commented Aug 11, 2017

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

This comment has been minimized.

Copy link
Contributor

commented Aug 11, 2017

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

This comment has been minimized.

Copy link
Contributor

commented Aug 11, 2017

@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

This comment has been minimized.

Copy link
Member

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

This comment has been minimized.

Copy link
Author

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

This comment has been minimized.

Copy link
Member

commented Aug 13, 2017

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

This comment has been minimized.

Copy link
Member

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

This comment has been minimized.

Copy link

commented Dec 8, 2017

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

This comment has been minimized.

Copy link

commented Mar 20, 2018

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

@BYK BYK referenced this issue May 3, 2018

Closed

🦁 Yarn 2.0 Working Group #5773

0 of 5 tasks complete
@ryancastle

This comment has been minimized.

Copy link

commented May 4, 2018

Would a potential path forward be something like this?

  1. yarn install in an interactive environment might prompt developer when any sort of auto-merge magic is going to be applied to the yarn.lock file. The prompt could be something like 'Are you sure you want to update your lock file? Y/n', where pressing return proceeds with the auto-merge.
  2. In a non-interactive environment (e.g. CI) the question is not asked and no magic ever happens.

This keeps CI builds properly deterministic and yarn.lock is entirely authoritative, but a fairly reasonable developer workflow is left in place basically as-is.

@tknerr

This comment has been minimized.

Copy link

commented Aug 22, 2018

Same here as @ProdigySim mentioned: yes having --frozen-lockfile enabled by default via .yarnrc is fine, but sometimes you actually need / want to update a package. In that case having to edit the .yarnrc to remove that default setting is really cumbersome. A --no-frozen-lockfile option would be really helpful here.

@raijinsetsu

This comment has been minimized.

Copy link

commented Sep 12, 2018

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

This comment has been minimized.

Copy link

commented Sep 12, 2018

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

This comment has been minimized.

Copy link
Author

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

This comment has been minimized.

Copy link
Member

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

This comment has been minimized.

Copy link

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?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.