diff --git a/Cargo.toml b/Cargo.toml index 68d154f..5c4235c 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -30,4 +30,5 @@ tempfile = "3.4" thiserror = "1.0" tokio = { version = "1.26", features = [ "full" ] } toml = "0.7.3" +topological-sort = "0.2.2" walkdir = "2.3" diff --git a/src/config.rs b/src/config.rs index eb82a7f..656651f 100644 --- a/src/config.rs +++ b/src/config.rs @@ -4,12 +4,97 @@ //! Configuration for a package. -use crate::package::{Package, PackageOutput}; +use crate::package::{Package, PackageOutput, PackageSource}; use crate::target::Target; use serde_derive::Deserialize; use std::collections::BTreeMap; use std::path::Path; use thiserror::Error; +use topological_sort::TopologicalSort; + +/// Describes a set of packages to act upon. +pub struct PackageMap<'a>(BTreeMap<&'a String, &'a Package>); + +// The name of a file which should be created by building a package. +#[derive(Clone, Eq, Hash, Ord, PartialEq, PartialOrd)] +struct OutputFile(String); + +impl<'a> PackageMap<'a> { + pub fn build_order(&self) -> PackageDependencyIter<'a> { + let lookup_by_output = self + .0 + .iter() + .map(|(name, package)| (OutputFile(package.get_output_file(name)), (*name, *package))) + .collect::>(); + + // Collect all packages, and sort them in dependency order, + // so we know which ones to build first. + let mut outputs = TopologicalSort::::new(); + for (package_output, (_, package)) in &lookup_by_output { + match &package.source { + PackageSource::Local { .. } + | PackageSource::Prebuilt { .. } + | PackageSource::Manual => { + // Skip intermediate leaf packages; if necessary they'll be + // added to the dependency graph by whatever composite package + // actually depends on them. + if !matches!( + package.output, + PackageOutput::Zone { + intermediate_only: true + } + ) { + outputs.insert(package_output.clone()); + } + } + PackageSource::Composite { packages: deps } => { + for dep in deps { + outputs.add_dependency(OutputFile(dep.clone()), package_output.clone()); + } + } + } + } + + PackageDependencyIter { + lookup_by_output, + outputs, + } + } +} + +/// Returns all packages in the order in which they should be built. +/// +/// Returns packages in batches that may be built concurrently. +pub struct PackageDependencyIter<'a> { + lookup_by_output: BTreeMap, + outputs: TopologicalSort, +} + +impl<'a> Iterator for PackageDependencyIter<'a> { + type Item = Vec<(&'a String, &'a Package)>; + + fn next(&mut self) -> Option { + if self.outputs.is_empty() { + return None; + } + let batch = self.outputs.pop_all(); + assert!( + !batch.is_empty() || self.outputs.is_empty(), + "cyclic dependency in package manifest!" + ); + + Some( + batch + .into_iter() + .map(|output| { + *self.lookup_by_output.get(&output).unwrap_or_else(|| { + panic!("Could not find a package which creates '{}'", output.0) + }) + }) + .collect(), + ) + } +} /// Describes the configuration for a set of packages. #[derive(Deserialize, Debug)] @@ -21,24 +106,28 @@ pub struct Config { impl Config { /// Returns target packages to be assembled on the builder machine. - pub fn packages_to_build(&self, target: &Target) -> BTreeMap<&String, &Package> { - self.packages - .iter() - .filter(|(_, pkg)| target.includes_package(pkg)) - .map(|(name, pkg)| (name, pkg)) - .collect() + pub fn packages_to_build(&self, target: &Target) -> PackageMap<'_> { + PackageMap( + self.packages + .iter() + .filter(|(_, pkg)| target.includes_package(pkg)) + .map(|(name, pkg)| (name, pkg)) + .collect(), + ) } /// Returns target packages which should execute on the deployment machine. - pub fn packages_to_deploy(&self, target: &Target) -> BTreeMap<&String, &Package> { - let all_packages = self.packages_to_build(target); - all_packages - .into_iter() - .filter(|(_, pkg)| match pkg.output { - PackageOutput::Zone { intermediate_only } => !intermediate_only, - PackageOutput::Tarball => true, - }) - .collect() + pub fn packages_to_deploy(&self, target: &Target) -> PackageMap<'_> { + let all_packages = self.packages_to_build(target).0; + PackageMap( + all_packages + .into_iter() + .filter(|(_, pkg)| match pkg.output { + PackageOutput::Zone { intermediate_only } => !intermediate_only, + PackageOutput::Tarball => true, + }) + .collect(), + ) } } @@ -61,3 +150,107 @@ pub fn parse>(path: P) -> Result { let contents = std::fs::read_to_string(path.as_ref())?; parse_manifest(&contents) } + +#[cfg(test)] +mod test { + use super::*; + + #[test] + fn test_order() { + let pkg_a_name = String::from("pkg-a"); + let pkg_a = Package { + service_name: String::from("a"), + source: PackageSource::Manual, + output: PackageOutput::Tarball, + only_for_targets: None, + setup_hint: None, + }; + + let pkg_b_name = String::from("pkg-b"); + let pkg_b = Package { + service_name: String::from("b"), + source: PackageSource::Composite { + packages: vec![pkg_a.get_output_file(&pkg_a_name)], + }, + output: PackageOutput::Tarball, + only_for_targets: None, + setup_hint: None, + }; + + let cfg = Config { + packages: BTreeMap::from([ + (pkg_a_name.clone(), pkg_a.clone()), + (pkg_b_name.clone(), pkg_b.clone()), + ]), + }; + + let mut order = cfg.packages_to_build(&Target::default()).build_order(); + // "pkg-a" comes first, because "pkg-b" depends on it. + assert_eq!(order.next(), Some(vec![(&pkg_a_name, &pkg_a)])); + assert_eq!(order.next(), Some(vec![(&pkg_b_name, &pkg_b)])); + } + + // We're kinda limited by the topological-sort library here, as this is a documented + // behavior from [TopologicalSort::pop_all]. + // + // Regardless, test that circular dependencies cause panics. + #[test] + #[should_panic(expected = "cyclic dependency in package manifest")] + fn test_cyclic_dependency() { + let pkg_a_name = String::from("pkg-a"); + let pkg_b_name = String::from("pkg-b"); + let pkg_a = Package { + service_name: String::from("a"), + source: PackageSource::Composite { + packages: vec![String::from("pkg-b.tar")], + }, + output: PackageOutput::Tarball, + only_for_targets: None, + setup_hint: None, + }; + let pkg_b = Package { + service_name: String::from("b"), + source: PackageSource::Composite { + packages: vec![String::from("pkg-a.tar")], + }, + output: PackageOutput::Tarball, + only_for_targets: None, + setup_hint: None, + }; + + let cfg = Config { + packages: BTreeMap::from([ + (pkg_a_name.clone(), pkg_a.clone()), + (pkg_b_name.clone(), pkg_b.clone()), + ]), + }; + + let mut order = cfg.packages_to_build(&Target::default()).build_order(); + order.next(); + } + + // Make pkg-a depend on pkg-b.tar, but don't include pkg-b.tar anywhere. + // + // Ensure that we see an appropriate panic. + #[test] + #[should_panic(expected = "Could not find a package which creates 'pkg-b.tar'")] + fn test_missing_dependency() { + let pkg_a_name = String::from("pkg-a"); + let pkg_a = Package { + service_name: String::from("a"), + source: PackageSource::Composite { + packages: vec![String::from("pkg-b.tar")], + }, + output: PackageOutput::Tarball, + only_for_targets: None, + setup_hint: None, + }; + + let cfg = Config { + packages: BTreeMap::from([(pkg_a_name.clone(), pkg_a.clone())]), + }; + + let mut order = cfg.packages_to_build(&Target::default()).build_order(); + order.next(); + } +} diff --git a/src/package.rs b/src/package.rs index b9df8af..20ec510 100644 --- a/src/package.rs +++ b/src/package.rs @@ -176,7 +176,7 @@ async fn add_package_to_zone_archive( /// the following path: /// /// -#[derive(Deserialize, Debug)] +#[derive(Clone, Deserialize, Debug, PartialEq)] pub struct PrebuiltBlob { pub repo: String, pub series: String, @@ -186,7 +186,7 @@ pub struct PrebuiltBlob { } /// Describes the origin of an externally-built package. -#[derive(Deserialize, Debug)] +#[derive(Clone, Deserialize, Debug, PartialEq)] #[serde(tag = "type", rename_all = "lowercase")] pub enum PackageSource { /// Describes a package which should be assembled locally. @@ -257,7 +257,7 @@ impl PackageSource { } /// Describes the output format of the package. -#[derive(Deserialize, Debug, Clone)] +#[derive(Deserialize, Debug, Clone, PartialEq)] #[serde(tag = "type", rename_all = "lowercase")] pub enum PackageOutput { /// A complete zone image, ready to be deployed to the target. @@ -274,7 +274,7 @@ pub enum PackageOutput { } /// A single package. -#[derive(Deserialize, Debug)] +#[derive(Clone, Deserialize, Debug, PartialEq)] pub struct Package { /// The name of the service name to be used on the target OS. pub service_name: String, @@ -821,7 +821,7 @@ impl Package { } /// Describes configuration for a package which contains a Rust binary. -#[derive(Deserialize, Debug)] +#[derive(Clone, Deserialize, Debug, PartialEq)] pub struct RustPackage { /// The name of the compiled binary to be used. // TODO: Could be extrapolated to "produced build artifacts", we don't @@ -869,7 +869,7 @@ impl RustPackage { } /// A string which can be modified with key-value pairs. -#[derive(Deserialize, Debug)] +#[derive(Clone, Deserialize, Debug, PartialEq)] pub struct InterpolatedString(String); impl InterpolatedString { @@ -891,7 +891,10 @@ impl InterpolatedString { }; let key = &input[..end_idx]; let Some(value) = target.0.get(key) else { - bail!("Key '{key}' not found in target, but required in '{}'", self.0); + bail!( + "Key '{key}' not found in target, but required in '{}'", + self.0 + ); }; output.push_str(&value); input = &input[end_idx + END_STR.len()..]; @@ -902,7 +905,7 @@ impl InterpolatedString { } /// A pair of paths, mapping from a directory on the host to the target. -#[derive(Deserialize, Debug)] +#[derive(Clone, Deserialize, Debug, PartialEq)] pub struct MappedPath { /// Source path. pub from: InterpolatedString, diff --git a/tests/mod.rs b/tests/mod.rs index 0836c1a..ad660c8 100644 --- a/tests/mod.rs +++ b/tests/mod.rs @@ -175,10 +175,16 @@ mod test { let cfg = config::parse("tests/service-e/cfg.toml").unwrap(); let out = tempfile::tempdir().unwrap(); + // Ask for the order of packages to-be-built + let packages = cfg.packages_to_build(&Target::default()); + let mut build_order = packages.build_order(); + // Build the dependencies first. - let package_dependencies = ["pkg-1", "pkg-2"]; - for package_name in package_dependencies { - let package = cfg.packages.get(package_name).unwrap(); + let batch = build_order.next().expect("Missing dependency batch"); + let mut batch_pkg_names: Vec<_> = batch.iter().map(|(name, _)| *name).collect(); + batch_pkg_names.sort(); + assert_eq!(batch_pkg_names, vec!["pkg-1", "pkg-2"]); + for (package_name, package) in batch { // Create the packaged file package .create_for_target(&Target::default(), package_name, out.path()) @@ -187,7 +193,10 @@ mod test { } // Build the composite package + let batch = build_order.next().expect("Missing dependency batch"); + let batch_pkg_names: Vec<_> = batch.iter().map(|(name, _)| *name).collect(); let package_name = "pkg-3"; + assert_eq!(batch_pkg_names, vec![package_name]); let package = cfg.packages.get(package_name).unwrap(); package .create_for_target(&Target::default(), package_name, out.path())