Add compatibility with ps from BusyBox
#173
Conversation
paescuj
changed the title
ps from BusyBox
| @@ -35,27 +35,16 @@ function parseTime (timestr, centisec) { | |||
| * @param {Function} done(err, stat) | |||
| */ | |||
| function ps (pids, options, done) { | |||
| const pArg = pids.join(',') | |||
| let args = ['-o', 'etime,pid,ppid,pcpu,rss,time', '-p', pArg] | |||
There was a problem hiding this comment.
my issue with this is that ps forwards way more data then needed no? Isn't there a way to do this only for busybox if there's no -p, or maybe that busybox has a procfs?
There was a problem hiding this comment.
Thanks for your quick review!
Yeah, I had the same concerns, but opted for this simple way because I assumed it wouldn't make much difference, as the output is usually very small since only the processes of the current user are printed.
That made me realize though, that the current solution wouldn't work on non-BusyBox ps because of this exact reason! We would have to pass the ax flags which indeed means the output can become quite large.
Two alternative ideas:
- Try to spawn
pswith the-pflag, and in case of error, check the output forps: unrecognized option: pand then respawn again without the-pflag. - Check if
pscomes from BusyBox by doing a preflightreadlinkcall on/bin/ps. The result should be/bin/busyboxin that case. Then we could apply the logic from this PR only if it's BusyBox.
The first option would mean no performance degradation for non-BusyBox platforms, but a little one for BusyBox. Therefore probably a bit more reasonable than the second option... WDYT?
There was a problem hiding this comment.
spawning is really slow, I'd go for 2nd
The
psimplementation of BusyBox doesn't support the-pargument. This PR replaces the usage of-pwith own filter logic in the line processing loop. This enablespidusage(with ps) to work on BusyBox based platforms like Alpine Linux.