Merge pull request 'Resolve config, state and work dirs via XDG paths' (#4) from worktree-xdg-paths into master

Reviewed-on: #4

docs/SPEC.md

@@ -330,6 +330,29 @@ Open questions on the schema:
 
 - **Config loader**: parses `packages.d/*.toml` into an in-memory package
   list. *(Implemented — `src/config.rs`.)*
+- **Paths**: where config, state and work files live, resolved by
+  `src/paths.rs` per the XDG base-directory spec rather than the current
+  working directory, so an installed binary behaves the same wherever it's
+  launched from. *(Implemented.)*
+
+  | What | Default | XDG variable | Override |
+  |---|---|---|---|
+  | Package declarations (`packages.d/*.toml`) | `~/.config/pkgwatch/packages.d` | `XDG_CONFIG_HOME` | `PKGWATCH_CONFIG_DIR` (the dir *containing* `packages.d`) |
+  | Last-published / pending versions | `~/.local/state/pkgwatch` | `XDG_STATE_HOME` | `PKGWATCH_STATE_DIR` |
+  | Downloads and build trees (safe to delete) | `~/.cache/pkgwatch` | `XDG_CACHE_HOME` | `PKGWATCH_WORK_DIR` |
+
+  Precedence per directory: override, then the XDG variable, then the
+  default under `$HOME`; an empty variable counts as unset. The overrides
+  are used verbatim (no `pkgwatch/` suffix) and exist for dry runs against
+  scratch directories, like `PKGWATCH_REPO_DIR` does for the pacman repo.
+  The XDG variables and `$HOME` must be absolute paths: a relative XDG
+  value is ignored, as the XDG spec requires, and a relative `$HOME` is an
+  error.
+
+  The checkout's `packages.d/` is no longer read on its own; it's just the
+  source to link from. Migrating from the old cwd-relative layout: move
+  `state/` to the state dir and copy or symlink `packages.d/` into the
+  config dir; `work/` is cache and can simply be dropped.
 - **Checker**: per source type, resolves "what's the latest version" —
   likely reuses `nvchecker`'s logic/sources conceptually for non-GitHub
   sources eventually. For GitHub sources, prefers the `github-atom` feed
@@ -419,8 +442,10 @@ Open questions on the schema:
     systemctl --user enable --now pkgwatch.timer
   ```
 
-  *`pkgwatch.service` sets `WorkingDirectory` to `~/dev/pkgwatch` because
+  *`pkgwatch.service` runs the release binary from the checkout and sets
-  `packages.d/`, `state/` and `work/` resolve relative to cwd.)*
+  no `WorkingDirectory`: config, state and work dirs come from the XDG
+  paths above, so the service needs `~/.config/pkgwatch/packages.d` set
+  up first — see the migration note under Paths.)*
 - **Notifications**: `notifier.rs` sends a desktop notification
   (`notify-send`) when a tier 4-6 release is newly queued for review or a
   tier 1-3 release is published; both are best-effort and never fail a

src/main.rs

@@ -9,6 +9,7 @@ mod fetcher;
 mod github;
 mod hash;
 mod notifier;
+mod paths;
 mod pipeline;
 mod publisher;
 mod sanity;

src/paths.rs

@@ -0,0 +1,184 @@
+//! Decides where pkgwatch's config, state, and work directories live on
+//! disk — the only module that reads the environment to answer that; every
+//! other module takes the directories it needs as parameters. (The pacman
+//! repo dir, `pipeline::custom_repo_dir`, is resolved separately.)
+//!
+//! These follow the XDG base-directory spec instead of the current working
+//! directory, so an installed `/usr/bin/pkgwatch` behaves the same
+//! wherever it's launched from (a systemd unit, a shell, another checkout)
+//! instead of only working from inside the repo.
+
+use anyhow::{Context, Result};
+use std::path::{Path, PathBuf};
+
+const APP_DIR: &str = "pkgwatch";
+
+#[derive(Debug, PartialEq, Eq)]
+pub struct Paths {
+    /// `*.toml` package declarations. Config: hand-edited, worth backing up.
+    pub packages_dir: PathBuf,
+    /// Last-published/pending versions. State: small, but losing it makes
+    /// every package look new, so it isn't cache.
+    pub state_dir: PathBuf,
+    /// Downloaded artifacts and build trees. Cache: safe to delete.
+    pub work_dir: PathBuf,
+}
+
+impl Paths {
+    pub fn from_env() -> Result<Self> {
+        Self::resolve(|key| std::env::var(key).ok())
+    }
+
+    /// `getenv` is injectable so tests don't mutate the process-global
+    /// environment, which would race with `cargo test`'s parallel threads.
+    ///
+    /// Each directory has a pkgwatch-specific override, then the matching
+    /// XDG variable, then the XDG default under `$HOME`; empty counts as
+    /// unset. The overrides exist for dry runs against scratch
+    /// directories, like `PKGWATCH_REPO_DIR` does for the pacman repo, so
+    /// they're used verbatim. The XDG variables and `$HOME` must be
+    /// absolute: the spec says to ignore a relative XDG value, and honoring
+    /// one would bring back the cwd dependence this module exists to remove.
+    fn resolve(getenv: impl Fn(&str) -> Option<String>) -> Result<Self> {
+        let base = |override_var: &str, xdg_var: &str, home_subpath: &str| -> Result<PathBuf> {
+            if let Some(dir) = non_empty(&getenv, override_var) {
+                return Ok(PathBuf::from(dir));
+            }
+            if let Some(dir) = absolute(&getenv, xdg_var) {
+                return Ok(Path::new(&dir).join(APP_DIR));
+            }
+            let home = absolute(&getenv, "HOME").context("HOME is not set to an absolute path")?;
+            Ok(Path::new(&home).join(home_subpath).join(APP_DIR))
+        };
+
+        Ok(Self {
+            packages_dir: base("PKGWATCH_CONFIG_DIR", "XDG_CONFIG_HOME", ".config")?
+                .join("packages.d"),
+            state_dir: base("PKGWATCH_STATE_DIR", "XDG_STATE_HOME", ".local/state")?,
+            work_dir: base("PKGWATCH_WORK_DIR", "XDG_CACHE_HOME", ".cache")?,
+        })
+    }
+}
+
+/// The XDG spec says an empty variable must be treated as unset.
+fn non_empty(getenv: &impl Fn(&str) -> Option<String>, key: &str) -> Option<String> {
+    getenv(key).filter(|v| !v.is_empty())
+}
+
+/// Like `non_empty`, but also drops relative values (an empty string isn't
+/// absolute either, so this subsumes the empty check).
+fn absolute(getenv: &impl Fn(&str) -> Option<String>, key: &str) -> Option<String> {
+    getenv(key).filter(|v| Path::new(v).is_absolute())
+}
+
+#[cfg(test)]
+mod tests {
+    use super::*;
+    use std::collections::HashMap;
+
+    fn env(pairs: &[(&str, &str)]) -> impl Fn(&str) -> Option<String> {
+        let map: HashMap<String, String> = pairs
+            .iter()
+            .map(|(k, v)| (k.to_string(), v.to_string()))
+            .collect();
+        move |key| map.get(key).cloned()
+    }
+
+    #[test]
+    fn defaults_live_under_home() {
+        let paths = Paths::resolve(env(&[("HOME", "/home/u")])).unwrap();
+        assert_eq!(
+            paths,
+            Paths {
+                packages_dir: "/home/u/.config/pkgwatch/packages.d".into(),
+                state_dir: "/home/u/.local/state/pkgwatch".into(),
+                work_dir: "/home/u/.cache/pkgwatch".into(),
+            }
+        );
+    }
+
+    #[test]
+    fn xdg_variables_override_home_defaults() {
+        let paths = Paths::resolve(env(&[
+            ("HOME", "/home/u"),
+            ("XDG_CONFIG_HOME", "/xdg/config"),
+            ("XDG_STATE_HOME", "/xdg/state"),
+            ("XDG_CACHE_HOME", "/xdg/cache"),
+        ]))
+        .unwrap();
+        assert_eq!(
+            paths.packages_dir,
+            Path::new("/xdg/config/pkgwatch/packages.d")
+        );
+        assert_eq!(paths.state_dir, Path::new("/xdg/state/pkgwatch"));
+        assert_eq!(paths.work_dir, Path::new("/xdg/cache/pkgwatch"));
+    }
+
+    #[test]
+    fn pkgwatch_overrides_win_and_are_used_verbatim() {
+        let paths = Paths::resolve(env(&[
+            ("HOME", "/home/u"),
+            ("XDG_STATE_HOME", "/xdg/state"),
+            ("PKGWATCH_CONFIG_DIR", "/scratch/cfg"),
+            ("PKGWATCH_STATE_DIR", "/scratch/state"),
+            ("PKGWATCH_WORK_DIR", "/scratch/work"),
+        ]))
+        .unwrap();
+        // No `pkgwatch/` suffix appended to an explicit override.
+        assert_eq!(paths.packages_dir, Path::new("/scratch/cfg/packages.d"));
+        assert_eq!(paths.state_dir, Path::new("/scratch/state"));
+        assert_eq!(paths.work_dir, Path::new("/scratch/work"));
+    }
+
+    #[test]
+    fn empty_variables_are_treated_as_unset() {
+        let paths = Paths::resolve(env(&[
+            ("HOME", "/home/u"),
+            ("XDG_STATE_HOME", ""),
+            ("PKGWATCH_WORK_DIR", ""),
+        ]))
+        .unwrap();
+        assert_eq!(paths.state_dir, Path::new("/home/u/.local/state/pkgwatch"));
+        assert_eq!(paths.work_dir, Path::new("/home/u/.cache/pkgwatch"));
+    }
+
+    #[test]
+    fn relative_xdg_variables_are_ignored() {
+        let paths = Paths::resolve(env(&[
+            ("HOME", "/home/u"),
+            ("XDG_CONFIG_HOME", "rel/config"),
+            ("XDG_STATE_HOME", "./state"),
+            ("XDG_CACHE_HOME", "cache"),
+        ]))
+        .unwrap();
+        assert_eq!(
+            paths.packages_dir,
+            Path::new("/home/u/.config/pkgwatch/packages.d")
+        );
+        assert_eq!(paths.state_dir, Path::new("/home/u/.local/state/pkgwatch"));
+        assert_eq!(paths.work_dir, Path::new("/home/u/.cache/pkgwatch"));
+    }
+
+    #[test]
+    fn relative_home_is_an_error() {
+        let err = Paths::resolve(env(&[("HOME", "relative/home")])).unwrap_err();
+        assert!(err.to_string().contains("HOME"));
+    }
+
+    #[test]
+    fn errors_when_nothing_locates_home() {
+        let err = Paths::resolve(env(&[])).unwrap_err();
+        assert!(err.to_string().contains("HOME"));
+    }
+
+    #[test]
+    fn no_home_needed_when_every_dir_is_overridden() {
+        let paths = Paths::resolve(env(&[
+            ("PKGWATCH_CONFIG_DIR", "/c"),
+            ("PKGWATCH_STATE_DIR", "/s"),
+            ("PKGWATCH_WORK_DIR", "/w"),
+        ]))
+        .unwrap();
+        assert_eq!(paths.state_dir, Path::new("/s"));
+    }
+}

src/pipeline.rs

@@ -11,6 +11,7 @@ use crate::config::{self, Package};
 use crate::fetcher::{self, DownloadedAsset};
 use crate::github::GithubEndpoints;
 use crate::notifier::{self, Event};
+use crate::paths::Paths;
 use crate::publisher;
 use crate::sanity;
 use crate::state;
@@ -18,9 +19,6 @@ use crate::verifier::{self, VerificationResult};
 use anyhow::{Context, Result, bail};
 use std::path::{Path, PathBuf};
 
-const PACKAGES_DIR: &str = "packages.d";
-const STATE_DIR: &str = "state";
-const WORK_DIR: &str = "work";
 /// Not a repo pkgwatch invents: this is the existing, already-registered
 /// local pacman repo on this box (see `[custom]` in /etc/pacman.conf and
 /// its `Server = file://...` line). pkgwatch adds packages to it; it does
@@ -44,14 +42,29 @@ fn custom_repo_dir() -> Result<PathBuf> {
     Ok(Path::new(&home).join(CUSTOM_REPO_SUBPATH))
 }
 
+/// Adds a hint to the bare "No such file" a missing config dir would give:
+/// the dir is no longer relative to the cwd, so a checkout's `packages.d/`
+/// isn't picked up on its own (see docs/SPEC.md > Paths).
+fn load_packages(packages_dir: &Path) -> Result<Vec<(String, Package)>> {
+    config::load_packages_dir(packages_dir).with_context(|| {
+        format!(
+            "no package config at {} (set PKGWATCH_CONFIG_DIR to the directory containing \
+             packages.d, or see docs/SPEC.md > Paths for moving a checkout's packages.d/ there)",
+            packages_dir.display()
+        )
+    })
+}
+
 pub fn run_check() -> Result<()> {
     let client = build_client()?;
     let endpoints = GithubEndpoints::default();
-    let packages_dir = Path::new(PACKAGES_DIR);
+    let Paths {
-    let state_dir = Path::new(STATE_DIR);
+        packages_dir,
-    let work_dir = Path::new(WORK_DIR);
+        state_dir,
+        work_dir,
+    } = Paths::from_env()?;
 
-    let packages = config::load_packages_dir(packages_dir)?;
+    let packages = load_packages(&packages_dir)?;
     if packages.is_empty() {
         println!("no packages configured under {}/", packages_dir.display());
         return Ok(());
@@ -60,7 +73,7 @@ pub fn run_check() -> Result<()> {
     let mut any_failed = false;
     for (name, pkg) in &packages {
         println!("== {name} ({}) ==", pkg.repo);
-        if let Err(err) = process_package(&client, &endpoints, state_dir, work_dir, name, pkg) {
+        if let Err(err) = process_package(&client, &endpoints, &state_dir, &work_dir, name, pkg) {
             eprintln!("  error: {err:#}");
             any_failed = true;
         }
@@ -267,16 +280,18 @@ fn build_and_publish(
 }
 
 pub fn run_review(args: &[String]) -> Result<()> {
-    let packages_dir = Path::new(PACKAGES_DIR);
+    let Paths {
-    let state_dir = Path::new(STATE_DIR);
+        packages_dir,
-    let work_dir = Path::new(WORK_DIR);
+        state_dir,
-    let packages = config::load_packages_dir(packages_dir)?;
+        work_dir,
+    } = Paths::from_env()?;
+    let packages = load_packages(&packages_dir)?;
 
     match args {
         [] => {
             let mut any = false;
             for (name, _) in &packages {
-                if let Some(pending) = state::load_pending_version(state_dir, name) {
+                if let Some(pending) = state::load_pending_version(&state_dir, name) {
                     println!(
                         "{name}: {pending} pending review (run `pkgwatch review {name} --approve`)"
                     );
@@ -292,9 +307,9 @@ pub fn run_review(args: &[String]) -> Result<()> {
             let (_, pkg) = packages.iter().find(|(n, _)| n == name).with_context(|| {
                 format!("no package named '{name}' in {}/", packages_dir.display())
             })?;
-            let tag = state::load_pending_version(state_dir, name)
+            let tag = state::load_pending_version(&state_dir, name)
                 .with_context(|| format!("'{name}' has no pending review"))?;
-            approve(state_dir, work_dir, name, pkg, &tag)
+            approve(&state_dir, &work_dir, name, pkg, &tag)
         }
         _ => bail!("usage: pkgwatch review [<name> --approve]"),
     }

src/state.rs

@@ -1,6 +1,6 @@
 //! Persists two independent per-package facts as plain files: the last
 //! published version, and any version currently pending human review.
-//! The only module that touches `state/` on disk.
+//! The only module that touches the state directory (see `paths.rs`) on disk.
 
 use anyhow::Result;
 use std::path::Path;

systemd/pkgwatch.service

@@ -1,9 +1,9 @@
 # User-level oneshot: one check -> fetch -> verify -> build -> publish pass.
 # Install: see docs/SPEC.md > Scheduling.
 #
-# WorkingDirectory matters: packages.d/, state/ and work/ are all resolved
+# No WorkingDirectory: config, state and work dirs come from the XDG paths
-# relative to the cwd, so this must be the main checkout, not a worktree.
+# in src/paths.rs (see docs/SPEC.md > Paths), not the cwd.
-# The binary is the release build in that same checkout (`cargo build
+# The binary is the release build in the main checkout (`cargo build
 # --release`), so a rebuild is what picks up code changes.
 [Unit]
 Description=pkgwatch: check tracked packages for new upstream releases
@@ -11,7 +11,6 @@ OnFailure=pkgwatch-failure.service
 
 [Service]
 Type=oneshot
-WorkingDirectory=%h/dev/pkgwatch
 ExecStart=%h/dev/pkgwatch/target/release/pkgwatch
 # Builds (makepkg, large Go/Rust binaries) can legitimately take a while.
 TimeoutStartSec=30min