-
-
Notifications
You must be signed in to change notification settings - Fork 326
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
Overhaul Apple disk implementation improvement #855
Overhaul Apple disk implementation improvement #855
Conversation
80b58de
to
20bff69
Compare
It looks like a great start. Mostly nits at this point. Thanks for working on this! |
Hi @complexspaces. Do you plan to continue working on this PR? |
Hey there @GuillaumeGomez. I do plan on working on this again, sorry. I have been busy enough with work that this one got de-prioritized. I'll try and have it cleaned up by the weekend at the latest. |
Don't worry, no hurry. Just wanted to check if you still planned to work on it. |
c323fae
to
9831f51
Compare
One comment to resolve and one comment to add and it'll be ready to go. I'm so excited about these changes to be merged. :) |
for obtaining disk info on macOS
9831f51
to
13c32c9
Compare
Thanks a lot for this work! |
Thank you for working through this with me, I'm glad to see it merged 🎉 |
This PR rewrites a significant portion of the Apple disk handling code, partially in response to #841. Some of the notable improvements are:
In more detail, this PR completely replaces
DiskArbitration.framework
withCoreFoundation
's volume handling instead. This was done for three reasons:CoreFoundation
allows accessing the "visibility" property of a volume/disk. This is very helpful for filtering out internal partitions and snapshots.DiskArbitration.framework
which is solely available on macOS.By utilizing
kCFURLVolumeIsBrowsableKey
, a volume property that tells you if its browsable on the system by default, which matches other Apple apps such as Finder volumes, disk utility, and system information's storage devices list. Using this framework also allows us to access better space estimation values that match what the rest of the system would report.In addition, I took the chance to remove the "hacky" implementation of disk medium type detection and replace it with something well-documented via IOKit. This change is also compatible with the default macOS sandbox, further reducing the feature gap of the crate with and without the
apple-sandbox
feature enabled.Finally, I removed the crate's build script to improve compilation times. Ever since Objective-C code was removed from the crate, it hasn't been strictly needed. The
#[link]
attribute can do the same thing the current version of it was doing to make sureIOKit
functions and constants are available.This PR was tested in and outside of the default macOS app sandbox with a variety of external drives attached, and inside the iOS simulator and on a real iOS device.
Closes #841
Footnotes
In order to make some use cases possible, the disk list now returns
/System/Volumes/Data
too. ↩Disk type detection is currently unsupported on iOS due to version requirements. ↩