Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: disable candidates #5

Merged
merged 6 commits into from
Nov 3, 2023
Merged
Show file tree
Hide file tree
Changes from 4 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
15 changes: 15 additions & 0 deletions src/internal/id.rs
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,21 @@ impl ArenaId for NameId {
}
}

/// The id associated with a generic string
#[repr(transparent)]
#[derive(Copy, Clone, Eq, PartialEq, Hash, Debug, Ord, PartialOrd)]
pub struct StringId(u32);

impl ArenaId for StringId {
fn from_usize(x: usize) -> Self {
Self(x as u32)
}

fn to_usize(self) -> usize {
self.0 as usize
}
}

/// The id associated with a VersionSet.
#[repr(transparent)]
#[derive(Clone, Default, Copy, Debug, Hash, Eq, PartialEq, Ord, PartialOrd)]
Expand Down
10 changes: 9 additions & 1 deletion src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@
//! comments.

#![deny(missing_docs)]

pub(crate) mod internal;
mod pool;
pub mod problem;
Expand All @@ -19,7 +20,7 @@ mod solver;
use itertools::Itertools;

pub use internal::{
id::{NameId, SolvableId, VersionSetId},
id::{NameId, SolvableId, StringId, VersionSetId},
mapping::Mapping,
};
pub use pool::Pool;
Expand All @@ -39,6 +40,7 @@ use std::{
///
/// A blanket trait implementation is provided for any type that implements [`Eq`] and [`Hash`].
pub trait PackageName: Eq + Hash {}

impl<N: Eq + Hash> PackageName for N {}

/// A [`VersionSet`] is describes a set of "versions". The trait defines whether a given version
Expand Down Expand Up @@ -104,6 +106,12 @@ pub struct Candidates {
/// solver doesnt actually need this information to form a solution. In general though, if the
/// 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 the
baszalmstra marked this conversation as resolved.
Show resolved Hide resolved
/// 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)>,
}

/// Holds information about the dependencies of a package.
Expand Down
30 changes: 29 additions & 1 deletion src/pool.rs
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
use std::fmt::{Display, Formatter};

use crate::internal::id::StringId;
use crate::{
internal::{
arena::Arena,
Expand All @@ -26,6 +27,12 @@ pub struct Pool<VS: VersionSet, N: PackageName = String> {
/// Map from package names to the id of their interned counterpart
pub(crate) names_to_ids: FrozenCopyMap<N, NameId>,

/// Interned strings
strings: Arena<StringId, String>,

/// Map from package names to the id of their interned counterpart
pub(crate) string_to_ids: FrozenCopyMap<String, StringId>,

/// Interned match specs
pub(crate) version_sets: Arena<VersionSetId, (NameId, VS)>,

Expand All @@ -40,9 +47,10 @@ impl<VS: VersionSet, N: PackageName> Default for Pool<VS, N> {

Self {
solvables,

names_to_ids: Default::default(),
package_names: Arena::new(),
strings: Arena::new(),
string_to_ids: Default::default(),
version_set_to_id: Default::default(),
version_sets: Arena::new(),
}
Expand All @@ -55,6 +63,26 @@ impl<VS: VersionSet, N: PackageName> Pool<VS, N> {
Self::default()
}

/// Interns a generic string into the `Pool` and returns its `StringId`. Strings are
/// deduplicated.
pub fn intern_string(&self, name: impl Into<String> + AsRef<str>) -> StringId {
if let Some(id) = self.string_to_ids.get_copy(name.as_ref()) {
return id;
}

let string = name.into();
let id = self.strings.alloc(string.clone());
self.string_to_ids.insert_copy(string, id);
id
}

/// Returns the string associated with the provided [`StringId`].
///
/// Panics if the string is not found in the pool.
pub fn resolve_string(&self, string_id: StringId) -> &str {
&self.strings[string_id]
}

/// Interns a package name into the `Pool`, returning its `NameId`. Names are deduplicated. If
/// the same name is inserted twice the same `NameId` will be returned.
///
Expand Down
62 changes: 50 additions & 12 deletions src/problem.rs
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ use petgraph::graph::{DiGraph, EdgeIndex, EdgeReference, NodeIndex};
use petgraph::visit::{Bfs, DfsPostOrder, EdgeRef};
use petgraph::Direction;

use crate::internal::id::StringId;
use crate::{
internal::id::{ClauseId, SolvableId, VersionSetId},
pool::Pool,
Expand Down Expand Up @@ -54,6 +55,12 @@ impl Problem {
let clause = &solver.clauses[*clause_id].kind;
match clause {
Clause::InstallRoot => (),
Clause::Disabled(solvable, reason) => {
tracing::info!("{solvable:?} is disabled");
let package_node = Self::add_node(&mut graph, &mut nodes, *solvable);
let conflict = ConflictCause::Disabled(*solvable, *reason);
graph.add_edge(root_node, package_node, ProblemEdge::Conflict(conflict));
}
Clause::Learnt(..) => unreachable!(),
&Clause::Requires(package_id, version_set_id) => {
let package_node = Self::add_node(&mut graph, &mut nodes, package_id);
Expand Down Expand Up @@ -214,6 +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),
}

/// Represents a node that has been merged with others
Expand Down Expand Up @@ -291,6 +300,9 @@ impl ProblemGraph {
| ProblemEdge::Conflict(ConflictCause::Locked(_)) => {
"already installed".to_string()
}
ProblemEdge::Conflict(ConflictCause::Disabled(_, reason)) => {
format!("disabled because {}", pool.resolve_string(*reason))
}
};

let target = match target {
Expand Down Expand Up @@ -399,6 +411,17 @@ impl ProblemGraph {
continue;
}

let disabling_edges = self.graph.edges_directed(nx, Direction::Incoming).any(|e| {
matches!(
e.weight(),
ProblemEdge::Conflict(ConflictCause::Disabled(_, _))
)
});
if disabling_edges {
// Nodes with incoming disabling edges aren't installable
continue;
}

let outgoing_conflicts = self
.graph
.edges_directed(nx, Direction::Outgoing)
Expand Down Expand Up @@ -645,6 +668,14 @@ impl<'pool, VS: VersionSet, N: PackageName + Display, M: SolvableDisplay<VS, N>>
.display_candidates(self.pool, &[solvable_id])
};

let disabled = graph
.edges_directed(candidate, Direction::Incoming)
.find_map(|e| match e.weight() {
ProblemEdge::Conflict(ConflictCause::Disabled(_, reason)) => {
Some(reason)
}
_ => None,
});
let already_installed = graph.edges(candidate).any(|e| {
e.weight() == &ProblemEdge::Conflict(ConflictCause::ForbidMultipleInstances)
});
Expand All @@ -656,7 +687,13 @@ impl<'pool, VS: VersionSet, N: PackageName + Display, M: SolvableDisplay<VS, N>>
});
let is_leaf = graph.edges(candidate).next().is_none();

if is_leaf {
if let Some(disabled_reason) = disabled {
writeln!(
f,
"{indent}{name} {version} is disabled because {reason}",
reason = self.pool.resolve_string(*disabled_reason)
)?;
} else if is_leaf {
writeln!(f, "{indent}{name} {version}")?;
} else if already_installed {
writeln!(f, "{indent}{name} {version}, which conflicts with the versions reported above.")?;
Expand Down Expand Up @@ -740,21 +777,22 @@ impl<VS: VersionSet, N: PackageName + Display, M: SolvableDisplay<VS, N>> fmt::D
};

// The only possible conflict at the root level is a Locked conflict
let locked_id = match conflict {
match conflict {
ConflictCause::Constrains(_) | ConflictCause::ForbidMultipleInstances => {
unreachable!()
}
&ConflictCause::Locked(solvable_id) => solvable_id,
&ConflictCause::Locked(solvable_id) => {
let locked = self.pool.resolve_solvable(solvable_id);
writeln!(
f,
"{indent}{} {} is locked, but another version is required as reported above",
locked.name.display(self.pool),
self.merged_solvable_display
.display_candidates(self.pool, &[solvable_id])
)?;
}
ConflictCause::Disabled(_, _) => continue,
};

let locked = self.pool.resolve_solvable(locked_id);
writeln!(
f,
"{indent}{} {} is locked, but another version is required as reported above",
locked.name.display(self.pool),
self.merged_solvable_display
.display_candidates(self.pool, &[locked_id])
)?;
}
}

Expand Down
24 changes: 24 additions & 0 deletions src/solver/clause.rs
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ use crate::{
PackageName, VersionSet,
};

use crate::internal::id::StringId;
use elsa::FrozenMap;
use std::fmt::{Debug, Display, Formatter};
use std::hash::Hash;
Expand Down Expand Up @@ -76,6 +77,9 @@ pub(crate) enum Clause {
/// The learnt clause id can be used to retrieve the clause's literals, which are stored
/// elsewhere to prevent the size of [`Clause`] from blowing up
Learnt(LearntClauseId),

/// A clause that forbids a package from being installed for an external reason.
Disabled(SolvableId, StringId),
}

impl Clause {
Expand Down Expand Up @@ -122,6 +126,10 @@ impl Clause {
(Clause::InstallRoot, None)
}

fn disable(candidate: SolvableId, reason: StringId) -> (Self, Option<[SolvableId; 2]>) {
(Clause::Disabled(candidate, reason), None)
}

fn lock(
locked_candidate: SolvableId,
other_candidate: SolvableId,
Expand Down Expand Up @@ -160,6 +168,12 @@ impl Clause {
) {
match *self {
Clause::InstallRoot => unreachable!(),
Clause::Disabled(solvable, _) => {
visit(Literal {
solvable_id: solvable,
negate: true,
});
}
Clause::Learnt(learnt_id) => {
for &literal in &learnt_clauses[learnt_id] {
visit(literal);
Expand Down Expand Up @@ -265,6 +279,11 @@ 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);
Self::from_kind_and_initial_watches(kind, watched_literals)
}

fn from_kind_and_initial_watches(
kind: Clause,
watched_literals: Option<[SolvableId; 2]>,
Expand Down Expand Up @@ -368,6 +387,7 @@ impl ClauseState {

match self.kind {
Clause::InstallRoot => unreachable!(),
Clause::Disabled(_, _) => 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 @@ -413,6 +433,7 @@ impl ClauseState {

match self.kind {
Clause::InstallRoot => unreachable!(),
Clause::Disabled(_, _) => unreachable!(),
Clause::Learnt(learnt_id) => learnt_clauses[learnt_id]
.iter()
.cloned()
Expand Down Expand Up @@ -486,6 +507,9 @@ 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::Learnt(learnt_id) => write!(f, "learnt clause {learnt_id:?}"),
Clause::Requires(solvable_id, match_spec_id) => {
let match_spec = self.pool.resolve_version_set(match_spec_id).to_string();
Expand Down
Loading