Resolve every base directory in one place, and on Windows

Five sites each read XDG_*_HOME and fell back to $HOME/.local/… on
their own, which is fine on Linux and wrong everywhere else: Windows
sets neither variable, so every one of them degraded to a path
relative to the working directory — for a Start Menu launch,
C:\Windows\System32. The models lookup walked XDG_DATA_DIRS the same
way.

dr_plat::dirs now holds the rule per platform: XDG on Unix, the known
folders on Windows — %APPDATA% for config, which roams, and
%LOCALAPPDATA% for data and state, which do not — and the executable's
own directory as the system data dir, which is where the installer
puts the models. The Android overrides stay where they were; only the
fallback behind them moved. Both rule sets are unit-tested on either
host, and the Windows one was confirmed by running the application
under Wine: its log landed in AppData\Local\darkroom\state and nothing
was written anywhere else.

core/dr-sync/src/account.rs

@@ -67,15 +67,21 @@ pub fn set_data_dir(dir: PathBuf) {
     let _ = DATA_DIR.set(dir);
 }
 
+/// The directory the platform entry point declared, if it declared one.
+///
+/// Android does; a desktop does not, and resolves through `dr_plat::dirs`
+/// instead. Exposed so a caller wanting the *data* directory can honour the
+/// same declaration without inheriting the config rule as its fallback.
+pub fn declared_data_dir() -> Option<PathBuf> {
+    DATA_DIR.get().cloned()
+}
+
 /// The directory configuration lives in.
 pub fn config_dir() -> PathBuf {
     if let Some(d) = DATA_DIR.get() {
         return d.clone();
     }
-    std::env::var_os("XDG_CONFIG_HOME")
+    dr_plat::base_dir(dr_plat::Base::Config)
-        .map(PathBuf::from)
-        .unwrap_or_else(|| PathBuf::from(std::env::var("HOME").unwrap_or_default()).join(".config"))
-        .join("darkroom")
 }
 
 /// TRACES: FR-NC-12

platform/dr-plat/src/crash.rs

@@ -106,12 +106,7 @@ pub fn state_dir() -> PathBuf {
     if let Some(d) = STATE_DIR.get() {
         return d.clone();
     }
-    std::env::var_os("XDG_STATE_HOME")
+    crate::dirs::base_dir(crate::dirs::Base::State)
-        .map(PathBuf::from)
-        .unwrap_or_else(|| {
-            PathBuf::from(std::env::var("HOME").unwrap_or_default()).join(".local/state")
-        })
-        .join("darkroom")
 }
 
 /// Where crash records are written.

platform/dr-plat/src/dirs.rs

@@ -0,0 +1,218 @@
+//! TRACES: FR-PLAT-LIN-1 | FR-PLAT-WIN-1 | NFR-PORT-1
+//! Where this application's files belong on this platform.
+//!
+//! One place for the rule, because it was in five. Each site read
+//! `XDG_*_HOME` and fell back to `$HOME/.local/...` on its own, which is fine
+//! on Linux and wrong everywhere else: Windows sets neither variable, so every
+//! one of them degraded to a path relative to the working directory — for a
+//! Start Menu launch, `C:\Windows\System32`. The callers keep their own
+//! override (`set_state_dir`, `set_data_dir`), which is how Android names its
+//! app-private directory; this answers the question those overrides do not.
+//!
+//! # The rules
+//!
+//! | | Config | Data | State |
+//! |---|---|---|---|
+//! | Unix | `$XDG_CONFIG_HOME` | `$XDG_DATA_HOME` | `$XDG_STATE_HOME` |
+//! | Unix default | `~/.config` | `~/.local/share` | `~/.local/state` |
+//! | Windows | `%APPDATA%` | `%LOCALAPPDATA%` | `%LOCALAPPDATA%`, then `state` |
+//! | Windows default | `%USERPROFILE%\AppData\Roaming` | `…\AppData\Local` | `…\AppData\Local` |
+//!
+//! then `darkroom` under each. Config roams on Windows and the rest does not,
+//! which is the same split XDG makes between config and everything else, and
+//! the reason `settings.json` is small and the thumbnail store is not.
+//!
+//! A relative value is ignored rather than resolved: the XDG specification
+//! says so, and the alternative is a `darkroom/` directory wherever the app
+//! was launched from. With nothing usable set at all the answer is the
+//! temporary directory — a container or a service unit with no home — because
+//! writing a log into `/tmp` is a poor outcome and refusing to write one, on
+//! exactly the machine nobody is sitting in front of, is a worse one.
+
+use std::ffi::OsString;
+use std::path::{Path, PathBuf};
+
+/// Which kind of directory. The distinction is the one every platform makes:
+/// what the user edits, what the application builds, and what it records.
+#[derive(Debug, Clone, Copy, PartialEq, Eq)]
+pub enum Base {
+    /// Settings and accounts. Small, and the user may back it up or edit it.
+    Config,
+    /// Catalogs, thumbnails, models. Large, rebuildable, never edited by hand.
+    Data,
+    /// Logs and crash records. Survives a cache sweep; not configuration.
+    State,
+}
+
+/// The directory of this kind on this machine, ending in `darkroom`.
+///
+/// Not created here: whoever writes into it creates it, so that merely asking
+/// leaves nothing behind.
+pub fn base_dir(kind: Base) -> PathBuf {
+    resolve(kind, |name| std::env::var_os(name))
+}
+
+/// Directories a package may have installed shared data into, most specific
+/// first, each ending in `darkroom`.
+///
+/// Unix: `$XDG_DATA_DIRS`, defaulting to `/usr/local/share:/usr/share` — what
+/// the Arch package and the Flatpak install under. Windows: the directory the
+/// executable is in, which is where the installer puts the models
+/// (FR-PLAT-WIN-2). Android: none; the APK's copy is unpacked into the user
+/// data directory instead, because an asset inside a package is not a path
+/// anything can read from.
+pub fn system_data_dirs() -> Vec<PathBuf> {
+    if cfg!(target_os = "android") {
+        return Vec::new();
+    }
+    if cfg!(windows) {
+        return std::env::current_exe()
+            .ok()
+            .and_then(|exe| exe.parent().map(Path::to_path_buf))
+            .into_iter()
+            .collect();
+    }
+    let dirs = std::env::var("XDG_DATA_DIRS")
+        .ok()
+        .filter(|v| !v.is_empty())
+        .unwrap_or_else(|| "/usr/local/share:/usr/share".into());
+    dirs.split(':')
+        .filter(|d| !d.is_empty())
+        .map(|d| PathBuf::from(d).join("darkroom"))
+        .collect()
+}
+
+/// The resolution as a function of its inputs rather than of the process
+/// environment, so it can be tested without `set_var` racing every other
+/// test in the binary — and so both platforms' rules are tested on either.
+fn resolve(kind: Base, env: impl Fn(&str) -> Option<OsString>) -> PathBuf {
+    let base = if cfg!(windows) {
+        windows_base(kind, &env)
+    } else {
+        xdg_base(kind, &env)
+    };
+    let dir = base.unwrap_or_else(std::env::temp_dir).join("darkroom");
+    match (kind, cfg!(windows)) {
+        // Data and state share `%LOCALAPPDATA%`; state gets its own level so
+        // a log and a catalog do not sit side by side.
+        (Base::State, true) => dir.join("state"),
+        _ => dir,
+    }
+}
+
+fn xdg_base(kind: Base, env: &impl Fn(&str) -> Option<OsString>) -> Option<PathBuf> {
+    let (var, under_home) = match kind {
+        Base::Config => ("XDG_CONFIG_HOME", ".config"),
+        Base::Data => ("XDG_DATA_HOME", ".local/share"),
+        Base::State => ("XDG_STATE_HOME", ".local/state"),
+    };
+    absolute(env(var)).or_else(|| absolute(env("HOME")).map(|h| h.join(under_home)))
+}
+
+fn windows_base(kind: Base, env: &impl Fn(&str) -> Option<OsString>) -> Option<PathBuf> {
+    let (var, under_profile) = match kind {
+        Base::Config => ("APPDATA", "AppData\\Roaming"),
+        Base::Data | Base::State => ("LOCALAPPDATA", "AppData\\Local"),
+    };
+    absolute(env(var)).or_else(|| absolute(env("USERPROFILE")).map(|p| p.join(under_profile)))
+}
+
+/// A value only if it names an absolute path. `is_absolute` is the host's
+/// notion, so a test of the Windows rule on Linux uses Unix-shaped paths;
+/// what is tested is the variable and the suffix, which is the part that
+/// was wrong.
+fn absolute(value: Option<OsString>) -> Option<PathBuf> {
+    value
+        .filter(|v| Path::new(v).is_absolute())
+        .map(PathBuf::from)
+}
+
+#[cfg(test)]
+mod tests {
+    use super::*;
+    use std::collections::HashMap;
+
+    fn env(pairs: &[(&str, &str)]) -> impl Fn(&str) -> Option<OsString> {
+        let map: HashMap<String, OsString> = pairs
+            .iter()
+            .map(|(k, v)| (k.to_string(), OsString::from(v)))
+            .collect();
+        move |name| map.get(name).cloned()
+    }
+
+    #[test]
+    fn xdg_honours_each_variable_and_defaults_under_home() {
+        let e = env(&[("XDG_CONFIG_HOME", "/etc/me"), ("HOME", "/home/someone")]);
+        assert_eq!(xdg_base(Base::Config, &e), Some(PathBuf::from("/etc/me")));
+        assert_eq!(
+            xdg_base(Base::Data, &e),
+            Some(PathBuf::from("/home/someone/.local/share"))
+        );
+        assert_eq!(
+            xdg_base(Base::State, &e),
+            Some(PathBuf::from("/home/someone/.local/state"))
+        );
+    }
+
+    #[test]
+    fn a_relative_value_is_ignored_rather_than_resolved() {
+        // The specification requires this, and the failure it prevents is a
+        // `darkroom/` directory appearing in whatever the working directory
+        // happened to be — including, on a desktop launcher, `/`.
+        let e = env(&[("XDG_STATE_HOME", "state"), ("HOME", "/home/someone")]);
+        assert_eq!(
+            xdg_base(Base::State, &e),
+            Some(PathBuf::from("/home/someone/.local/state"))
+        );
+        assert_eq!(xdg_base(Base::State, &env(&[("HOME", "")])), None);
+    }
+
+    #[test]
+    fn windows_splits_config_from_the_rest() {
+        // Config roams, data and state do not. The variable is what was wrong
+        // before this module: neither XDG_* nor HOME exists on Windows.
+        let e = env(&[("APPDATA", "/r"), ("LOCALAPPDATA", "/l")]);
+        assert_eq!(windows_base(Base::Config, &e), Some(PathBuf::from("/r")));
+        assert_eq!(windows_base(Base::Data, &e), Some(PathBuf::from("/l")));
+        assert_eq!(windows_base(Base::State, &e), Some(PathBuf::from("/l")));
+    }
+
+    #[test]
+    fn windows_falls_back_to_the_profile() {
+        let e = env(&[("USERPROFILE", "/u")]);
+        assert_eq!(
+            windows_base(Base::Config, &e),
+            Some(PathBuf::from("/u").join("AppData\\Roaming"))
+        );
+        assert_eq!(
+            windows_base(Base::Data, &e),
+            Some(PathBuf::from("/u").join("AppData\\Local"))
+        );
+    }
+
+    #[test]
+    fn nothing_set_is_still_absolute() {
+        // A container or a service unit: the answer is somewhere writable,
+        // never a relative path.
+        let dir = resolve(Base::State, env(&[]));
+        assert!(dir.is_absolute());
+        assert!(dir.ends_with("darkroom") || dir.ends_with("state"));
+    }
+
+    #[test]
+    fn every_kind_ends_in_the_application_name() {
+        let e = env(&[
+            ("HOME", "/home/someone"),
+            ("APPDATA", "/r"),
+            ("LOCALAPPDATA", "/l"),
+        ]);
+        for kind in [Base::Config, Base::Data, Base::State] {
+            let dir = resolve(kind, &e);
+            assert!(
+                dir.components().any(|c| c.as_os_str() == "darkroom"),
+                "{kind:?} resolved to {}",
+                dir.display()
+            );
+        }
+    }
+}

platform/dr-plat/src/lib.rs

@@ -12,6 +12,7 @@
 // arrangement this crate exists to prevent.
 pub mod crash;
 pub mod diagnostics;
+pub mod dirs;
 pub mod display;
 pub mod input;
 pub mod secrets;
@@ -20,6 +21,7 @@ pub mod storage;
 pub mod volumes;
 
 pub use diagnostics::{Installed, LogFile};
+pub use dirs::{base_dir, system_data_dirs, Base};
 pub use display::{
     Bounds, DisplayInfo, DisplayProfile, DisplayServer, DisplaySurvey, FallbackReason,
     ProfileSource,

platform/dr-plat/src/state.rs

@@ -44,8 +44,7 @@
 //! reason [`crate::diagnostics`] redacts at the sink rather than trusting call
 //! sites: everything written here is readable by anyone holding the device.
 
-use std::ffi::OsString;
+use std::path::PathBuf;
-use std::path::{Path, PathBuf};
 use std::sync::OnceLock;
 
 /// Declared once by the platform entry point; a guess otherwise.
@@ -72,66 +71,13 @@ pub fn state_dir() -> PathBuf {
     if let Some(dir) = STATE_DIR.get() {
         return dir.clone();
     }
-    xdg_state_dir(std::env::var_os("XDG_STATE_HOME"), std::env::var_os("HOME"))
+    crate::dirs::base_dir(crate::dirs::Base::State)
-}
-
-/// The XDG resolution, as a function of its inputs rather than of the process
-/// environment, so it can be tested without `set_var` racing every other test
-/// in the binary.
-///
-/// Relative values are ignored rather than resolved against the working
-/// directory: the base-directory specification says so explicitly, and the
-/// alternative is a `darkroom/` directory appearing wherever the app was
-/// launched from.
-fn xdg_state_dir(xdg_state_home: Option<OsString>, home: Option<OsString>) -> PathBuf {
-    xdg_state_home
-        .filter(|value| Path::new(value).is_absolute())
-        .map(PathBuf::from)
-        .or_else(|| {
-            home.filter(|value| Path::new(value).is_absolute())
-                .map(|value| PathBuf::from(value).join(".local/state"))
-        })
-        // A container or a systemd unit with neither variable set. Writing a
-        // log into `/tmp` is a poor outcome; refusing to log at all, on the
-        // one kind of machine nobody is sitting in front of, is a worse one.
-        .unwrap_or_else(std::env::temp_dir)
-        .join("darkroom")
 }
 
 #[cfg(test)]
 mod tests {
     use super::*;
 
-    #[test]
-    fn the_log_goes_under_the_state_directory_the_user_named() {
-        // NFR-OPS-1 says "the XDG state directory", and honouring
-        // $XDG_STATE_HOME is the whole of what that means to a user who has
-        // moved theirs.
-        let dir = xdg_state_dir(Some("/var/lib/dr".into()), Some("/home/someone".into()));
-        assert_eq!(dir, PathBuf::from("/var/lib/dr/darkroom"));
-    }
-
-    #[test]
-    fn without_the_variable_it_is_the_specifications_default() {
-        let dir = xdg_state_dir(None, Some("/home/someone".into()));
-        assert_eq!(dir, PathBuf::from("/home/someone/.local/state/darkroom"));
-    }
-
-    #[test]
-    fn a_relative_value_is_ignored_rather_than_resolved() {
-        // The specification requires this, and the failure it prevents is a
-        // `darkroom/` directory appearing in whatever the working directory
-        // happened to be — including, on a desktop launcher, `/`.
-        let dir = xdg_state_dir(Some("state".into()), Some("/home/someone".into()));
-        assert_eq!(dir, PathBuf::from("/home/someone/.local/state/darkroom"));
-
-        let dir = xdg_state_dir(Some("".into()), Some("".into()));
-        assert!(
-            dir.is_absolute(),
-            "an empty HOME must not produce a relative state directory"
-        );
-    }
-
     #[test]
     fn a_declared_directory_is_declared_once() {
         // The property the entry points rely on: two callers cannot split the

ui/dr-ui/src/library.rs

@@ -1155,18 +1155,16 @@ pub fn place_path(account: &Account) -> PathBuf {
 /// reached the server, is the worst failure this application can have, and
 /// it would be silent.
 ///
-/// `AccountStore::data_dir()` is the persistent per-app directory the
+/// `dr_sync::account::declared_data_dir` is the persistent per-app directory
-/// Android entry point establishes before anything opens a store. On a
+/// the Android entry point establishes before anything opens a store. A
-/// desktop it is the XDG config directory, and the two lines below keep the
+/// desktop declares nothing and takes the platform's data directory from
-/// established XDG *data* location there rather than moving anyone's
+/// `dr_plat::dirs` — XDG on Linux, `%LOCALAPPDATA%` on Windows — which keeps
-/// catalog.
+/// the established location on Linux rather than moving anyone's catalog.
 fn data_root() -> PathBuf {
-    let base = std::env::var_os("XDG_DATA_HOME")
+    match dr_sync::account::declared_data_dir() {
-        .map(PathBuf::from)
+        Some(declared) => declared.join("darkroom"),
-        .or_else(|| std::env::var_os("HOME").map(|h| PathBuf::from(h).join(".local/share")))
+        None => dr_plat::base_dir(dr_plat::Base::Data),
-        .unwrap_or_else(dr_sync::AccountStore::data_dir);
+    }
-
-    base.join("darkroom")
 }
 
 /// TRACES: FR-NC-10 | NFR-R1
@@ -4696,25 +4694,16 @@ pub fn scene_model(account: &Account) -> Option<(PathBuf, PathBuf, PathBuf)> {
 
 /// Where a *package* may have installed the models.
 ///
-/// `$XDG_DATA_DIRS` rather than a hard-coded `/usr/share`, because that is the
+/// `dr_plat::system_data_dirs` has the rule per platform: `$XDG_DATA_DIRS` on
-/// variable a distribution, a prefix install, or a Nix-style store sets to say
+/// Linux, the executable's own directory on Windows, nothing on Android —
-/// where its data went, and the default it falls back to is exactly the pair of
+/// there the APK's copy is unpacked into the shared user directory instead,
-/// paths that would otherwise have been hard-coded.
+/// because an asset inside a package is not a path anything can read from
-///
+/// (ARCH §6.9). Last in the search order on every platform, so a pair the
-/// Empty on Android, which has no such directories: there the APK's copy is
+/// user placed by hand outranks the installed one.
-/// unpacked into the shared user directory instead, because an asset inside a
-/// package is not a path anything can read from (ARCH §6.9).
 fn system_face_models_dirs() -> Vec<PathBuf> {
-    if cfg!(target_os = "android") {
+    dr_plat::system_data_dirs()
-        return Vec::new();
+        .into_iter()
-    }
+        .map(|d| d.join("models"))
-    let dirs = std::env::var("XDG_DATA_DIRS")
-        .ok()
-        .filter(|v| !v.is_empty())
-        .unwrap_or_else(|| "/usr/local/share:/usr/share".into());
-    dirs.split(':')
-        .filter(|d| !d.is_empty())
-        .map(|d| PathBuf::from(d).join("darkroom").join("models"))
         .collect()
 }
 

ui/dr-ui/src/settings_store.rs

@@ -25,17 +25,12 @@ pub struct SettingsStore {
 impl SettingsStore {
     /// Open the store at the platform config location.
     ///
-    /// Linux: `$XDG_CONFIG_HOME/darkroom/settings.json`, falling back to
+    /// `settings.json` in the platform's config directory — `dr_plat::dirs`
-    /// `~/.config` (FR-PLAT-LIN-1) — the same resolution `SessionStore` does,
+    /// has the rule per platform — which is the same resolution `SessionStore`
-    /// so the two files sit together and a user backing up one takes both.
+    /// makes, so the two files sit together and a user backing up one takes
+    /// both.
     pub fn open() -> Self {
-        let dir = std::env::var_os("XDG_CONFIG_HOME")
+        Self::open_at(dr_plat::base_dir(dr_plat::Base::Config).join("settings.json"))
-            .map(PathBuf::from)
-            .unwrap_or_else(|| {
-                PathBuf::from(std::env::var("HOME").unwrap_or_default()).join(".config")
-            })
-            .join("darkroom");
-        Self::open_at(dir.join("settings.json"))
     }
 
     /// Open at an explicit path — for tests, and for a non-default location.