diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index f19b0b1b2..c861dc888 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -72,6 +72,8 @@ jobs: with: tool: cargo-tarpaulin - run: cargo tarpaulin -o lcov --output-dir coverage + - run: brew trust coverallsapp/coveralls + if: matrix.os == 'macos-latest' - uses: coverallsapp/github-action@v2 with: path-to-lcov: coverage/lcov.info @@ -234,6 +236,8 @@ jobs: -Xdemangler=rustfilt \ > lcov.info + - run: brew trust coverallsapp/coveralls + if: matrix.os == 'macos-latest' - uses: coverallsapp/github-action@v2 with: path-to-lcov: lcov.info diff --git a/crates/cli/src/resolve.rs b/crates/cli/src/resolve.rs index dbac0e3bd..8a54efb38 100644 --- a/crates/cli/src/resolve.rs +++ b/crates/cli/src/resolve.rs @@ -87,7 +87,7 @@ pub async fn resolve( let mut installations = resolution.installed; if !resolution.pending.is_empty() { if env::var("PKGX_NO_INSTALL").is_ok() { - return Err("PKGX_NO_INSTALL is set, refusing to install pending packages")?; + Err("PKGX_NO_INSTALL is set, refusing to install pending packages")?; } let installed = install_multi(&resolution.pending, config, spinner.arc()).await?; installations.extend(installed); diff --git a/crates/lib/src/env.rs b/crates/lib/src/env.rs index 36d0abf8e..eda81f64c 100644 --- a/crates/lib/src/env.rs +++ b/crates/lib/src/env.rs @@ -211,7 +211,7 @@ pub fn expand_moustaches(input: &str, pkg: &Installation, deps: &Vec bool { + MULTI_VERSION_PROJECTS.contains(&project) +} + +/// Record an extra constraint for a multi-version project, merging into an +/// existing additional entry when the ranges intersect. +fn push_additional(additional: &mut Vec, pkg: PackageReq) { + for existing in additional.iter_mut().filter(|p| p.project == pkg.project) { + if let Ok(constraint) = intersect_constraints(&existing.constraint, &pkg.constraint) { + existing.constraint = constraint; + return; + } + } + additional.push(pkg); +} + #[derive(Clone)] struct Node { parent: Option>, pkg: PackageReq, - children: HashSet, } impl Node { fn new(pkg: PackageReq, parent: Option>) -> Self { - Self { - parent, - pkg, - children: HashSet::new(), - } + Self { parent, pkg } } fn count(&self) -> usize { @@ -38,36 +61,55 @@ pub async fn hydrate( where F: Fn(String) -> Result, Box>, { - let dry = condense(input); + let dry = condense(input)?; let mut graph: HashMap> = HashMap::new(); let mut stack: Vec> = vec![]; - let mut additional_unicodes: Vec = vec![]; + let mut additional: Vec = vec![]; for pkg in dry.iter() { - let node = graph - .entry(pkg.project.clone()) - .or_insert_with(|| Box::new(Node::new(pkg.clone(), None))); - node.pkg.constraint = intersect_constraints(&node.pkg.constraint, &pkg.constraint) - .map_err(|e| format!("{} for {}", e, pkg.project))?; - stack.push(node.clone()); + if let Some(node) = graph.get_mut(&pkg.project) { + match intersect_constraints(&node.pkg.constraint, &pkg.constraint) { + Ok(constraint) => { + node.pkg.constraint = constraint; + stack.push(node.clone()); + } + Err(e) => { + if is_multi_version(&pkg.project) { + push_additional(&mut additional, pkg.clone()); + } else { + return Err(format!("{} for {}", e, pkg.project).into()); + } + } + } + } else { + let node = Box::new(Node::new(pkg.clone(), None)); + graph.insert(pkg.project.clone(), node.clone()); + stack.push(node); + } } - while let Some(mut current) = stack.pop() { + while let Some(current) = stack.pop() { for child_pkg in get_deps(current.pkg.project.clone())? { + let was_new = !graph.contains_key(&child_pkg.project); let child_node = graph .entry(child_pkg.project.clone()) .or_insert_with(|| Box::new(Node::new(child_pkg.clone(), Some(current.clone())))); + + if was_new { + // Fresh node already carries child_pkg.constraint. + stack.push(child_node.clone()); + continue; + } + + // Already have a graph node: try the primary constraint, then any + // additional lines for this multi-version project. let intersection = intersect_constraints(&child_node.pkg.constraint, &child_pkg.constraint); if let Ok(constraint) = intersection { child_node.pkg.constraint = constraint; - current.children.insert(child_node.pkg.project.clone()); stack.push(child_node.clone()); - } else if child_pkg.project == "unicode.org" { - // we handle unicode.org for now to allow situations like: - // https://github.com/pkgxdev/pantry/issues/4104 - // https://github.com/pkgxdev/pkgx/issues/899 - additional_unicodes.push(child_pkg.constraint); + } else if is_multi_version(&child_pkg.project) { + push_additional(&mut additional, child_pkg); } else { return Err( format!("{} for {}", intersection.unwrap_err(), child_pkg.project).into(), @@ -80,30 +122,45 @@ where pkgs.sort_by_key(|node| node.count()); let mut pkgs: Vec = pkgs.into_iter().map(|node| node.pkg.clone()).collect(); - // see above explanation - for constraint in additional_unicodes { - let pkg = PackageReq { - project: "unicode.org".to_string(), - constraint, - }; - pkgs.push(pkg); - } + pkgs.extend(additional); Ok(pkgs) } -/// Condenses a list of `PackageRequirement` by intersecting constraints for duplicates. -fn condense(pkgs: &Vec) -> Vec { +/// Condenses a list of `PackageReq` by intersecting constraints for duplicates. +/// Multi-version projects keep non-intersecting constraints as separate entries. +fn condense(pkgs: &Vec) -> Result, Box> { let mut out: Vec = vec![]; for pkg in pkgs { if let Some(existing) = out.iter_mut().find(|p| p.project == pkg.project) { - existing.constraint = intersect_constraints(&existing.constraint, &pkg.constraint) - .expect("Failed to intersect constraints"); + match intersect_constraints(&existing.constraint, &pkg.constraint) { + Ok(constraint) => existing.constraint = constraint, + Err(e) => { + if is_multi_version(&pkg.project) { + // merge into a later non-intersecting sibling if possible + let mut merged = false; + for sibling in out.iter_mut().filter(|p| p.project == pkg.project).skip(1) { + if let Ok(constraint) = + intersect_constraints(&sibling.constraint, &pkg.constraint) + { + sibling.constraint = constraint; + merged = true; + break; + } + } + if !merged { + out.push(pkg.clone()); + } + } else { + return Err(format!("{} for {}", e, pkg.project).into()); + } + } + } } else { out.push(pkg.clone()); } } - out + Ok(out) } /// Intersects two version constraints. diff --git a/crates/lib/src/lib.rs b/crates/lib/src/lib.rs index c99bc8f44..c0387e196 100644 --- a/crates/lib/src/lib.rs +++ b/crates/lib/src/lib.rs @@ -11,6 +11,8 @@ pub mod pantry_db; pub mod platform_case_aware_env_key; pub mod resolve; pub mod sync; +#[cfg(test)] +mod tests; pub mod types; pub mod utils; diff --git a/crates/lib/src/sync.rs b/crates/lib/src/sync.rs index 950867985..af3922c95 100644 --- a/crates/lib/src/sync.rs +++ b/crates/lib/src/sync.rs @@ -39,9 +39,7 @@ pub async fn update(config: &Config, conn: &mut Connection) -> Result<(), Box PackageReq { + PackageReq { + project: project.to_string(), + constraint: VersionReq::parse(constraint).unwrap(), + } +} + +fn pkgs_for<'a>(pkgs: &'a [PackageReq], project: &str) -> Vec<&'a PackageReq> { + pkgs.iter().filter(|p| p.project == project).collect() +} + +/// True if some hydrated line for `project` intersects `range` (same ABI line). +fn has_line(pkgs: &[PackageReq], project: &str, range: &str) -> bool { + let want = VersionReq::parse(range).unwrap(); + pkgs_for(pkgs, project) + .into_iter() + .any(|p| p.constraint.intersect(&want).is_ok()) +} + +/// Assert two ranges remain disjoint (cannot be collapsed). +fn assert_disjoint(a: &str, b: &str) { + assert!(VersionReq::parse(a) + .unwrap() + .intersect(&VersionReq::parse(b).unwrap()) + .is_err()); +} + +#[tokio::test] +async fn hydrates_unicode_multi() { + let input = vec![req("npmjs.com", "*"), req("python.org", "~3.9")]; + let pkgs = hydrate(&input, |project| match project.as_str() { + "python.org" => Ok(vec![req("unicode.org", "^73")]), + "npmjs.com" => Ok(vec![req("unicode.org", "^71")]), + _ => Ok(vec![]), + }) + .await + .unwrap(); + + assert_eq!(pkgs_for(&pkgs, "unicode.org").len(), 2); + assert!(has_line(&pkgs, "unicode.org", "^71")); + assert!(has_line(&pkgs, "unicode.org", "^73")); + assert_disjoint("^71", "^73"); +} + +#[tokio::test] +async fn hydrates_openssl_multi() { + // python locks ^1.1; cryptography needs ^3 — must coexist + let input = vec![req("python.org", "*"), req("cryptography.io", "*")]; + let pkgs = hydrate(&input, |project| match project.as_str() { + "python.org" => Ok(vec![req("openssl.org", "^1.1")]), + "cryptography.io" => Ok(vec![req("openssl.org", "^3")]), + _ => Ok(vec![]), + }) + .await + .unwrap(); + + assert_eq!(pkgs_for(&pkgs, "openssl.org").len(), 2); + assert!(has_line(&pkgs, "openssl.org", "^1.1")); + assert!(has_line(&pkgs, "openssl.org", "^3")); + assert_disjoint("^1.1", "^3"); +} + +#[tokio::test] +async fn hydrates_abseil_multi() { + // re2 on one LTS line, grpc on another + let input = vec![req("github.com/google/re2", "*"), req("grpc.io", "*")]; + let pkgs = hydrate(&input, |project| match project.as_str() { + "github.com/google/re2" => Ok(vec![req("abseil.io", "^20250127")]), + "grpc.io" => Ok(vec![req("abseil.io", ">=20250512")]), + _ => Ok(vec![]), + }) + .await + .unwrap(); + + assert_eq!(pkgs_for(&pkgs, "abseil.io").len(), 2); + assert!(has_line(&pkgs, "abseil.io", "^20250127")); + assert!(has_line(&pkgs, "abseil.io", ">=20250512")); + assert_disjoint("^20250127", ">=20250512"); +} + +#[tokio::test] +async fn hydrates_openssl_dry_input() { + // explicit +openssl^1.1 +openssl^3 + let input = vec![req("openssl.org", "^1.1"), req("openssl.org", "^3")]; + let pkgs = hydrate(&input, |_| Ok(vec![])).await.unwrap(); + + assert_eq!(pkgs_for(&pkgs, "openssl.org").len(), 2); + assert!(has_line(&pkgs, "openssl.org", "^1.1")); + assert!(has_line(&pkgs, "openssl.org", "^3")); +} + +#[tokio::test] +async fn hydrates_multi_version_three_way() { + // three consumers, two openssl lines (two share ^1.1, one needs ^3) + // order cryptography first so ^3 is the graph node and both ^1.1 merge + // into a single additional entry. + let input = vec![ + req("cryptography.io", "*"), + req("python.org", "*"), + req("curl.se", "*"), + ]; + let pkgs = hydrate(&input, |project| match project.as_str() { + "python.org" | "curl.se" => Ok(vec![req("openssl.org", "^1.1")]), + "cryptography.io" => Ok(vec![req("openssl.org", "^3")]), + _ => Ok(vec![]), + }) + .await + .unwrap(); + + assert_eq!(pkgs_for(&pkgs, "openssl.org").len(), 2); + assert!(has_line(&pkgs, "openssl.org", "^1.1")); + assert!(has_line(&pkgs, "openssl.org", "^3")); +} + +#[tokio::test] +async fn hydrates_cannot_intersect() { + let input = vec![req("npmjs.com", "*"), req("python.org", "~3.9")]; + let err = hydrate(&input, |project| match project.as_str() { + "python.org" => Ok(vec![req("nodejs.com", "^73")]), + "npmjs.com" => Ok(vec![req("nodejs.com", "^71")]), + _ => Ok(vec![]), + }) + .await + .unwrap_err(); + + assert!(err.to_string().contains("nodejs.com")); +} + +#[tokio::test] +async fn hydrates_compatible_intersect() { + let input = vec![req("pipenv.pypa.io", "*"), req("python.org", "~3.9")]; + let pkgs = hydrate(&input, |project| match project.as_str() { + "pipenv.pypa.io" => Ok(vec![req("python.org", ">=3.7")]), + _ => Ok(vec![]), + }) + .await + .unwrap(); + + let pythons = pkgs_for(&pkgs, "python.org"); + assert_eq!(pythons.len(), 1); + // dry ~3.9 wins over looser >=3.7 + assert!(has_line(&pkgs, "python.org", "~3.9")); + assert!(!has_line(&pkgs, "python.org", "~3.10")); + assert!(!has_line(&pkgs, "python.org", "~3.8")); +} + +#[tokio::test] +async fn hydrates_multi_version_dry_condense_compatible() { + // two compatible dry openssl constraints still collapse to one + let input = vec![req("openssl.org", "^1.1"), req("openssl.org", ">=1.1.1")]; + let pkgs = hydrate(&input, |_| Ok(vec![])).await.unwrap(); + + assert_eq!(pkgs_for(&pkgs, "openssl.org").len(), 1); + assert!(has_line(&pkgs, "openssl.org", "^1.1")); + assert!(!has_line(&pkgs, "openssl.org", "^3")); +} diff --git a/crates/lib/src/tests/mod.rs b/crates/lib/src/tests/mod.rs new file mode 100644 index 000000000..39fab0739 --- /dev/null +++ b/crates/lib/src/tests/mod.rs @@ -0,0 +1 @@ +mod hydrate; diff --git a/crates/lib/src/types.rs b/crates/lib/src/types.rs index 3de100e28..4b090171e 100644 --- a/crates/lib/src/types.rs +++ b/crates/lib/src/types.rs @@ -19,7 +19,7 @@ pub struct Package { impl fmt::Display for Package { fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { - write!(f, "{}={}", self.project, &self.version) + write!(f, "{}={}", self.project, self.version) } } @@ -63,7 +63,7 @@ impl fmt::Display for PackageReq { if self.constraint.raw == "*" { write!(f, "{}", self.project) } else { - write!(f, "{}{}", self.project, &self.constraint) + write!(f, "{}{}", self.project, self.constraint) } } }