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

NCU detects NPM version of installed nodejs, not the actual global installed version (Windows) #163

Closed
vkbansal opened this issue Nov 11, 2015 · 29 comments · Fixed by #297
Closed

Comments

@vkbansal
Copy link

When I do ncu -g, I get the following

> ncu -g              
 npm  3.3.6  →  3.3.12

but actually it should be (at time of writting this 11-11-2015 07:53:00 IST)

electron-prebuilt 0.34.2  → 0.34.3
harp 0.17.0 → 0.19.0
http-server 0.8.0 → 0.8.5
metalsmith 2.0.1 → 2.1.0
stylint 1.2.2  → 1.3.2
tsd 0.6.4 → 0.6.5

npm list -g is follows (at time of writting this 11-11-2015 07:53:00 IST)

> npm list -g --depth=0
├── electron-prebuilt@0.34.2
├── gulp@3.9.0
├── harp@0.17.0
├── hexo-cli@0.1.8
├── http-server@0.8.0
├── metalsmith@2.0.1
├── node-gyp@3.0.3
├── npm@3.3.12
├── npm-check-updates@2.3.4
├── stylint@1.2.2
├── tsd@0.6.4
└── typescript@1.6.2
@raineorshine
Copy link
Owner

It seems like it is a bug, but unfortunately I cannot reproduce. I installed stylint@1.2.2 globally on my machine (OSX) and ncu picked it up correctly.

 stylint      1.2.2  →   1.3.2 

If anyone else can reproduce and debug that would be helpful.

@vkbansal
Copy link
Author

@metaraine I reported it for windows not OSX.

@raineorshine
Copy link
Owner

Yes, exactly. This is why I am unable to reproduce on my machine. If there is someone else who can test on Windows, that would be great!

@XhmikosR
Copy link
Collaborator

@metaraine: have a look at AppVeyor. I use it for my projects like grunt-contrib-plugins. Have a look there for configs.

@raineorshine
Copy link
Owner

Been working on a fix. Try this:

$ npm install -g npm-check-updates@2.5.4-spawnglobal

(Duplicate of #146)

@vkbansal
Copy link
Author

vkbansal commented Dec 7, 2015

getting following error:

Unhandled rejection Error: spawn npm ENOENT
    at exports._errnoException (util.js:860:11)
    at Process.ChildProcess._handle.onexit (internal/child_process.js:178:32)
    at onErrorNT (internal/child_process.js:344:16)
    at doNTCallback2 (node.js:450:9)
    at process._tickCallback (node.js:364:17)

@raineorshine
Copy link
Owner

spawn is used to invoke npm so that the correct global prefix is used. ENOENT suggests that the npm command is not available in the environment that spawn is invoked. Do you have npm installed globally? Is npm available only in shell in certain contexts? It's hard for me to troubleshoot without being able to reproduce the issues, so any debugging that people getting this error are willing to do would be helpful.

@tw0517tw
Copy link
Contributor

ncu seems to use the npm from nodejs installation (C:\Program Files\nodejs\npm) rather than the one installed globally (C:\Users\<user name>\AppData\Roaming\npm\npm)

@raineorshine
Copy link
Owner

Thanks! If anyone is willing to make a pull request for this, it could help a lot of people! I haven't been able to tackled it since I am unable to reproduce it myself.

@tw0517tw
Copy link
Contributor

I've been trying using https://github.com/dracupid/global-npm or https://github.com/h2non/requireg to get the global npm but still found some problems. Anyone would like to help with this?

@nielsgl
Copy link
Contributor

nielsgl commented Apr 26, 2016

@tw0517tw I am not running windows, but I can probably help fix it with your help (I already fixed a similar issue for people who installed it on osx via homebrew #146 and #231), let me know!

@tw0517tw
Copy link
Contributor

@nielsgl Thanks. I believe the issue on Windows is that we got 2 different npm (one comes with node installation, one comes with npm i npm -g)

I got these on Windows:

$ npm config get prefix -g
C:\Users\tw0517tw\AppData\Roaming\npm

$ npm root -g
C:\Users\tw0517tw\AppData\Roaming\npm\node_modules

and with adding your console.log lines in #146 (comment) into lib/package-manages/npm.js I got these:

$ ./bin/ncu -g
C:\Program Files\nodejs
C:\Program Files\nodejs\node_modules
undefined
C:\Program Files\nodejs\node_modules

 npm  3.8.3  →  3.8.7

@tw0517tw
Copy link
Contributor

tw0517tw commented Apr 26, 2016

I can get correct result with modifying the prefix

$ PREFIX="$APPDATA\npm" ncu -g

 azure-cli            0.9.15  →   0.9.20
 babel-eslint          4.1.5  →    6.0.4
 browser-sync         2.10.0  →   2.12.5
blahblah

@nielsgl
Copy link
Contributor

nielsgl commented Apr 26, 2016

@tw0517tw thanks a lot for the info, that's really helpful. It looks like the issue is the opposite of the homebrew issue, I'm going to have a look at it in a few hours.

@glen-84
Copy link

glen-84 commented Apr 29, 2016

This might be relevant:

Does this have anything to do with the fact that there is an npmrc file in C:\Program Files\nodejs\node_modules\npm that points to prefix=${APPDATA}\npm (the actual location of my global packages)?

@PaulBaugher
Copy link

Windows here.
ncu -g works as noted

ncu
just hangs and does nothing in my project folder.

@raineorshine
Copy link
Owner

raineorshine commented Aug 18, 2016

@digitalskyline Have you checked the known issues? Try --loglevel verbose to see if it's the stdin problem.

@PaulBaugher
Copy link

It does appear to be the "stdin" problem. Thanks...

ncu --loglevel verbose
Initializing...
Running in local mode...
Finding package file data...
Waiting for package data on stdin...

@tw0517tw
Copy link
Contributor

tw0517tw commented Aug 19, 2016

@digitalskyline if you're using Git Bash, try winpty ncu.cmd.
ref. #136 (comment)

@PaulBaugher
Copy link

Using mobaxterm, but I'll give that a shot.

@emigenix
Copy link

emigenix commented Aug 20, 2016

Any updates on the issue of @digitalskyline ? I have the same exact issue under my (otherwise working) Cygwin installation. @raineorshine What is expected from stdin? Also where to get winpty? EDIT From here.

@emigenix
Copy link

@tw0517tw winpty ncu doesn't work.

$ winpty ncu
Could not start 'ncu': The system cannot find the file specified. (error 0x2)

But this does, kind of:

$ winpty `which ncu.cmd`
No package.json
Please add a package.json to the current directory, specify the --packageFile or --packageData options, or pipe a package.json to stdin.

@tw0517tw
Copy link
Contributor

@emigenix Thanks, I've updated the comment above.

@raineorshine
Copy link
Owner

This ticket is for the global npm prefix issue on Windows. I'm going to ask that discussion related to the stdin issue be discontinued here and directed towards #136. @emigenix You can find more details about TTY on windows (which is used to detect stdin) in that issue.

TLDR; ncu appears to be using the appropriate and expected method of detecting stdin, but certain Windows terminals do not set isTTY correctly, causing the miscommunication. I would be surprised if there was no good cross-platform solution, but so far no one has suggested one.

As mentioned above, please continue discussion of the stdin issue on #136. Thanks!

@maitredede
Copy link

Hi,
I have windows and this problem too.

C:\Users\memyselfandi>ncu -g
 npm  2.15.9  →  3.10.8

C:\Users\memyselfandi>npm list -g --depth=0
C:\Users\memyselfandi\AppData\Roaming\npm
+-- bower@1.7.9
+-- bower-update@0.2.0
+-- coffee-script@1.10.0
+-- generator-aspnet@0.2.3
+-- graceful-fs@4.1.3
+-- grunt@1.0.1
+-- grunt-cli@1.2.0
+-- gulp@3.9.1
+-- lodash@4.11.1
+-- npm@3.10.8
+-- npm-check@5.2.1
+-- npm-check-updates@2.8.0
+-- source-map@0.5.5
+-- supports-color@3.1.2
`-- yo@1.8.5

NCU detects NPM version of installed nodejs, not the actual global installed version.

C:\Users\memyselfandi>npm -g prefix
C:\Users\memyselfandi\AppData\Roaming\npm

C:\Users\memyselfandi>npm root -g
C:\Users\memyselfandi\AppData\Roaming\npm\node_modules

I double-checked that my PATH has the good npm version first.

@raineorshine raineorshine changed the title ncu -g not working on windows NCU detects NPM version of installed nodejs, not the actual global installed version (Windows) Oct 2, 2016
@anantoghosh
Copy link
Contributor

I've been trying to figure this for hours. Here's how I understand it now, please feel free to correct me where ever you feel I am wrong.
First of all ncu spawns the npm process when looking for local packages, but uses npm api for global packages.

From lib/package-managers/npm.js

// if packageFile is specified, spawn an npm process so that installed modules can be read from the same directotry as the package file (#201)
        return options.cwd ?
            spawn('npm', ['ls', '--json', '-depth=0'], {cwd: options.cwd})
                .then(JSON.parse)
                // transform results into a similar format as the API
                .then(function (results) {
                    return {
                        dependencies: cint.mapObject(results.dependencies, function (name, info) {
                            return cint.keyValue(name, {
                                name: name,
                                version: info.version
                            });
                        })
                    };
                }) :
            npm.commands.listAsync(args || [], true); // silent:true

The npm api is not supposed to be used anymore by the anyone else but the npm developers npm/npm#8283 as it does not follow semver and may change anytime.

I looked in the bundled npm and found that
On windows, npm creates a npm.cmd executable script to set prefix and then run npm-cli.js

Output from bundled npm.cmd (from node_modules/npm)

./npm.cmd config ls -g
; cli configs
global = true
user-agent = "npm/3.10.8 node/v6.9.1 win32 x64"

; builtin config undefined
prefix = "C:\\Users\\Ananto\\AppData\\Roaming\\npm"

; node bin location = D:\Installs\Nodejs\node.exe
; cwd = C:\Users\Ananto\Documents\GitHub\npm-check-updates
; HOME = C:\Users\Ananto
; "npm config ls -l" to show all defaults.

vs
Output from npm-cli.js (again from node_modules/npm)

 node .\node_modules\npm\bin\npm-cli.js config ls -g
; cli configs
global = true
user-agent = "npm/3.10.10 node/v6.9.1 win32 x64"

; node bin location = D:\Installs\Nodejs\node.exe
; cwd = C:\Users\Ananto\Documents\GitHub\npm-check-updates
; HOME = C:\Users\Ananto
; "npm config ls -l" to show all defaults.

Notice the missing environment "prefix" variable.
Also the npm.cmd is running npm-cli.js from the node installed location and npm-cli.js is running the local npm version.

In npm\lib\config\default.js

var globalPrefix
Object.defineProperty(exports, 'defaults', {get: function () {
  if (defaults) return defaults

  if (process.env.PREFIX) {
    globalPrefix = process.env.PREFIX
  } else if (process.platform === 'win32') {
    // c:\node\node.exe --> prefix=c:\node\
    globalPrefix = path.dirname(process.execPath)
  } else {
    // /usr/local/bin/node --> prefix=/usr/local
    globalPrefix = path.dirname(path.dirname(process.execPath))

    // destdir only is respected on Unix
    if (process.env.DESTDIR) {
      globalPrefix = path.join(process.env.DESTDIR, globalPrefix)
    }
  }

On Windows, when "prefix" is missing, npm assigns the global prefix same value as the node install location which should have been %appdata%.
Notice the for the *nix part, in case the prefix was not set, use /urs/local/ folder, which actually is the correct default path for global npm package installation for *nix platform.

In my opinion the better move would be to stop using the npm api directly and spawn the npm process for global installs as well. (it may also solve stdout errors)

Currently, as a workaround, we can manually set the prefix to appdata on windows.
In lib/package-managers/npm.js after npm.loadAsync

if (process.platform === 'win32' && npm.config.get('global')) {
    npm.config.set('prefix', process.env.AppData + '\\npm');
}

This works on windows 10, I am not sure about older versions of windows.
@raineorshine
I can create a pull request if you like.

sidenote: ncu is failing a lot of tests on windows.

@raineorshine
Copy link
Owner

@anantoghosh First of all, THANK YOU for taking the time to really delve into the problem. Nobody wants to spend hours troubleshooting someone else's library, but it's what makes open source possible. I especially appreciate it since I don't have an easy way to test on Windows. ncu is being used by a lot of people to only have me actively working on it.

I am in agreement with you; ncu should spawn npm in all instances. It really should never have used the unofficial npm api to begin with, but I was naive at the time and, well, legacy code...

To be fully clear, switching to a spawned npm would clear up this problem completely, yes?

Would love your pull request in the interim.

Thanks!!! 🤗

@anantoghosh
Copy link
Contributor

@raineorshine Thank you for spending your time on making such a useful product. I saw that you are managing the whole project yourself! You have helped so many people!

spawn, on Windows, actually doesn't work when just using .cmd script's name (npm on windows is actually npm.cmd). It crashes with ENOENT error.
So for windows, we must write spawn('npm.cmd' .... or for node v6+ pass {shell: true} https://nodejs.org/api/child_process.html#child_process_spawning_bat_and_cmd_files_on_windows
There is also a spawn related package https://www.npmjs.com/package/cross-spawn

executing npm.cmd should set the required prefix variable and clear up this problem.

Earlier, I think, I understood it wrong that spawn was executing for local packages, but I see now that it only executes when --packageFileDir option is set.
In that case it does crash on windows as expected.

Unhandled rejection Error: spawn npm ENOENT
    at exports._errnoException (util.js:1026:11)
    at Process.ChildProcess._handle.onexit (internal/child_process.js:193:32)
    at onErrorNT (internal/child_process.js:359:16)
    at _combinedTickCallback (internal/process/next_tick.js:74:11)
    at process._tickCallback (internal/process/next_tick.js:98:9)

Here is a test file

var spawn = require('child_process').spawn;
// Create a child process, npm.cmd for windows, just writing 'npm' will crash
var child = spawn(process.platform === 'win32'? 'npm.cmd' : 'npm', ['config', 'ls', '-g']);

child.stdout.on('data',
    function (data) {
        console.log('output: ' + data);
    });

child.stderr.on('data', function (data) {
    console.log('stderr: ' + data);
});

child.on('close', function (code) {
    console.log('child process exited with code ' + code);
});

Output on Windows 10 x64:

output: ; cli configs
global = true
user-agent = "npm/3.10.8 node/v6.9.1 win32 x64"

; builtin config undefined
prefix = "C:\\Users\\Ananto\\AppData\\Roaming\\npm"

; node bin location = D:\Installs\Nodejs\node.exe
; cwd = C:\Users\Ananto\Documents\GitHub\npm-check-updates
; HOME = C:\Users\Ananto
; "npm config ls -l" to show all defaults.


child process exited with code 0

prefix was set as expected.
In short, if npm cli works then this will work as well.


Do you have a separate fork or branch for development? I don't think it would be a good idea to add the workaround to master without testing it in other versions of windows.

@raineorshine
Copy link
Owner

raineorshine commented Nov 26, 2016 via email

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

Successfully merging a pull request may close this issue.

10 participants