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

Arguments like --dist-url/--nodedir not respected with npm 3.10.10 #300

Open
springmeyer opened this issue Jun 11, 2017 · 3 comments
Open

Comments

@springmeyer
Copy link
Contributor

Context

When someone runs npm install --some-option to install a module that uses node-pre-gyp, then node-pre-gyp notices the arguments passed and forwards to node-gyp (when a source compile is needed). The same thing is done when node-pre-gyp is called directly (rather than via npm). This argument forwarding is critical to be able to control node-gyp behavior directly (since modules that use node-pre-gyp don't directly call node-gyp, by design).

Problem

This argument passing works in all cases when arguments are passed directly to node-pre-gyp. For example node-pre-gyp build --dist-url=foo will result in a call to node-gyp build --dist-url=foo.

And this argument passing works in all cases when arguments are passed to npm and npm calls node-pre-gyp except on windows with npm 3.10.10. What I've seen is:

mac linux windows
node v6/npm 3.10.10
node v6/npm 2.15.11 (downgraded)
node v4/npm 2.15.11
node v0.10/npm2.15.1
@springmeyer
Copy link
Contributor Author

Looks like this bug also impacts npm 5.3.0 / node v8.4.0 on windows: https://ci.appveyor.com/project/Mapbox/node-pre-gyp/build/1.0.625/job/m02cx3p7chk0q4km#L53

And with those versions another problem manifests that looks like:

not ok 29 Error: Command failed 'npm install --clang=1 --msvs_version=2015 --fallback-to-build=false'
  ---
    operator: error
    expected: |-
      undefined
    actual: |-
      [Error: Command failed 'npm install --clang=1 --msvs_version=2015   --fallback-to-build=false']
    at: maybeClose (internal/child_process.js:927:16)
    stack: |-
      module.js:491
          throw err;
          ^
      
      Error: Cannot find module 'C:\projects\node-pre-gyp\test\app1\node_modules\node-pre-gyp\bin\node-pre-gyp'
          at Function.Module._resolveFilename (module.js:489:15)
          at Function.Module._load (module.js:439:25)
          at Function.Module.runMain (module.js:609:10)
          at startup (bootstrap_node.js:158:16)
          at bootstrap_node.js:598:3
      npm ERR! code ELIFECYCLE
      npm ERR! errno 1
      npm ERR! [email protected] install: `node-pre-gyp install --fallback-to-build`
      npm ERR! Exit status 1
      npm ERR! 
      npm ERR! Failed at the [email protected] install script.
      npm ERR! This is probably not a problem with npm. There is likely additional logging output above.
      
      npm ERR! A complete log of this run can be found in:
      npm ERR!     C:\Users\appveyor\AppData\Roaming\npm-cache\_logs\2017-09-08T16_49_52_844Z-debug.log
      
  ...
ok 30 should not be equal

https://ci.appveyor.com/project/Mapbox/node-pre-gyp/build/1.0.625/job/m02cx3p7chk0q4km#L112

@springmeyer
Copy link
Contributor Author

springmeyer commented May 1, 2018

I dig into this a bit more and have figured out why the tests are failing, but not yet how to fix. Here is what I've learned:

  • On unix with you can do npm -C test/app1 install and it will: a) change into the test/app1 directory and install the tool.
  • On windows with npm v2.x it works the same as on unix
  • But on windows with > npm v2.x the npm -C test/app1 install behaves differently. Instead of attempting to install app1 instead it installs node-pre-gyp.

The fact that it installs node-pre-gyp makes no sense to me given that node-pre-gyp is not a dependency of test/app1 (it is not listed in the deps for that app). So I have no idea why this is happening.

But it is the cause of the test failures. What happens is that the tests expect npm -C test/app1 install to fail when an invalid argument is passed like --nodedir=invalid-value. But because that command just goes and installs node-pre-gyp it passes, which is super bizzarre.

Here is the output of npm -C test/app1/ install on windows with npm 3.10.10:

C:\projects\node-pre-gyp\test\app1\node-pre-gyp -> C:\projects\node-pre-gyp\test\app1\node_modules\node-pre-gyp\bin\node-pre-gyp
[email protected] C:\projects\node-pre-gyp\test\app1
`-- [email protected] 
  +-- [email protected] 
  +-- [email protected] 
  | `-- [email protected] 
  +-- [email protected] 
  | +-- [email protected] 
  | | `-- [email protected] 
  | +-- [email protected] 
  | | `-- [email protected] 
  | `-- [email protected] 
  +-- [email protected] 
  | +-- [email protected] 
  | `-- [email protected] 
  |   +-- [email protected] 
  |   `-- [email protected] 
  +-- [email protected] 
  | +-- [email protected] 
  | | `-- [email protected] 
  | |   `-- [email protected] 
  | |     +-- [email protected] 
  | |     `-- [email protected] 
  | `-- [email protected] 
  +-- [email protected] 
  | +-- [email protected] 
  | | +-- [email protected] 
  | | `-- [email protected] 
  | |   +-- [email protected] 
  | |   +-- [email protected] 
  | |   +-- [email protected] 
  | |   +-- [email protected] 
  | |   `-- [email protected] 
  | +-- [email protected] 
  | +-- [email protected] 
  | | +-- [email protected] 
  | | +-- [email protected] 
  | | +-- [email protected] 
  | | +-- [email protected] 
  | | +-- [email protected] 
  | | | +-- [email protected] 
  | | | `-- [email protected] 
  | | |   `-- [email protected] 
  | | +-- [email protected] 
  | | | `-- [email protected] 
  | | `-- [email protected] 
  | `-- [email protected] 
  +-- [email protected] 
  | +-- [email protected] 
  | +-- [email protected] 
  | +-- [email protected] 
  | `-- [email protected] 
  +-- [email protected] 
  | `-- [email protected] 
  |   +-- [email protected] 
  |   +-- [email protected] 
  |   | `-- [email protected] 
  |   +-- [email protected] 
  |   +-- [email protected] 
  |   `-- [email protected] 
  +-- [email protected] 
  `-- [email protected] 
    +-- [email protected] 
    +-- [email protected] 
    +-- [email protected] 
    +-- [email protected] 
    +-- [email protected] 
    `-- [email protected] 

/cc @mapsam

@springmeyer
Copy link
Contributor Author

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

1 participant