-
Notifications
You must be signed in to change notification settings - Fork 324
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
Comments
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.
If anyone else can reproduce and debug that would be helpful. |
@metaraine I reported it for windows not OSX. |
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! |
@metaraine: have a look at AppVeyor. I use it for my projects like grunt-contrib-plugins. Have a look there for configs. |
Been working on a fix. Try this:
(Duplicate of #146) |
getting following error:
|
|
ncu seems to use the |
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. |
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 Thanks. I believe the issue on Windows is that we got 2 different I got these on Windows:
and with adding your
|
I can get correct result with modifying the prefix
|
@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. |
This might be relevant:
|
Windows here. ncu |
@digitalskyline Have you checked the known issues? Try |
It does appear to be the "stdin" problem. Thanks... ncu --loglevel verbose |
@digitalskyline if you're using Git Bash, try |
Using mobaxterm, but I'll give that a shot. |
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 |
@tw0517tw $ 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. |
@emigenix Thanks, I've updated the comment above. |
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! |
Hi,
NCU detects NPM version of installed nodejs, not the actual global installed version.
I double-checked that my PATH has the good npm version first. |
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. 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 Output from bundled npm.cmd (from node_modules/npm)
vs
Notice the missing environment "prefix" variable. 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 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. 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. sidenote: ncu is failing a lot of tests on windows. |
@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 To be fully clear, switching to a spawned Would love your pull request in the interim. Thanks!!! 🤗 |
@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. executing Earlier, I think, I understood it wrong that spawn was executing for local packages, but I see now that it only executes when
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:
prefix was set as expected. 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. |
Okay, that sounds good! If you open a pull-request, it will have a separate
branch automatically. We can test it before the merge.
…On Fri, Nov 25, 2016 at 4:06 PM anantoghosh ***@***.***> wrote:
@raineorshine <https://github.com/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 crashvar 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.
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
<#163 (comment)>,
or mute the thread
<https://github.com/notifications/unsubscribe-auth/AAtyxFN4x0nYijfV_bx-LDEjCL-hB2Ynks5rB2nogaJpZM4Gf87U>
.
|
When I do
ncu -g
, I get the followingbut actually it should be (at time of writting this 11-11-2015 07:53:00 IST)
npm list -g
is follows (at time of writting this 11-11-2015 07:53:00 IST)The text was updated successfully, but these errors were encountered: