diff --git a/src/internal/id.rs b/src/internal/id.rs index 47fe226..e3b160a 100644 --- a/src/internal/id.rs +++ b/src/internal/id.rs @@ -46,6 +46,12 @@ impl ArenaId for StringId { #[cfg_attr(feature = "serde", serde(transparent))] pub struct VersionSetId(pub u32); +impl From<(VersionSetId, Option)> for VersionSetId { + fn from((id, _): (VersionSetId, Option)) -> Self { + id + } +} + impl ArenaId for VersionSetId { fn from_usize(x: usize) -> Self { Self(x as u32) diff --git a/src/requirement.rs b/src/requirement.rs index 56a7bc5..9641e27 100644 --- a/src/requirement.rs +++ b/src/requirement.rs @@ -71,6 +71,15 @@ impl From for ConditionalRequirement { } } +impl From<(VersionSetId, Option)> for ConditionalRequirement { + fn from((requirement, condition): (VersionSetId, Option)) -> Self { + Self { + condition, + requirement: requirement.into(), + } + } +} + /// Specifies the dependency of a solvable on a set of version sets. #[derive(Clone, Copy, Debug, Hash, Eq, PartialEq, Ord, PartialOrd)] #[cfg_attr(feature = "serde", derive(serde::Serialize, serde::Deserialize))] diff --git a/src/solver/mod.rs b/src/solver/mod.rs index e794091..e025566 100644 --- a/src/solver/mod.rs +++ b/src/solver/mod.rs @@ -814,12 +814,9 @@ impl Solver { clauses.iter().map(|(r, c)| (*r, *c)).collect::>(), ) }); - + for (solvable_id, condition, requirements) in requires_iter.chain(conditional_iter) { - let is_explicit_requirement = match condition { - None => solvable_id == VariableId::root(), - Some(_) => solvable_id == VariableId::root(), - }; + let is_explicit_requirement = solvable_id == VariableId::root(); if let Some(best_decision) = &best_decision { // If we already have an explicit requirement, there is no need to evaluate @@ -836,15 +833,24 @@ impl Solver { // For conditional clauses, check that at least one conditional variable is true if let Some(condition) = condition { + tracing::trace!("condition o kir: {:?}", condition); let condition_requirement: Requirement = condition.into(); - let conditional_candidates = - &self.requirement_to_sorted_candidates[&condition_requirement]; + let conditional_candidates = match self.requirement_to_sorted_candidates.get(&condition_requirement) { + Some(candidates) => candidates, + None => continue, + }; - if !conditional_candidates.iter().any(|candidates| { + // Check if any candidate that matches the condition's version set is installed + let condition_met = conditional_candidates.iter().any(|candidates| { candidates.iter().any(|&candidate| { + // Only consider the condition met if a candidate that exactly matches + // the condition's version set is installed self.decision_tracker.assigned_value(candidate) == Some(true) }) - }) { + }); + + // If the condition is not met, skip this requirement entirely + if !condition_met { continue; } } @@ -1936,13 +1942,6 @@ async fn add_clauses_for_solvables( } if let Some((condition, condition_candidates)) = condition { - tracing::trace!( - "Adding conditional clauses for {} with condition {}", - requirement.display(cache.provider()), - std::convert::Into::::into(condition) - .display(cache.provider()), - ); - let condition_version_set_variables = version_set_to_variables.insert( condition, condition_candidates diff --git a/tests/solver.rs b/tests/solver.rs index 5691003..173881b 100644 --- a/tests/solver.rs +++ b/tests/solver.rs @@ -217,11 +217,16 @@ impl BundleBoxProvider { .expect("package missing") } - pub fn requirements>(&self, requirements: &[&str]) -> Vec { + pub fn requirements)>>(&self, requirements: &[&str]) -> Vec { requirements .iter() .map(|dep| Spec::from_str(dep).unwrap()) - .map(|spec| self.intern_version_set(&spec)) + .map(|spec| { + ( + self.intern_version_set(&spec), + spec.condition.as_ref().map(|c| self.intern_version_set(c)), + ) + }) .map(From::from) .collect() } @@ -1497,23 +1502,28 @@ fn test_conditional_requirements() { } #[test] +#[traced_test] fn test_conditional_requirements_not_met() { let mut provider = BundleBoxProvider::new(); provider.add_package("b", 1.into(), &[], &[]); // Add b=1 as a candidate provider.add_package("b", 2.into(), &[], &[]); // Different version of b provider.add_package("c", 1.into(), &[], &[]); // Simple package c - provider.add_package("a", 1.into(), &["b 2"], &[]); // a depends on b + provider.add_package("a", 1.into(), &["b 2"], &[]); // a depends on b=2 specifically // Create conditional requirement: if b=1 is installed, require c - let requirements = provider.requirements(&["a", "c 1; if b 1"]); + let requirements = provider.requirements(&[ + "a", // Require package a + "c 1; if b 1" // If b=1 is installed, require c (note the exact version) + ]); let mut solver = Solver::new(provider); let problem = Problem::new().requirements(requirements); let solved = solver.solve(problem).unwrap(); let result = transaction_to_string(solver.provider(), &solved); - // Since b=1 is not installed (b=2 is), c should not be installed + // Since b=2 is installed (not b=1), c should not be installed insta::assert_snapshot!(result, @r###" a=1 + b=2 "###); } @@ -1527,30 +1537,13 @@ fn test_nested_conditional_dependencies() { provider.add_package("c", 1.into(), &[], &[]); // Second level conditional provider.add_package("d", 1.into(), &[], &[]); // Third level conditional - // Create nested conditional requirements: - // If a is installed, require b - // If b is installed, require c - // If c is installed, require d - let a_spec = Spec::parse_union("a 1").next().unwrap().unwrap(); - let b_spec = Spec::parse_union("b 1").next().unwrap().unwrap(); - let c_spec = Spec::parse_union("c 1").next().unwrap().unwrap(); - let d_spec = Spec::parse_union("d 1").next().unwrap().unwrap(); - - let a_version_set = provider.intern_version_set(&a_spec); - let b_version_set = provider.intern_version_set(&b_spec); - let c_version_set = provider.intern_version_set(&c_spec); - let d_version_set = provider.intern_version_set(&d_spec); - - let cond_req1 = ConditionalRequirement::new(a_version_set, b_version_set.into()); - let cond_req2 = ConditionalRequirement::new(b_version_set, c_version_set.into()); - let cond_req3 = ConditionalRequirement::new(c_version_set, d_version_set.into()); - - let requirements = vec![ - cond_req1, - cond_req2, - cond_req3, - a_version_set.into(), // Require package a - ]; + // Create nested conditional requirements using the parser + let requirements = provider.requirements(&[ + "a", // Require package a + "b 1; if a 1", // If a is installed, require b + "c 1; if b 1", // If b is installed, require c + "d 1; if c 1" // If c is installed, require d + ]); let mut solver = Solver::new(provider); let problem = Problem::new().requirements(requirements); @@ -1575,28 +1568,13 @@ fn test_multiple_conditions_same_package() { provider.add_package("c", 1.into(), &[], &[]); provider.add_package("target", 1.into(), &[], &[]); - // Create multiple conditions that all require the same package - let a_spec = Spec::parse_union("a 1").next().unwrap().unwrap(); - let b_spec = Spec::parse_union("b 1").next().unwrap().unwrap(); - let c_spec = Spec::parse_union("c 1").next().unwrap().unwrap(); - let target_spec = Spec::parse_union("target 1").next().unwrap().unwrap(); - - let a_version_set = provider.intern_version_set(&a_spec); - let b_version_set = provider.intern_version_set(&b_spec); - let c_version_set = provider.intern_version_set(&c_spec); - let target_version_set = provider.intern_version_set(&target_spec); - - // If any of a, b, or c is installed, require target - let cond_req1 = ConditionalRequirement::new(a_version_set, target_version_set.into()); - let cond_req2 = ConditionalRequirement::new(b_version_set, target_version_set.into()); - let cond_req3 = ConditionalRequirement::new(c_version_set, target_version_set.into()); - - let requirements = vec![ - cond_req1, - cond_req2, - cond_req3, - b_version_set.into(), // Only require package b - ]; + // Create multiple conditions that all require the same package using the parser + let requirements = provider.requirements(&[ + "b", // Only require package b + "target 1; if a 1", // If a is installed, require target + "target 1; if b 1", // If b is installed, require target + "target 1; if c 1" // If c is installed, require target + ]); let mut solver = Solver::new(provider); let problem = Problem::new().requirements(requirements); @@ -1617,23 +1595,12 @@ fn test_circular_conditional_dependencies() { provider.add_package("a", 1.into(), &[], &[]); provider.add_package("b", 1.into(), &[], &[]); - // Create circular conditional dependencies: - // If a is installed, require b - // If b is installed, require a - let a_spec = Spec::parse_union("a 1").next().unwrap().unwrap(); - let b_spec = Spec::parse_union("b 1").next().unwrap().unwrap(); - - let a_version_set = provider.intern_version_set(&a_spec); - let b_version_set = provider.intern_version_set(&b_spec); - - let cond_req1 = ConditionalRequirement::new(a_version_set, b_version_set.into()); - let cond_req2 = ConditionalRequirement::new(b_version_set, a_version_set.into()); - - let requirements = vec![ - cond_req1, - cond_req2, - a_version_set.into(), // Require package a - ]; + // Create circular conditional dependencies using the parser + let requirements = provider.requirements(&[ + "a", // Require package a + "b 1; if a 1", // If a is installed, require b + "a 1; if b 1" // If b is installed, require a + ]); let mut solver = Solver::new(provider); let problem = Problem::new().requirements(requirements);