From 09b198b96dc0ac54a63679ecb266a3e5ef5b4845 Mon Sep 17 00:00:00 2001 From: Austin Schaefer Date: Sun, 20 Sep 2026 10:01:54 +0200 Subject: [PATCH 1/3] Resolve config, state and work dirs via XDG, not the cwd An installed pkgwatch has no checkout to run from, so packages.d/, state/ and work/ can no longer be relative to the working directory. New paths module resolves them per the XDG base-directory spec, with PKGWATCH_{CONFIG,STATE,WORK}_DIR overrides for dry runs. Co-Authored-By: Claude Sonnet 5 --- docs/SPEC.md | 16 +++++ src/main.rs | 1 + src/paths.rs | 151 ++++++++++++++++++++++++++++++++++++++++++++++++ src/pipeline.rs | 32 +++++----- 4 files changed, 185 insertions(+), 15 deletions(-) create mode 100644 src/paths.rs diff --git a/docs/SPEC.md b/docs/SPEC.md index b6cfa09..f513d99 100644 --- a/docs/SPEC.md +++ b/docs/SPEC.md @@ -330,6 +330,22 @@ 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` | + + 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. 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 diff --git a/src/main.rs b/src/main.rs index 300ac6b..ca86015 100644 --- a/src/main.rs +++ b/src/main.rs @@ -8,6 +8,7 @@ mod config; mod fetcher; mod github; mod hash; +mod paths; mod pipeline; mod publisher; mod sanity; diff --git a/src/paths.rs b/src/paths.rs new file mode 100644 index 0000000..3f14ac1 --- /dev/null +++ b/src/paths.rs @@ -0,0 +1,151 @@ +//! 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. +//! +//! 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, Clone, 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::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`. The overrides exist + /// for dry runs against scratch directories, like `PKGWATCH_REPO_DIR` + /// does for the pacman repo. + fn resolve(getenv: impl Fn(&str) -> Option) -> Result { + let base = |override_var: &str, xdg_var: &str, home_subpath: &str| -> Result { + if let Some(dir) = non_empty(&getenv, override_var) { + return Ok(PathBuf::from(dir)); + } + if let Some(dir) = non_empty(&getenv, xdg_var) { + return Ok(Path::new(&dir).join(APP_DIR)); + } + let home = non_empty(&getenv, "HOME").context("HOME is not set")?; + 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, key: &str) -> Option { + getenv(key).filter(|v| !v.is_empty()) +} + +#[cfg(test)] +mod tests { + use super::*; + use std::collections::HashMap; + + fn env(pairs: &[(&str, &str)]) -> impl Fn(&str) -> Option { + let map: HashMap = 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 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")); + } +} diff --git a/src/pipeline.rs b/src/pipeline.rs index 8d0b037..413e5e9 100644 --- a/src/pipeline.rs +++ b/src/pipeline.rs @@ -10,6 +10,7 @@ use crate::checker; use crate::config::{self, Package}; use crate::fetcher::{self, DownloadedAsset}; use crate::github::GithubEndpoints; +use crate::paths::Paths; use crate::publisher; use crate::sanity; use crate::state; @@ -17,9 +18,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 @@ -46,11 +44,13 @@ fn custom_repo_dir() -> Result { pub fn run_check() -> Result<()> { let client = build_client()?; let endpoints = GithubEndpoints::default(); - let packages_dir = Path::new(PACKAGES_DIR); - let state_dir = Path::new(STATE_DIR); - let work_dir = Path::new(WORK_DIR); + let Paths { + packages_dir, + state_dir, + work_dir, + } = Paths::from_env()?; - let packages = config::load_packages_dir(packages_dir)?; + let packages = config::load_packages_dir(&packages_dir)?; if packages.is_empty() { println!("no packages configured under {}/", packages_dir.display()); return Ok(()); @@ -59,7 +59,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; } @@ -258,16 +258,18 @@ fn build_and_publish( } pub fn run_review(args: &[String]) -> Result<()> { - let packages_dir = Path::new(PACKAGES_DIR); - let state_dir = Path::new(STATE_DIR); - let work_dir = Path::new(WORK_DIR); - let packages = config::load_packages_dir(packages_dir)?; + let Paths { + packages_dir, + state_dir, + work_dir, + } = Paths::from_env()?; + let packages = config::load_packages_dir(&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`)" ); @@ -283,9 +285,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 [ --approve]"), } From 6188f7f0b7be72cda5efab3e0eb6216f7296d821 Mon Sep 17 00:00:00 2001 From: Austin Schaefer Date: Sun, 20 Sep 2026 10:03:24 +0200 Subject: [PATCH 2/3] Drop the now-unneeded WorkingDirectory from the service Paths no longer resolve relative to the cwd, so the unit's comment and the SPEC's explanation of it were stale. Co-Authored-By: Claude Sonnet 5 --- docs/SPEC.md | 6 ++++-- systemd/pkgwatch.service | 7 +++---- 2 files changed, 7 insertions(+), 6 deletions(-) diff --git a/docs/SPEC.md b/docs/SPEC.md index 15b3273..637eada 100644 --- a/docs/SPEC.md +++ b/docs/SPEC.md @@ -435,8 +435,10 @@ Open questions on the schema: systemctl --user enable --now pkgwatch.timer ``` - *`pkgwatch.service` sets `WorkingDirectory` to `~/dev/pkgwatch` because - `packages.d/`, `state/` and `work/` resolve relative to cwd.)* + *`pkgwatch.service` runs the release binary from the checkout and sets + 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 diff --git a/systemd/pkgwatch.service b/systemd/pkgwatch.service index 232a5d4..0bb604d 100644 --- a/systemd/pkgwatch.service +++ b/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 -# relative to the cwd, so this must be the main checkout, not a worktree. -# The binary is the release build in that same checkout (`cargo build +# No WorkingDirectory: config, state and work dirs come from the XDG paths +# in src/paths.rs (see docs/SPEC.md > Paths), not the cwd. +# 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 From d0ab2525e4ecbed1ba6bd9ea324dd66051e2f7d9 Mon Sep 17 00:00:00 2001 From: Austin Schaefer Date: Sun, 20 Sep 2026 10:19:12 +0200 Subject: [PATCH 3/3] Apply review feedback on XDG paths Ignore relative XDG_* values and reject a relative HOME, per the XDG spec; add a hint to the missing-config error; tighten docs and comments. Co-Authored-By: Claude Sonnet 5 --- docs/SPEC.md | 13 ++++++++++--- src/paths.rs | 49 +++++++++++++++++++++++++++++++++++++++++-------- src/pipeline.rs | 17 +++++++++++++++-- src/state.rs | 2 +- 4 files changed, 67 insertions(+), 14 deletions(-) diff --git a/docs/SPEC.md b/docs/SPEC.md index 637eada..3361cbe 100644 --- a/docs/SPEC.md +++ b/docs/SPEC.md @@ -341,9 +341,16 @@ Open questions on the schema: | 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` | - 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. Migrating from the old cwd-relative layout: move + 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" — diff --git a/src/paths.rs b/src/paths.rs index 3f14ac1..d4ec016 100644 --- a/src/paths.rs +++ b/src/paths.rs @@ -1,6 +1,7 @@ //! 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. +//! 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 @@ -12,7 +13,7 @@ use std::path::{Path, PathBuf}; const APP_DIR: &str = "pkgwatch"; -#[derive(Debug, Clone, PartialEq, Eq)] +#[derive(Debug, PartialEq, Eq)] pub struct Paths { /// `*.toml` package declarations. Config: hand-edited, worth backing up. pub packages_dir: PathBuf, @@ -32,18 +33,21 @@ impl Paths { /// 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`. The overrides exist - /// for dry runs against scratch directories, like `PKGWATCH_REPO_DIR` - /// does for the pacman repo. + /// 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) -> Result { let base = |override_var: &str, xdg_var: &str, home_subpath: &str| -> Result { if let Some(dir) = non_empty(&getenv, override_var) { return Ok(PathBuf::from(dir)); } - if let Some(dir) = non_empty(&getenv, xdg_var) { + if let Some(dir) = absolute(&getenv, xdg_var) { return Ok(Path::new(&dir).join(APP_DIR)); } - let home = non_empty(&getenv, "HOME").context("HOME is not set")?; + let home = absolute(&getenv, "HOME").context("HOME is not set to an absolute path")?; Ok(Path::new(&home).join(home_subpath).join(APP_DIR)) }; @@ -61,6 +65,12 @@ fn non_empty(getenv: &impl Fn(&str) -> Option, key: &str) -> Option Option, key: &str) -> Option { + getenv(key).filter(|v| Path::new(v).is_absolute()) +} + #[cfg(test)] mod tests { use super::*; @@ -132,6 +142,29 @@ mod tests { 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(); diff --git a/src/pipeline.rs b/src/pipeline.rs index 146eac1..753698e 100644 --- a/src/pipeline.rs +++ b/src/pipeline.rs @@ -42,6 +42,19 @@ fn custom_repo_dir() -> Result { 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> { + 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(); @@ -51,7 +64,7 @@ pub fn run_check() -> Result<()> { 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(()); @@ -272,7 +285,7 @@ pub fn run_review(args: &[String]) -> Result<()> { state_dir, work_dir, } = Paths::from_env()?; - let packages = config::load_packages_dir(&packages_dir)?; + let packages = load_packages(&packages_dir)?; match args { [] => { diff --git a/src/state.rs b/src/state.rs index 70dbbb2..6158ebd 100644 --- a/src/state.rs +++ b/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;