From d7d09735d54352d84542dbe5bb711926b84d4182 Mon Sep 17 00:00:00 2001 From: Rito Rhymes Date: Sun, 3 May 2026 20:35:08 -0400 Subject: [PATCH 1/2] Add repository manifest identity Add the .build-eips.repo.toml schema, loader, validation rules, and manifest tests for active proposal repositories and declared sibling repositories. Drop concept of staging/production --- Cargo.lock | 43 +++++- Cargo.toml | 3 + src/changed.rs | 6 +- src/cli.rs | 4 - src/config.rs | 330 +++++++++++++++++++++++++++++++++++++---------- src/context.rs | 2 +- src/find_root.rs | 26 ++-- src/git.rs | 73 +++-------- src/main.rs | 52 ++++---- 9 files changed, 371 insertions(+), 168 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index dbca5a3..aa9d0aa 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -260,6 +260,7 @@ dependencies = [ "serde_json", "sha3", "snafu", + "tempfile", "tokio", "toml 0.9.11+spec-1.1.0", "toml_datetime 0.7.5+spec-1.1.0", @@ -785,6 +786,12 @@ dependencies = [ "regex-syntax", ] +[[package]] +name = "fastrand" +version = "2.4.1" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "9f1f227452a390804cdb637b74a86990f2a7d7ba4b7d5693aac9b4dd6defd8d6" + [[package]] name = "figment" version = "0.10.19" @@ -1341,9 +1348,9 @@ checksum = "bbd2bcb4c963f2ddae06a2efc7e9f3591312473c50c6685e1f298068316e66fe" [[package]] name = "libc" -version = "0.2.180" +version = "0.2.185" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "bcc35a38544a891a5f7c865aca548a982ccb3b8650a5b06d0fd33a10283c56fc" +checksum = "52ff2c0fe9bc6cb6b14a0592c2ff4fa9ceb83eea9db979b0487cd054946a2b8f" [[package]] name = "libgit2-sys" @@ -1395,6 +1402,12 @@ dependencies = [ "vcpkg", ] +[[package]] +name = "linux-raw-sys" +version = "0.12.1" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "32a66949e030da00e8c7d4434b251670a91556f4144941d37452769c25d58a53" + [[package]] name = "litemap" version = "0.8.1" @@ -2007,6 +2020,19 @@ version = "0.1.27" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "b50b8869d9fc858ce7266cce0194bd74df58b9d0e3f6df3a9fc8eb470d95c09d" +[[package]] +name = "rustix" +version = "1.1.4" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "b6fe4565b9518b83ef4f91bb47ce29620ca828bd32cb7e408f0062e9930ba190" +dependencies = [ + "bitflags", + "errno", + "libc", + "linux-raw-sys", + "windows-sys 0.61.2", +] + [[package]] name = "rustversion" version = "1.0.22" @@ -2424,6 +2450,19 @@ dependencies = [ "libc", ] +[[package]] +name = "tempfile" +version = "3.27.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "32497e9a4c7b38532efcdebeef879707aa9f794296a4f0244f6f69e9bc8574bd" +dependencies = [ + "fastrand", + "getrandom 0.3.4", + "once_cell", + "rustix", + "windows-sys 0.61.2", +] + [[package]] name = "tendril" version = "0.4.3" diff --git a/Cargo.toml b/Cargo.toml index d87e3e7..4529c21 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -53,3 +53,6 @@ iref = "3.2.2" [features] backtrace = [ "snafu/backtrace", "eipw-lint/backtrace" ] + +[dev-dependencies] +tempfile = "3.23.0" diff --git a/src/changed.rs b/src/changed.rs index 533ac26..c30f882 100644 --- a/src/changed.rs +++ b/src/changed.rs @@ -13,7 +13,7 @@ use std::{ use snafu::{ResultExt, Whatever}; -use crate::{cli::ChangedFormat, config::Config, git, layout::REPO_DIR}; +use crate::{cli::ChangedFormat, config::Manifest, git, layout::REPO_DIR}; pub(crate) fn is_proposal_path(mut p: PathBuf) -> bool { // Only lint `content/00001.md` and `content/00001/index.md` files. @@ -55,13 +55,13 @@ pub(crate) fn is_proposal_path(mut p: PathBuf) -> bool { pub(crate) fn run( root_path: &Path, build_path: &Path, - config: &Config, + manifest: Manifest, all: bool, format: &ChangedFormat, ) -> Result<(), Whatever> { let repo_path = build_path.join(REPO_DIR); - let both = git::Fresh::new(root_path, &repo_path, &config.locations) + let both = git::Fresh::new(root_path, &repo_path, manifest) .whatever_context("initializing build repo")? .clone_src() .whatever_context("cloning source repo")? diff --git a/src/cli.rs b/src/cli.rs index f7a2480..ff93bbf 100644 --- a/src/cli.rs +++ b/src/cli.rs @@ -20,10 +20,6 @@ pub(crate) struct Args { #[clap(short = 'C')] pub(crate) root: Option, - /// Use the staging repositories (for testing) - #[clap(long = "staging")] - pub(crate) staging: bool, - #[clap(subcommand)] pub(crate) operation: Operation, } diff --git a/src/config.rs b/src/config.rs index 26c4ad4..3c0f2ca 100644 --- a/src/config.rs +++ b/src/config.rs @@ -4,12 +4,49 @@ * file, You can obtain one at https://mozilla.org/MPL/2.0/. */ -use std::collections::HashMap; +use std::{borrow::Borrow, collections::HashMap, path::PathBuf, str::FromStr}; +use regex::Regex; use serde::{Deserialize, Serialize}; +use snafu::{ensure, Backtrace, IntoError, ResultExt, Snafu}; use url::Url; -#[derive(Debug, Clone, Serialize, Deserialize)] +pub const MANIFEST_FILE: &str = "Build.toml"; + +#[derive(Debug, Snafu)] +#[non_exhaustive] +pub enum Error { + #[snafu(display("i/o error while accessing `{}`", path.to_string_lossy()))] + Io { + path: PathBuf, + source: std::io::Error, + backtrace: Backtrace, + }, + + #[snafu(display( + "unable to parse repo manifest `{}`", + manifest_path.to_string_lossy() + ))] + Parse { + manifest_path: PathBuf, + #[snafu(source(from(toml::de::Error, Box::new)))] + source: Box, + backtrace: Backtrace, + }, + + #[snafu(display( + "repo manifest `{}` is invalid: {reason}", + manifest_path.to_string_lossy() + ))] + Invalid { + manifest_path: PathBuf, + reason: String, + backtrace: Backtrace, + }, +} + +#[derive(Debug, Clone, Serialize, Deserialize, PartialEq, Eq)] +#[serde(deny_unknown_fields, rename_all = "kebab-case")] pub struct Theme { /// Where to fetch the theme from. pub repository: Url, @@ -18,91 +55,254 @@ pub struct Theme { pub commit: String, } -#[derive(Debug, Clone, Serialize, Deserialize)] +/// Location-specific repository metadata for an active proposal repo or sibling repo. +#[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize)] +#[serde(deny_unknown_fields, rename_all = "kebab-case")] pub struct Location { - /// Git repository to fetch proposals from. + /// Git repository to fetch proposal content from. pub repository: Url, - /// Location where the rendered HTML and assets will end up. + /// Base URL where rendered HTML and assets for this repository are served. pub base_url: Url, +} - /// A commit hash that exists solely in this repository. - /// - /// Use to determine which repository is being rendered. Pick a commit after every other - /// location/working group/etc. split off. - pub identifying_commit: String, +#[derive(Debug, Snafu)] +#[snafu(display( + "invalid location name `{name}`; only letters/numbers/dashes/underscores are allowed" +))] +pub struct NameError { + name: String, + backtrace: Backtrace, } -#[derive(Debug, Clone, Serialize, Deserialize)] -#[serde(transparent)] -pub struct Locations(pub HashMap); +lazy_static::lazy_static! { + static ref RE_LOC_NAME: Regex = Regex::new(r"^[a-zA-Z0-9_-]+$").unwrap(); +} -#[derive(Debug, Clone, Serialize, Deserialize)] -pub struct Config { - pub theme: Theme, +#[derive(Debug, Clone, Eq, PartialEq, Hash, Ord, PartialOrd, Serialize, Deserialize)] +#[serde(try_from = "String", into = "String")] +pub struct LocName(String); + +impl Borrow for LocName { + fn borrow(&self) -> &str { + self.0.as_str() + } +} + +impl Borrow for LocName { + fn borrow(&self) -> &String { + &self.0 + } +} + +impl PartialEq for LocName { + fn eq(&self, other: &str) -> bool { + self.0 == other + } +} + +impl PartialEq for LocName { + fn eq(&self, other: &String) -> bool { + self.0 == other.as_str() + } +} + +impl FromStr for LocName { + type Err = NameError; + + fn from_str(name: &str) -> Result { + Self::try_from(name.to_owned()) + } +} + +impl TryFrom for LocName { + type Error = NameError; + + fn try_from(name: String) -> Result { + if RE_LOC_NAME.is_match(&name) { + Ok(Self(name)) + } else { + NameSnafu { name }.fail() + } + } +} + +impl From for String { + fn from(value: LocName) -> Self { + value.0 + } +} + +impl std::fmt::Display for LocName { + fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { + std::fmt::Display::fmt(&self.0, f) + } +} + +pub type Locations = HashMap; + +#[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize)] +#[serde(deny_unknown_fields, rename_all = "kebab-case")] +struct InnerManifest { + name: LocName, + + #[serde(default, skip_serializing_if = "Locations::is_empty")] + locations: Locations, + + theme: Theme, +} + +#[derive(Debug, Clone, PartialEq, Eq)] +pub struct Manifest { + pub manifest_path: PathBuf, + pub name: LocName, pub locations: Locations, + pub theme: Theme, } -impl Config { - pub fn production() -> Self { - let mut locations = HashMap::new(); +impl Manifest { + pub fn load>(path: P) -> Result { + let path = path.into(); + match std::fs::read_to_string(&path) { + Ok(contents) => Self::from_contents(path, &contents), + Err(e) => Err(IoSnafu { path }.into_error(e)), + } + } - locations.insert( - "EIPs".into(), - Location { - repository: "https://github.com/ethereum/EIPs.git".try_into().unwrap(), - base_url: "https://eips.ethereum.org/".try_into().unwrap(), - identifying_commit: "0f44e2b94df4e504bb7b912f56ebd712db2ad396".into(), - }, + fn from_inner(manifest_path: PathBuf, inner: InnerManifest) -> Result { + ensure!( + inner.locations.contains_key(&inner.name), + InvalidSnafu { + manifest_path: &manifest_path, + reason: format!( + "this locations's name (`{}`) must appear in `locations`", + inner.name + ), + } ); + Ok(Self { + manifest_path, + name: inner.name, + locations: inner.locations, + theme: inner.theme, + }) + } - locations.insert( - "ERCs".into(), - Location { - repository: "https://github.com/ethereum/ERCs.git".try_into().unwrap(), - base_url: "https://ercs.ethereum.org/".try_into().unwrap(), - identifying_commit: "8dd085d159cb123f545c272c0d871a5339550e79".into(), - }, - ); + fn from_contents(manifest_path: PathBuf, contents: &str) -> Result { + let new = toml::from_str::(contents).context(ParseSnafu { + manifest_path: &manifest_path, + })?; + + Self::from_inner(manifest_path, new) + } +} + +#[cfg(test)] +mod tests { + use std::path::{Path, PathBuf}; + + use tempfile::TempDir; + + use super::{Error, Manifest, MANIFEST_FILE}; + + struct TestRepo { + tempdir: TempDir, + } + + impl TestRepo { + fn new() -> Self { + Self { + tempdir: TempDir::new().unwrap(), + } + } + + fn root(&self) -> &Path { + self.tempdir.path() + } - Self { - theme: Theme { - repository: "https://github.com/ethereum/eips-theme.git" - .try_into() - .unwrap(), - commit: "0ddac35da36d311a8401c6cfb79c9991f78b647d".into(), - }, - locations: Locations(locations), + fn path(&self, relative: impl AsRef) -> PathBuf { + self.root().join(relative) } + + fn write_file(&self, relative: impl AsRef, contents: &str) -> PathBuf { + let path = self.path(relative); + if let Some(parent) = path.parent() { + std::fs::create_dir_all(parent).unwrap(); + } + std::fs::write(&path, contents).unwrap(); + path + } + } + + #[test] + fn malformed_repo_manifest_reports_parse_error() { + let repo = TestRepo::new(); + let manifest_path = repo.write_file(MANIFEST_FILE, "repo_id = ["); + + let error = Manifest::load(&manifest_path).unwrap_err(); + + assert!(matches!(error, Error::Parse { .. })); } - pub fn staging() -> Self { - let mut locations = HashMap::new(); + #[test] + fn parses_repo_manifest() { + let repo = TestRepo::new(); + let manifest_path = repo.write_file( + MANIFEST_FILE, + r#" +name = "Core" + +[locations.Core] +repository = "https://example.test/EIPs.git" +base-url = "https://example.test/EIPs/" - locations.insert( - "EIPs".into(), - Location { - repository: "https://github.com/eips-wg/EIPs.git".try_into().unwrap(), - base_url: "https://eips-wg.github.io/EIPs/".try_into().unwrap(), - identifying_commit: "0f44e2b94df4e504bb7b912f56ebd712db2ad396".into(), - }, +[theme] +repository = "https://example.test/theme.git" +commit = "aaa" +"#, ); - locations.insert( - "ERCs".into(), - Location { - repository: "https://github.com/eips-wg/ERCs.git".try_into().unwrap(), - base_url: "https://eips-wg.github.io/ERCs/".try_into().unwrap(), - identifying_commit: "8dd085d159cb123f545c272c0d871a5339550e79".into(), - }, + let manifest = Manifest::load(&manifest_path).expect("loaded successfully"); + + assert_eq!(&manifest.name, "Core"); + assert_eq!(manifest.locations.len(), 1); + let core = &manifest.locations["Core"]; + + assert_eq!(core.base_url.as_str(), "https://example.test/EIPs/"); + } + + #[test] + fn repo_manifest_rejects_unsafe_names() { + let repo = TestRepo::new(); + let manifest_path = repo.write_file(MANIFEST_FILE, r#"name = "^^^^""#); + + let Err(Error::Parse { source, .. }) = Manifest::load(&manifest_path) else { + panic!("expected parse error"); + }; + + let reason = source.to_string(); + + assert!(reason.contains("invalid location name")); + } + + #[test] + fn repo_manifest_requires_self() { + let repo = TestRepo::new(); + let manifest_path = repo.write_file( + MANIFEST_FILE, + r#" +name = "banana" + +[theme] +repository = "https://example.test/theme.git" +commit = "aaa" +"#, ); - Self { - theme: Theme { - repository: "https://github.com/eips-wg/theme.git".try_into().unwrap(), - commit: "0ddac35da36d311a8401c6cfb79c9991f78b647d".into(), - }, - locations: Locations(locations), - } + let Err(Error::Invalid { reason, .. }) = Manifest::load(&manifest_path) else { + panic!("expected invalid error"); + }; + + assert!(reason.contains("this locations's name (`banana`) must appear in `locations`")); } } diff --git a/src/context.rs b/src/context.rs index 0fffb43..f371837 100644 --- a/src/context.rs +++ b/src/context.rs @@ -12,7 +12,7 @@ use crate::{cli::Args, find_root}; pub(crate) fn root(args: &Args) -> Result { let dir = match &args.root { - None => find_root::find_root().whatever_context("cannot find repository root")?, + None => find_root::find_root().whatever_context("cannot find root")?, Some(p) => p.to_path_buf(), }; find_root::is_root(&dir).whatever_context("invalid root directory")?; diff --git a/src/find_root.rs b/src/find_root.rs index c2db1e0..812ff70 100644 --- a/src/find_root.rs +++ b/src/find_root.rs @@ -5,6 +5,8 @@ */ use super::CONTENT_DIR; +use crate::config::MANIFEST_FILE; + use snafu::{ResultExt, Snafu}; use std::{ backtrace::Backtrace, @@ -27,17 +29,23 @@ pub enum Error { backtrace: Backtrace, }, - #[snafu(display("could not find root directory (containing `.git` and `{CONTENT_DIR}`)"))] + #[snafu(display( + "could not find root directory (containing `{MANIFEST_FILE}` and `{CONTENT_DIR}`)" + ))] NoRoot { backtrace: Backtrace }, } -pub fn is_root(path: &Path) -> Result<(), Error> { - let git = path.join(".git"); - let contents = path.join(CONTENT_DIR); - if git.is_dir() && contents.is_dir() { - Ok(()) - } else { +pub fn is_root(path: &Path) -> Result { + let manifest_path = path.join(MANIFEST_FILE); + let content_path = path.join(CONTENT_DIR); + let git_path = path.join(".git"); + + if manifest_path.is_file() && content_path.is_dir() { + Ok(true) + } else if git_path.is_dir() { NoRootSnafu.fail() + } else { + Ok(false) } } @@ -50,8 +58,8 @@ pub fn find_root() -> Result { while let Some(candidate) = current { match is_root(candidate) { - Ok(()) => return Ok(candidate.to_path_buf()), - Err(Error::NoRoot { .. }) => (), + Ok(true) => return Ok(candidate.to_path_buf()), + Ok(false) => (), Err(e) => return Err(e), } current = candidate.parent(); diff --git a/src/git.rs b/src/git.rs index 357743f..5679a06 100644 --- a/src/git.rs +++ b/src/git.rs @@ -12,7 +12,7 @@ use std::{ use crate::{ cache::Cache, - config::{Location, Locations}, + config::{LocName, Location, Manifest}, progress::{Git, ProgressIteratorExt}, }; use git2::{ @@ -40,13 +40,8 @@ pub enum Error { source: git2::Error, backtrace: Backtrace, }, - #[snafu(display("unable to determine which repository is being built (could be: {})", titles.join(", ")))] - AmbiguousIdentify { - titles: Vec, - backtrace: Backtrace, - }, #[snafu(display("unable to determine which repository is being built (none match)"))] - NoIdentify { backtrace: Backtrace }, + NoIdentify { name: LocName, backtrace: Backtrace }, #[snafu(display("working tree or index has uncommitted modifications"))] Dirty { backtrace: Backtrace }, #[snafu(display("unable to update tree ({msg})"))] @@ -65,54 +60,22 @@ pub struct RepositoryUse { pub other_repos: HashMap, } -impl Locations { - pub fn identify_repository(&self, path: &Path) -> Result { - let repo = - git2::Repository::open_ext(path, RepositoryOpenFlags::NO_SEARCH, &[] as &[&OsStr]) - .context(GitSnafu { - what: "identify open", - })?; - - let containing_locations: Vec<_> = self - .0 - .iter() - .filter_map(|(k, v)| match repo.revparse_single(&v.identifying_commit) { - Ok(_) => Some((k, v)), - _ => None, - }) - .collect(); +impl TryFrom for RepositoryUse { + type Error = Error; - ensure!( - containing_locations.len() < 2, - AmbiguousIdentifySnafu { - titles: containing_locations - .into_iter() - .map(|x| x.0) - .cloned() - .collect::>(), - } - ); - ensure!(containing_locations.len() == 1, NoIdentifySnafu); - - let (title, location) = containing_locations[0]; - - // TODO: this is a bit weird, and is a leftover from the previous architecture. - let other_repos = self - .0 - .iter() - .filter_map(|(k, v)| { - if k == title || v.repository == location.repository { - None - } else { - Some((k.clone(), v.repository.clone())) - } - }) - .collect(); + fn try_from(mut value: Manifest) -> Result { + let Some(location) = value.locations.remove(&value.name) else { + return NoIdentifySnafu { name: value.name }.fail(); + }; - Ok(RepositoryUse { - title: title.clone(), - location: location.clone(), - other_repos, + Ok(Self { + title: value.name.into(), + location, + other_repos: value + .locations + .into_iter() + .map(|(k, v)| (k.into(), v.repository)) + .collect(), }) } } @@ -176,10 +139,10 @@ pub struct Fresh { } impl Fresh { - pub fn new(root_path: &Path, build_path: &Path, locations: &Locations) -> Result { + pub fn new(root_path: &Path, build_path: &Path, manifest: Manifest) -> Result { let root_path = absolute(root_path).context(IoSnafu { path: root_path })?; check_dirty(&root_path)?; - let src_repo_use = locations.identify_repository(&root_path)?; + let src_repo_use = RepositoryUse::try_from(manifest)?; let src_repo_url = Url::from_directory_path(&root_path) .ok() .context(PathUrlSnafu { path: root_path })?; diff --git a/src/main.rs b/src/main.rs index bc01379..29c7d72 100644 --- a/src/main.rs +++ b/src/main.rs @@ -28,7 +28,8 @@ use snafu::{Report, ResultExt, Whatever}; use crate::{ cli::{Args, Operation}, - config::Config, + config::Manifest, + git::RepositoryUse, layout::{BUILD_DIR, CONTENT_DIR, OUTPUT_DIR, REPO_DIR}, }; @@ -62,16 +63,15 @@ fn make_build_dir(root: &Path) -> Result { #[derive(Debug)] struct Prepared { cache: cache::Cache, - root_path: PathBuf, repo_path: PathBuf, output_path: PathBuf, - config: Config, + manifest: Manifest, } impl Prepared { fn prepare( eipw: lint::CmdArgs, - config: Config, + manifest: Manifest, root_path: PathBuf, build_path: PathBuf, ) -> Result { @@ -81,7 +81,7 @@ impl Prepared { let content_path = repo_path.join(CONTENT_DIR); let output_path = build_path.join(OUTPUT_DIR); - let both = git::Fresh::new(&root_path, &repo_path, &config.locations) + let both = git::Fresh::new(&root_path, &repo_path, manifest.clone()) .whatever_context("initializing build repo")? .clone_src() .whatever_context("cloning source repo")? @@ -102,8 +102,8 @@ impl Prepared { let cache = cache::Cache::open().whatever_context("unable to open cache")?; lint::eipw( - config.theme.repository.as_str(), - &config.theme.commit, + manifest.theme.repository.as_str(), + &manifest.theme.commit, &cache, &root_path, &repo_path, @@ -115,8 +115,7 @@ impl Prepared { markdown::preprocess(&content_path).whatever_context("unable to preprocess markdown")?; Ok(Prepared { - config, - root_path, + manifest, cache, repo_path, output_path, @@ -124,14 +123,11 @@ impl Prepared { } fn build(self) -> Result<(), Whatever> { - let repository_use = self - .config - .locations - .identify_repository(&self.root_path) + let repository_use = RepositoryUse::try_from(self.manifest.clone()) .whatever_context("cannot identify repository use")?; zola::build( - self.config.theme.repository.as_str(), - &self.config.theme.commit, + self.manifest.theme.repository.as_str(), + &self.manifest.theme.commit, &self.cache, &self.repo_path, &self.output_path, @@ -143,8 +139,8 @@ impl Prepared { fn serve(self) -> Result<(), Whatever> { zola::serve( - self.config.theme.repository.as_str(), - &self.config.theme.commit, + self.manifest.theme.repository.as_str(), + &self.manifest.theme.commit, &self.cache, &self.repo_path, &self.output_path, @@ -155,8 +151,8 @@ impl Prepared { fn check(self) -> Result<(), Whatever> { zola::check( - self.config.theme.repository.as_str(), - &self.config.theme.commit, + self.manifest.theme.repository.as_str(), + &self.manifest.theme.commit, &self.cache, &self.repo_path, ) @@ -172,13 +168,11 @@ fn run() -> Result<(), Whatever> { return Ok(()); } - let config = if args.staging { - Config::staging() - } else { - Config::production() - }; - let root_path = context::root(&args)?; + + let manifest_path = root_path.join(config::MANIFEST_FILE); + let manifest = Manifest::load(&manifest_path).whatever_context("unable to read manifest")?; + let build_path = make_build_dir(&root_path)?; let mut lock_file = lock(&build_path)?; @@ -196,16 +190,16 @@ fn run() -> Result<(), Whatever> { return Ok(()); } Operation::Check { eipw } => { - Prepared::prepare(eipw, config, root_path, build_path)?.check()?; + Prepared::prepare(eipw, manifest, root_path, build_path)?.check()?; } Operation::Build { eipw } => { - Prepared::prepare(eipw, config, root_path, build_path)?.build()?; + Prepared::prepare(eipw, manifest, root_path, build_path)?.build()?; } Operation::Serve { eipw } => { - Prepared::prepare(eipw, config, root_path, build_path)?.serve()?; + Prepared::prepare(eipw, manifest, root_path, build_path)?.serve()?; } Operation::Changed { all, format } => { - changed::run(&root_path, &build_path, &config, all, &format)?; + changed::run(&root_path, &build_path, manifest, all, &format)?; } } From 953b695e7eddce34d94bd468ee0595df2a8c88e1 Mon Sep 17 00:00:00 2001 From: Sam Wilson Date: Fri, 19 Jun 2026 10:23:05 -0400 Subject: [PATCH 2/2] use RepositoryUse as the unit of configuration --- src/changed.rs | 6 ++-- src/config.rs | 81 ++++++++++++++++++++++++++++++++++++++++---------- src/git.rs | 44 +++++++-------------------- src/main.rs | 12 +++++--- 4 files changed, 88 insertions(+), 55 deletions(-) diff --git a/src/changed.rs b/src/changed.rs index c30f882..4472b59 100644 --- a/src/changed.rs +++ b/src/changed.rs @@ -13,7 +13,7 @@ use std::{ use snafu::{ResultExt, Whatever}; -use crate::{cli::ChangedFormat, config::Manifest, git, layout::REPO_DIR}; +use crate::{cli::ChangedFormat, config::RepositoryUse, git, layout::REPO_DIR}; pub(crate) fn is_proposal_path(mut p: PathBuf) -> bool { // Only lint `content/00001.md` and `content/00001/index.md` files. @@ -55,13 +55,13 @@ pub(crate) fn is_proposal_path(mut p: PathBuf) -> bool { pub(crate) fn run( root_path: &Path, build_path: &Path, - manifest: Manifest, + repo_use: RepositoryUse, all: bool, format: &ChangedFormat, ) -> Result<(), Whatever> { let repo_path = build_path.join(REPO_DIR); - let both = git::Fresh::new(root_path, &repo_path, manifest) + let both = git::Fresh::new(root_path, &repo_path, repo_use) .whatever_context("initializing build repo")? .clone_src() .whatever_context("cloning source repo")? diff --git a/src/config.rs b/src/config.rs index 3c0f2ca..bb01cef 100644 --- a/src/config.rs +++ b/src/config.rs @@ -8,7 +8,7 @@ use std::{borrow::Borrow, collections::HashMap, path::PathBuf, str::FromStr}; use regex::Regex; use serde::{Deserialize, Serialize}; -use snafu::{ensure, Backtrace, IntoError, ResultExt, Snafu}; +use snafu::{Backtrace, IntoError, OptionExt, ResultExt, Snafu}; use url::Url; pub const MANIFEST_FILE: &str = "Build.toml"; @@ -35,13 +35,15 @@ pub enum Error { }, #[snafu(display( - "repo manifest `{}` is invalid: {reason}", - manifest_path.to_string_lossy() + "repo manifest `{}` is invalid: {}", + manifest_path.to_string_lossy(), + source, ))] Invalid { manifest_path: PathBuf, - reason: String, backtrace: Backtrace, + #[snafu(source(from(NoIdentityError, Box::new)))] + source: Box, }, } @@ -170,16 +172,12 @@ impl Manifest { } fn from_inner(manifest_path: PathBuf, inner: InnerManifest) -> Result { - ensure!( - inner.locations.contains_key(&inner.name), - InvalidSnafu { - manifest_path: &manifest_path, - reason: format!( - "this locations's name (`{}`) must appear in `locations`", - inner.name - ), - } - ); + if !inner.locations.contains_key(&inner.name) { + return NoIdentitySnafu { name: inner.name } + .fail() + .context(InvalidSnafu { manifest_path }); + } + Ok(Self { manifest_path, name: inner.name, @@ -197,6 +195,43 @@ impl Manifest { } } +#[derive(Debug, Snafu)] +#[snafu(display("this locations's name (`{name}`) must appear in `locations`"))] +pub struct NoIdentityError { + name: LocName, + backtrace: Backtrace, +} + +#[derive(Debug, Clone)] +pub struct RepositoryUse { + pub title: String, + pub location: Location, + pub other_repos: HashMap, +} + +impl TryFrom for RepositoryUse { + type Error = NoIdentityError; + + fn try_from(mut value: Manifest) -> Result { + let location = value + .locations + .remove(&value.name) + .with_context(|| NoIdentitySnafu { + name: value.name.clone(), + })?; + + Ok(Self { + title: value.name.into(), + location, + other_repos: value + .locations + .into_iter() + .map(|(k, v)| (k.into(), v.repository)) + .collect(), + }) + } +} + #[cfg(test)] mod tests { use std::path::{Path, PathBuf}; @@ -285,6 +320,20 @@ commit = "aaa" assert!(reason.contains("invalid location name")); } + #[test] + fn repo_manifest_rejects_empty_names() { + let repo = TestRepo::new(); + let manifest_path = repo.write_file(MANIFEST_FILE, r#"name = """#); + + let Err(Error::Parse { source, .. }) = Manifest::load(&manifest_path) else { + panic!("expected parse error"); + }; + + let reason = source.to_string(); + + assert!(reason.contains("invalid location name")); + } + #[test] fn repo_manifest_requires_self() { let repo = TestRepo::new(); @@ -299,10 +348,12 @@ commit = "aaa" "#, ); - let Err(Error::Invalid { reason, .. }) = Manifest::load(&manifest_path) else { + let Err(Error::Invalid { source, .. }) = Manifest::load(&manifest_path) else { panic!("expected invalid error"); }; + let reason = source.to_string(); + assert!(reason.contains("this locations's name (`banana`) must appear in `locations`")); } } diff --git a/src/git.rs b/src/git.rs index 5679a06..275c828 100644 --- a/src/git.rs +++ b/src/git.rs @@ -5,14 +5,13 @@ */ use std::{ - collections::HashMap, ffi::OsStr, path::{absolute, Path, PathBuf}, }; use crate::{ cache::Cache, - config::{LocName, Location, Manifest}, + config::{NoIdentityError, RepositoryUse}, progress::{Git, ProgressIteratorExt}, }; use git2::{ @@ -40,8 +39,11 @@ pub enum Error { source: git2::Error, backtrace: Backtrace, }, - #[snafu(display("unable to determine which repository is being built (none match)"))] - NoIdentify { name: LocName, backtrace: Backtrace }, + #[snafu(context(false))] + NoIdentity { + #[snafu(backtrace)] + source: NoIdentityError, + }, #[snafu(display("working tree or index has uncommitted modifications"))] Dirty { backtrace: Backtrace }, #[snafu(display("unable to update tree ({msg})"))] @@ -53,33 +55,6 @@ pub enum Error { }, } -#[derive(Debug, Clone)] -pub struct RepositoryUse { - pub title: String, - pub location: Location, - pub other_repos: HashMap, -} - -impl TryFrom for RepositoryUse { - type Error = Error; - - fn try_from(mut value: Manifest) -> Result { - let Some(location) = value.locations.remove(&value.name) else { - return NoIdentifySnafu { name: value.name }.fail(); - }; - - Ok(Self { - title: value.name.into(), - location, - other_repos: value - .locations - .into_iter() - .map(|(k, v)| (k.into(), v.repository)) - .collect(), - }) - } -} - pub fn check_dirty(root_path: &Path) -> Result<(), Error> { let repo = git2::Repository::open(root_path).context(GitSnafu { what: "open root repository", @@ -139,10 +114,13 @@ pub struct Fresh { } impl Fresh { - pub fn new(root_path: &Path, build_path: &Path, manifest: Manifest) -> Result { + pub fn new( + root_path: &Path, + build_path: &Path, + src_repo_use: RepositoryUse, + ) -> Result { let root_path = absolute(root_path).context(IoSnafu { path: root_path })?; check_dirty(&root_path)?; - let src_repo_use = RepositoryUse::try_from(manifest)?; let src_repo_url = Url::from_directory_path(&root_path) .ok() .context(PathUrlSnafu { path: root_path })?; diff --git a/src/main.rs b/src/main.rs index 29c7d72..0b86029 100644 --- a/src/main.rs +++ b/src/main.rs @@ -28,8 +28,7 @@ use snafu::{Report, ResultExt, Whatever}; use crate::{ cli::{Args, Operation}, - config::Manifest, - git::RepositoryUse, + config::{Manifest, RepositoryUse}, layout::{BUILD_DIR, CONTENT_DIR, OUTPUT_DIR, REPO_DIR}, }; @@ -81,7 +80,10 @@ impl Prepared { let content_path = repo_path.join(CONTENT_DIR); let output_path = build_path.join(OUTPUT_DIR); - let both = git::Fresh::new(&root_path, &repo_path, manifest.clone()) + let repository_use = RepositoryUse::try_from(manifest.clone()) + .whatever_context("cannot identify repository use")?; + + let both = git::Fresh::new(&root_path, &repo_path, repository_use) .whatever_context("initializing build repo")? .clone_src() .whatever_context("cloning source repo")? @@ -199,7 +201,9 @@ fn run() -> Result<(), Whatever> { Prepared::prepare(eipw, manifest, root_path, build_path)?.serve()?; } Operation::Changed { all, format } => { - changed::run(&root_path, &build_path, manifest, all, &format)?; + let repository_use = RepositoryUse::try_from(manifest) + .whatever_context("cannot identify repository use")?; + changed::run(&root_path, &build_path, repository_use, all, &format)?; } }