diff --git a/SPEC.md b/SPEC.md index 5e2f8c9..b793611 100644 --- a/SPEC.md +++ b/SPEC.md @@ -377,10 +377,19 @@ Open questions on the schema: repo rather than one pkgwatch creates — this box already has one at `~/.local/share/pacman/custom`, registered as `[custom]` in `/etc/pacman.conf` (`SigLevel = Optional TrustAll`) and already in use - for a hand-packaged AppImage, resolving the "where does the repo live / - how does it get registered" open question below without pkgwatch ever - touching pacman.conf. Deliberately stops at `repo-add`: getting the new - version onto the running system is a separate, deliberate + for a hand-packaged AppImage. But that repo directory/registration isn't + guaranteed to exist on every box this ever runs on, so it isn't just + assumed: `publisher::ensure_registered` checks `/etc/pacman.conf` for an + active `[]` section before a build even starts, failing fast + with the exact snippet to add if it's missing, rather than wasting a + `makepkg` build on a repo pacman will never sync from. The repo + *directory* and its database file, by contrast, are fully self-healing — + `publish` creates the directory if missing and `repo-add` creates the + database on its first run. What's deliberately not automatic, and can't + safely be: writing the `[section]` into `/etc/pacman.conf` itself — that + needs root, which this process doesn't have and shouldn't grab for + itself. Similarly, publish deliberately stops at `repo-add`: getting the + new version onto the running system is a separate, deliberate `pacman -Syu`/`pacman -S ` step left to the operator, not run automatically.)* - **Reviewer queue**: for tiers 4–6, records the detected change instead of diff --git a/src/main.rs b/src/main.rs index cfbb055..73885a5 100644 --- a/src/main.rs +++ b/src/main.rs @@ -180,6 +180,12 @@ fn build_and_publish( tag: &str, fetched: &FetchVerifyResult, ) -> Result<()> { + // Fail fast if the repo isn't registered in pacman.conf, before + // spending several seconds on a makepkg build that would otherwise + // succeed and then publish somewhere pacman never syncs from. + let repo_dir = custom_repo_dir()?; + publisher::ensure_registered(CUSTOM_REPO_NAME, &repo_dir)?; + let build_dir = fetched.dest_dir.join("build"); let req = builder::BuildRequest { pkg_name: name, @@ -200,7 +206,6 @@ fn build_and_publish( println!(" sanity check passed"); } - let repo_dir = custom_repo_dir()?; let published = publisher::publish(&built.package_path, &repo_dir, CUSTOM_REPO_NAME)?; println!( " added to {} repo: {}", diff --git a/src/publisher.rs b/src/publisher.rs index 3cd675b..2fb65d9 100644 --- a/src/publisher.rs +++ b/src/publisher.rs @@ -2,16 +2,21 @@ use anyhow::{Context, Result, bail}; use std::path::{Path, PathBuf}; use std::process::Command; +/// Pacman's system-wide config — hardcoded like the rest of this tool's +/// Arch/Manjaro-specific assumptions (see SPEC.md > Scope). +const PACMAN_CONF: &str = "/etc/pacman.conf"; + /// Copies the built package into `repo_dir` and runs `repo-add` against -/// `.db.tar.gz` there. +/// `.db.tar.gz` there. Creates `repo_dir` if it doesn't exist +/// yet — `repo-add` itself creates the database file on its first run, so +/// everything filesystem-side is self-healing. /// -/// `repo_dir`/`repo_name` are expected to already be a real, registered -/// pacman repo (see SPEC.md > Architecture > Publisher and -/// `/etc/pacman.conf`'s `[custom]` section on this box) — pkgwatch doesn't -/// create the repo or touch pacman.conf, only adds packages to an -/// already-registered one. Getting the new version into an installed -/// system is a separate, deliberate `pacman -Syu`/`pacman -S` step left to -/// the operator, not run automatically here. +/// What is *not* self-healing, and can't safely be: registering +/// `repo_name` in `/etc/pacman.conf` (see `ensure_registered`) — that +/// needs root, which this process doesn't have and shouldn't grab for +/// itself. Getting a published version onto the running system is a +/// separate, deliberate `pacman -Syu`/`pacman -S` step too, also left to +/// the operator. pub fn publish(package_path: &Path, repo_dir: &Path, repo_name: &str) -> Result { publish_with(Path::new("repo-add"), package_path, repo_dir, repo_name) } @@ -51,6 +56,38 @@ fn publish_with( Ok(dest) } +/// Verifies `repo_name` is registered as an active `[section]` in +/// `/etc/pacman.conf`, so a build isn't wasted on a repo pacman will never +/// actually sync from. Call this before `publish` — ideally before even +/// starting the build, so a missing repo fails fast instead of after +/// several seconds of `makepkg` work. +/// +/// Doesn't check that the section's `Server =`/`Include =` line points at +/// `repo_dir` specifically — just that a repo by this name exists at all. +/// A same-named repo pointed somewhere else is a rare, easily-diagnosed +/// misconfiguration, not worth the parsing complexity to catch here. +pub fn ensure_registered(repo_name: &str, repo_dir: &Path) -> Result<()> { + ensure_registered_at(Path::new(PACMAN_CONF), repo_name, repo_dir) +} + +fn ensure_registered_at(pacman_conf: &Path, repo_name: &str, repo_dir: &Path) -> Result<()> { + let conf = std::fs::read_to_string(pacman_conf) + .with_context(|| format!("reading {}", pacman_conf.display()))?; + let header = format!("[{repo_name}]"); + let registered = conf.lines().map(str::trim).any(|line| line == header); + if !registered { + bail!( + "'{repo_name}' is not registered in {} — add this once, as root, then re-run:\n\n\ + [{repo_name}]\n\ + SigLevel = Optional TrustAll\n\ + Server = file://{}\n", + pacman_conf.display(), + repo_dir.display() + ); + } + Ok(()) +} + #[cfg(test)] mod tests { use super::*; @@ -120,4 +157,47 @@ mod tests { publish_with(&repo_add, &package_path, &repo_dir, "custom").unwrap(); assert!(repo_dir.join("thing-1.0.0-1-x86_64.pkg.tar.zst").exists()); } + + fn write_pacman_conf(dir: &Path, contents: &str) -> PathBuf { + let path = dir.join("pacman.conf"); + std::fs::write(&path, contents).unwrap(); + path + } + + #[test] + fn ensure_registered_passes_when_section_present() { + let dir = tempfile::tempdir().unwrap(); + let conf = write_pacman_conf( + dir.path(), + "[options]\nArchitecture = auto\n\n[extra]\nInclude = /etc/pacman.d/mirrorlist\n\n[custom]\nSigLevel = Optional TrustAll\nServer = file:///home/austin/.local/share/pacman/custom\n", + ); + assert!(ensure_registered_at(&conf, "custom", Path::new("/repo")).is_ok()); + } + + #[test] + fn ensure_registered_fails_when_section_missing() { + let dir = tempfile::tempdir().unwrap(); + let conf = write_pacman_conf(dir.path(), "[options]\nArchitecture = auto\n\n[extra]\n"); + let err = ensure_registered_at(&conf, "custom", Path::new("/repo")).unwrap_err(); + let msg = err.to_string(); + assert!(msg.contains("not registered")); + assert!(msg.contains("[custom]")); + assert!(msg.contains("/repo")); + } + + #[test] + fn ensure_registered_does_not_match_substring_of_another_section() { + // "custom" must match the whole section header, not just appear + // as a substring of e.g. "[custom-extra]". + let dir = tempfile::tempdir().unwrap(); + let conf = write_pacman_conf(dir.path(), "[custom-extra]\nServer = file:///elsewhere\n"); + assert!(ensure_registered_at(&conf, "custom", Path::new("/repo")).is_err()); + } + + #[test] + fn ensure_registered_errors_when_pacman_conf_missing() { + let dir = tempfile::tempdir().unwrap(); + let missing = dir.path().join("does-not-exist.conf"); + assert!(ensure_registered_at(&missing, "custom", Path::new("/repo")).is_err()); + } }