[BUG] On macOS, total disk size is reported incorrectly for APFS drives about archey4 HOT 9 CLOSED

So, it appears APFS has containers which macOS gives device paths to that look like disks - it seems disk1 and disk3 are two APFS containers so inside them all volumes get the full free space but have separate used space.

Unfortunately I'm not sure the idea of using the available space to calculate used space will work since we still need some way to distinguish between APFS and say an HFS partition on any disk.

The only way I can really think to do this would be to try and execute diskutil list (or diskutil apfs list?) and parse its output for the filesystem type to discard APFS volumes as "duplicates" of their containers while keeping their used space. @HorlogeSkynet any thoughts on that? Since it may be a little clumsy, it's kind of like an ifdef macOS escape hatch 😆

from archey4.

I've written a few commits as the start of an approach to this issue with the diskutil approach - luckily the Python standard library has a nice parser for Apple's property list formats. Looking at the tests they still need modifying to pass when run in macOS too.

@a-hurst would it be possible for you to test the latest commit on bug/macos-disk? I don't have a mac so I can't say for definite whether using diskutil in this way is particularly slow or not (...or even whether it actually works at all, in fact).

@HorlogeSkynet I had a look at what neofetch and screenfetch do - it looks like they solved it by only ever considering / by default, unless you specify other directories. Perhaps this isn't such a bad idea after all...
Since macOS has supported APFS for a while then this bug would also be present on any mac using a disk with more than one APFS volume on it, so unfortunately it needs applying to all macs rather than arm only.

Anyway, let me know what you think, whether it's worth approaching it like this or maybe going the neofetch/screenfetch way.

from archey4.

@HorlogeSkynet Yep it seems to work now, thanks! Not sure where the "invalid field" warning is coming from, but it happens on the master branch of @ingrinder's fork too so I don't think it's a result of the patch.

from archey4.

Hmm, it's been a while since I touched any of the code here, but IIRC this is similar to the issues with btrfs which was solvable by just eliminating duplicate mountpoint entries for the same device path/filesystem. However, it looks as though all of the entries are going to be needed here - the total usage (based on the Avail column) is all of the volumes added together:

I'll have a proper look at this later tonight. By the way, you can probably set a config file specifying just the / mountpoint as a workaround for the time being.

Quick edit to add (before I forget!) - perhaps this calculation could be taken advantage of, and we use the available space to calculate the usage? I'm not sure if this may be problematic on other filesystems, I'll have to check.

from archey4.

Thanks for digging this up again @ingrinder !
I'm not against parsing diskutil output (JSON support ? 🙃), but here are my concerns about doing this, for this entry or any other one :

• Apple binaries may be very slow to run (as system_profiler IIRC), so if we really need to do this, we might have to implement disk caching ;
• Indeed we would need to protect such code by an if depending on the platform (if only M1 and onwards are concerned, we should take this into account too) ;
• We should also check how neofetch/screenfetch (will ?) solve this issue, I'll take a look if needed.

Bye, thanks again 👋

from archey4.

Hey @ingrinder ! Many thanks for your findings and your work 👌

@a-hurst any chance you've been able to try the bug/macos-disk branch ? 🙂
If it actually fixes your issue, I will give a proper review to #116.

Thank to both of you, bye 👋

from archey4.

Many thanks for your reply, I'll handle #116.
About the warning, this may be one of two added bugs on master, targeting the next version. I'll take a look at this too. Bye 👋

from archey4.

Closing here as #116 is ready for review/merge 🙏

from archey4.

Released on Thursday as v4.14.0.0 @a-hurst ! Thanks, bye 👋

from archey4.