Use XDG base directories and Known Folders on Windows - #1809
Conversation
This changes logic that previously read/wrote from `~/.pkl` to use XDG base directories (all OSes), and Known Folders locations on Windows. For example, Pkl will look for `settings.pkl` in: 1. `$XDG_CONFIG_HOME/pkl/settings.pkl` 2. `%APPDATA/pkl/settings.pkl` 3. `~/.pkl/settings.pkl` 4. Path pkl/settings/pkl within `$XDG_CONFIG_DIRS` 5. `/etc/xdg/pkl/settings.pkl` The settings and cacerts dirs will still read from the legacy `~/.pkl/` directory if not found in the new locations.
8ac55d6 to
ec92a9a
Compare
|
|
||
| === Changes | ||
|
|
||
| * Default the CLI's package cache, settings file, CA certificates directory, and REPL history to XDG-style locations instead of `~/.pkl`: `~/.cache/pkl`, `~/.config/pkl/settings.pkl`, `~/.config/pkl/cacerts`, and `~/.local/state/pkl/repl-history` (pr:https://github.com/apple/pkl/pull/1674[]). |
There was a problem hiding this comment.
Nit: took me a couple scans to parse "default" as a verb
| * Default the CLI's package cache, settings file, CA certificates directory, and REPL history to XDG-style locations instead of `~/.pkl`: `~/.cache/pkl`, `~/.config/pkl/settings.pkl`, `~/.config/pkl/cacerts`, and `~/.local/state/pkl/repl-history` (pr:https://github.com/apple/pkl/pull/1674[]). | |
| * Use XDG-style locations by default for the CLI's package cache, settings file, CA certificates directory, and REPL history instead of `~/.pkl`: `~/.cache/pkl`, `~/.config/pkl/settings.pkl`, `~/.config/pkl/cacerts`, and `~/.local/state/pkl/repl-history` (pr:https://github.com/apple/pkl/pull/1674[]). |
| | Package cache | ||
| | `~/.cache/pkl` | ||
| | `$LOCALAPPDATA/pkl/Cache` | ||
| | none |
There was a problem hiding this comment.
I wonder if it might make sense to have a one-time "new cache not found, but old cache is present" copy/link migration to make some attempt to reuse existing cache dirs. Cost to re-build is low, but still non-zero, especially on slow connections.
|
|
||
| private fun HttpClient.Builder.addDefaultCliCertificates() { | ||
| val caCertsDir = IoUtils.getPklHomeDir().resolve("cacerts") | ||
| val caCertsDir = IoUtils.getSystemCaCertsDir() |
There was a problem hiding this comment.
The change to cacerts means that the certs can now be silently diverted via an env var. I'm not sure this is a good call security-wise.
There was a problem hiding this comment.
Is this any different from diverting the path of the settings.pkl via an env var? Both have security implications.
| Path candidate; | ||
| if (home != null) { | ||
| candidate = home.resolve(subpath); | ||
| if (Files.exists(candidate)) { |
There was a problem hiding this comment.
File.exists means a file named cacerts will return a candidate, but later we check if it's a directory, which will fail, meaning we'll use the default bundled certs even if there's a valid one somewhere else.
Worth adding another parameter or predicate to check the correct expectation.
There was a problem hiding this comment.
Good point; I changed this to accept a predicate.
| /// Unless CLI commands and build tool plugins are explicitly configured with a settings file, | ||
| /// they will use `~/.pkl/settings.pkl` or the defaults specified in this module. | ||
| /// looks in the following locations in order of precedence: |
There was a problem hiding this comment.
Wording is strange here.
What looks in the following locations?
There was a problem hiding this comment.
Updated this; the subject is "CLI commands and build tool plugins"
87d7852 to
62eddae
Compare
4 participants
This changes logic that previously read/wrote from
~/.pklto use XDG base directories (all OSes), and Known Folders locations on Windows.For example, Pkl will look for
settings.pklin:$XDG_CONFIG_HOME/pkl/settings.pkl%APPDATA/pkl/settings.pklpkl/settings.pklwithin$XDG_CONFIG_DIRS/etc/xdg/pkl/settings.pkl~/.pkl/settings.pklThe settings and cacerts dirs will still read from the legacy
~/.pkl/directory if not found in the new locations.See https://specifications.freedesktop.org/basedir/latest/