greenkeeperio / greenkeeper-lockfile

:lock: Your lockfile, up to date, all the time
https://greenkeeper.io
183 stars 73 forks source link

update-lockfile fails if npm install changed the package.json #108

Closed 125m125 closed 6 years ago

125m125 commented 6 years ago

It seems like update-lockfile fails if npm install changes the package.json.

In the following example, npm install added an empty line at the end of the package.json, which causes update-lockfile to fail: https://travis-ci.org/125m125/ktapi-js/jobs/335830733

cdanielsen commented 6 years ago

I'm seeing similar behavior on a repo I recently setup with greenkeeper. @125m125 I'm curious about the empty line cause - how do you know that's happening? Looking at your commit and mine, I only see the version bump?

125m125 commented 6 years ago

The newline was removed in a previous commit: https://github.com/125m125/ktapi-js/commit/76ac12251dce30766276cbfa65cd2d0291355f6d#diff-b9cfc7f2cdf78a7f4b91a753d10865a2R52 (the autoformatting of my editor removed the empty line at the end of the file). npm always inserts the new line at the end of the file when installing or otherwise modifying the package.json. This causes the revert to fail.

I just found pull request #61 , which should solve this problem.

brunocodutra commented 6 years ago

I don't follow the reasoning blaming an empty line at the end of the file, because if greenkeeper-lockfile is supposed to run on a PR where a dependency has just been bumped by greenkeeper, isn't it then to be expected that the package-lock.json file will always be modified by the previous npm install?

I get the very same error here and frankly I can't see how it's supposed to work, because the first thing greenkeeper-lockfile does is to run git revert -n HEAD, which will always fail due to the conflicting package-lock.json.

I even manually attempted to reproduce locally what greenkeeper-lockfile does and, as expected, I get the very same error.

Do I miss anything obvious?

Realtin commented 6 years ago

Hey,

I briefly checked the repositories and noticed that sometimes when adding or removing packages, the package-lock.json was not kept in-sync. Therefore running an npm install would update it in the CI build even before greenkeeper-lockfile starts (Those are the uncommitted changes then).

I also sometimes forget to run npm install after removing packages from the package.json before committing the change so the package-lock is not updated.

Regarding accidental package.json changes from greenkeeper-lockfile, I'll have a look at https://github.com/greenkeeperio/greenkeeper-lockfile/pull/61 again!

brunocodutra commented 6 years ago

@Realtin maybe I'm just being obtuse, but I'm having a bit of hard time understanding the reasoning here. Isn't greenkeeper-lockfile supposed to run on a PR by greenkeeper where a package version is bumped and therefore npm install expected to always change package-lock prior to greenkeeper-lockfile running?

Realtin commented 6 years ago

Yes, so what happened is that a human updated the package.json but not the package-lock some time before greenkeeper made it's branch. The next one, who would run npm install, would get the update of the package-lock with changes made before. Then when greenkeeper created it's branch and the CI sets up the project by installing the dependencies (with npm install), so it is the one that updates the package-lock. We now have uncommitted changes in the git history. Greenkeeper lockfile tries to do the git revert -n HEAD but that fails due to the uncommited changes.

So if the package.json and the package-lock.json are out-of-sync greenkeeper-lockfile can't update correctly.

brunocodutra commented 6 years ago

I understand what you are saying, but I think it misses the point. To be clear, I double checked and my package.json is in sync with package-lock.json on the master branch, so the problem is not due to a human forgetting to push changes to package-log.json, but rather the fact that, by design, greenkeeper updates just the package.json when it creates a PR and, therefore, by definition greenkeeper-lock will always run on a package-lock.json that is out of sync, unless I grossly misunderstood the design?

I'm new to greenkeeper-lockfile, so it's very well possible that I miss configured something, but I did follow the docs to the best of my knowledge and I don't see where I could possibly have done something wrong.

Googling I found the issue below on another project that describes the exact same situation I have, but sadly they eventually gave up and closed it without a solution. Maybe the discussion there is relevant?

stylelint/stylelint#2766

Anyways, thanks for taking the time to go over this.