Add xdg_basedir API

[CAD97]

View all comments

Implements an API providing easy access to the XDG Base Directories.

I chose to defer runtime_dir (XDG_RUNTIME_DIR) since the lack of a specified default (but specified expectation for applications to determine an alternative option somehow) makes the correct usage (and thus ideal API) less obvious.

• Accepted ACP: ACP: Add XDG Basedirs API to std::os::unix::xdg libs-team#805
• Tracking issue: Tracking Issue for xdg_basedir #157515

[rustbot]

r? @aapoalas

rustbot has assigned @aapoalas.
They will have a look at your PR within the next two weeks and either review your PR or reassign to another reviewer.

Use r? to explicitly pick a reviewer

Why was this reviewer chosen?

The reviewer was selected based on:

• Owners of files modified in this PR: @ChrisDenton, libs
• @ChrisDenton, libs expanded to 11 candidates
• Random selection from LawnGnome, Mark-Simulacrum, aapoalas, clarfonthey, jhpratt

[CAD97]

cc @BurntSushi (+1 on ACP)

[Kobzol]

Might be interesting to @rami3l.

[rami3l]

@Kobzol Thanks for the ping!

Our current plan with @Cloud0310 regarding rust-lang/rustup#247 is slightly more complicated than this because it involves handling both old and new home definitions shared by rustup and cargo. That said, having std APIs for the XDG paths will definitely make this easier.

[aapoalas]

Looks good to me - I had a bunch of comments here and there but nothing that I'd consider blocking.

Let me know your thoughts on the comments and one we've handled them one way or another (made changes or decided to ignore) then we can put this on the train.

View changes since this review

[CAD97]

The bit of the doc comments that touch on installation rather than normal program execution are the last bit I'm not super confident in. Installation is a entirely separate can of worms that I'm not qualified to speak to, so the best I can do is try to recontectualize the GNU Make assuming spec to the Cargo build system here. (Which doesn't even have a concept of build or installation artifacts other than the executable bundle.)

View changes since this review

[CAD97]

{system_data_dir} here is our version of the XDG spec's

$datadir [...] with $datadir defaulting to /usr/share.

AIUI, $datadir doesn't refer to an environment variable like the other $XDG_* variables, but to a conventional GNU Make variable for installation directories. The build-system agnostic version of this would be "program installation data directory base" or similar.

It feels like a cop-out to provide the XDG paths API but still leave an undefined template variable here, but I don't know how we could do better. This specifically is a concern of the installer, not of the installed program, as a properly configured system will ensure that this path is in $XDG_DATA_DIRS so that the installed data is actually found during lookup.

[aapoalas]

Does this suggest that there should be an API to create an installer datadir path, given a $datadir template variable value?

[CAD97]

The current API surface is deliberately not giving app-specific paths, only the base directories that the app-specific folders go in. So such an API would just spit $datadir back out unchanged. $sysconfdir would just append /xdg. In any case, installation isn't really part of the XDG basedir spec, and if application file lookup is a contentious floating target, installers is even more so.

Another way to put the difference is that only the program that creates installable packages or whatever delivery mechanism needs to understand the conventions for installation, whereas every program that isn't just a single stateless executable needs to understand how to access auxiliary files.

[CAD97]

{system_config_dir} here is our version of the XDG spec's

$sysconfdir [...] with $sysconfdir defaulting to /etc.

AIUI, $sysconfdir doesn't refer to an environment variable like the other $XDG_* variables, but to a conventional GNU Make variable for installation directories. The build-system agnostic version of this would be "program installation configuration directory base" or similar.

It feels like a cop-out to provide the XDG paths API but still leave an undefined template variable here, but I don't know how we could do better. This specifically is a concern of the installer, not of the installed program, as a properly configured system will ensure that this path is in $XDG_CONFIG_DIRS so that the installed data is actually found during lookup.

[aapoalas]

The $datadir and $sysconfdir points are interesting.

I'd be fine to merge this as-is, but do you think that is a good idea @CAD97 ? Also what's @orlp 's opinion?

View changes since this review

[rustbot]

Reminder, once the PR becomes ready for a review, use @rustbot ready.

[orlp]

@aapoalas My main concerns I expressed earlier are still unaddressed.

1. XDG basedirs is an unstable specification at 0.8, last updated ~5 years ago. It could change tomorrow but we can't update the Rust standard library.

2. There's no justification given why for example the xdg crate is insufficient for this purpose. I think inclusion in the standard library has a high bar which isn't met here.

Typically inclusion in the standard library is motivated through one of four forms (note, my observation - not official policy):

1. The standard library forms a lingua franca that lets code talk to each other in a common language. E.g. Future, Vec, String Error, Hash, Default, Borrow, Option, Result fall under this.

2. The functionality is fundamental for the language, e.g. UnsafeCell or Any.

3. The functionality provided abstracts over platform-specific details for commonly needed functionality. E.g. std::fs, std::net, GlobalAlloc, std::thread, std::time, etc.

4. The functionality is a platform-specific intrinsic or API. E.g. std::arch or std::os.

5. The functionality is just so common and universally used, it should be available out of the box. E.g. [T]::sort, str::contains, BinaryHeap, etc.

I just don't see this functionality really falling under any of the four categories. It doesn't form a common language, it's not fundamental, it doesn't provide an abstraction and isn't used all that commonly. The closest is nr. 4, but it's not really an API, more a convention, and it's not all that platform-specific even.

[CAD97]

The ACP being accepted means that at least one T-libs-api member has +1'd the inclusion of this API unstably in std. It does not necessarily confer intent to stabilize. What it says is more along the lines that the given problem statement is interesting enough to experiment and learn what a solution provided inside std would look like, and if there are any unforeseen issues.

But to at least partially address some of your points with my reasoning for opening the ACP, @orip:

• While the formal standard version is 0.8, that doesn't mean the standard is unstable. Rather, it has become a living standard adopted by Linux operating system components and popular tooling. Nothing would change if the Freedesktop project were to rubber stamp the current text as "specification version 1.0" because what matters is actual adoption of the conventions, which we have in practice now. Lack of support for XDG paths in developer-facing tooling is generally seen as a bug (albeit often a low priority one made difficult by the need to support existing systems).
• Yes, the stability story of a function returning a path is rough; changing what path is produced in any case could result in changing program behavior due to file lookup locations changing. But because XDG paths are used in the wild, any update to the specification would necessarily include guidance on how to handle the older systems, and we can follow that in updating what our API provides, even if that means changing how some path discovery works.
• This functionality is necessarily part of a platform-independent API exposing auxiliary directories for a program to use. T-libs-api has stated that such an API is a problem worth exploring what a solution in std would look like. This is a necessary part of such a solution. Furthermore, a properly platform-independent API will necessarily expose a simplified "least common denominator" that works everywhere; exposing the platform-specific API allows developers who want to put in that extra work to make their program feel just that little bit more native to actually do so.
• If you're combining multiple libraries that utilize auxiliary data files (as opposed to just compiling everything into the executable), you want them to all look for those files in a consistent location, not via multiple independent ad-hoc conventions.

I, personally, believe that there isn't a one-size-fits-all solution to application paths. Existing applications each have their own unique backwards-compatibility considerations, and even brand new ones have widely different constraints and expectations. But the standard library absolutely can and should provide the pieces that each application can assemble into their domain-appropriate solution.

Aside: your point 4 allows for platform-specific API to be put into std. Does this not qualify because its an interface defined in terms of environment variables, which are technically platform-independent? Does that mean that an API providing the value of the %LOCALAPPDATA% environment variable on Windows would be prohibited, but an API fetching that same OS path by calling SHGetKnownFolderPath(FOLDERID_LocalAppData, 0, NULL, out_ptr) would be allowed?

[CAD97]

I do believe this is reasonable to merge as-is. I also understand that this might get reverted, depending on what T-libs-api decides w.r.t. this problem space. But I don't see any benefit in holding back this PR. It's completely fine to remove unstable API if we determine it's not a good fit for std.

@rustbot ready

View changes since this review

[orlp]

While the formal standard version is 0.8, that doesn't mean the standard is unstable.

I think it is. It was version 0.6 in 2003, and then in 2010 0.7 came out, and in 2021 0.8 came out. There is no indication the standard is finalized. The standard has changed multiple times after appearing stable for a couple years.

Yes, the stability story of a function returning a path is rough; changing what path is produced in any case could result in changing program behavior due to file lookup locations changing.

The stability story with it therefore is "it literally cannot change". Again, not a good fit for the standard library if you predict changes to this could occur in the future.

I, personally, believe that there isn't a one-size-fits-all solution to application paths.

I think that's a strong indicator it doesn't belong in the standard library then, unless you can abstract all possible choices into a one-configured-sized-fits-all solution.

This functionality is necessarily part of a platform-independent API exposing auxiliary directories for a program to use. T-libs-api has stated that such an API is a problem worth exploring what a solution in std would look like.

Yes, that falls under category 3 "provides abstraction". I would not be concerned for fit-for-purpose in that case for the stdlib (although my other concerns around stability would remain).

I don't buy the argument that "because this functionality would be part of an abstraction, it should be provided as a piece". You could apply that for literally all the platform-specific implementations of std::fs, std::net, std::thread, etc, most of which simply aren't exposed in the non-abstract form unless there's a very specific reason to.

If you're combining multiple libraries that utilize auxiliary data files (as opposed to just compiling everything into the executable), you want them to all look for those files in a consistent location, not via multiple independent ad-hoc conventions.

I don't want to use libraries that directly look in specific configuration file locations rather than having their configuration through their API in the first place. If that is the code pattern this is supposed to enable I even more strongly oppose to it.

Aside: your point 4 allows for platform-specific API to be put into std. Does this not qualify because its an interface defined in terms of environment variables, which are technically platform-independent?

Essentially, yes. The point of nr. 4 is to provide nice safe Rust abstractions and avoid having to write FFI or inline assembly. If there's a clear, safe and portable (to all relevant platforms) alternative I don't think you qualify under nr. 4.

Does that mean that an API providing the value of the %LOCALAPPDATA% environment variable on Windows would be prohibited, but an API fetching that same OS path by calling SHGetKnownFolderPath(FOLDERID_LocalAppData, 0, NULL, out_ptr) would be allowed?

I don't know enough about the precise definition of Windows folders to answer this. The latter method sounds safer against accidental environment pollution though.

[ChrisDenton]

I think it is. It was version 0.6 in 2003, and then in 2010 0.7 came out, and in 2021 0.8 came out. There is no indication the standard is finalized. The standard has changed multiple times after appearing stable for a couple years.

That is misleading. The thing added in 0.8 is XDG_STATE_HOME but this is well within rust's normal stability guarantees. No existing application using XDG base directories would be broken by adding new XDG_ variables.

[orlp]

@ChrisDenton XDG Base Directory specification 0.8 also changed the separator of $XDG_DATA_DIRS and $XDG_CONFIG_DIRS. In 0.7 it was always :, in 0.8 it is the separator used for $PATH on the platform (typically this is a colon :). This is a breaking change.

[ChrisDenton]

@ChrisDenton XDG Base Directory specification 0.8 also changed the separator of $XDG_DATA_DIRS and $XDG_CONFIG_DIRS. In 0.7 it was always :, in 0.8 it is the separator used for $PATH on the platform (typically this is a colon :). This is a breaking change.

That is not in the published specification. It is in the repository but is explicitly noted as not being a breaking change: https://gitlab.freedesktop.org/xdg/xdg-specs/-/commit/22a6d100a1c28d5fb725fb1d3d2f872cf4308769

I don’t think this can be a backwards compatibility break, as due to colons being used for drive identifiers on Windows, it’s never really been possible to use colons as search path separators.

And this definitely won't be a breaking change for the rust standard library because the API lives in std::os::unix.

[orlp]

Is https://specifications.freedesktop.org/basedir/ not "published"? It's there and marked as version 0.8... If that's not version 0.8 it should be marked as 0.9-draft or whatever.

[ChrisDenton]

Oh huh, "0.8" and "latest" are different. Interesting. My other points still stands though.

[aapoalas]

@aapoalas My main concerns I expressed earlier are still unaddressed.

As I think I said in one comment, I believe these are worthwhile concerns for blocking stabilisation of the API but I don't see them as worthwhile blockers for this PR: now, I'm just a first-time libs reviewer and my understanding and "feel" for this is lacking indeed, but the way I see it is:

1. An ACP has been accepted, so opening a PR to test out the proposed API change is good and okay.
2. As the PR code-wise looks okay, as a libs reviewer I don't have reasons to block it (I do not believe it is my job to decide on whether the API is good or not).
3. The next steps after this are unstable experimentation on this API and gathering feedback, very much including yours, then an eventual stabilisation or revert of the ACP based on that feedback.

I do have some points that I agree with you on: if the eventual point is for the std to offer an OS-independent "minimal API", it does not necessarily mean that each os library would get its own "maximal API". ... Though eg. fs offers extensions per OS (eg. PermissionsExt), so maybe there is some reason to think that is normal? And it can absolutely be argued that the spec is not stable enough to be included in the std, and/or that it's not the kind of stuff that std should include. Those are absolutely things that we should discuss, but in the tracking issue and not in a PR introducing an unstable API for testing out :)

As a long-time Linux user, I'm personally also quite happy to see this stuff proposed: we'll make The Year of the Linux Desktop yet! :D

@bors r+ rollup

[rust-bors]

📌 Commit 378da80 has been approved by aapoalas

It is now in the queue for this repository.

🌲 The tree is currently closed for pull requests below priority 5. This pull request will be tested once the tree is reopened.

[rust-bors]

⌛ Testing commit 378da80 with merge 876e334...

Workflow: https://github.com/rust-lang/rust/actions/runs/27469971600

[JonathanBrouwer]

@bors yield
Yielding to enclosing rollup

[rust-bors]

Auto build was cancelled. Cancelled workflows:

• https://github.com/rust-lang/rust/actions/runs/27469971600

The next pull request likely to be tested is #157863.