From 4acc1f570e795220716681d9020dce93ade14588 Mon Sep 17 00:00:00 2001 From: Bas Zalmstra Date: Fri, 3 Nov 2023 10:14:05 +0100 Subject: [PATCH] chore: rename disabled to excluded --- src/lib.rs | 6 ++-- src/problem.rs | 34 ++++++++++--------- src/solver/clause.rs | 20 +++++------ src/solver/mod.rs | 10 +++--- ...r__disabled.snap => solver__excluded.snap} | 4 +-- ...abled.snap => solver__merge_excluded.snap} | 4 +-- ...sabled.snap => solver__root_excluded.snap} | 2 +- tests/solver.rs | 30 ++++++++-------- 8 files changed, 56 insertions(+), 54 deletions(-) rename tests/snapshots/{solver__disabled.snap => solver__excluded.snap} (75%) rename tests/snapshots/{solver__merge_disabled.snap => solver__merge_excluded.snap} (61%) rename tests/snapshots/{solver__root_disabled.snap => solver__root_excluded.snap} (76%) diff --git a/src/lib.rs b/src/lib.rs index 332458d..a2f8211 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -107,11 +107,11 @@ pub struct Candidates { /// dependencies can easily be provided one should provide them up-front. pub hint_dependencies_available: Vec, - /// A list of solvables that are available but have been disabled for reasons outside of the - /// solver. For example, a package might be disabled because it is not compatible with the + /// A list of solvables that are available but have been excluded from the solver. For example, + /// a package might be excluded from the solver because it is not compatible with the /// runtime. The solver will not consider these solvables when forming a solution but will use /// them in the error message if no solution could be found. - pub disabled: Vec<(SolvableId, StringId)>, + pub excluded: Vec<(SolvableId, StringId)>, } /// Holds information about the dependencies of a package. diff --git a/src/problem.rs b/src/problem.rs index feb42a0..e15a455 100644 --- a/src/problem.rs +++ b/src/problem.rs @@ -55,10 +55,10 @@ impl Problem { let clause = &solver.clauses[*clause_id].kind; match clause { Clause::InstallRoot => (), - Clause::Disabled(solvable, reason) => { - tracing::info!("{solvable:?} is disabled"); + Clause::Excluded(solvable, reason) => { + tracing::info!("{solvable:?} is excluded"); let package_node = Self::add_node(&mut graph, &mut nodes, *solvable); - let conflict = ConflictCause::Disabled(*solvable, *reason); + let conflict = ConflictCause::Excluded(*solvable, *reason); graph.add_edge(root_node, package_node, ProblemEdge::Conflict(conflict)); } Clause::Learnt(..) => unreachable!(), @@ -221,8 +221,8 @@ pub enum ConflictCause { Constrains(VersionSetId), /// It is forbidden to install multiple instances of the same dependency ForbidMultipleInstances, - /// The node was disabled - Disabled(SolvableId, StringId), + /// The node was excluded + Excluded(SolvableId, StringId), } /// Represents a node that has been merged with others @@ -300,8 +300,8 @@ impl ProblemGraph { | ProblemEdge::Conflict(ConflictCause::Locked(_)) => { "already installed".to_string() } - ProblemEdge::Conflict(ConflictCause::Disabled(_, reason)) => { - format!("disabled because {}", pool.resolve_string(*reason)) + ProblemEdge::Conflict(ConflictCause::Excluded(_, reason)) => { + format!("excluded because {}", pool.resolve_string(*reason)) } }; @@ -411,13 +411,15 @@ impl ProblemGraph { continue; } - let disabling_edges = self.graph.edges_directed(nx, Direction::Incoming).any(|e| { + // Determine any incoming "exclude" edges to the node. This would indicate that the + // node is disabled for external reasons. + let excluding_edges = self.graph.edges_directed(nx, Direction::Incoming).any(|e| { matches!( e.weight(), - ProblemEdge::Conflict(ConflictCause::Disabled(_, _)) + ProblemEdge::Conflict(ConflictCause::Excluded(_, _)) ) }); - if disabling_edges { + if excluding_edges { // Nodes with incoming disabling edges aren't installable continue; } @@ -668,10 +670,10 @@ impl<'pool, VS: VersionSet, N: PackageName + Display, M: SolvableDisplay> .display_candidates(self.pool, &[solvable_id]) }; - let disabled = graph + let excluded = graph .edges_directed(candidate, Direction::Incoming) .find_map(|e| match e.weight() { - ProblemEdge::Conflict(ConflictCause::Disabled(_, reason)) => { + ProblemEdge::Conflict(ConflictCause::Excluded(_, reason)) => { Some(reason) } _ => None, @@ -687,11 +689,11 @@ impl<'pool, VS: VersionSet, N: PackageName + Display, M: SolvableDisplay> }); let is_leaf = graph.edges(candidate).next().is_none(); - if let Some(disabled_reason) = disabled { + if let Some(excluded_reason) = excluded { writeln!( f, - "{indent}{name} {version} is disabled because {reason}", - reason = self.pool.resolve_string(*disabled_reason) + "{indent}{name} {version} is excluded because {reason}", + reason = self.pool.resolve_string(*excluded_reason) )?; } else if is_leaf { writeln!(f, "{indent}{name} {version}")?; @@ -791,7 +793,7 @@ impl> fmt::D .display_candidates(self.pool, &[solvable_id]) )?; } - ConflictCause::Disabled(_, _) => continue, + ConflictCause::Excluded(_, _) => continue, }; } } diff --git a/src/solver/clause.rs b/src/solver/clause.rs index 48fdc06..7226068 100644 --- a/src/solver/clause.rs +++ b/src/solver/clause.rs @@ -79,7 +79,7 @@ pub(crate) enum Clause { Learnt(LearntClauseId), /// A clause that forbids a package from being installed for an external reason. - Disabled(SolvableId, StringId), + Excluded(SolvableId, StringId), } impl Clause { @@ -126,8 +126,8 @@ impl Clause { (Clause::InstallRoot, None) } - fn disable(candidate: SolvableId, reason: StringId) -> (Self, Option<[SolvableId; 2]>) { - (Clause::Disabled(candidate, reason), None) + fn exclude(candidate: SolvableId, reason: StringId) -> (Self, Option<[SolvableId; 2]>) { + (Clause::Excluded(candidate, reason), None) } fn lock( @@ -168,7 +168,7 @@ impl Clause { ) { match *self { Clause::InstallRoot => unreachable!(), - Clause::Disabled(solvable, _) => { + Clause::Excluded(solvable, _) => { visit(Literal { solvable_id: solvable, negate: true, @@ -279,8 +279,8 @@ impl ClauseState { Self::from_kind_and_initial_watches(kind, watched_literals) } - pub fn disable(candidate: SolvableId, reason: StringId) -> Self { - let (kind, watched_literals) = Clause::disable(candidate, reason); + pub fn exclude(candidate: SolvableId, reason: StringId) -> Self { + let (kind, watched_literals) = Clause::exclude(candidate, reason); Self::from_kind_and_initial_watches(kind, watched_literals) } @@ -387,7 +387,7 @@ impl ClauseState { match self.kind { Clause::InstallRoot => unreachable!(), - Clause::Disabled(_, _) => unreachable!(), + Clause::Excluded(_, _) => unreachable!(), Clause::Learnt(learnt_id) => { // TODO: we might want to do something else for performance, like keeping the whole // literal in `self.watched_literals`, to avoid lookups... But first we should @@ -433,7 +433,7 @@ impl ClauseState { match self.kind { Clause::InstallRoot => unreachable!(), - Clause::Disabled(_, _) => unreachable!(), + Clause::Excluded(_, _) => unreachable!(), Clause::Learnt(learnt_id) => learnt_clauses[learnt_id] .iter() .cloned() @@ -507,8 +507,8 @@ impl Debug for ClauseDebug<'_, VS, N> fn fmt(&self, f: &mut Formatter<'_>) -> std::fmt::Result { match self.kind { Clause::InstallRoot => write!(f, "install root"), - Clause::Disabled(_, reason) => { - write!(f, "disabled because {}", self.pool.resolve_string(reason)) + Clause::Excluded(_, reason) => { + write!(f, "excluded because {}", self.pool.resolve_string(reason)) } Clause::Learnt(learnt_id) => write!(f, "learnt clause {learnt_id:?}"), Clause::Requires(solvable_id, match_spec_id) => { diff --git a/src/solver/mod.rs b/src/solver/mod.rs index bfc048e..8cfc83f 100644 --- a/src/solver/mod.rs +++ b/src/solver/mod.rs @@ -285,15 +285,15 @@ impl> Sol } } - // Disable any solvable that was externally disabled - for (solvable, reason) in package_candidates.disabled.iter().copied() { - let clause_id = self.clauses.alloc(ClauseState::disable(solvable, reason)); + // Add a clause for solvables that are externally excluded. + for (solvable, reason) in package_candidates.excluded.iter().copied() { + let clause_id = self.clauses.alloc(ClauseState::exclude(solvable, reason)); // Immediately add the decision to unselect this solvable. This should be fine because // no other decision should have been made about this solvable in the first place. - let disable_decision = Decision::new(solvable, false, clause_id); + let exclude_descision = Decision::new(solvable, false, clause_id); self.decision_tracker - .try_add_fixed_assignment(disable_decision) + .try_add_fixed_assignment(exclude_descision) .expect("already decided about a solvable that wasn't properly added yet."); // No need to add watches for this clause because the clause is an assertion on which diff --git a/tests/snapshots/solver__disabled.snap b/tests/snapshots/solver__excluded.snap similarity index 75% rename from tests/snapshots/solver__disabled.snap rename to tests/snapshots/solver__excluded.snap index ebdb7f0..9bae558 100644 --- a/tests/snapshots/solver__disabled.snap +++ b/tests/snapshots/solver__excluded.snap @@ -6,8 +6,8 @@ The following packages are incompatible |-- a * cannot be installed because there are no viable options: |-- a 2 would require |-- b *, which cannot be installed because there are no viable options: - |-- b 1 is disabled because it is externally disabled + |-- b 1 is excluded because it is externally excluded |-- a 1 would require |-- c *, which cannot be installed because there are no viable options: - |-- c 1 is disabled because it is externally disabled + |-- c 1 is excluded because it is externally excluded diff --git a/tests/snapshots/solver__merge_disabled.snap b/tests/snapshots/solver__merge_excluded.snap similarity index 61% rename from tests/snapshots/solver__merge_disabled.snap rename to tests/snapshots/solver__merge_excluded.snap index a10f060..3370895 100644 --- a/tests/snapshots/solver__merge_disabled.snap +++ b/tests/snapshots/solver__merge_excluded.snap @@ -4,6 +4,6 @@ expression: "solve_snapshot(provider, &[\"a\"])" --- The following packages are incompatible |-- a * cannot be installed because there are no viable options: - |-- a 2 is disabled because it is externally disabled - |-- a 1 is disabled because it is externally disabled + |-- a 2 is excluded because it is externally excluded + |-- a 1 is excluded because it is externally excluded diff --git a/tests/snapshots/solver__root_disabled.snap b/tests/snapshots/solver__root_excluded.snap similarity index 76% rename from tests/snapshots/solver__root_disabled.snap rename to tests/snapshots/solver__root_excluded.snap index c797a9d..d9a1002 100644 --- a/tests/snapshots/solver__root_disabled.snap +++ b/tests/snapshots/solver__root_excluded.snap @@ -4,5 +4,5 @@ expression: "solve_snapshot(provider, &[\"a\"])" --- The following packages are incompatible |-- a * cannot be installed because there are no viable options: - |-- a 1 is disabled because it is externally disabled + |-- a 1 is excluded because it is externally excluded diff --git a/tests/solver.rs b/tests/solver.rs index 1fbaed4..01a0384 100644 --- a/tests/solver.rs +++ b/tests/solver.rs @@ -111,7 +111,7 @@ struct BundleBoxProvider { packages: IndexMap>, favored: HashMap, locked: HashMap, - disabled: HashMap>, + excluded: HashMap>, } struct BundleBoxPackageDependencies { @@ -148,8 +148,8 @@ impl BundleBoxProvider { self.favored.insert(package_name.to_owned(), Pack(version)); } - pub fn set_disabled(&mut self, package_name: &str, version: u32, reason: impl Into) { - self.disabled + pub fn exclude(&mut self, package_name: &str, version: u32, reason: impl Into) { + self.excluded .entry(package_name.to_owned()) .or_default() .insert(Pack(version), reason.into()); @@ -219,7 +219,7 @@ impl DependencyProvider> for BundleBoxProvider { }; let favor = self.favored.get(package_name); let locked = self.locked.get(package_name); - let disabled = self.disabled.get(package_name); + let excluded = self.excluded.get(package_name); for pack in package.keys() { let solvable = self.pool.intern_solvable(name, *pack); candidates.candidates.push(solvable); @@ -229,10 +229,10 @@ impl DependencyProvider> for BundleBoxProvider { if Some(pack) == locked { candidates.locked = Some(solvable); } - if let Some(disabled) = disabled.and_then(|d| d.get(pack)) { + if let Some(excluded) = excluded.and_then(|d| d.get(pack)) { candidates - .disabled - .push((solvable, self.pool.intern_string(disabled))); + .excluded + .push((solvable, self.pool.intern_string(excluded))); } } @@ -785,29 +785,29 @@ fn test_incremental_crash() { } #[test] -fn test_disabled() { +fn test_excluded() { let mut provider = BundleBoxProvider::from_packages(&[ ("a", 2, vec!["b"]), ("a", 1, vec!["c"]), ("b", 1, vec![]), ("c", 1, vec![]), ]); - provider.set_disabled("b", 1, "it is externally disabled"); - provider.set_disabled("c", 1, "it is externally disabled"); + provider.exclude("b", 1, "it is externally excluded"); + provider.exclude("c", 1, "it is externally excluded"); insta::assert_snapshot!(solve_snapshot(provider, &["a"])); } #[test] -fn test_merge_disabled() { +fn test_merge_excluded() { let mut provider = BundleBoxProvider::from_packages(&[("a", 1, vec![]), ("a", 2, vec![])]); - provider.set_disabled("a", 1, "it is externally disabled"); - provider.set_disabled("a", 2, "it is externally disabled"); + provider.exclude("a", 1, "it is externally excluded"); + provider.exclude("a", 2, "it is externally excluded"); insta::assert_snapshot!(solve_snapshot(provider, &["a"])); } #[test] -fn test_root_disabled() { +fn test_root_excluded() { let mut provider = BundleBoxProvider::from_packages(&[("a", 1, vec![])]); - provider.set_disabled("a", 1, "it is externally disabled"); + provider.exclude("a", 1, "it is externally excluded"); insta::assert_snapshot!(solve_snapshot(provider, &["a"])); }