Check the custom repo is registered in pacman.conf before building
PR feedback: the local repo isn't guaranteed to exist on every box this runs on, so it shouldn't just be assumed. publisher::ensure_registered checks /etc/pacman.conf for an active [<repo_name>] section before a build even starts, failing fast with the exact snippet to add if it's missing — instead of spending several seconds on a makepkg build that would succeed and then publish into a repo pacman never syncs from. The repo directory and its database file were already self-healing (publish creates the dir if missing, repo-add creates the db on first run) — the actual gap was the pacman.conf registration, which can't be made self-healing without root, so this fails loud with instructions instead of trying to write to /etc/pacman.conf itself. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
This commit is contained in:
parent
68fa648010
commit
6305743e4d
3 changed files with 107 additions and 13 deletions
17
SPEC.md
17
SPEC.md
|
|
@ -377,10 +377,19 @@ Open questions on the schema:
|
||||||
repo rather than one pkgwatch creates — this box already has one at
|
repo rather than one pkgwatch creates — this box already has one at
|
||||||
`~/.local/share/pacman/custom`, registered as `[custom]` in
|
`~/.local/share/pacman/custom`, registered as `[custom]` in
|
||||||
`/etc/pacman.conf` (`SigLevel = Optional TrustAll`) and already in use
|
`/etc/pacman.conf` (`SigLevel = Optional TrustAll`) and already in use
|
||||||
for a hand-packaged AppImage, resolving the "where does the repo live /
|
for a hand-packaged AppImage. But that repo directory/registration isn't
|
||||||
how does it get registered" open question below without pkgwatch ever
|
guaranteed to exist on every box this ever runs on, so it isn't just
|
||||||
touching pacman.conf. Deliberately stops at `repo-add`: getting the new
|
assumed: `publisher::ensure_registered` checks `/etc/pacman.conf` for an
|
||||||
version onto the running system is a separate, deliberate
|
active `[<repo_name>]` 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 <pkg>` step left to the operator, not run
|
`pacman -Syu`/`pacman -S <pkg>` step left to the operator, not run
|
||||||
automatically.)*
|
automatically.)*
|
||||||
- **Reviewer queue**: for tiers 4–6, records the detected change instead of
|
- **Reviewer queue**: for tiers 4–6, records the detected change instead of
|
||||||
|
|
|
||||||
|
|
@ -180,6 +180,12 @@ fn build_and_publish(
|
||||||
tag: &str,
|
tag: &str,
|
||||||
fetched: &FetchVerifyResult,
|
fetched: &FetchVerifyResult,
|
||||||
) -> Result<()> {
|
) -> 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 build_dir = fetched.dest_dir.join("build");
|
||||||
let req = builder::BuildRequest {
|
let req = builder::BuildRequest {
|
||||||
pkg_name: name,
|
pkg_name: name,
|
||||||
|
|
@ -200,7 +206,6 @@ fn build_and_publish(
|
||||||
println!(" sanity check passed");
|
println!(" sanity check passed");
|
||||||
}
|
}
|
||||||
|
|
||||||
let repo_dir = custom_repo_dir()?;
|
|
||||||
let published = publisher::publish(&built.package_path, &repo_dir, CUSTOM_REPO_NAME)?;
|
let published = publisher::publish(&built.package_path, &repo_dir, CUSTOM_REPO_NAME)?;
|
||||||
println!(
|
println!(
|
||||||
" added to {} repo: {}",
|
" added to {} repo: {}",
|
||||||
|
|
|
||||||
|
|
@ -2,16 +2,21 @@ use anyhow::{Context, Result, bail};
|
||||||
use std::path::{Path, PathBuf};
|
use std::path::{Path, PathBuf};
|
||||||
use std::process::Command;
|
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
|
/// Copies the built package into `repo_dir` and runs `repo-add` against
|
||||||
/// `<repo_name>.db.tar.gz` there.
|
/// `<repo_name>.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
|
/// What is *not* self-healing, and can't safely be: registering
|
||||||
/// pacman repo (see SPEC.md > Architecture > Publisher and
|
/// `repo_name` in `/etc/pacman.conf` (see `ensure_registered`) — that
|
||||||
/// `/etc/pacman.conf`'s `[custom]` section on this box) — pkgwatch doesn't
|
/// needs root, which this process doesn't have and shouldn't grab for
|
||||||
/// create the repo or touch pacman.conf, only adds packages to an
|
/// itself. Getting a published version onto the running system is a
|
||||||
/// already-registered one. Getting the new version into an installed
|
/// separate, deliberate `pacman -Syu`/`pacman -S` step too, also left to
|
||||||
/// system is a separate, deliberate `pacman -Syu`/`pacman -S` step left to
|
/// the operator.
|
||||||
/// the operator, not run automatically here.
|
|
||||||
pub fn publish(package_path: &Path, repo_dir: &Path, repo_name: &str) -> Result<PathBuf> {
|
pub fn publish(package_path: &Path, repo_dir: &Path, repo_name: &str) -> Result<PathBuf> {
|
||||||
publish_with(Path::new("repo-add"), package_path, repo_dir, repo_name)
|
publish_with(Path::new("repo-add"), package_path, repo_dir, repo_name)
|
||||||
}
|
}
|
||||||
|
|
@ -51,6 +56,38 @@ fn publish_with(
|
||||||
Ok(dest)
|
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)]
|
#[cfg(test)]
|
||||||
mod tests {
|
mod tests {
|
||||||
use super::*;
|
use super::*;
|
||||||
|
|
@ -120,4 +157,47 @@ mod tests {
|
||||||
publish_with(&repo_add, &package_path, &repo_dir, "custom").unwrap();
|
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());
|
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());
|
||||||
|
}
|
||||||
}
|
}
|
||||||
|
|
|
||||||
Loading…
Reference in a new issue