1465164 - eslint modifies the srcdir (sourcedir, srctree, sourcetree)

Status: RESOLVED DUPLICATE of bug 1476467

(Developer Infrastructure :: Lint and Formatting, defect)

Description

[Boris Zbarsky [:bzbarsky]]

I've now had multiple cases where I ran eslint locally and it modified package-lock.json in the source tree as a side-effect.  The diff look like a bunch of:

-      "integrity": "sha512-xcJpopdamTuY5duC/KnTTNBraPK54YwpenP4lzxU8H91GudWpFv38u0CKjclE1Wi2EH2EDz5LRcHcKbCIzqGyg==",
+      "integrity": "sha1-/wS9/AEO5UfXgL7DjhrBwnd9JTo=",

with different hash values.  Why is this tool touching the srcdir at all?

Comment 1

[Dave Townsend [:mossop]]

I thought that with bug 1456085 fixed we could remove package-lock.json?

Comment 2

[Mark Banner (:standard8)]

The intention of package-lock.json is to try to ensure that we get the exact same trees installed across everyone's installations.

package-lock.json should be the same cross platform, and mostly the recent issues with npm seem to have been fixed, except for this sha issue. I'm not yet sure if this is an npm bug or something weird.

Boris, can we get a bit more detail about your setup please? It might help find where this lies.

- Node version
- npm version
- OS & version

Also, can you try from the top level source directory:

$ rm -r node_modules
<check there's no diffs to package-lock.json>
$ npm install

and check if package-lock.json is modified or not. If there are diffs, please can you post the diffs, the output from `npm install`, and also `npm cache verify` ?

Hopefully something there will help us track this down.

Comment 3

[Boris Zbarsky [:bzbarsky]]

Node version: 8.11.0
npm version: 5.6.0
OS: Fedora 27

> $ npm install

Running this does not cause a change to package-lock.json.

Further, running eslint after doing that also does not change package-lock.json.  That matches my past experience: if I eslint modify package-lock.json and just revert those changes and run it again, it doesn't get modified again...

Comment 4

[Boris Zbarsky [:bzbarsky]]

And I did check the "running eslint" step there both in the condition that there is an error to find and in the condition that there is not.

Comment 5

[Boris Zbarsky [:bzbarsky]]

This looks like https://github.com/npm/npm/issues/17749 for what it's worth.  Which is basically resolved "incomplete"...  That said, the steps you had me run per comment 2 likely mean I personally won't see this again until "something" changes, per the comments in that thread....

Comment 6

[Mark Banner (:standard8)]

Yeah, unfortunately it does look like the same issue.

It is unclear if you removed node_modules or not. The intent behind that was to force an install that is one of the likely times it might alter package-lock.json.

The other thing I didn't mention to do, which is mentioned a lot there, is to clear the cache: `npm cache clear --force`, then remove node_modules, then re-install. Like you say, we'll only see if that fully works next time we do an update.

Note: some of this might change as we move to supporting node in the build system, but a lot of that is not fully defined at the moment so we'll have to wait and see.

Comment 7

[Boris Zbarsky [:bzbarsky]]

> It is unclear if you removed node_modules or not.

I did.

Comment 8

[Mark Banner (:standard8)]

To summarize here:

- We have package-lock.json to try and ensure that developer's installs of the modules for ESLint & their dependencies are consistent with the builders.
- npm has a bug in which may or may not be resolved in the latest versions (https://github.com/npm/npm/issues/17749). The bug causes package-lock.json changes to weaker integrity keys, that sometimes gets checked in.
- The npm bug is possibly eased by doing `npm cache clear --force`
- We're slowly working towards having node as a build dependency and node_modules would likely be moved in-tree.

Maybe a possible option is to have `./mach eslint --setup` detect package-lock.json changes and undo them, and print a warning to clear cache / node_modules.

If those with node experience have any other ideas, please comment!

Comment 9

[Mark Banner (:standard8)]

Just remembered, we worked around this in bug 1475504 and then bug 1476467 fixed it properly - for node >= 5.8 we only use package-lock.json combined with the npm ci command. For < 5.8 we clean up package-lock.json afterwards.

In future we're likely to vendor node_modules from mozilla-central directly so, this would even less of an issue.