search

balena-io-modules / drivelist

List all connected drives in your computer, in all major operating systems
Apache License 2.0
243 stars 89 forks source link

Support for CentOS 7 #316

Open jgbradley1 opened 5 years ago

jgbradley1 commented 5 years ago

drivelist is a dependency for one of the extensions in the Eclipse Theia project. I've been building a CentOS 7 docker image of Theia and see error messages that appear to indicate inconsistent behavior based on the OS. Those error messages do not show up under alpine linux.

Can someone take a look at the error message and tell me if the problem is with drivelist or another package?

lurch commented 5 years ago

I think that's already been fixed - if you look at https://github.com/balena-io-modules/drivelist/blob/master/lib/lsblk/index.js you can see that it first tries to do lsblk --bytes --all --json --paths --output-all and if that fails it then tries lsblk --bytes --pairs --all. Are you using the latest version of drivelist?

lurch commented 5 years ago

And BTW I'm not sure the way you're using drivelist in https://github.com/theia-ide/theia/blob/master/packages/filesystem/src/node/node-filesystem.ts#L336 is "correct"? A "drive" in drivelist is a physical drive (e.g. /dev/sda in Linux) and a drive may have multiple partitions (e.g. /dev/sda1, /dev/sda2, /dev/sda3, etc. each of which might be mounted at a different location); whereas your code seems to be assuming that each drive will only map to a single filesystem URI?

Although it's been a long time since I've written any NodeJS code, so apologies if you are doing things correctly and I simply didn't understand your code properly! :slightly_smiling_face:

jgbradley1 commented 5 years ago

I forced yarn to use the latest version of drivelist (v7.0.1 currently) and I still get the same error about lsblk failing. I do see in the source code where it falls back to lsblk --bytes --pairs --all which should work under centos7. Will need to do some more testing to track down where the problem is.

jgbradley1 commented 5 years ago

@kittaakos It looks like you contributed this portion of the code. Will getDrives() fail when a disk has multiple partitions? Also, how are volumes handled?

kittaakos commented 5 years ago

Will getDrives() fail when a disk has multiple partitions?

It should not.

Also, how are volumes handled?

I do not understand this. If you feel, we can improve this in the Theia project; please open a PR there. I am happy to review your changes.

BTW, after creating a few dummy APFS volumes on my mac, I can see them. I have never tried it on CentOS, but some else has already expressed his concern about my code. See here. If you can figure out what is wrong, we can fix it.

frankLife commented 5 years ago

It looks like centos7's lsblk don't have the options of --json and --output-all image image

and It can run with the options of --bytes --all --paths of --bytes --pairs --all image image

and the the options of all image