Add std::fs::{Home|Media}Dirs#158936
Open
CAD97 wants to merge 43 commits into
Open
Conversation
This could reasonably be expected to load the OS-specific defaults. Remove the impl to force users to explicitly choose. This also allows us to impl Default with that behavior later, if we so desire.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
Collaborator
|
r? @JohnTitor rustbot has assigned @JohnTitor. Use Why was this reviewer chosen?The reviewer was selected based on:
|
Collaborator
|
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
Contributor
Author
|
That took longer to resolve CI than I would've liked… but we're green now and ready for review without any caveats. @rustbot ready I might come back to this in anger and rebase and squash these commits to present a cleaner history, or I might not. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
View all comments
std::fs::UserDirsor similar libs-team#830Replacement for
std::os::unix::xdgas suggested by libs-api in #157515 (comment). Exposes media directories common between the three big OSes in addition to the cache/config/data/state directories under a separate feature gate. API summary:This implementation diverges from the
directoriescrate's mapping in that we setstate_dirin the non-unix constructors (to~/Library/Application Supporton Darwin and%APPDATA%on Windows). This mapping is derived from the idea that "state" files are application support files that are not important nor portable enough to the user to be "data" files.The XDG paths are as described in the XDG Base Directories Specification and the xdg-user-dirs tool.
$XDG_CONFIG_DIR/user-dirs.dirsis parsed directly to avoid delegating to potentially arbitrary shell execution.The Darwin paths are loaded via the
sysdir(3)API fromlibSystem.dylib(introduced in macOS 10.12 with a similar timeline for other Darwin OSes, deprecating the earlierNSSystemDirectories.hAPI). Using the File System Effectively points to preferring the Foundation framework'sNSSearchPathForDirectoriesInDomain(_:_:_:)orNSFileManager.URLForDirectoryinstead, but calling those correctly requires an active Objective C autorelease pool, IIUC. TheLibrary/Application Supportdirectory is used forconfig_home,data_home, andstate_home; the Apple documentationThe Library Directory Stores App-Specific Filesdirectly calls out placing data and configuration files inLibrary/Application Support, and state files are just less user-meaningful data files.The Windows paths are loaded via the Known Folders API (introduced in Vista).
config_homeanddata_homeare placed inAppData\Roamingas files intended to be important and portable to the user, whilecache_homeandstate_homeare placed inAppData\Localas files that aren't.I'm not fully confident about the handling of the XDG base directory paths which don't have good cross-platform analogs, as well as the exact API for the search path dealing functions, but I'm confident that the shape of the rest of the API does match the stdlib API style. Common paths are platform-independent enough of a needed concept to be exposed by std, IMHO, but platform-specific that a struct with public fields (even
#[non_exhaustive]) seems incorrect, specifically because of platform-specific paths that we may want to expose like is already done for XDG.The one API change I could see doing is moving
state_dirinto the XDGUserDirsExt. I chose not to do this for this initial implementation, though, as getting the ideal choice of fallback for both Darwin and Windows can't be achieved in an OS-agnostic way:~/Library/Caches~/AppData/Local~/Library/Application Support~/AppData/Roaming~/Library/Application Support~/AppData/Roaming~/Library/Application Support~/AppData/LocalA more drastic change would be to move all four onto the unix
UserDirsExt, addingcaches/application_supportto the DarwinUserDirsExtandroaming_app_data/local_app_datato the WindowsUserDirsExt. This would be more "correct" but seems a bit heavy-handed, as it would mean applications need to pull in OS-specific extension traits just to place their support files in something more appropriate than a~/.appnamedirectory.We could also separate the "home" directory API from the "media" directory API. I'm neutral on this with one relevant note: the app-specific cache/config/data/state files need a subdirectory named after the application, so "
ProjectDirs" would exclude the media directories; it could make sense to have a type with just those and apush_application_subdirmethod.Switching the impl to using a pal
imp::UserDirscould be reasonable, but seems at odds with the desire to have the target agnostic way to "build your own"UserDirs. AnExtraUserDirsinstead of the#[allow(dead_code)]fields would make sense, I just didn't know how to best set up that in the pal layer.Disclaimer: This was worked on as part of my employment at Canonical. I initially proposed it independently of my employment, but improving std's functionality is part of my job description, so Canonical told me I should use work time on it.
AI Disclosure: I did not use AI to generate any of the code, with a partial exception for VSCode's AI-assisted smart autocomplete helping with the repetitive parts of the code. All nontrivial code was handwritten. As an experiment, I did use some AI to assist in exploring the problem and API design spaces.
I tested locally on my Ubuntu developer machine, but am relying on CI for Darwin and Windows tests. 🤞
user_home(toNSHomeDirectoryandFOLDERID_Profilerespectively) instead ofenv::home_dir? (Shouldenv::home_dirbe changed to call those?)SHGetKnownFolderPatheagerly? If so, how? Addstd::fs::{Home|Media}Dirs#158936 (comment)set_*methods do any kind of validation, such as ensuring the path is non-empty or even absolute?set_*methods takeimpl AsRef<Path>instead ofPathBuf? (Would introduce needless copies without a separate ownership-taking option likereplace_*below.)fn replace_*(&mut self, x: Option<PathBuf>) -> Option<PathBuf>style methods to allow transferring ownership and setting paths back toNone?NSSearchPathDirectorymake sense to expose in the DarwinUserDirsExt?KNOWNFOLDERIDmake sense to expose in the WindowsUserDirsExt?cc @joshtriplett @nia-e