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

Preserve "integrity" entries when using Yarn 1.9 #6440

Closed
rarkins opened this issue Sep 27, 2018 · 10 comments
Closed

Preserve "integrity" entries when using Yarn 1.9 #6440

rarkins opened this issue Sep 27, 2018 · 10 comments
Assignees
Labels

Comments

@rarkins
Copy link
Contributor

rarkins commented Sep 27, 2018

Can a patch version 1.9.5 be released where Yarn does not strip out every single "integrity" link (added in v1.10.0) from a yarn.lock if found?

Due to some problems with Yarn v1.10.x, I rolled Yarn back to v1.9.4. Now I see any small change to the lock file results in the integrity field being stripped from every single package.

Although ideally everyone in a team uses the exact same version of Yarn at all times, this is not always feasible and perhaps sometimes the entire team needs to roll back. Ideally this wouldn't result in massive flip/flopping of lock file contents as a result, if they could be left as-is without impact.

Cc @edmorley

@ghost ghost assigned arcanis Sep 27, 2018
@ghost ghost added the triaged label Sep 27, 2018
@rarkins rarkins changed the title Preserve "integrity" entries when using Yarn 1.9x Preserve "integrity" entries when using Yarn 1.9 Sep 27, 2018
@arcanis
Copy link
Member

arcanis commented Sep 27, 2018 via email

@rarkins
Copy link
Contributor Author

rarkins commented Sep 27, 2018

@arcanis thanks for considering it and the tip re: yarn-path.

FYI, I'm the author of "Renovate Bot", so that's why I've been exposed to this challenge right away. Currently the bot bundles its own yarn dependency and uses that for all repos, so I have reverted it to v1.9.4 for now until I have a solution to dynamically switch Yarn versions for the longer term. Having a v1.9.5 that does not strip integrity would be a great benefit to any repos already on v1.10 using the bot (such as probably this one).

@rarkins
Copy link
Contributor Author

rarkins commented Sep 27, 2018

Here's an example to reproduce this locally:

  1. Run "yarn install" using v1.10.0 with this package.json:
{
  "name": "yarnintegrity1",
  "version": "0.0.1",
  "dependencies": {
    "chalk": "2.0.0",
    "left-pad": "1.0.0"
  }
}

Result, as expected: yarn.lock exists with each entry including an integrity hash.

  1. Using [email protected], run yarn add [email protected] --exact

Result: Every integrity field has been stripped from the yarn.lock for chalk and its dependents

Expectation: Only left-pad's integrity field would be removed.

The [email protected] I used is installed via brew on OSX, while the [email protected] I used is the compiled JS downloaded from this repository's releases page. Technically I ran node ~/yarn-1.9.4.js add [email protected] --exact but I suspect that none of these details really change anything.

Cc @imsnif as per our Twitter thread

yarn.lock diff
diff --git a/yarn.lock b/yarn.lock
index 486cce3..04f319c 100644
--- a/yarn.lock
+++ b/yarn.lock
@@ -5,14 +5,12 @@
 ansi-styles@^3.1.0:
   version "3.2.1"
   resolved "https://registry.yarnpkg.com/ansi-styles/-/ansi-styles-3.2.1.tgz#41fbb20243e50b12be0f04b8dedbf07520ce841d"
-  integrity sha512-VT0ZI6kZRdTh8YyJw3SMbYm/u+NqfsAxEpWO0Pf9sq8/e94WxxOpPKx9FR1FlyCtOVDNOQ+8ntlqFxiRc+r5qA==
   dependencies:
     color-convert "^1.9.0"

 [email protected]:
   version "2.0.0"
   resolved "https://registry.yarnpkg.com/chalk/-/chalk-2.0.0.tgz#c25c5b823fedff921aa5d83da3ecb5392e84e533"
-  integrity sha512-7jy/5E6bVCRhLlvznnsbVPjsARuVC9HDkBjUKVaOmUrhsp6P3ExUUcW09htM7/qieRH+D2lHVpNbuYh7GjVJ0g==
   dependencies:
     ansi-styles "^3.1.0"
     escape-string-regexp "^1.0.5"
@@ -21,33 +19,27 @@ [email protected]:
 color-convert@^1.9.0:
   version "1.9.3"
   resolved "https://registry.yarnpkg.com/color-convert/-/color-convert-1.9.3.tgz#bb71850690e1f136567de629d2d5471deda4c1e8"
-  integrity sha512-QfAUtd+vFdAtFQcC8CCyYt1fYWxSqAiK2cSD6zDB8N3cpsEBAvRxp9zOGg6G/SHHJYAT88/az/IuDGALsNVbGg==
   dependencies:
     color-name "1.1.3"

 [email protected]:
   version "1.1.3"
   resolved "https://registry.yarnpkg.com/color-name/-/color-name-1.1.3.tgz#a7d0558bd89c42f795dd42328f740831ca53bc25"
-  integrity sha1-p9BVi9icQveV3UIyj3QIMcpTvCU=

 escape-string-regexp@^1.0.5:
   version "1.0.5"
   resolved "https://registry.yarnpkg.com/escape-string-regexp/-/escape-string-regexp-1.0.5.tgz#1b61c0562190a8dff6ae3bb2cf0200ca130b86d4"
-  integrity sha1-G2HAViGQqN/2rjuyzwIAyhMLhtQ=

 has-flag@^2.0.0:
   version "2.0.0"
   resolved "https://registry.yarnpkg.com/has-flag/-/has-flag-2.0.0.tgz#e8207af1cc7b30d446cc70b734b5e8be18f88d51"
-  integrity sha1-6CB68cx7MNRGzHC3NLXovhj4jVE=

-[email protected]:
-  version "1.0.0"
-  resolved "https://registry.yarnpkg.com/left-pad/-/left-pad-1.0.0.tgz#c84e2417581bbb8eaf2b9e3d7a122e572ab1af37"
-  integrity sha1-yE4kF1gbu46vK549ehIuVyqxrzc=
+[email protected]:
+  version "1.1.0"
+  resolved "https://registry.yarnpkg.com/left-pad/-/left-pad-1.1.0.tgz#47a2daf581ede454334dee6c6036cae00d912e4d"

 supports-color@^4.0.0:
   version "4.5.0"
   resolved "https://registry.yarnpkg.com/supports-color/-/supports-color-4.5.0.tgz#be7a0de484dec5c5cddf8b3d59125044912f635b"
-  integrity sha1-vnoN5ITexcXN34s9WRJQRJEvY1s=
   dependencies:
     has-flag "^2.0.0"

@edmorley
Copy link
Contributor

edmorley commented Sep 27, 2018

Given this issue and also #6430 / heroku/heroku-buildpack-nodejs#569 (comment) it is starting to feel like the new integrity feature needs to be changed to a more staged rollout.

ie:

  1. Keep support for it in yarn 1.10.x, but default it to off (eg equivalent of unsafe-disable-integrity-migration true). In this "off" state, any existing integrity fields should be preserved, but no new ones added.
  2. In a later version of yarn, start adding integrity for any newly changed packages (eg those affected by yarn add), but don't mass-fetch all dependencies and try to update the lockfile in one go
  3. In an even later version of yarn, default to adding any missing integrity fields in the lockfile whenever performing any operation on it (and hopefully by this point many hashes will have already been added, reducing the number of packages that need fetching)

@rarkins
Copy link
Contributor Author

rarkins commented Sep 27, 2018

Is there a way to specify the behaviour of unsafe-disable-integrity-migration via CLI option rather than .yarnrc ?

@arcanis
Copy link
Member

arcanis commented Sep 27, 2018

@edmorley Yeah, I think we might do that. The integrity check isn't a breaking change per-se, but the behavior is too unexpected to be by-default right now. So I concur with your plan, except for one point:

  • unsafe-disable-integrity-migration will default to true until the 2.0, but we'll continue adding the integrity field to newly added packages (it doesn't have any extra cost - the cost you see is because we need to redo all the resolutions requests to populate the field)

  • Starting from the 2.0, unsafe-disable-integrity-migration will default to false. A major bump is a good signal that this kind of things might happen, so it should be fine.

Depending on whether the CLI mode is implemented by someone before the 2.0 lands, it might make sense to add an intermediary step where unsafe-disable-integrity-migration would default to gentle (not implemented), where it would only re-resolve at most X dependencies at each install (X=2?). The end result would be the same, but it would feel less intrusive for most users. Not sure about it, feel free to argue against that 🙂

@arcanis
Copy link
Member

arcanis commented Sep 27, 2018

Is there a way to specify the behaviour of unsafe-disable-integrity-migration via CLI option rather than .yarnrc ?

@rarkins Yes and no. You must have a yarnrc file, but you can tell Yarn to consider a specific yarnrc file using --use-yarnrc <path> (it will still also read the regular yarnrc unless you use --no-default-rc, cf #6007).

@glennreyes
Copy link

As a workaround, you can specify the yarn version in the engines field of your package.json:

"engines": {
  "yarn": ">=1.10.1"
}

@rarkins
Copy link
Contributor Author

rarkins commented Sep 28, 2018

Although I think this issue is still a good idea generally, I wanted to mention that it's no longer a compelling problem for Renovate or hence for me personally.

Renovate now auto-switches between 1.9 and 1.10 versions depending on presence of existing integrity hashes, which pretty much solves the compatibility challenge. e.g. in this repo once integrity hashes were added to yarn.lock in master, all open Renovate PRs were updated to include/update hashes.

Example:

image

@rjmunro
Copy link
Contributor

rjmunro commented Oct 8, 2018

@arcanis The "gentle" idea is IMHO terrible. I don't want 2 lines changed in my yarn.lock file every time I install that I then have to commit. I want either all or nothing.

The behaviour should be to detect and preserve the type of the lock file:

  • If there is no yarn.lock file or one that already contains integrity entries, use them and generate new ones.
  • If there is a 1.9 style yarn.lock file (i.e. it doesn't contain integrity entries), keep it as a 1.9 style lock file, don't add integrity entries. Print a warning, something like "Using yarn 1.9 compatible lock file; run yarn upgrade-lock-file to upgrade [and enable integrity checking]".

In general, never change the lock file unless I either ask for it, or have changed something relevant in the package.json file. Detect the version of the lock file and keep it consistent with that version.

@rarkins rarkins closed this as not planned Won't fix, can't repro, duplicate, stale Nov 10, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

5 participants