diff --git a/Cargo.lock b/Cargo.lock index 3c3515a..1fa6c5f 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -233,30 +233,6 @@ version = "1.1.1" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "1c133bc6a41be0d194c306b5506d15e6feeea7b1d6604bd3f8310dfb2ca96486" -[[package]] -name = "cmd_lib" -version = "2.0.1" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "dc93f60133215cad2ef47e47db5a386c78874a2630c2c2be68f0f543b3ce0c2c" -dependencies = [ - "cmd_lib_macros", - "env_logger", - "faccess", - "log", - "os_pipe", -] - -[[package]] -name = "cmd_lib_macros" -version = "2.0.1" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "bd39b5c2657c1a0064aa5d73636dac4a217edeeb6c5f8a5f52c123a758462ece" -dependencies = [ - "proc-macro2", - "quote", - "syn 2.0.119", -] - [[package]] name = "colorchoice" version = "1.0.5" @@ -538,17 +514,6 @@ dependencies = [ "windows-sys 0.61.2", ] -[[package]] -name = "faccess" -version = "0.2.4" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "59ae66425802d6a903e268ae1a08b8c38ba143520f227a205edf4e9c7e3e26d5" -dependencies = [ - "bitflags 1.3.2", - "libc", - "winapi", -] - [[package]] name = "fastrand" version = "2.5.0" @@ -1488,16 +1453,6 @@ version = "0.2.0" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "04744f49eae99ab78e0d5c0b603ab218f515ea8cfe5a456d7629ad883a3b6e7d" -[[package]] -name = "os_pipe" -version = "1.2.3" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "7d8fae84b431384b68627d0f9b3b1245fcf9f46f6c0e3dc902e9dce64edd1967" -dependencies = [ - "libc", - "windows-sys 0.61.2", -] - [[package]] name = "parking_lot" version = "0.12.5" @@ -1546,7 +1501,6 @@ dependencies = [ "bzip2-rs", "chrono", "clap", - "cmd_lib", "crossterm", "csv", "directories", @@ -1558,7 +1512,6 @@ dependencies = [ "hex", "indicatif", "indicatif-log-bridge", - "lazy_static", "libc", "log", "md-5", @@ -1576,6 +1529,7 @@ dependencies = [ "tar", "tempfile", "test-log", + "thiserror", "tokio", "unicode-width", "xz2", @@ -2288,9 +2242,7 @@ dependencies = [ "bytes 1.12.1", "libc", "mio", - "parking_lot", "pin-project-lite", - "signal-hook-registry", "socket2", "tokio-macros", "windows-sys 0.61.2", diff --git a/Cargo.toml b/Cargo.toml index b596e8b..5097224 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -10,7 +10,6 @@ readme = "README.md" [dependencies] clap = { version = "4.5.51", features = ["cargo"] } -cmd_lib = "2.0.0" crossterm = "0.28" flate2 = "1.1.5" serde = { version = "1.0.228", features = ["derive"] } @@ -20,7 +19,7 @@ reqwest = { version = "0.12.9", features = ["blocking", "json", "stream"] } git2 = "0.20.2" regex = "1" chrono = "0.4" -tokio = { version = "1.41.1", features = ["full"] } +tokio = { version = "1.41.1", features = ["rt-multi-thread", "macros", "time", "sync"] } sha2 = "0.10.8" sha1 = "0.10" md-5 = "0.10" @@ -37,11 +36,11 @@ directories = "6.0.0" ssh2 = "0.9.5" gpgme = "0.11" serde_yaml = "0.9" -lazy_static = "1.4.0" unicode-width = "0.2" parking_lot = "0.12" suppaftp = "12" bzip2-rs = "0.1.2" +thiserror = "2" [dev-dependencies] test-log = "0.2.19" diff --git a/src/apt/release.rs b/src/apt/release.rs index 412c695..35d1482 100644 --- a/src/apt/release.rs +++ b/src/apt/release.rs @@ -39,13 +39,14 @@ //! the missing authenticity only produces a `warn!`. //! //! Only actual tampering evidence — an invalid signature, or an index whose -//! hash does not match the signed Release — is reported as [`VerifyError`]. +//! hash does not match the signed Release — is reported as an error: +//! [`VerifyError`] for the suite signature, [`FileVerifyError`] for an +//! index that fails its Release checksums. //! //! ASCII-armored keyrings (Debian distributes `.asc` keyrings, and //! keyserver responses are armored) are dearmored in-process, so no `gpg` //! binary is needed. -use lazy_static::lazy_static; use log::{debug, warn}; use serde::Deserialize; use std::collections::HashMap; @@ -53,6 +54,7 @@ use std::error::Error; use std::fmt; use std::path::Path; use std::path::PathBuf; +use std::sync::LazyLock; use std::sync::Mutex; use std::sync::PoisonError; @@ -227,48 +229,101 @@ impl VerifiedRelease { /// or, for flat repositories, 'Sources.xz'). Both the digest and the /// size are checked; a file not listed in the Release file is refused, /// as it is not covered by the repository signature. - pub fn verify_file(&self, rel_path: &str, data: &[u8]) -> Result<(), String> { + pub fn verify_file(&self, rel_path: &str, data: &[u8]) -> Result<(), FileVerifyError> { if !self.available { - return Err(format!( - "no Release file was found for '{}': cannot checksum-verify '{rel_path}'", - self.suite_url - )); + return Err(FileVerifyError::NoReleaseFile { + suite_url: self.suite_url.clone(), + path: rel_path.to_string(), + }); } - let entry = self.checksums.get(rel_path).ok_or_else(|| { - format!( - "'{rel_path}' is not listed in the Release file of '{}'; refusing \ - to use an index that is not covered by the repository metadata", - self.suite_url - ) - })?; + let entry = self + .checksums + .get(rel_path) + .ok_or_else(|| FileVerifyError::NotListed { + path: rel_path.to_string(), + suite_url: self.suite_url.clone(), + })?; let digest = entry.kind.digest(data); if !digest.eq_ignore_ascii_case(&entry.hash) { - return Err(format!( - "checksum mismatch for '{rel_path}' of '{}': the Release file \ - expects {} {} but the downloaded file hashes to {}; the index \ - does not match the signed repository metadata", - self.suite_url, - entry.kind.name(), - entry.hash, - digest - )); + return Err(FileVerifyError::ChecksumMismatch { + path: rel_path.to_string(), + suite_url: self.suite_url.clone(), + algorithm: entry.kind.name().to_string(), + expected: entry.hash.clone(), + actual: digest, + }); } if entry.size > 0 && entry.size != data.len() as u64 { - return Err(format!( - "size mismatch for '{rel_path}' of '{}': expected {} bytes, got {}", - self.suite_url, - entry.size, - data.len() - )); + return Err(FileVerifyError::SizeMismatch { + path: rel_path.to_string(), + suite_url: self.suite_url.clone(), + expected: entry.size, + actual: data.len(), + }); } Ok(()) } } +/// Failure of [`VerifiedRelease::verify_file`]: the downloaded index is not +/// covered by the suite's signed Release metadata. +#[derive(Debug, Clone, PartialEq, Eq, thiserror::Error)] +pub enum FileVerifyError { + /// No Release file was seen on the suite, so there is nothing to + /// verify against. + #[error("no Release file was found for '{suite_url}': cannot checksum-verify '{path}'")] + NoReleaseFile { + /// URL of the suite directory. + suite_url: String, + /// The path that was to be verified. + path: String, + }, + /// The Release file does not list the path: it is not covered by the + /// repository signature. + #[error( + "'{path}' is not listed in the Release file of '{suite_url}'; refusing to use an index that is not covered by the repository metadata" + )] + NotListed { + /// The unlisted path. + path: String, + /// URL of the suite directory. + suite_url: String, + }, + /// The downloaded file does not hash to the Release entry. + #[error( + "checksum mismatch for '{path}' of '{suite_url}': the Release file expects {algorithm} {expected} but the downloaded file hashes to {actual}; the index does not match the signed repository metadata" + )] + ChecksumMismatch { + /// The verified path. + path: String, + /// URL of the suite directory. + suite_url: String, + /// Name of the checksum algorithm (`SHA256`, ...). + algorithm: String, + /// The digest the Release file lists. + expected: String, + /// The digest of the downloaded bytes. + actual: String, + }, + /// The downloaded file's length differs from the Release entry (a + /// zeroed entry skips the check). + #[error("size mismatch for '{path}' of '{suite_url}': expected {expected} bytes, got {actual}")] + SizeMismatch { + /// The verified path. + path: String, + /// URL of the suite directory. + suite_url: String, + /// The size the Release file lists. + expected: u64, + /// The size of the downloaded bytes. + actual: usize, + }, +} + /// Parse a Release file body (the signed payload of an InRelease, or a /// plain Release file) into a [`VerifiedRelease`] fn parse_release_body(suite_url: &str, body: &str, authenticated: bool) -> VerifiedRelease { @@ -793,11 +848,10 @@ async fn fetch_optional(url: &str) -> Option> { None } -lazy_static! { - /// Downloaded keyrings, keyed by URL: keyrings are small but would - /// otherwise be re-downloaded once per series/pocket lookup - static ref KEYRING_CACHE: Mutex>> = Mutex::new(HashMap::new()); -} +/// Downloaded keyrings, keyed by URL: keyrings are small but would +/// otherwise be re-downloaded once per series/pocket lookup +static KEYRING_CACHE: LazyLock>>> = + LazyLock::new(|| Mutex::new(HashMap::new())); /// Read the keyring cache, ignoring poisoning (the map is only corrupted on /// panics, in which case a fresh map is just as good) @@ -914,9 +968,33 @@ pub async fn ppa_keyring_bytes( Ok(keyring) } +/// Failure of the OpenPGP armor and base64 decoders ([`dearmor`], +/// [`base64_decode`]). +#[derive(Debug, Clone, PartialEq, Eq, thiserror::Error)] +pub enum ArmorError { + /// The armored input is not valid UTF-8. + #[error("the armored keyring is not valid UTF-8")] + NotUtf8, + /// The armor CRC-24 does not match the decoded payload. + #[error("the armor CRC-24 checksum does not match")] + CrcMismatch, + /// An armor marker was present but no payload followed it. + #[error("no armored OpenPGP data found")] + NoArmoredData, + /// A byte outside the base64 alphabet. + #[error("invalid base64 character '{c}'")] + InvalidBase64 { + /// The offending character. + c: char, + }, + /// The base64 payload ends mid-group. + #[error("truncated base64 data")] + Truncated, +} + /// Decode an OpenPGP ASCII-armored keyring into its binary form (the /// equivalent of 'gpg --dearmor'). Binary input passes through unchanged. -fn dearmor(data: &[u8]) -> Result, String> { +fn dearmor(data: &[u8]) -> Result, ArmorError> { // Only input starting with an armor marker is armored; anything else is // assumed to already be a binary keyring let first = data @@ -927,8 +1005,7 @@ fn dearmor(data: &[u8]) -> Result, String> { return Ok(data.to_vec()); } - let text = std::str::from_utf8(data) - .map_err(|_| "the armored keyring is not valid UTF-8".to_string())?; + let text = std::str::from_utf8(data).map_err(|_| ArmorError::NotUtf8)?; #[derive(PartialEq)] enum State { @@ -964,7 +1041,7 @@ fn dearmor(data: &[u8]) -> Result, String> { | (u32::from(expected[1]) << 8) | u32::from(expected[2]); if value != crc24(&decoded) { - return Err("the armor CRC-24 checksum does not match".to_string()); + return Err(ArmorError::CrcMismatch); } } } @@ -982,21 +1059,21 @@ fn dearmor(data: &[u8]) -> Result, String> { } if out.is_empty() { - return Err("no armored OpenPGP data found".to_string()); + return Err(ArmorError::NoArmoredData); } Ok(out) } /// Decode standard base64, ignoring whitespace; '=' padding is optional -fn base64_decode(input: &str) -> Result, String> { - fn value(byte: u8) -> Result { +fn base64_decode(input: &str) -> Result, ArmorError> { + fn value(byte: u8) -> Result { match byte { b'A'..=b'Z' => Ok(u32::from(byte - b'A')), b'a'..=b'z' => Ok(u32::from(byte - b'a' + 26)), b'0'..=b'9' => Ok(u32::from(byte - b'0' + 52)), b'+' => Ok(62), b'/' => Ok(63), - _ => Err(format!("invalid base64 character '{}'", byte as char)), + _ => Err(ArmorError::InvalidBase64 { c: byte as char }), } } @@ -1016,7 +1093,7 @@ fn base64_decode(input: &str) -> Result, String> { } 3 => (value(chunk[0])? << 18) | (value(chunk[1])? << 12) | (value(chunk[2])? << 6), 2 => (value(chunk[0])? << 18) | (value(chunk[1])? << 12), - _ => return Err("truncated base64 data".to_string()), + _ => return Err(ArmorError::Truncated), }; out.push((n >> 16) as u8); if chunk.len() >= 3 { @@ -1155,20 +1232,23 @@ garbage // Tampered content fails with a checksum mismatch let err = verified .verify_file("main/source/Sources.gz", b"tampered!") - .unwrap_err(); + .unwrap_err() + .to_string(); assert!(err.contains("checksum mismatch"), "got: {err}"); // A correct hash with a wrong size also fails let err = verified .verify_file("main/source/Sizes.gz", b"test") - .unwrap_err(); + .unwrap_err() + .to_string(); assert!(err.contains("size mismatch"), "got: {err}"); // A file not listed in the Release file is refused: it is not // covered by the repository signature let err = verified .verify_file("evil/source/Sources.gz", b"test") - .unwrap_err(); + .unwrap_err() + .to_string(); assert!(err.contains("not listed"), "got: {err}"); // Without a Release file nothing can be verified diff --git a/src/build/env.rs b/src/build/env.rs index 30113f8..96ef4e8 100644 --- a/src/build/env.rs +++ b/src/build/env.rs @@ -81,7 +81,7 @@ pub fn build_env( /// (`*_ARCH`, `*_OS`, `*_CPU`, `*_MULTIARCH`, `*_GNU_TYPE`, ...), exactly as /// `dpkg-buildpackage` does. pub fn arch_env(host_arch: Option<&str>) -> Result, String> { - crate::debian::arch::arch_env(host_arch) + crate::debian::arch::arch_env(host_arch).map_err(|e| e.to_string()) } /// Read the current vendor name from the active dpkg origins `default` file diff --git a/src/build/mod.rs b/src/build/mod.rs index 0573c43..30d1d54 100644 --- a/src/build/mod.rs +++ b/src/build/mod.rs @@ -2284,13 +2284,13 @@ Provides: virtual-thing (= 2.0), plain-virtual Some(dist), None, None, - None, + &crate::report::Quiet, )) .unwrap_or_else(|e| panic!("package lookup failed for {}: {}", package, e)); rt.block_on(crate::pull::pull( &package_info, Some(fetch_dir.path()), - None, + &crate::report::Quiet, true, )) .unwrap_or_else(|e| panic!("pull failed for {}: {}", package, e)); diff --git a/src/data.rs b/src/data.rs index e750ec7..970b66c 100644 --- a/src/data.rs +++ b/src/data.rs @@ -13,7 +13,7 @@ //! data/quirks.yml) through the [`embed_data!`] macro below, so data //! and its accessors stay together and a diff touching one domain cannot //! half-touch another. The macro embeds the file at compile time and -//! parses it once into a `lazy_static` on first use; since the data ships +//! parses it once into a [`LazyLock`] on first use; since the data ships //! inside the binary, a parse failure is a build-time bug that cannot be //! recovered from at runtime, and the macro panics on it. //! @@ -30,8 +30,9 @@ /// the invoking source file (`"../data/distro_info.yml"` from /// `src/distro_info.rs`, `"../../data/host_keys.yml"` from /// `src/put/ssh.rs`, ...), and expands to the house `include_str!` → -/// `lazy_static` → parse pattern — only the embed+parse boilerplate is -/// generated. +/// parse-on-first-use pattern — only the embed+parse boilerplate is +/// generated. The `static ref` spelling is kept from the macro's +/// `lazy_static!` ancestry; it expands to a plain `LazyLock` static. /// /// ```ignore /// embed_data! { @@ -39,31 +40,19 @@ /// } /// ``` macro_rules! embed_data { - // Internal arm: the visibility arrives wrapped in parentheses (empty for - // private statics) because `lazy_static!` only re-matches literal - // `pub`/`pub(...)` token sequences, not an opaque forwarded `vis`. - (@expand ($($vis:tt)*) static ref $name:ident : $ty:ty = $path:literal) => { - lazy_static::lazy_static! { - // The YAML is include_str!'d at compile time and statically - // valid; if it ever failed to parse it would be a build-time bug - // that cannot be recovered from at runtime, so panicking here is - // acceptable. - $($vis)* static ref $name: $ty = serde_yaml::from_str(include_str!($path)) + ($vis:vis static ref $name:ident : $ty:ty = $path:literal) => { + // The YAML is include_str!'d at compile time and statically + // valid; if it ever failed to parse it would be a build-time bug + // that cannot be recovered from at runtime, so panicking here is + // acceptable. + $vis static $name: ::std::sync::LazyLock<$ty> = ::std::sync::LazyLock::new(|| { + serde_yaml::from_str(include_str!($path)) .expect(concat!( "built-in ", $path, " data is statically valid and must parse" - )); - } - }; - (static ref $name:ident : $ty:ty = $path:literal) => { - $crate::data::embed_data!(@expand () static ref $name : $ty = $path); - }; - (pub static ref $name:ident : $ty:ty = $path:literal) => { - $crate::data::embed_data!(@expand (pub) static ref $name : $ty = $path); - }; - (pub ($($vis:tt)+) static ref $name:ident : $ty:ty = $path:literal) => { - $crate::data::embed_data!(@expand (pub ($($vis)+)) static ref $name : $ty = $path); + )) + }); }; } diff --git a/src/deb/mod.rs b/src/deb/mod.rs index 0c27e1a..2f62376 100644 --- a/src/deb/mod.rs +++ b/src/deb/mod.rs @@ -1360,11 +1360,19 @@ mod tests { log::debug!("Created temporary directory: {}", cwd.display()); log::info!("Pulling package {} from {}...", package, series); - let package_info = - crate::package_info::lookup(package, None, Some(series), "", dist, None, None, None) - .await - .expect("Cannot lookup package information"); - crate::pull::pull(&package_info, Some(cwd), None, true) + let package_info = crate::package_info::lookup( + package, + None, + Some(series), + "", + dist, + None, + None, + &crate::report::Quiet, + ) + .await + .expect("Cannot lookup package information"); + crate::pull::pull(&package_info, Some(cwd), &crate::report::Quiet, true) .await .expect("Cannot pull package"); log::info!("Successfully pulled package {}", package); diff --git a/src/debian/arch.rs b/src/debian/arch.rs index 9d47e1e..cb775aa 100644 --- a/src/debian/arch.rs +++ b/src/debian/arch.rs @@ -682,14 +682,45 @@ pub fn is_invalid(arch: &str, positive: bool) -> bool { } } +/// Failure of the architecture helpers: an invalid list entry, an +/// undetectable native architecture, or an architecture unknown to dpkg's +/// tables. +#[derive(Debug, Clone, PartialEq, Eq, thiserror::Error)] +pub enum ArchError { + /// An entry of the list is not a valid architecture name. + #[error("'{arch}' is not a valid architecture in list '{list}'")] + InvalidInList { + /// The offending architecture name. + arch: String, + /// The whole list. + list: String, + }, + /// Neither dpkg nor uname could name the native architecture. + #[error("cannot determine native Debian architecture")] + NativeUndetermined, + /// The architecture is not in dpkg's tables. + #[error("unknown Debian architecture '{arch}'")] + Unknown { + /// The unknown architecture name. + arch: String, + }, + /// A GNU triplet without the expected `cpu-system` shape. + #[error("invalid GNU triplet '{triplet}'")] + InvalidTriplet { + /// The malformed triplet. + triplet: String, + }, +} + /// Parse a whitespace-separated architecture list, validating every entry. -pub fn list_parse(list: &str) -> Result, String> { +pub fn list_parse(list: &str) -> Result, ArchError> { let arches: Vec = list.split_whitespace().map(str::to_string).collect(); for arch in &arches { if is_invalid(arch, false) { - return Err(format!( - "'{arch}' is not a valid architecture in list '{list}'" - )); + return Err(ArchError::InvalidInList { + arch: arch.clone(), + list: list.to_string(), + }); } } Ok(arches) @@ -802,7 +833,7 @@ fn from_uname() -> Option { /// the `DEB_BUILD_*` variables): the authoritative answer comes from the /// dpkg database itself; if the `dpkg` frontend cannot be executed, the /// architecture is derived from `uname` through the same tables. -pub fn native() -> Result { +pub fn native() -> Result { if let Ok(output) = Command::new("dpkg").arg("--print-architecture").output() && output.status.success() { @@ -811,7 +842,7 @@ pub fn native() -> Result { return Ok(arch); } } - from_uname().ok_or_else(|| "cannot determine native Debian architecture".to_string()) + from_uname().ok_or(ArchError::NativeUndetermined) } /// Compute the complete architecture environment, the equivalent of @@ -821,7 +852,7 @@ pub fn native() -> Result { /// The target architecture defaults to the host architecture, and the host /// architecture defaults to the native build architecture, exactly like /// dpkg-architecture. -pub fn arch_env(host_arch: Option<&str>) -> Result, String> { +pub fn arch_env(host_arch: Option<&str>) -> Result, ArchError> { let build_arch = native()?; let host_arch = host_arch.unwrap_or(&build_arch).to_string(); let target_arch = host_arch.clone(); @@ -832,8 +863,9 @@ pub fn arch_env(host_arch: Option<&str>) -> Result, Str ("HOST", host_arch), ("TARGET", target_arch), ] { - let tuple = debarch_to_debtuple(&arch) - .ok_or_else(|| format!("unknown Debian architecture '{arch}'"))?; + let tuple = debarch_to_debtuple(&arch).ok_or_else(|| ArchError::Unknown { + arch: arch.to_string(), + })?; env.insert(format!("DEB_{role}_ARCH"), arch.clone()); env.insert(format!("DEB_{role}_ARCH_ABI"), tuple.abi.clone()); @@ -841,20 +873,26 @@ pub fn arch_env(host_arch: Option<&str>) -> Result, Str env.insert(format!("DEB_{role}_ARCH_OS"), tuple.os.clone()); env.insert(format!("DEB_{role}_ARCH_CPU"), tuple.cpu.clone()); - let (bits, endian) = - abi_attrs(&arch).ok_or_else(|| format!("unknown Debian architecture '{arch}'"))?; + let (bits, endian) = abi_attrs(&arch).ok_or_else(|| ArchError::Unknown { + arch: arch.to_string(), + })?; env.insert(format!("DEB_{role}_ARCH_BITS"), bits.to_string()); env.insert(format!("DEB_{role}_ARCH_ENDIAN"), endian.to_string()); - let multi = - multiarch(&arch).ok_or_else(|| format!("unknown Debian architecture '{arch}'"))?; + let multi = multiarch(&arch).ok_or_else(|| ArchError::Unknown { + arch: arch.to_string(), + })?; env.insert(format!("DEB_{role}_MULTIARCH"), multi); - let gnu_type = debarch_to_gnutriplet(&arch) - .ok_or_else(|| format!("unknown Debian architecture '{arch}'"))?; - let (gnu_cpu, gnu_system) = gnu_type - .split_once('-') - .ok_or_else(|| format!("invalid GNU triplet '{gnu_type}'"))?; + let gnu_type = debarch_to_gnutriplet(&arch).ok_or_else(|| ArchError::Unknown { + arch: arch.to_string(), + })?; + let (gnu_cpu, gnu_system) = + gnu_type + .split_once('-') + .ok_or_else(|| ArchError::InvalidTriplet { + triplet: gnu_type.clone(), + })?; env.insert(format!("DEB_{role}_GNU_CPU"), gnu_cpu.to_string()); env.insert(format!("DEB_{role}_GNU_SYSTEM"), gnu_system.to_string()); env.insert(format!("DEB_{role}_GNU_TYPE"), gnu_type); diff --git a/src/debian/checksums.rs b/src/debian/checksums.rs index be041cb..021135e 100644 --- a/src/debian/checksums.rs +++ b/src/debian/checksums.rs @@ -100,6 +100,48 @@ pub struct FileChecksums { index: HashMap, } +/// Failure of [`FileChecksums::parse_field`]: a malformed checksum field +/// line, naming the field, the offending line and what is wrong with it. +#[derive(Debug, Clone, PartialEq, Eq, thiserror::Error)] +pub enum ChecksumParseError { + /// The line has other than `checksum size name` columns. + #[error( + "malformed '{field}' line (expected 'checksum size name', got {columns} columns): '{line}'" + )] + MalformedLine { + /// The checksum field the line came from (`Checksums-Sha256`, ...). + field: String, + /// The actual number of whitespace-separated columns. + columns: usize, + /// The offending line. + line: String, + }, + /// The size column is not a number. + #[error("malformed '{field}' line (size '{size}' is not a number): '{line}'")] + BadSize { + /// The checksum field the line came from. + field: String, + /// The non-numeric size text. + size: String, + /// The offending line. + line: String, + }, + /// The digest is not the expected number of lowercase hex characters. + #[error( + "malformed '{field}' line (digest '{digest}' is not {len} lowercase hex characters): '{line}'" + )] + BadDigest { + /// The checksum field the line came from. + field: String, + /// The malformed digest. + digest: String, + /// The expected digest length. + len: usize, + /// The offending line. + line: String, + }, +} + impl FileChecksums { /// Create an empty registry. pub fn new() -> Self { @@ -234,7 +276,11 @@ impl FileChecksums { /// `build::parse_checksum_field` helper (which additionally accepts the /// legacy 5-column `Files` layout); the two are intentionally not unified /// across modules. - pub fn parse_field(kind: ChecksumKind, value: &str) -> Result, String> { + pub fn parse_field( + kind: ChecksumKind, + value: &str, + ) -> Result, ChecksumParseError> { + let field = kind.field_name().to_string(); let mut entries = Vec::new(); for line in value.lines() { if line.trim().is_empty() { @@ -242,30 +288,28 @@ impl FileChecksums { } let tokens: Vec<&str> = line.split_whitespace().collect(); let [digest, size, name] = tokens.as_slice() else { - return Err(format!( - "malformed '{}' line (expected 'checksum size name', got {} \ - columns): '{line}'", - kind.field_name(), - tokens.len() - )); + return Err(ChecksumParseError::MalformedLine { + field, + columns: tokens.len(), + line: line.to_string(), + }); }; - let size: u64 = size.parse().map_err(|_| { - format!( - "malformed '{}' line (size '{size}' is not a number): '{line}'", - kind.field_name() - ) + let size: u64 = size.parse().map_err(|_| ChecksumParseError::BadSize { + field: field.clone(), + size: size.to_string(), + line: line.to_string(), })?; let digest_ok = digest.len() == kind.digest_len() && digest .bytes() .all(|b| b.is_ascii_digit() || (b'a'..=b'f').contains(&b)); if !digest_ok { - return Err(format!( - "malformed '{}' line (digest '{digest}' is not {} lowercase \ - hex characters): '{line}'", - kind.field_name(), - kind.digest_len() - )); + return Err(ChecksumParseError::BadDigest { + field, + digest: digest.to_string(), + len: kind.digest_len(), + line: line.to_string(), + }); } let mut entry = Entry { size, @@ -401,27 +445,32 @@ mod tests { assert_eq!(entries[0].1.size, 12); // 2 columns: missing the name. - let err = - FileChecksums::parse_field(ChecksumKind::Sha512, &format!("{digest} 12")).unwrap_err(); + let err = FileChecksums::parse_field(ChecksumKind::Sha512, &format!("{digest} 12")) + .unwrap_err() + .to_string(); assert!(err.contains("Checksums-Sha512"), "{err}"); assert!(err.contains(&format!("{digest} 12")), "{err}"); // 4 columns. let err = FileChecksums::parse_field(ChecksumKind::Sha512, &format!(" {digest} 12 bogus a.txt")) - .unwrap_err(); + .unwrap_err() + .to_string(); assert!(err.contains("Checksums-Sha512"), "{err}"); assert!(err.contains("a.txt"), "{err}"); // Non-numeric size. let err = FileChecksums::parse_field(ChecksumKind::Sha512, &format!(" {digest} twelve a.txt")) - .unwrap_err(); + .unwrap_err() + .to_string(); assert!(err.contains("not a number"), "{err}"); assert!(err.contains("twelve"), "{err}"); // Digest that is not 128 lowercase hex characters. - let err = FileChecksums::parse_field(ChecksumKind::Sha512, " abc123 12 a.txt").unwrap_err(); + let err = FileChecksums::parse_field(ChecksumKind::Sha512, " abc123 12 a.txt") + .unwrap_err() + .to_string(); assert!(err.contains("lowercase hex"), "{err}"); } diff --git a/src/debian/control.rs b/src/debian/control.rs index 91b4401..cf7de59 100644 --- a/src/debian/control.rs +++ b/src/debian/control.rs @@ -403,6 +403,21 @@ pub struct ControlInfo { pub binaries: Vec, } +/// Failure of [`ControlInfo::parse_content`]: the content is not shaped +/// like a `debian/control` file. +#[derive(Debug, Clone, PartialEq, Eq, thiserror::Error)] +pub enum ControlParseError { + /// The content has no paragraphs at all. + #[error("control file has no paragraphs")] + NoParagraphs, + /// The first (source) stanza carries no `Source` field. + #[error("first control paragraph has no 'Source' field")] + NoSourceField, + /// A binary stanza carries no `Package` field. + #[error("binary control paragraph has no 'Package' field")] + NoBinaryPackageField, +} + impl ControlInfo { /// Parse a `debian/control` file. pub fn parse(path: &Path) -> Result> { @@ -416,19 +431,17 @@ impl ControlInfo { /// Parse control content from a string. /// /// Prefer [`std::str::FromStr`] (`"...".parse::()`). - pub fn parse_content(content: &str) -> Result { + pub fn parse_content(content: &str) -> Result { let paragraphs = parse_paragraphs(content); let mut iter = paragraphs.into_iter(); - let source = iter - .next() - .ok_or_else(|| "control file has no paragraphs".to_string())?; + let source = iter.next().ok_or(ControlParseError::NoParagraphs)?; if source.get("Source").is_none() { - return Err("first control paragraph has no 'Source' field".to_string()); + return Err(ControlParseError::NoSourceField); } let binaries: Vec = iter.collect(); for bin in &binaries { if bin.get("Package").is_none() { - return Err("binary control paragraph has no 'Package' field".to_string()); + return Err(ControlParseError::NoBinaryPackageField); } } Ok(ControlInfo { source, binaries }) @@ -451,7 +464,7 @@ impl ControlInfo { } impl std::str::FromStr for ControlInfo { - type Err = String; + type Err = ControlParseError; fn from_str(content: &str) -> Result { ControlInfo::parse_content(content) diff --git a/src/debian/deps.rs b/src/debian/deps.rs index 535f7bb..76d0f76 100644 --- a/src/debian/deps.rs +++ b/src/debian/deps.rs @@ -167,22 +167,96 @@ fn profile_name_regex() -> &'static Regex { }) } -/// Parse one simple dependency alternative. -/// -/// Fails when the text does not match the grammar, when `:native` appears in -/// a non-build-dependency context, or when the architecture/version/profile -/// syntax is invalid. -pub fn parse_simple(dep: &str, build_dep: bool) -> Result { +/// Failure of the dependency parsers: a syntactically invalid relation or +/// restriction formula, an invalid parse option, or an unreadable dpkg +/// status file. +#[derive(Debug, thiserror::Error)] +pub enum ParseError { + /// The dependency text does not match the package relation grammar. + #[error("cannot parse dependency '{dep}'")] + Syntax { + /// The unparsable dependency text. + dep: String, + }, + /// `:native` appeared outside a build-dependency context. + #[error("':native' qualifier only allowed in build dependencies: '{dep}'")] + NativeOutsideBuildDep { + /// The offending dependency text. + dep: String, + }, + /// The version relation operator is not one of `<< <= = >= >>`. + #[error("invalid relation '{relation}' in '{dep}'")] + InvalidRelation { + /// The unrecognized operator. + relation: String, + /// The dependency text carrying it. + dep: String, + }, + /// The constrained version does not parse. + #[error("invalid version in dependency '{dep}': {source}")] + InvalidVersion { + /// The dependency text carrying it. + dep: String, + /// The version parse failure. + source: crate::debian::version::VersionParseError, + }, + /// The `[...]` architecture list does not parse. + #[error("invalid architecture list in '{dep}': {source}")] + InvalidArchList { + /// The dependency text carrying it. + dep: String, + /// The architecture list failure. + source: crate::debian::arch::ArchError, + }, + /// A `[...]` restriction formula reduced to nothing. + #[error("empty restriction formula in '{dep}'")] + EmptyRestrictions { + /// The dependency text carrying it. + dep: String, + }, + /// A `<...>` formula carries a non-profile term. + #[error("'{formula}' is not a valid build profile restriction formula")] + InvalidProfileFormula { + /// The offending formula. + formula: String, + }, + /// [`ParseOpts::host_arch`] is empty or not a valid architecture. + #[error("invalid host_arch {host_arch}")] + InvalidHostArch { + /// The offending host architecture. + host_arch: String, + }, + /// `union` mode saw a clause with several alternatives. + #[error("an union dependency can only contain simple dependencies")] + UnionNotSimple, + /// The dpkg status file could not be read. + #[error("cannot read status file '{path}': {source}")] + ReadStatus { + /// The status file path. + path: String, + /// The read failure. + source: std::io::Error, + }, +} + +/// Parse one simple (no alternatives) package relation: `pkg` +/// (`pkg:qualifier`, `pkg (op version)`, `pkg [arch list]`, +/// `pkg ` combinations), validating the pieces: `:native` +/// is rejected in a non-build-dependency context, or when the +/// architecture/version/profile syntax is invalid. +pub fn parse_simple(dep: &str, build_dep: bool) -> Result { let caps = dep_regex() .captures(dep) - .ok_or_else(|| format!("cannot parse dependency '{dep}'"))?; + .ok_or_else(|| ParseError::Syntax { + dep: dep.to_string(), + })?; let package = caps[2].to_string(); let arch_qualifier = caps.get(3).map(|m| m.as_str().to_string()); if arch_qualifier.as_deref() == Some("native") && !build_dep { - return Err(format!( - "':native' qualifier only allowed in build dependencies: '{dep}'" - )); + return Err(ParseError::NativeOutsideBuildDep { + dep: dep.to_string(), + }); } let constraint = match (caps.get(5), caps.get(6)) { @@ -197,36 +271,49 @@ pub fn parse_simple(dep: &str, build_dep: bool) -> Result { "=" => Relation::Eq, ">" | ">=" => Relation::Ge, ">>" => Relation::Gt, - other => return Err(format!("invalid relation '{other}' in '{dep}'")), + other => { + return Err(ParseError::InvalidRelation { + relation: other.to_string(), + dep: dep.to_string(), + }); + } }; - let version = DebianVersion::parse(version.as_str()) - .map_err(|e| format!("invalid version in dependency '{dep}': {e}"))?; + let version = DebianVersion::parse(version.as_str()).map_err(|source| { + ParseError::InvalidVersion { + dep: dep.to_string(), + source, + } + })?; Some(VersionConstraint { relation, version }) } _ => None, }; - let arches = match caps.get(8) { - Some(list) => Some( - arch::list_parse(list.as_str()) - .map_err(|e| format!("invalid architecture list in '{dep}': {e}"))?, - ), - None => None, - }; + let arches = + match caps.get(8) { + Some(list) => Some(arch::list_parse(list.as_str()).map_err(|source| { + ParseError::InvalidArchList { + dep: dep.to_string(), + source, + } + })?), + None => None, + }; let mut restrictions = Vec::new(); if let Some(formula) = caps.get(9) { for group in restriction_group_regex().captures_iter(formula.as_str()) { let terms: Vec = group[1].split_whitespace().map(str::to_string).collect(); if terms.is_empty() { - return Err(format!("empty restriction formula in '{dep}'")); + return Err(ParseError::EmptyRestrictions { + dep: dep.to_string(), + }); } for term in &terms { if !profile_name_regex().is_match(term) { - return Err(format!( - "'{}' is not a valid build profile restriction formula", - formula.as_str() - )); + return Err(ParseError::InvalidProfileFormula { + formula: formula.as_str().to_string(), + }); } } restrictions.push(terms); @@ -291,13 +378,19 @@ impl Deps { /// alternatives whose architecture or profile restrictions do not apply /// are dropped first; clauses losing all their alternatives disappear, /// and a fully reduced field yields an empty [`Deps`]. - pub fn parse(input: &str, opts: &ParseOpts) -> Result { + pub fn parse(input: &str, opts: &ParseOpts) -> Result { Self::parse_inner(input, opts, false) } - fn parse_inner(input: &str, opts: &ParseOpts, reduce_arch_only: bool) -> Result { + fn parse_inner( + input: &str, + opts: &ParseOpts, + reduce_arch_only: bool, + ) -> Result { if opts.host_arch.is_empty() || arch::is_invalid(&opts.host_arch, true) { - return Err(format!("invalid host_arch {}", opts.host_arch)); + return Err(ParseError::InvalidHostArch { + host_arch: opts.host_arch.clone(), + }); } // Merge continuation lines and normalize whitespace. @@ -336,7 +429,7 @@ impl Deps { continue; } if opts.union && alternatives.len() > 1 { - return Err("an union dependency can only contain simple dependencies".to_string()); + return Err(ParseError::UnionNotSimple); } clauses.push(alternatives); } @@ -652,9 +745,15 @@ impl Facts { /// Load a dpkg status file (e.g. `/var/lib/dpkg/status`), binding the /// facts to the given host/build architectures. - pub fn load_status(path: &Path, host_arch: &str, build_arch: &str) -> Result { - let content = std::fs::read_to_string(path) - .map_err(|e| format!("cannot read status file '{}': {}", path.display(), e))?; + pub fn load_status( + path: &Path, + host_arch: &str, + build_arch: &str, + ) -> Result { + let content = std::fs::read_to_string(path).map_err(|source| ParseError::ReadStatus { + path: path.display().to_string(), + source, + })?; Ok(Facts::from_status(&content, host_arch, build_arch)) } @@ -936,7 +1035,10 @@ impl UnmetReport { /// /// On success the report is empty; otherwise its [`UnmetReport::message`] /// carries the dpkg-compatible diagnostics. -pub fn check_build_depends(control: &ControlInfo, opts: &CheckOpts) -> Result { +pub fn check_build_depends( + control: &ControlInfo, + opts: &CheckOpts, +) -> Result { let source = &control.source; let mut bd_parts: Vec<&str> = Vec::new(); diff --git a/src/debian/files.rs b/src/debian/files.rs index 08dda15..a855df8 100644 --- a/src/debian/files.rs +++ b/src/debian/files.rs @@ -121,6 +121,23 @@ pub struct FilesList { files: BTreeMap, } +/// Failure of [`FilesList::parse`]: a malformed `debian/files` line. +#[derive(Debug, Clone, PartialEq, Eq, thiserror::Error)] +pub enum FilesParseError { + /// The line has fewer than `filename section priority` columns. + #[error("badly formed line: {line}")] + BadLine { + /// The offending line. + line: String, + }, + /// The file name does not match any known artifact shape. + #[error("badly formed file name: {name}")] + BadFileName { + /// The offending file name. + name: String, + }, +} + impl FilesList { /// An empty registry. pub fn new() -> Self { @@ -141,7 +158,7 @@ impl FilesList { /// Parse a `debian/files` registry from its textual content /// (`filename section priority [key=value...]` lines). - pub fn parse(content: &str) -> Result { + pub fn parse(content: &str) -> Result { let mut list = FilesList::new(); for line in content.lines() { if line.trim().is_empty() { @@ -149,10 +166,14 @@ impl FilesList { } let tokens: Vec<&str> = line.split_whitespace().collect(); if tokens.len() < 3 { - return Err(format!("badly formed line: {line}")); + return Err(FilesParseError::BadLine { + line: line.to_string(), + }); } - let mut entry = parse_filename(tokens[0]) - .ok_or_else(|| format!("badly formed file name: {}", tokens[0]))?; + let mut entry = + parse_filename(tokens[0]).ok_or_else(|| FilesParseError::BadFileName { + name: tokens[0].to_string(), + })?; entry.section = tokens[1].to_string(); entry.priority = tokens[2].to_string(); for attr in &tokens[3..] { diff --git a/src/debian/tests_control.rs b/src/debian/tests_control.rs index 66f6a77..a619b87 100644 --- a/src/debian/tests_control.rs +++ b/src/debian/tests_control.rs @@ -54,6 +54,30 @@ pub struct TestsControl { pub tests: Vec, } +/// Failure of [`TestsControl::parse_content`]: a malformed stanza, naming +/// it by its 1-based position. +#[derive(Debug, Clone, PartialEq, Eq, thiserror::Error)] +pub enum TestsControlParseError { + /// The stanza carries both `Tests:` and `Test-Command:`. + #[error("stanza {stanza}: Tests and Test-Command are mutually exclusive")] + MutuallyExclusive { + /// The 1-based stanza position. + stanza: usize, + }, + /// The stanza carries neither `Tests:` nor `Test-Command:`. + #[error("stanza {stanza}: missing a Tests or Test-Command field")] + MissingField { + /// The 1-based stanza position. + stanza: usize, + }, + /// `test-name:` appeared on a `Tests:` (script list) stanza. + #[error("stanza {stanza}: test-name only applies to a Test-Command stanza")] + TestNameOnScripts { + /// The 1-based stanza position. + stanza: usize, + }, +} + impl TestsControl { /// Parse the test control file at `path`. pub fn parse(path: &Path) -> Result> { @@ -69,7 +93,7 @@ impl TestsControl { /// `Test-Command:` stanza. A file without stanzas parses to an empty /// [`TestsControl`] — whether the package declares tests at all is /// the caller's check (`Testsuite: autopkgtest`). - pub fn parse_content(content: &str) -> Result { + pub fn parse_content(content: &str) -> Result { let mut tests_control = TestsControl::default(); let mut command_index = 0usize; for (index, paragraph) in parse_paragraphs(content).iter().enumerate() { @@ -85,20 +109,14 @@ impl TestsControl { match (tests_field, command_field) { (Some(_), Some(_)) => { - return Err(format!( - "stanza {stanza}: Tests and Test-Command are mutually exclusive" - )); + return Err(TestsControlParseError::MutuallyExclusive { stanza }); } (None, None) => { - return Err(format!( - "stanza {stanza}: missing a Tests or Test-Command field" - )); + return Err(TestsControlParseError::MissingField { stanza }); } (Some(scripts), None) => { if test_name.is_some() { - return Err(format!( - "stanza {stanza}: test-name only applies to a Test-Command stanza" - )); + return Err(TestsControlParseError::TestNameOnScripts { stanza }); } for script in scripts.split_whitespace() { tests_control.tests.push(Test { @@ -259,7 +277,9 @@ mod tests { "only applies", ), ] { - let error = TestsControl::parse_content(content).unwrap_err(); + let error = TestsControl::parse_content(content) + .unwrap_err() + .to_string(); assert!(error.contains(expected), "{error}"); } } diff --git a/src/debian/version.rs b/src/debian/version.rs index 47f7e95..f2a54f7 100644 --- a/src/debian/version.rs +++ b/src/debian/version.rs @@ -12,19 +12,65 @@ pub struct DebianVersion { pub debian_revision: Option, } +/// Failure of [`DebianVersion::parse`]: the string is outside the +/// `[epoch:]upstream[-revision]` syntax. +#[derive(Debug, Clone, PartialEq, Eq, thiserror::Error)] +pub enum VersionParseError { + /// The string, once trimmed, is empty. + #[error("empty version string")] + Empty, + /// The epoch (before the first `:`) is not a number. + #[error("invalid epoch '{epoch}' in version '{raw}'")] + InvalidEpoch { + /// The non-numeric epoch text. + epoch: String, + /// The whole version string. + raw: String, + }, + /// Nothing precedes the revision separator: no upstream version. + #[error("missing upstream version in '{raw}'")] + MissingUpstream { + /// The whole version string. + raw: String, + }, + /// A character outside dpkg's upstream alphabet. + #[error("invalid character '{c}' in version '{raw}'")] + InvalidUpstreamChar { + /// The offending character. + c: char, + /// The whole version string. + raw: String, + }, + /// A trailing hyphen: the revision number is empty (dpkg rejects it). + #[error("empty debian revision in '{raw}'")] + EmptyRevision { + /// The whole version string. + raw: String, + }, + /// A character outside dpkg's revision alphabet. + #[error("invalid character '{c}' in revision of version '{raw}'")] + InvalidRevisionChar { + /// The offending character. + c: char, + /// The whole version string. + raw: String, + }, +} + impl DebianVersion { /// Parse and validate a Debian version string. - pub fn parse(raw: &str) -> Result { + pub fn parse(raw: &str) -> Result { let raw = raw.trim(); if raw.is_empty() { - return Err("empty version string".to_string()); + return Err(VersionParseError::Empty); } let (epoch, rest) = match raw.split_once(':') { Some((e, r)) => { - let epoch: u32 = e - .parse() - .map_err(|_| format!("invalid epoch '{}' in version '{}'", e, raw))?; + let epoch: u32 = e.parse().map_err(|_| VersionParseError::InvalidEpoch { + epoch: e.to_string(), + raw: raw.to_string(), + })?; (Some(epoch), r) } None => (None, raw), @@ -37,14 +83,19 @@ impl DebianVersion { }; if upstream.is_empty() { - return Err(format!("missing upstream version in '{}'", raw)); + return Err(VersionParseError::MissingUpstream { + raw: raw.to_string(), + }); } for c in upstream.chars() { if !(c.is_ascii_alphanumeric() || matches!(c, '.' | '+' | '-' | '~' | ':') || !c.is_ascii()) { - return Err(format!("invalid character '{}' in version '{}'", c, raw)); + return Err(VersionParseError::InvalidUpstreamChar { + c, + raw: raw.to_string(), + }); } } if let Some(rev) = &debian_revision { @@ -52,14 +103,16 @@ impl DebianVersion { // dpkg rejects a trailing hyphen: "bad syntax: revision // number is empty". Native versions (no `-` at all) are // handled above and stay valid. - return Err(format!("empty debian revision in '{}'", raw)); + return Err(VersionParseError::EmptyRevision { + raw: raw.to_string(), + }); } for c in rev.chars() { if !(c.is_ascii_alphanumeric() || matches!(c, '.' | '+' | '~') || !c.is_ascii()) { - return Err(format!( - "invalid character '{}' in revision of version '{}'", - c, raw - )); + return Err(VersionParseError::InvalidRevisionChar { + c, + raw: raw.to_string(), + }); } } } @@ -340,7 +393,7 @@ mod tests { /// the *last* hyphen (upstream `1.0-` + revision `1`). #[test] fn version_empty_revision() { - let err = DebianVersion::parse("1.0-").unwrap_err(); + let err = DebianVersion::parse("1.0-").unwrap_err().to_string(); assert!(err.contains("empty"), "unexpected message: {err}"); assert!(DebianVersion::parse("1.0-").is_err()); diff --git a/src/distro_info.rs b/src/distro_info.rs index 9e7fb6c..45505af 100644 --- a/src/distro_info.rs +++ b/src/distro_info.rs @@ -1,10 +1,10 @@ use crate::data::embed_data; use chrono::NaiveDate; -use lazy_static::lazy_static; use serde::Deserialize; use std::collections::HashMap; use std::error::Error; use std::path::Path; +use std::sync::LazyLock; use std::time::Duration; #[derive(Debug, Clone)] @@ -104,20 +104,20 @@ embed_data! { static ref DATA: Data = "../data/distro_info.yml" } -lazy_static! { - // Shared HTTP client used for all outgoing plain requests: timeouts keep - // a hanging remote (connect or transfer) from stalling pkh indefinitely. - // The short pool idle timeout and TCP keepalive avoid reusing keep-alive - // connections that the remote closed in the meantime, which surfaces as - // spurious 'error sending request' failures on busy mirrors/CDNs. - static ref HTTP_CLIENT: reqwest::Client = reqwest::Client::builder() +// Shared HTTP client used for all outgoing plain requests: timeouts keep +// a hanging remote (connect or transfer) from stalling pkh indefinitely. +// The short pool idle timeout and TCP keepalive avoid reusing keep-alive +// connections that the remote closed in the meantime, which surfaces as +// spurious 'error sending request' failures on busy mirrors/CDNs. +static HTTP_CLIENT: LazyLock = LazyLock::new(|| { + reqwest::Client::builder() .connect_timeout(Duration::from_secs(10)) .timeout(Duration::from_secs(30)) .pool_idle_timeout(Duration::from_secs(10)) .tcp_keepalive(Duration::from_secs(30)) .build() - .expect("building the shared HTTP client with static options cannot fail"); -} + .expect("building the shared HTTP client with static options cannot fail") +}); /// Shared HTTP client with a connect timeout (10s) and a total request /// timeout (30s), to be used for all outgoing plain HTTP(S) requests diff --git a/src/lib.rs b/src/lib.rs index 1c8854b..a1d7f4b 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -63,13 +63,6 @@ pub(crate) mod test_support; /// Utility functions pub(crate) mod utils; -/// Optional callback function (taking 4 arguments) -/// - Name of the current main operation (e.g. pulling package) -/// - Name of the current nested operation (e.g. cloning git repo) -/// - Progress, position, index of current operation (e.g. amount of data downloaded) -/// - Total amount for current operation (e.g. size of the file to download) -pub type ProgressCallback<'a> = Option<&'a dyn Fn(&str, &str, usize, usize)>; - /// Returns the architecture of current CPU, debian-compatible pub fn get_current_arch() -> String { match std::env::consts::ARCH { diff --git a/src/main.rs b/src/main.rs index 8005243..dff078c 100644 --- a/src/main.rs +++ b/src/main.rs @@ -670,7 +670,7 @@ fn main() { .unwrap_or(""); let archive = sub_matches.get_one::("archive").unwrap_or(&false); - let (pb, progress_callback) = pkh::ui::create_progress_bar(&multi); + let view = pkh::ui::PullView::new(&multi); // Convert PPA to base URL if provided let base_url = match ppa.map(pkh::package_info::split_ppa) { @@ -692,17 +692,17 @@ fn main() { dist, base_url.as_deref(), repository, - Some(&progress_callback), + &view, ) .await?; - pkh::pull::pull(&package_info, None, Some(&progress_callback), *archive).await + pkh::pull::pull(&package_info, None, &view, *archive).await }) { - pb.finish_and_clear(); + // exit(1) below would skip the view's Drop + drop(view); error!("{}", e); std::process::exit(1); } - pb.finish_and_clear(); - multi.remove(&pb); + drop(view); info!("Done."); } Some(("chlog", sub_matches)) => { diff --git a/src/new/options.rs b/src/new/options.rs index 12c9917..f40ad22 100644 --- a/src/new/options.rs +++ b/src/new/options.rs @@ -63,22 +63,12 @@ impl TemplateId { } /// Parse a CLI identifier, accepting exactly the registered spellings. - pub fn parse(s: &str) -> Result { + pub fn parse(s: &str) -> Result { Self::all() .iter() .copied() .find(|id| id.as_str() == s) - .ok_or_else(|| { - format!( - "Unknown language/template '{}'. Supported values are: {}.", - s, - Self::all() - .iter() - .map(|id| id.as_str()) - .collect::>() - .join(", ") - ) - }) + .ok_or_else(|| ValidationError::UnknownTemplate { id: s.to_string() }) } /// Human-readable menu label of this template, as offered by the wizard @@ -407,21 +397,132 @@ pub struct NewCli { pub defaults: bool, } +/// Failure of one of the scaffold option validators ([`validate_source_name`], +/// [`validate_command`], [`validate_upstream_version`], [`parse_maintainer`], +/// [`validate_homepage`], [`validate_depends`], [`validate_orig_path`], +/// [`check_file_collisions`], [`TemplateId::parse`]): what the wizard re-asks +/// about, carrying the pieces callers may want to match on. +#[derive(Debug, thiserror::Error)] +pub enum ValidationError { + /// The source package name is shorter than dpkg's 2-character minimum. + #[error("'{name}' is too short: package names need at least 2 characters")] + SourceNameTooShort { + /// The rejected name. + name: String, + }, + /// The source package name is outside dpkg's charset or shape. + #[error( + "'{}' is not a valid package name: names must be lowercase, start with a letter or digit and may only contain [a-z0-9+.-]", + name + )] + InvalidSourceName { + /// The rejected name. + name: String, + }, + /// No installed command was given. + #[error("the command name must not be empty")] + EmptyCommand, + /// The command name is not a lowercase identifier. + #[error( + "'{}' is not a valid command name: commands must be a lowercase identifier (letters, digits, + - . _), e.g. 'mytool'", + command + )] + InvalidCommand { + /// The rejected command name. + command: String, + }, + /// Upstream versions should start with a digit (dpkg recommendation, + /// enforced here). + #[error("upstream versions should start with a digit")] + UpstreamNotDigit, + /// `-` is the Debian revision separator. + #[error( + "upstream versions must not contain '-': the Debian revision is appended automatically" + )] + UpstreamContainsDash, + /// The upstream version composed with the Debian revision does not + /// parse as a Debian version. + #[error("'{version}' is not a valid Debian version: {source}")] + InvalidVersion { + /// The composed `upstream-revision` string. + version: String, + /// The version parse failure. + source: crate::debian::version::VersionParseError, + }, + /// The maintainer is not of the form `Name `. + #[error("'{s}' is not a valid maintainer: expected the form 'Name '")] + MaintainerShape { + /// The rejected maintainer string. + s: String, + }, + /// The maintainer name (before the angle brackets) is empty. + #[error("'{s}': the maintainer name must not be empty")] + MaintainerNameEmpty { + /// The rejected maintainer string. + s: String, + }, + /// The maintainer email does not carry exactly one `@`. + #[error("'{s}': the maintainer email must contain exactly one '@'")] + MaintainerEmailAt { + /// The rejected maintainer string. + s: String, + }, + /// The maintainer email carries whitespace. + #[error("'{s}': the maintainer email must not contain whitespace")] + MaintainerEmailWhitespace { + /// The rejected maintainer string. + s: String, + }, + /// The homepage is not an http(s) URL. + #[error("'{url}' is not a valid homepage: expected an http:// or https:// URL")] + InvalidHomepage { + /// The rejected URL. + url: String, + }, + /// The Depends list does not parse as package relations. + #[error("Invalid Depends list '{list}': {source}")] + InvalidDepends { + /// The rejected list. + list: String, + /// The relation parse failure. + source: crate::debian::deps::ParseError, + }, + /// The orig tarball is neither an http(s) URL nor an existing file. + #[error( + "'{path}' is not an http(s) URL and not an existing file: --orig-path must point at a tarball to repack" + )] + InvalidOrigPath { + /// The rejected path or URL. + path: String, + }, + /// The language/template id is not one of the registered spellings. + #[error("Unknown language/template '{}'. Supported values are: {}.", id, TemplateId::all().iter().map(|id| id.as_str()).collect::>().join(", "))] + UnknownTemplate { + /// The unknown id. + id: String, + }, + /// A template generated the same file twice. + #[error("internal error: file '{path}' was generated more than once")] + GeneratedTwice { + /// The duplicated path. + path: String, + }, +} + /// Validate a Debian source package name: `^[a-z0-9][a-z0-9+.\-]+$` with a /// minimum length of 2 (dpkg rules). -pub fn validate_source_name(name: &str) -> Result<(), String> { +pub fn validate_source_name(name: &str) -> Result<(), ValidationError> { static NAME_REGEX: std::sync::OnceLock = std::sync::OnceLock::new(); let regex = NAME_REGEX.get_or_init(|| Regex::new(r"^[a-z0-9][a-z0-9+.\-]+$").unwrap()); if name.len() < 2 { - return Err(format!( - "'{name}' is too short: package names need at least 2 characters" - )); + return Err(ValidationError::SourceNameTooShort { + name: name.to_string(), + }); } if !regex.is_match(name) { - return Err(format!( - "'{name}' is not a valid package name: names must be lowercase, \ - start with a letter or digit and may only contain [a-z0-9+.-]" - )); + return Err(ValidationError::InvalidSourceName { + name: name.to_string(), + }); } Ok(()) } @@ -473,17 +574,16 @@ pub fn sanitize_name(input: &str) -> Option { /// `meson.build` and the `[project.scripts]` table, so uppercase letters, /// whitespace, quotes and shell metacharacters are rejected here instead of /// generating broken install lines and build files. -pub fn validate_command(command: &str) -> Result<(), String> { +pub fn validate_command(command: &str) -> Result<(), ValidationError> { static COMMAND_REGEX: std::sync::OnceLock = std::sync::OnceLock::new(); let regex = COMMAND_REGEX.get_or_init(|| Regex::new(r"^[a-z0-9][a-z0-9+.\-_]*$").unwrap()); if command.is_empty() { - return Err("the command name must not be empty".to_string()); + return Err(ValidationError::EmptyCommand); } if !regex.is_match(command) { - return Err(format!( - "'{command}' is not a valid command name: commands must be a \ - lowercase identifier (letters, digits, + - . _), e.g. 'mytool'" - )); + return Err(ValidationError::InvalidCommand { + command: command.to_string(), + }); } Ok(()) } @@ -492,65 +592,60 @@ pub fn validate_command(command: &str) -> Result<(), String> { /// recommendation, enforced here) and survive [`DebianVersion::parse`] once /// composed with the Debian revision. It must not contain `-` (the revision /// separator). -pub fn validate_upstream_version(upstream: &str, revision: u32) -> Result<(), String> { +pub fn validate_upstream_version(upstream: &str, revision: u32) -> Result<(), ValidationError> { if !upstream.starts_with(|c: char| c.is_ascii_digit()) { - return Err("upstream versions should start with a digit".to_string()); + return Err(ValidationError::UpstreamNotDigit); } if upstream.contains('-') { - return Err( - "upstream versions must not contain '-': the Debian revision is \ - appended automatically" - .to_string(), - ); + return Err(ValidationError::UpstreamContainsDash); } let composed = format!("{}-{}", upstream, revision); DebianVersion::parse(&composed) .map(|_| ()) - .map_err(|e| format!("'{composed}' is not a valid Debian version: {e}")) + .map_err(|source| ValidationError::InvalidVersion { + version: composed.clone(), + source, + }) } /// Parse a `Name ` maintainer string: the name must be non-empty and /// the email must contain exactly one `@` with no whitespace inside the /// angle brackets. -pub fn parse_maintainer(s: &str) -> Result<(String, String), String> { +pub fn parse_maintainer(s: &str) -> Result<(String, String), ValidationError> { static MAINTAINER_REGEX: std::sync::OnceLock = std::sync::OnceLock::new(); let regex = MAINTAINER_REGEX.get_or_init(|| Regex::new(r"^(.+?)\s*<([^<>]*)>\s*$").unwrap()); - let caps = regex.captures(s.trim()).ok_or_else(|| { - format!("'{s}' is not a valid maintainer: expected the form 'Name '") - })?; + let caps = regex + .captures(s.trim()) + .ok_or_else(|| ValidationError::MaintainerShape { s: s.to_string() })?; let name = caps[1].trim().to_string(); let email = caps[2].trim().to_string(); if name.is_empty() { - return Err(format!("'{s}': the maintainer name must not be empty")); + return Err(ValidationError::MaintainerNameEmpty { s: s.to_string() }); } if !email.contains('@') || email.matches('@').count() > 1 { - return Err(format!( - "'{s}': the maintainer email must contain exactly one '@'" - )); + return Err(ValidationError::MaintainerEmailAt { s: s.to_string() }); } if email.chars().any(char::is_whitespace) { - return Err(format!( - "'{s}': the maintainer email must not contain whitespace" - )); + return Err(ValidationError::MaintainerEmailWhitespace { s: s.to_string() }); } Ok((name, email)) } /// Validate a homepage URL: it must carry an `http(s)://` prefix. -pub fn validate_homepage(url: &str) -> Result<(), String> { +pub fn validate_homepage(url: &str) -> Result<(), ValidationError> { if url.starts_with("http://") || url.starts_with("https://") { Ok(()) } else { - Err(format!( - "'{url}' is not a valid homepage: expected an http:// or https:// URL" - )) + Err(ValidationError::InvalidHomepage { + url: url.to_string(), + }) } } /// Validate a comma-separated runtime Depends list (the metapackage flavor) /// by parsing it with [`Deps::parse`] and re-rendering every clause /// canonically. -pub fn validate_depends(list: &str) -> Result, String> { +pub fn validate_depends(list: &str) -> Result, ValidationError> { let opts = ParseOpts { host_arch: crate::get_current_arch(), build_arch: crate::get_current_arch(), @@ -559,8 +654,10 @@ pub fn validate_depends(list: &str) -> Result, String> { union: false, build_dep: false, }; - let deps = - Deps::parse(list, &opts).map_err(|e| format!("Invalid Depends list '{list}': {e}"))?; + let deps = Deps::parse(list, &opts).map_err(|source| ValidationError::InvalidDepends { + list: list.to_string(), + source, + })?; Ok(deps .clauses() .map(|clause| { @@ -620,7 +717,7 @@ pub async fn resolve(cli: NewCli) -> Result { // (nothing to detect for a fresh skeleton), else --defaults picks the // empty template. let template = match &cli.lang { - Some(lang) => Some(TemplateId::parse(lang)?), + Some(lang) => Some(TemplateId::parse(lang).map_err(|e| e.to_string())?), None => match &source_dir { SourceDir::Skeleton => { if cli.defaults { @@ -706,7 +803,7 @@ pub async fn resolve(cli: NewCli) -> Result { } // Everything below has a default and is validated as it is resolved. - validate_source_name(&name)?; + validate_source_name(&name).map_err(|e| e.to_string())?; // Source format: explicit flags win (--native / --quilt, mutually // exclusive), otherwise the mode decides — a fresh, self-authored @@ -769,7 +866,7 @@ pub async fn resolve(cli: NewCli) -> Result { let homepage = match &cli.homepage { Some(h) => { - validate_homepage(h)?; + validate_homepage(h).map_err(|e| e.to_string())?; Some(h.clone()) } None => None, @@ -780,10 +877,10 @@ pub async fn resolve(cli: NewCli) -> Result { let command = cli.command.unwrap_or_else(|| name.clone()); // The default is the already-validated package name (a subset of the // command charset), so only an explicit --command/wizard answer can fail. - validate_command(&command)?; + validate_command(&command).map_err(|e| e.to_string())?; let maintainer = match &cli.maintainer { - Some(m) => parse_maintainer(m)?, + Some(m) => parse_maintainer(m).map_err(|e| e.to_string())?, None => crate::changelog::get_maintainer_info().map_err(|e| { format!( "Could not determine the maintainer ({e}). \ @@ -815,7 +912,7 @@ pub async fn resolve(cli: NewCli) -> Result { let depends = if joined.trim().is_empty() { Vec::new() } else { - validate_depends(&joined)? + validate_depends(&joined).map_err(|e| e.to_string())? }; Ok(NewOptions { @@ -874,7 +971,7 @@ fn resolve_orig_origin( let source = cli.orig_path.clone().ok_or_else(|| { "--orig-from path requires --orig-path ".to_string() })?; - validate_orig_path(&source)?; + validate_orig_path(&source).map_err(|e| e.to_string())?; OrigOrigin::Provided { source } } Some("git") => { @@ -935,27 +1032,26 @@ fn resolve_orig_origin( /// Validate a `--orig-path` value (and the wizard's tarball answer): either /// an http(s) URL (downloaded at scaffold time) or an existing local /// tarball. -pub fn validate_orig_path(source: &str) -> Result<(), String> { +pub fn validate_orig_path(source: &str) -> Result<(), ValidationError> { if source.starts_with("http://") || source.starts_with("https://") { return Ok(()); } if Path::new(source).is_file() { return Ok(()); } - Err(format!( - "'{source}' is not an http(s) URL and not an existing file: \ - --orig-path must point at a tarball to repack" - )) + Err(ValidationError::InvalidOrigPath { + path: source.to_string(), + }) } /// Set of names no template may produce twice (collision check). -pub(crate) fn check_file_collisions(paths: &[String]) -> Result<(), String> { +pub(crate) fn check_file_collisions(paths: &[String]) -> Result<(), ValidationError> { let mut seen: HashSet<&String> = HashSet::new(); for path in paths { if !seen.insert(path) { - return Err(format!( - "internal error: file '{path}' was generated more than once" - )); + return Err(ValidationError::GeneratedTwice { + path: path.to_string(), + }); } } Ok(()) @@ -1222,7 +1318,9 @@ mod tests { #[test] fn collision_detection() { assert!(check_file_collisions(&["a".into(), "b".into()]).is_ok()); - let err = check_file_collisions(&["a".into(), "b".into(), "a".into()]).unwrap_err(); + let err = check_file_collisions(&["a".into(), "b".into(), "a".into()]) + .unwrap_err() + .to_string(); assert!(err.contains('a')); } diff --git a/src/new/questions.rs b/src/new/questions.rs index 6dc14fa..57a7c80 100644 --- a/src/new/questions.rs +++ b/src/new/questions.rs @@ -96,12 +96,9 @@ async fn run_wizard( // basename of the current directory. if cli.name.is_none() { let default = default_package_name(&cwd, probe.as_ref()); - let answer = ask_text( - prompter, - "Package name", - &default, - options::validate_source_name, - )?; + let answer = ask_text(prompter, "Package name", &default, |name: &str| { + options::validate_source_name(name).map_err(|e| e.to_string()) + })?; cli.name = Some(answer); } @@ -265,7 +262,9 @@ async fn run_wizard( .or_else(|| origin.as_ref().and_then(GitOrigin::git_version)) .unwrap_or_else(|| "0.1.0".to_string()); let revision = cli.revision.unwrap_or(1); - let validate = move |version: &str| options::validate_upstream_version(version, revision); + let validate = move |version: &str| { + options::validate_upstream_version(version, revision).map_err(|e| e.to_string()) + }; let answer = ask_text(prompter, "Upstream version", &default, validate)?; cli.upstream_version = Some(answer.clone()); @@ -321,7 +320,8 @@ async fn run_wizard( .expect("answer comes from the choice list"); cli.orig_from = Some(chosen.to_string()); if chosen == "path" { - let validator = |path: &str| options::validate_orig_path(path); + let validator = + |path: &str| options::validate_orig_path(path).map_err(|e| e.to_string()); let path = prompter.text("Tarball path or URL", "", Some(&validator))?; cli.orig_path = Some(path); } @@ -354,7 +354,7 @@ async fn run_wizard( if url.is_empty() { Ok(()) } else { - options::validate_homepage(url) + options::validate_homepage(url).map_err(|e| e.to_string()) } }; let answer = ask_text(prompter, "Homepage (blank to skip)", &default, validate)?; @@ -400,12 +400,9 @@ async fn run_wizard( .as_ref() .and_then(|p| p.command.clone()) .unwrap_or_else(|| cli.name.clone().unwrap_or_default()); - let command = ask_text( - prompter, - "Command name", - &default, - options::validate_command, - )?; + let command = ask_text(prompter, "Command name", &default, |command: &str| { + options::validate_command(command).map_err(|e| e.to_string()) + })?; cli.command = Some(command); } @@ -430,7 +427,11 @@ async fn run_wizard( DEBEMAIL and no git user config): answer as 'Name '" ); } - let validate = |answer: &str| options::parse_maintainer(answer).map(|_| ()); + let validate = |answer: &str| { + options::parse_maintainer(answer) + .map(|_| ()) + .map_err(|e| e.to_string()) + }; cli.maintainer = Some(ask_text(prompter, "Maintainer", &default, validate)?); } @@ -480,7 +481,11 @@ async fn run_wizard( // 13. Metapackage Depends (empty template only). if template == TemplateId::EMPTY && cli.depends.is_empty() { - let validate = |answer: &str| options::validate_depends(answer).map(|_| ()); + let validate = |answer: &str| { + options::validate_depends(answer) + .map(|_| ()) + .map_err(|e| e.to_string()) + }; let answer = ask_text( prompter, "Depends (metapackage, comma-separated, blank for an empty base)", @@ -1496,7 +1501,9 @@ mod tests { assert!(options::validate_upstream_version("v1.0", 1).is_err()); assert!(options::validate_upstream_version("1.0.0", 1).is_ok()); - let validate = |version: &str| options::validate_upstream_version(version, 1); + let validate = |version: &str| { + options::validate_upstream_version(version, 1).map_err(|e| e.to_string()) + }; // Invalid probes are not offered as the default... assert_eq!(offered_default("1.0-2", &validate), ""); assert_eq!(offered_default("v1.0", &validate), ""); @@ -1532,7 +1539,7 @@ mod tests { // The upstream-version scenario end to end: Enter on an invalid // probed default (offered as none) fails instead of taking it // verbatim, while a valid default is taken. - let version = |v: &str| options::validate_upstream_version(v, 1); + let version = |v: &str| options::validate_upstream_version(v, 1).map_err(|e| e.to_string()); assert!(accept_answer("", "1.0-2", &version).is_err()); assert_eq!( accept_answer("", "1.0.0", &version), diff --git a/src/new/templates/mod.rs b/src/new/templates/mod.rs index 9e2bdde..9576183 100644 --- a/src/new/templates/mod.rs +++ b/src/new/templates/mod.rs @@ -52,6 +52,7 @@ pub mod rust; pub mod shell; use std::path::{Path, PathBuf}; +use std::sync::LazyLock; use serde::Deserialize; @@ -478,15 +479,13 @@ static TEMPLATE_SOURCES: &[TemplateSources] = &[ }, ]; -lazy_static::lazy_static! { - /// The parsed registry: manifest data plus resolved hooks, in the - /// order of [`TEMPLATE_SOURCES`] (wizard menu = detection priority). - static ref REGISTRY: Vec