Skip to content

Commit

Permalink
chore: rename disabled to excluded
Browse files Browse the repository at this point in the history
  • Loading branch information
baszalmstra committed Nov 3, 2023
1 parent 4a90222 commit 4acc1f5
Show file tree
Hide file tree
Showing 8 changed files with 56 additions and 54 deletions.
6 changes: 3 additions & 3 deletions src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -107,11 +107,11 @@ pub struct Candidates {
/// dependencies can easily be provided one should provide them up-front.
pub hint_dependencies_available: Vec<SolvableId>,

/// 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.
Expand Down
34 changes: 18 additions & 16 deletions src/problem.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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!(),
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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))
}
};

Expand Down Expand Up @@ -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;
}
Expand Down Expand Up @@ -668,10 +670,10 @@ impl<'pool, VS: VersionSet, N: PackageName + Display, M: SolvableDisplay<VS, N>>
.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,
Expand All @@ -687,11 +689,11 @@ impl<'pool, VS: VersionSet, N: PackageName + Display, M: SolvableDisplay<VS, N>>
});
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}")?;
Expand Down Expand Up @@ -791,7 +793,7 @@ impl<VS: VersionSet, N: PackageName + Display, M: SolvableDisplay<VS, N>> fmt::D
.display_candidates(self.pool, &[solvable_id])
)?;
}
ConflictCause::Disabled(_, _) => continue,
ConflictCause::Excluded(_, _) => continue,
};
}
}
Expand Down
20 changes: 10 additions & 10 deletions src/solver/clause.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down Expand Up @@ -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(
Expand Down Expand Up @@ -168,7 +168,7 @@ impl Clause {
) {
match *self {
Clause::InstallRoot => unreachable!(),
Clause::Disabled(solvable, _) => {
Clause::Excluded(solvable, _) => {
visit(Literal {
solvable_id: solvable,
negate: true,
Expand Down Expand Up @@ -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)
}

Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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()
Expand Down Expand Up @@ -507,8 +507,8 @@ impl<VS: VersionSet, N: PackageName + Display> 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) => {
Expand Down
10 changes: 5 additions & 5 deletions src/solver/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -285,15 +285,15 @@ impl<VS: VersionSet, N: PackageName + Display, D: DependencyProvider<VS, N>> 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
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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

Original file line number Diff line number Diff line change
Expand Up @@ -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

Original file line number Diff line number Diff line change
Expand Up @@ -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

30 changes: 15 additions & 15 deletions tests/solver.rs
Original file line number Diff line number Diff line change
Expand Up @@ -111,7 +111,7 @@ struct BundleBoxProvider {
packages: IndexMap<String, IndexMap<Pack, BundleBoxPackageDependencies>>,
favored: HashMap<String, Pack>,
locked: HashMap<String, Pack>,
disabled: HashMap<String, HashMap<Pack, String>>,
excluded: HashMap<String, HashMap<Pack, String>>,
}

struct BundleBoxPackageDependencies {
Expand Down Expand Up @@ -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<String>) {
self.disabled
pub fn exclude(&mut self, package_name: &str, version: u32, reason: impl Into<String>) {
self.excluded
.entry(package_name.to_owned())
.or_default()
.insert(Pack(version), reason.into());
Expand Down Expand Up @@ -219,7 +219,7 @@ impl DependencyProvider<Range<Pack>> 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);
Expand All @@ -229,10 +229,10 @@ impl DependencyProvider<Range<Pack>> 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)));
}
}

Expand Down Expand Up @@ -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"]));
}

0 comments on commit 4acc1f5

Please sign in to comment.