greenkeeperio / greenkeeper

:robot: :palm_tree: Real-time automated dependency updates for npm and GitHub
https://greenkeeper.io/
Apache License 2.0
1.6k stars 95 forks source link

Do not remove integrity property in Yarn lockfiles #955

Open hassankhan opened 6 years ago

hassankhan commented 6 years ago

It seems Greenkeeper is (somewhat unnecessarily) stripping the integrity hashes from Yarn lockfiles. Could this behaviour be changed? Thanks for running such a great service! ๐Ÿ‘

Koslun commented 6 years ago

@hassankhan I believe that the cause is that yarn before version 1.10.0 do not update the integrity hashes automatically.

We solved usage of the wrong version by forcibly installing the latest yarn version in our build pipeline at CodeShip. All the new Greenkeeper PRs since have stopped stripping the integrity hashes away.

Though since GreenKeeper is the one triggering the lockfile I am not quite sure why it is working now as opposed to before or if there even was something else that happened to change.

hassankhan commented 6 years ago

Hi @Koslun, thanks for the response! I'll wait till the next PRs come through on any of my repos and will update the issue as appropriate ๐Ÿ‘

foray1010 commented 5 years ago

@Koslun seems problem is still there, this pr just created a few hours before https://github.com/foray1010/didyoumean2/pull/35/files

btw, I actually doubt that greenkeeper should use newest yarn as some users may not ready to upgrade, lot of LoC will be introduced after user yarn install again maybe we should detect yarn version in package.json engines.yarn or we can parse yarn.lock to see if any integrity field in order to pick right version

hassankhan commented 5 years ago

+1 @foray1010 , I've seen the same issue rear its head on a few of my repositories again

Koslun commented 5 years ago

@foray1010 For our part, we did not have an engines.yarn field as I believe older versions of yarn we were using seemed to support the integrity field. I am however not exactly sure in which version this field was introduced but do think it's curious that it started being added as a default in a minor version update. But given its introduction in a minor version bump I'd assume the integrity field would be supported since v1 of yarn.

Either way I think your suggestion of checking for the presence of the integrity field is more ideal as it assumes less about the developer environment.

But workaround right now seems to be to be simply use version ^1.10.1 of yarn.

ylemkimon commented 5 years ago

The integrity field was added in https://github.com/yarnpkg/yarn/pull/5042 released in v1.10.0, but since they realized it may be a breaking change, it was disabled for existing entries in https://github.com/yarnpkg/yarn/pull/6465.

ylemkimon commented 5 years ago

So the best approach for Greenkeeper seems to be upgrading to 1.12.1, and then it will not add integrity fields to existing lockfiles but update those with integrity fields.

kylemh commented 5 years ago

I'm using yarn@1.12.1 in CI, and yet Greenkeeper still removes integrity hashes. Really want to get this resolved!

I searched this repo and the lockfile repo for text instances of "yarn" in order to submit a PR, but it looks like the version of yarn used in CI is not part of the code base.

lfender6445 commented 5 years ago

Is it not possible to control the yarn engine or have green keeper respect the one specified in package.json?

willdurand commented 5 years ago

We also have issues with Greenkeeper's 2nd commit on every PR: integrity fields are removed for every single dependency. We use Yarn 1.12.3. Is there any way to disable the lock file update by chance? That'd be better than merging a PR without the integrity fields by accident.

edmorley commented 5 years ago

@willdurand hi fellow Mozillian! Quite a few Mozilla projects are using Renovate instead of Greenkeeper now, since its Yarn lockfile integration works much better amongst other things. If you'd like to chat about it ping me on IRC (:emorley) :-)

baldurh commented 5 years ago

Has Greenkeeper updated itโ€™s yarn version? Iโ€™m seeing the integrity fields being added in the latest PRs.