diff --git a/crates/spk-schema/crates/foundation/src/version/compat.rs b/crates/spk-schema/crates/foundation/src/version/compat.rs index f8e537d8a4..ccf711751c 100644 --- a/crates/spk-schema/crates/foundation/src/version/compat.rs +++ b/crates/spk-schema/crates/foundation/src/version/compat.rs @@ -90,6 +90,10 @@ impl Ord for CompatRule { /// Denotes whether or not something is compatible. #[derive(Clone, Debug, Eq, PartialEq)] pub enum IncompatibleReason { + AlreadyEmbeddedPackage { + embedded: PkgNameBuf, + embedded_by: PkgNameBuf, + }, ConflictingEmbeddedPackage(PkgNameBuf), Other(String), } @@ -97,6 +101,15 @@ pub enum IncompatibleReason { impl std::fmt::Display for IncompatibleReason { fn fmt(&self, f: &mut std::fmt::Formatter) -> std::fmt::Result { match self { + IncompatibleReason::AlreadyEmbeddedPackage { + embedded, + embedded_by, + } => { + write!( + f, + "embedded package {embedded} already embedded by another package in solve: {embedded_by}" + ) + } IncompatibleReason::ConflictingEmbeddedPackage(pkg) => { write!( f, diff --git a/crates/spk-solve/src/solver.rs b/crates/spk-solve/src/solver.rs index 53fe02192e..4614fadb55 100644 --- a/crates/spk-solve/src/solver.rs +++ b/crates/spk-solve/src/solver.rs @@ -643,13 +643,44 @@ impl Solver { // solve. Jump back to before the conflicting // package was added and try adding this // build again. - let (_, _, state_id) = node + let (conflicting_pkg, conflicting_pkg_source, state_id) = node .state .get_current_resolve(&conflicting_package_name) .map_err(|_| { Error::String("package not found in resolve".into()) })?; + // Is the conflicting package already embedded + // by some other package? + if conflicting_pkg.ident().is_embedded() { + notes.push(Note::SkipPackageNote(SkipPackageNote::new( + spec.ident().to_any(), + Compatibility::Incompatible({ + match conflicting_pkg_source { + PackageSource::Embedded { parent } => { + IncompatibleReason::AlreadyEmbeddedPackage { + embedded: conflicting_package_name, + embedded_by: parent.name().to_owned(), + } + } + _ => { + // As the solver exhausts + // all possibilities it + // eventually tries to + // resolve an embedded stub + // for a package that is + // already in the solve. + IncompatibleReason::ConflictingEmbeddedPackage( + conflicting_package_name, + ) + } + } + }), + ))); + self.number_builds_skipped += 1; + continue; + } + let graph_lock = graph.read().await; let target_state_node = graph_lock diff --git a/crates/spk-solve/src/solver_test.rs b/crates/spk-solve/src/solver_test.rs index 84d53b4fdd..79de61e10d 100644 --- a/crates/spk-solve/src/solver_test.rs +++ b/crates/spk-solve/src/solver_test.rs @@ -1421,6 +1421,62 @@ async fn test_solver_impossible_request_but_embedded_package_makes_solvable(mut }; } +/// When multiple packages try to embed the same package the solver doesn't +/// panic. +#[rstest] +#[tokio::test] +async fn test_multiple_packages_embed_same_package( + mut solver: Solver, + #[values(true, false)] resolve_validation_impossible_checks: bool, +) { + init_logging(); + + let repo = make_repo!( + [ + { + "pkg": "embedded-pkg/1.0.0", + "build": {"script": "echo BUILD"}, + }, + { + "pkg": "dep1/1.0.0", + "build": {"script": "echo BUILD"}, + "install": {"embedded": [{"pkg": "embedded-pkg/1.0.0"}]}, + }, + { + "pkg": "dep2/1.0.0", + "build": {"script": "echo BUILD"}, + "install": {"embedded": [{"pkg": "embedded-pkg/1.0.0"}]}, + }, + { + "pkg": "top-level/1.0.0", + "build": {"script": "echo BUILD"}, + "install": {"requirements": [ + // Resolve the "real" package first + {"pkg": "embedded-pkg"}, + // dep1 embeds 'embedded-pkg' and should eject the real one + {"pkg": "dep1"}, + // dep2 also embeds 'embedded-pkg' and in the past would + // cause the solver to panic + {"pkg": "dep2"}]}, + }, + ] + ); + + solver.add_repository(Arc::new(repo)); + solver.add_request(request!("top-level")); + solver.set_resolve_validation_impossible_checks(resolve_validation_impossible_checks); + + match run_and_print_resolve_for_tests(&solver).await { + Err(Error::GraphError(spk_solve_graph::Error::FailedToResolve(_))) => {} + Ok(_) => { + panic!("No solution expected"); + } + Err(err) => { + panic!("Expected FailedToResolve, but got this error: {err}"); + } + }; +} + #[rstest] #[tokio::test] async fn test_solver_with_impossible_checks_in_build_keys(mut solver: Solver) {