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

npm install commands fail on Android 8 - os.cpus() returns undefined #1798

Closed
shannon opened this issue Nov 15, 2017 · 35 comments
Closed

npm install commands fail on Android 8 - os.cpus() returns undefined #1798

shannon opened this issue Nov 15, 2017 · 35 comments
Labels
nodejs Issue is about Node.js related stuff, including npm

Comments

@shannon
Copy link

shannon commented Nov 15, 2017

I don't know if this a node issue or a termux issue but I figured I would start here.

If I install nodejs via pkg install nodejs whenever I run npm commands I get an error

$ npm i -g http-server
npm ERR!  Cannot read property 'length' of undefined

npm ERR!  A complete log of this run can be found in:
npm ERR!       /data/data/com.termux/files/home/.npm/_logs/2017-11-15T15_21_06_474Z-debug.log

After reviewing the logs I tracked the error down to this line in a npm dependency.

It seems that os.cpus() returns undefined. I confirmed this using the node command prompt.

> var os = require('os')
undefined
> os.cpus()
undefined

I also tried nodejs-current but no luck. Could this be an issue with termux?

@shannon
Copy link
Author

shannon commented Nov 15, 2017

I should also note, other os methods seem to work ok.

> os.arch()
'arm64'
> os.release()
'4.4.56-g594d847d09a1'
> os.platform()
'android'
> os.type()
'Linux'

@Quasic
Copy link
Contributor

Quasic commented Nov 15, 2017

Well, the wrapper function is rather simple, so I'm not sure if this will help, but you can try this test script:

const cpuValues = new Float64Array(6 * process.binding("util").pushValToArrayMax);
function f(){console.log(arguments);}
console.log(process.binding("os").getCPUs(f,cpuValues,[]));
console.log(cpuValues);

@shannon, I edited this to log the [], as well, so now this should log an Arguments array of cpus, then the [], then an array of numbers, if everything's working. If it lists undefined instead of one or more of these, it might narrow down the problem a bit.

@fornwall
Copy link
Member

@shannon Are you running the latest version of the nodejs package (node --version == v8.9.1)?

I tested os.cpus() on an aarch64 device and it worked, but this was with a 3.10.84-perf-g99119bc kernel. Could it be kernel specific (or perhaps depending on SELinux rules)?

@shannon
Copy link
Author

shannon commented Nov 15, 2017

@Quasic I tried this and it outputs the following:

(I truncated all the 0s)

$ node test.js
undefined
Float64Array [
  0,
  0,
  0,
...
]

@fornwall yes I tried version 8.9.1 and 9.2.0. Both result in the same issue.

I just got this phone last week and I haven't done anything in termux yet so I don't think it was anything I did. I even uninstalled an reinstalled termux to see if it would resolve it.

I did notice that my os.arch() says "arm64" and not "aarch64" but I assume this is ok.

@Quasic
Copy link
Contributor

Quasic commented Nov 15, 2017

Just for the record, it's working in mine, which is

os.arch()
'arm'
os.release()
'3.10.49-gecffe94'

$ node --version
v8.9.1

@shannon
Copy link
Author

shannon commented Nov 15, 2017

So I tried it on my wife's Pixel XL (not 2) and I get the same error.

os.arch()
"arm64"
os.release()
"3.18.52-g99dda0323132"

$ node --version
v8.9.1

@Quasic
Copy link
Contributor

Quasic commented Nov 15, 2017

@shannon
Well, I doubt it's setting the timing array, with no nonzero values.
It also appears to be ignoring the callback?

You could see if "callback" is logged (should be on the first line) and see if the 0s are all replaced by -1 when running this:

const L=6 * process.binding("util").pushValToArrayMax;
const cpuValues = new Float64Array(L);
for(var i=0;i<L;i++)cpuValues[i]=-1;
function f(){console.log("callback");console.log(arguments);}
console.log(process.binding("os").getCPUs(f,cpuValues,[]));
console.log(cpuValues);

I think the native function is basically acting empty.

@shannon
Copy link
Author

shannon commented Nov 16, 2017

@Quasic This script outputs all -1 now so it does seem to be calling the callback.

I found a similar bug reported against freebsd here:

https://bugs.freebsd.org/bugzilla/show_bug.cgi?id=221762

It seems that if the something doesn't work in libuv it might get be causing it to returned undefined. I don't think the patch provided in that bug applies here but it could give us a lead. I don't really know how to go about testing this functionality in libuv though.

@brianppoole
Copy link

I seem to be running into the same issue on my Pixel 2 running Node v.8.9.1.

$ npm i -g http-server
npm ERR! Cannot read property 'length' of undefined

npm ERR! A complete log of this run can be found in:
npm ERR!     /data/data/com.termux/files/home/.npm/_logs/2017-11-16T00_08_35_087Z-debug.log
$ node
> var os = require("os")
undefined
> os.arch()
'arm64'
> os.release()
'4.4.56-g594d847d09a1'

Anything I can do to help diagnose?

@Quasic
Copy link
Contributor

Quasic commented Nov 16, 2017

@shannon, the -1 test was to verify that it wasn't changing the Float64Array. The callback should log the word "callback" if it is called.

I found that the Windows variant apparently used to return -1 ?? before nodejs/node-v0.x-archive@9557020

@shannon
Copy link
Author

shannon commented Nov 16, 2017

@Quasic oh sorry I misread what you wrote. "callback" is NOT output so you are correct.

node test.js
undefined
Float64Array [
  -1,
  -1,
  -1,
  -1,
...

@Quasic
Copy link
Contributor

Quasic commented Nov 16, 2017

The patch attached to the last post of the bugzilla report looks promising...

@Quasic
Copy link
Contributor

Quasic commented Nov 16, 2017

@Quasic
Copy link
Contributor

Quasic commented Nov 16, 2017

@shannon, @brianppoole Is this on Android Nougat or Oreo? https://github.com/nodejs/node/blob/master/deps/uv/src/unix/linux-core.c uses /proc/stat to retrieve the cpu info, which means termux/termux-app#299 could apply here.

@shannon
Copy link
Author

shannon commented Nov 16, 2017

@Quasic, yea this is on Oreo 8.0.0. That looks like the problem alright :-/

@Quasic
Copy link
Contributor

Quasic commented Nov 16, 2017

libuv/libuv#1459

Quasic added a commit to Quasic/node-worker-farm that referenced this issue Nov 16, 2017
@shannon
Copy link
Author

shannon commented Nov 16, 2017

thanks @Quasic

I've opened an issue here npm/npm#19170. It would be good if npm worked without relying on this functionality or gracefully handled the lack of it.

I was able to workaround the issue myself by changing /data/data/com.termux/files/usr/lib/node_modules/npm/node_modules/worker-farm/lib/farm.js to use a hardcoded 4 cpus. This is not really ideal though for obvious reasons.

@shannon shannon changed the title npm install commands fail on Pixel 2 XL - os.cpus() returns undefined npm install commands fail on Android 8 - os.cpus() returns undefined Nov 16, 2017
@Quasic
Copy link
Contributor

Quasic commented Nov 16, 2017

I made a PR that falls back to 1, but hopefully someone who knows the code will know a better way.

rvagg pushed a commit to rvagg/node-worker-farm that referenced this issue Nov 16, 2017
@Quasic
Copy link
Contributor

Quasic commented Nov 19, 2017

npm using the current node-worker-farm won't fail, but it will disable concurrent workers. I started patching libuv, which requires more /proc/stat info be mapped to Android sources than just virtual cpu number. If it works, maybe we could use proot or something to generate /proc/stat? I also have perhaps a less ambitious option, in case this doesn't work, suggested at node-worker-farm.

@SDRausty

This comment was marked as spam.

@Quasic
Copy link
Contributor

Quasic commented Dec 2, 2017

It was reported in npm/npm#19170

@fornwall fornwall added the nodejs Issue is about Node.js related stuff, including npm label Dec 3, 2017
@F1LT3R
Copy link

F1LT3R commented Feb 15, 2018

Thanks @shannon, I did what you did. But I changed: /data/data/com.termux/files/usr/lib/node_modules/npm/node_modules/worker-farm/lib/farm.js to use 4 maxConcurrentWorkers in DEFAULT_OPTIONS. Works perfectly on the Pixel 2 XL.

@gund
Copy link

gund commented Mar 12, 2018

Same error on Pixel 2 device and both stable and current versions of NodeJs.

Patching farm.js works for now but obviously is not a good solution...

@F0RIS
Copy link

F0RIS commented Sep 4, 2018

Why npm still is not fixed!!?

@fornwall
Copy link
Member

The nodejs package has been updated to the current 10.10.0 version of node. Does this work better now?

@MaxPelly
Copy link

The nodejs package has been updated to the current 10.10.0 version of node. Does this work better now?

No, still returning undefined
screenshot_termux_20180916-200025

@ghost
Copy link

ghost commented Sep 16, 2018

This error is related to unreadable /proc/stat file.

If I copy this file from another device (PC) and bind it with proot, then os.cpus() will work:

  1. Test script (./test.js):
var os = require("os")
console.log(os.cpus())
  1. Dummy file (./stat) for /proc/stat:
cpu  5115690 2798653 1880760 7948594 13293 227304 70323 0 0 0
cpu0 2441336 1338502 1065622 6886762 11873 173371 50618 0 0 0
cpu1 2674353 1460151 815138 1061832 1420 53933 19705 0 0 0
intr 325594253 9 40544 0 0 0 0 0 0 1 26465 0 0 24398555 0 0 0 1484163 0 0 0 0 0 0 149 0 0 666186 3551761 21 722 61752469 5569088 8993 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 
0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 
0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 
0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 
0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 
0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 
0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0
ctxt 801588897
btime 1536941305
processes 2420179
procs_running 1
procs_blocked 0
softirq 130904467 1473985 60414110 76040 726869 3416486 5 3515545 29874514 145808 31261105
  1. When using original /proc/stat (this will fail on Android 8):
$ node test.js
undefined
  1. Binding custom file to /proc/stat with proot (this will work):
$ proot -b ./stat:/proc/stat node test.js
[ { model: 'unknown',
    speed: 442,
    times:
     { user: 244133600,
       nice: 133850200,
       sys: 106562200,
       idle: 688676200,
       irq: 17337100 } },
  { model: 'unknown',
    speed: 442,
    times:
     { user: 267435300,
       nice: 146015100,
       sys: 81513800,
       idle: 106183200,
       irq: 5393300 } } ]

@daveball
Copy link

This problem still exist but I think if you use yarn instead of npm and clone node-red from github the it work, just trying this now do will update you all once I've confirmed

@imkiva

This comment has been minimized.

@ghost

This comment has been minimized.

@imkiva

This comment has been minimized.

@ghost

This comment has been minimized.

@imkiva

This comment has been minimized.

@ghost

This comment has been minimized.

@ghost ghost deleted a comment from imkiva Sep 23, 2019
@ghost
Copy link

ghost commented Sep 23, 2019

Closing since npm no longer produces mentioned error. os.cpus() still returns undefined, but not so much can be done with that.

This issue was closed.
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
nodejs Issue is about Node.js related stuff, including npm
Projects
None yet
Development

No branches or pull requests