Depedencies: node-pre-gyp drags in a whole lot of new of dependencies

[bpasero]

Would it be possible to put node-pre-gyp as devDependency? Since 1.0.0 fsevents drags in more than 30 (just a guess) new modules via the dependency to node-pre-gyp. Since chokidar updated to fsevents recently, this has quite a big impact on shipping with chokidar 1.2.0.

[es128]

It isn't a devDependency. It's necessary for downloading and installing the pre-built binary instead of compiling locally - which is way faster and should resolve the issues many people have had with a messed up local compile chain.

has quite a big impact

What is the impact?

[bpasero]

I was always compiling fsevents from source and never had issues. So you say because other users have issues setting up their environment to build fsevents, I have to depend on 30+ new modules? This is bad, I would prefer if fsevents was rather shipping the prebuilt binaries for each platform if I had to choose.

The impact is as I said, 30+ new dependencies. Depending on where fsevents is being used this can have a number of issues (overall product size, LCA approvals for open source node modules, etc).

[es128]

I hear you registering your disapproval of this change, and I'm sorry you're unhappy with it. But I do believe it is a net benefit to chokidar's userbase. If complex/deep dep trees give you pause, the npm registry must be a pretty perilous place for you.

It would be nice if this could be configurable so you could opt out, but I don't see a clean way to provide that with npm. You can lock to an earlier version obviously, but you knew that. A manipulated shrinkwrap file maybe, but it will probably lead to maintenance headaches eventually.

I suppose we could consider a PR that converts node-pre-gyp to a devDependency and adds scripting or some alternate means to handle installation of the pre-built binaries. Is that something you'd be interested in working on?

Any other suggestions? (aside from just reverting)

@indieisaconcept do you have any perspective to share on this by any chance?

[bpasero]

@es128 I am trying to understand the code dependency to node-pre-gyp in fsevents.js, I assume that one could be replaced with something else or does it need all of node-pre-gyp to be there?

If the code dependency could go away, then I think it would be nicer to give a user the choice of node-gyp vs. node-pre-gyp. E.g. if the normal install fails, provide a script that installs node-pre-gyp and runs the install again.

I see the value of granular dependencies, even if there are many for one module but I cannot really understand a dependency to 30+ modules that are only being used during npm install, and not as a code dependency.

[es128]

It's not a code dependency, it's an install dependency. See https://github.com/strongloop/fsevents/blob/master/package.json#L17

What if you were to add some scripting to your project (postinstall or similar) that removes it when done so you don't have to worry about shipping it? Maybe that's something we could consider for addition here if it can be demonstrated to work solidly. (node-pre-gyp install --fallback-to-build && npm uninstall node-pre-gyp perhaps?)

if the normal install fails, provide a script that installs node-pre-gyp and runs the install again.

Defeats the gains we've achieved in installation time using this method.

[bpasero]

@es128 what about https://github.com/strongloop/fsevents/blob/master/fsevents.js#L10 ?

[es128]

Ah, good point. The only thing it's doing there is finding the installed binary, which would probably be possible to shim with something simpler, but obviously this idea is getting more complicated.

It all depends on how much motivation there is to trim down the dependencies.

[indieisaconcept]

I'll review later today

On Tue, Oct 6, 2015 at 9:42 PM, Elan Shanker notifications@github.com wrote:

Ah, good point. The only thing it's doing there is finding the installed binary, which would probably be possible to shim with something simpler, but obviously this idea is getting more complicated.

It all depends on how much motivation there is to trim down the dependencies.

Reply to this email directly or view it on GitHub: https://github.com/strongloop/fsevents/issues/93#issuecomment-145818941

[sheerun]

@es128 Some projects use node-pre-gyp as optionalDependency. Maybe that's the way?

[sheerun]

Or maybe a peerDependency, and allow user to install it by himself if he wants it.

[es128]

Example? I don't see how to do that without defeating some major benefits and/or not having any positive impact on the dep tree concerns.

[sheerun]

node-pry-gyp is not working for me again for electron, and I don't want to use it at all. I want source builds..

[es128]

Is there any open source electron app using chokidar/fsevents someone could point me to?

[es128]

Also, can you describe the failure better? Breakage is different front hand-wringing about too many deps and maybe should be discussed in a separate issue.

[sheerun]

Popular OS projects using chokidar are listed in their README :) For me it's notably webpack and babel (but also less popular xpc-connection).

Specifically it happens when using electron-rebuild for recent electron version, against projects that indirectly depend on chokidar/fsevents/node-pre-gyp (i.e. most of front-end projects).

The error shows as follows:

Unsupported target version: 0.33.3

node-pre-gyp ERR! install error
node-pre-gyp ERR! stack Error: Unsupported target version: 0.33.3
...
node-pre-gyp ERR! node -v v4.1.1
node-pre-gyp ERR! node-pre-gyp -v v0.6.12
node-pre-gyp ERR! not ok

npm ERR! Darwin 14.5.0
npm ERR! argv "/.../npm-cli.js" "rebuild" "--target=0.33.3" "--arch=x64"
npm ERR! node v4.1.1
npm ERR! npm  v2.14.6
npm ERR! code ELIFECYCLE
npm ERR! fsevents@1.0.2 install: `node-pre-gyp install --fallback-to-build`

(https://github.com/mapbox/node-pre-gyp/pull/175 somewhat solves it, when using electron-rebuild 1.0.1).

As a consumer of such open source modules I'd like to choose whether to compile them with node-pre-gyp and node-gyp (with fallback), but I don't feel like ping each and every project that uses them when something breaks..

So I guess ideally fsevents wouldn't depend on either of them, but only require either of them. This can be checked directly in install step. No need to list it in either dependencies, devDependencies, optionalDependencies, peerDependencies or bundledDependencies. But that's my opinion. Ping @springmeyer

[es128]

@sheerun I asked if there were any projects I could look at using electron and chokidar.

[sheerun]

Personally I know only non-public projects, but I see that https://github.com/kitematic/kitematic uses electron and babel (that depends on chokidar). I bet half of the projects from https://github.com/sindresorhus/awesome-electron use either webpack, babel, or browserify.

[es128]

If that's the case then why aren't a lot more people having the problems you've described?

[es128]

@sheerun see if fsevents 1.0.3 improves the situation for you

[es128]

@bpasero I attempted a fix for your concern in the [fix-93 branch](https://github.com/strongloop/fsevents/tree/fix-93 branch), but it has a fatal flaw in that removing node-pre-gyp breaks npm rebuild.

But perhaps just adding pre-gyp-find and eliminating the node-pre-gyp runtime dependency is good enough, allowing you to add your own npm uninstall node-pre-gyp step before creating a distribution of your software to cull all those extra deps.

Please take a look and let me know what you think.

[svnm]

It would be great to see this dependency removed. I also get the warning npm WARN package.json node-pre-gyp@0.6.15 No README data Maybe it could be updated to use a more recent version of node-pre-gyp

[stdavis]

I use chokidar (1.2.0) which uses fsevents (1.0.5) in an atom editor package. After upgrading to Atom 1.2.0 I get this error message when trying to build the package:

node-pre-gyp ERR! install error 
node-pre-gyp ERR! stack Error: Unsupported target version: 0.34.0
node-pre-gyp ERR! stack     at get_runtime_abi (/Users/stdavis/Documents/Projects/atom-amdbutler/node_modules/chokidar/node_modules/fsevents/node_modules/node-pre-gyp/lib/util/versioning.js:156:23)
node-pre-gyp ERR! stack     at Object.module.exports.evaluate (/Users/stdavis/Documents/Projects/atom-amdbutler/node_modules/chokidar/node_modules/fsevents/node_modules/node-pre-gyp/lib/util/versioning.js:265:19)
node-pre-gyp ERR! stack     at install (/Users/stdavis/Documents/Projects/atom-amdbutler/node_modules/chokidar/node_modules/fsevents/node_modules/node-pre-gyp/lib/install.js:138:31)
node-pre-gyp ERR! stack     at Object.self.commands.(anonymous function) [as install] (/Users/stdavis/Documents/Projects/atom-amdbutler/node_modules/chokidar/node_modules/fsevents/node_modules/node-pre-gyp/lib/node-pre-gyp.js:48:37)
node-pre-gyp ERR! stack     at run (/Users/stdavis/Documents/Projects/atom-amdbutler/node_modules/chokidar/node_modules/fsevents/node_modules/node-pre-gyp/bin/node-pre-gyp:79:30)
node-pre-gyp ERR! stack     at Object.<anonymous> (/Users/stdavis/Documents/Projects/atom-amdbutler/node_modules/chokidar/node_modules/fsevents/node_modules/node-pre-gyp/bin/node-pre-gyp:131:1)
node-pre-gyp ERR! stack     at Module._compile (module.js:456:26)
node-pre-gyp ERR! stack     at Object.Module._extensions..js (module.js:474:10)
node-pre-gyp ERR! stack     at Module.load (module.js:356:32)
node-pre-gyp ERR! stack     at Function.Module._load (module.js:312:12)
node-pre-gyp ERR! System Darwin 15.0.0
node-pre-gyp ERR! command "node" "/Users/stdavis/Documents/Projects/atom-amdbutler/node_modules/chokidar/node_modules/fsevents/node_modules/.bin/node-pre-gyp" "install" "--fallback-to-build"
node-pre-gyp ERR! cwd /Users/stdavis/Documents/Projects/atom-amdbutler/node_modules/chokidar/node_modules/fsevents
node-pre-gyp ERR! node -v v0.10.40
node-pre-gyp ERR! node-pre-gyp -v v0.6.15
node-pre-gyp ERR! not ok 

npm ERR! Darwin 15.0.0
npm ERR! argv "/Applications/Atom.app/Contents/Resources/app/apm/bin/node" "/Applications/Atom.app/Contents/Resources/app/apm/node_modules/npm/bin/npm-cli.js" "--globalconfig" "/Users/stdavis/.atom/.apm/.apmrc" "--userconfig" "/Users/stdavis/.atom/.apmrc" "rebuild" "--target=0.34.0" "--arch=x64"
npm ERR! node v0.10.40
npm ERR! npm  v2.13.3
npm ERR! code ELIFECYCLE
npm ERR! fsevents@1.0.5 install: `node-pre-gyp install --fallback-to-build`
npm ERR! Exit status 1
npm ERR! 
npm ERR! Failed at the fsevents@1.0.5 install script 'node-pre-gyp install --fallback-to-build'.
npm ERR! This is most likely a problem with the fsevents package,
npm ERR! not with npm itself.
npm ERR! Tell the author that this fails on your system:
npm ERR!     node-pre-gyp install --fallback-to-build
npm ERR! You can get their info via:
npm ERR!     npm owner ls fsevents
npm ERR! There is likely additional logging output above.

npm ERR! Please include the following file with any support request:
npm ERR!     /Users/stdavis/Documents/Projects/atom-amdbutler/npm-debug.log

I assume that this is related to this issue?

[bpasero]

@es128 tried out the branch and it seems to work nicely for me! The only warning I get during npm install is:

npm WARN installMany node-pre-gyp was bundled with fsevents@1.0.3, but bundled package wasn't found in unpacked tree

[es128]

@bpasero the branch I had made would create unacceptable side effects, but if you can confirm that you'd be able to add your own npm uninstall node-pre-gyp step in your build (which will be safe to do if we add and use pre-gyp-find), then this could help out your use case. I didn't want to go ahead without your feedback or the result might be just adding yet another dependency.

[bpasero]

I can give that a try, are the side effects going to impact us as well or are we ok once we have built chokidar?

[es128]

My proposal creates no ill side effects other than adding one more small dependency. The suggestion I'm making is that you take that and uninstall node-pre-gyp with all its dependencies on your own since it will no longer be needed at runtime. I think the only impact is breaking npm rebuild, which I believe you wouldn't need after you've created your bundle. You could also always choose to execute an npm install instead if you did need to perform an action like that.

[bpasero]

Right, we do not run npm rebuild ever as far as I know.

[hedefalk]

I'm having the same issue as @stdavis with my Atom package https://github.com/ensime/ensime-atom. I've seen the error before and asked on atom channel first: https://discuss.atom.io/t/apm-using-ancient-versions-of-node-npm-and-i-cant-seem-to-fix-it/26400

Stuff did seem to work anyway even though apm rebuild failed but today it started failing totally. Atom wont load my package since:

Failed to require the main module of 'Ensime' because it requires an incompatible native module. Run apm rebuild in the package directory to resolve.

Running apm rebuild gives me

~/d/p/atom-ensime ❯❯❯ apm rebuild master ✱ Rebuilding modules ✗ node-pre-gyp ERR! install error node-pre-gyp ERR! stack Error: Unsupported target version: 0.34.5 node-pre-gyp ERR! stack at get_runtime_abi (/Users/viktor/dev/projects/atom-ensime/node_modules/fsevents/node_modules/node-pre-gyp/lib/util/versioning.js:156:23) …

Just like @stdavis above.

To reproduce using Atom, you just need:

mkdir fsevents-apm-rebuild
cd fsevents-apm-rebuild
apm init --package .
npm install -s fsevents
apm rebuild

which fails as I reported here https://github.com/atom/apm/issues/531

I think the problem stems from the fact that apm bundles ancient node/npm as described here: https://discuss.atom.io/t/apm-using-ancient-versions-of-node-npm-and-i-cant-seem-to-fix-it/26400/4

I'm super-stuck and don't know what I could do to release a new version of my package. I guess I should look into what @stdavis did to fix his problem since were in the same space…

Any tips highly appreciated!

[hedefalk]

Oh, @stdavis removed chokidar https://github.com/agrc/atom-amdbutler/commit/288f01ca604cd37e985bca9fe782157ebba0256d

[stdavis]

Yep. Removing chokidar was my fix.

[hedefalk]

@stdavis Thanks! I went with gaze too https://github.com/ensime/ensime-node/commit/463caa2c59c85879874fcceeb23a073f4560726b

[bpasero]

@es128 I tried to npm uninstall node-pre-gyp but it will not work as long as I am not the package that depends on node-pre-gyp directly. This script would have to be added to the postinstall script of fsevents itself to work.

When I manually run npm uninstall node-pre-gyp from the fsevents folder in node_modules, the contents of node_modules within fsevents nicely go away.

Also I am hitting a blocker with Electron and fsevents: https://github.com/strongloop/fsevents/issues/131

[es128]

This script would have to be added to the postinstall script of fsevents itself to work.

Would break other users' attempts at npm rebuild, so not an option I'm afraid.

Are you using npm@2? On v3 npm uninstall node-pre-gyp works for me, even from the parent.

How about

"postinstall": "cd node_modules/fsevents; npm uninstall node-pre-gyp"

[bpasero]

@es128 we recently switched to using npm@3. Interestingly all dependencies of fsevents are put within a node_modules folder of fsevents and not flat outside. Not sure if that has an impact.

Actually this might not be a blocker for us after all because we already have custom code that only takes the fse.node from chokidar during build time.

@es128 can you confirm that deleting the entire node_modules folder of fsevents (all its dependencies) is OK as long as you have built already and you do not need npm rebuild?

[es128]

Must be because of the bundled dependencies that it's not fully flattening, although I thought I was installing from the registry when I tested this...

No, you cannot delete all of node_modules. For this idea to work we need to add a dependency - the much lighter pre-gyp-find. Also pretty sure nan needs to be kept around.

[es128]

Ah, so I'm seeing cases where npm uninstall node-pre-gyp on npm@3 just exits silently without removing anything. The solution, a bit paradoxically, seems to be to directly depend on node-pre-gyp from the top level, and then npm uninstall node-pre-gyp at the end.

[bpasero]

@es128 interestingly when I run cd node_modules/fsevents; npm uninstall node-pre-gyp the entire contents of the node_modules within fsevents goes empty :)

[es128]

Ah, because the other deps have been flattened. So maybe your earlier suggestion actually will work for you.

[bpasero]

Yup, I can verify this once I get fse.node to be loaded properly.

[jwheare]

Bundling this dependency is now a security issue, it bundles minimatch@3.0.0 (vuln details: https://snyk.io/vuln/npm:minimatch:20160620) via sub dependencies, with no maintainable way (beyond shrinkwrap hacking) to fix it at my project level (as far as I'm aware):

> npm ls minimatch
fsevents@1.0.12
└─┬ node-pre-gyp@0.6.25
  ├─┬ rimraf@2.5.2
  │ └─┬ glob@7.0.3
  │   └── minimatch@3.0.0
  └─┬ tar-pack@3.1.3
    └─┬ fstream-ignore@1.0.3
      └── minimatch@3.0.0

I've checked all the package.json version patterns in the above tree, and they would allow resolving to a patched version of minimatch with a fresh npm install, but I can't work out how to override the bundling to allow that.

[sheerun]

Maybe you can take a look at https://github.com/sheerun/npm-packer I've made.

It preserves all paths inside package, and bundles properly even for old npm versions that re-install even if they are marked as in bundledDependencies.

[bnoordhuis]

Bundling this dependency is now a security issue, it bundles minimatch@3.0.0 (vuln details: https://snyk.io/vuln/npm:minimatch:20160620) via sub dependencies

I'm aware of the minimatch issue but is there reason for concern here? If you have a credible attack that would affect fsevents, I'd like to hear it.

Have you contacted the node-pre-gyp project? What did they say?

[jwheare]

I'm not aware of a credible attack in this case, but it does worry me that patched vulnerabilities can exist without the ability to apply patches in a maintainable way, especially given the dependency tree version patterns would specifically allow it.

[jwheare]

To be clear, the node-pre-gyp dependency tree isn't vulnerable. A fresh npm install would pull in the patched version of minimatch.

[pipobscure]

I’ve had some conversations at NodeConf.EU which lead to me porting to N-API. However that requires a few experimental things to be backported to 6.x and 8.x.

Once that is done, the prebuilt should only be needed once per version (since OS X and nose are then ABI compatible across versions). At that point it would be worth while to remove node-pre-gyp and ship the binary directly. On install we could then try loading that and on failure recompile.

[pipobscure]

It's being removed in #237