Skip to content

Commit

Permalink
feat: display merged candidates (#326)
Browse files Browse the repository at this point in the history
feat: adds functionality that gives the dependency provider the option
to display the merged candidates for errors in a way it sees fit

A default display provider is given. The conda code overwrites this
provider

Also removes `VersionTrait` as we no longer need this. Removed this both
from the test and conda code
  • Loading branch information
tdejager authored Sep 12, 2023
1 parent 01564e3 commit 55c6f11
Show file tree
Hide file tree
Showing 7 changed files with 155 additions and 109 deletions.
56 changes: 28 additions & 28 deletions crates/rattler_conda_types/src/generic_virtual_package.rs
Original file line number Diff line number Diff line change
@@ -1,28 +1,28 @@
use crate::{PackageName, Version};
use std::fmt::{Display, Formatter};

/// A `GenericVirtualPackage` is a Conda package description that contains a `name` and a
/// `version` and a `build_string`.
#[derive(Clone, Debug, Eq, PartialEq, Hash)]
pub struct GenericVirtualPackage {
/// The name of the package
pub name: PackageName,

/// The version of the package
pub version: Version,

/// The build identifier of the package.
pub build_string: String,
}

impl Display for GenericVirtualPackage {
fn fmt(&self, f: &mut Formatter<'_>) -> std::fmt::Result {
write!(
f,
"{}={}={}",
&self.name.as_normalized(),
&self.version,
&self.build_string
)
}
}
use crate::{PackageName, Version};
use std::fmt::{Display, Formatter};

/// A `GenericVirtualPackage` is a Conda package description that contains a `name` and a
/// `version` and a `build_string`.
#[derive(Clone, Debug, Eq, PartialEq, Hash, Ord, PartialOrd)]
pub struct GenericVirtualPackage {
/// The name of the package
pub name: PackageName,

/// The version of the package
pub version: Version,

/// The build identifier of the package.
pub build_string: String,
}

impl Display for GenericVirtualPackage {
fn fmt(&self, f: &mut Formatter<'_>) -> std::fmt::Result {
write!(
f,
"{}={}={}",
&self.name.as_normalized(),
&self.version,
&self.build_string
)
}
}
45 changes: 32 additions & 13 deletions crates/rattler_libsolv_rs/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@ mod solver;
mod transaction;

pub use id::{NameId, SolvableId, VersionSetId};
use itertools::Itertools;
pub use pool::Pool;
pub use solvable::PackageSolvable;
pub use solve_jobs::SolveJobs;
Expand All @@ -36,26 +37,16 @@ pub use mapping::Mapping;
pub trait PackageName: Eq + Hash {}
impl<N: Eq + Hash> PackageName for N {}

/// Version is a name and a version specification.
pub trait VersionTrait: Display {
/// The version associated with this record.
type Version: Display + Ord + Clone;

/// Returns the version associated with this record
// TODO: We could maybe get rid of this, but would need to know what is generic to display and replace sorting in `problem.rs`
fn version(&self) -> Self::Version;
}

/// Trait describing sets of versions.
pub trait VersionSet: Debug + Display + Clone + Eq + Hash {
/// Version type associated with the sets manipulated.
type V: VersionTrait;
type V: Display + Ord;

/// Evaluate membership of a version in this set.
fn contains(&self, v: &Self::V) -> bool;
}

/// Bla
/// Describes how to sort tentative candidates, for a specific dependency provider. E.g conda
/// pypi, etc.
pub trait DependencyProvider<VS: VersionSet, N: PackageName = String> {
/// Sort the specified solvables based on which solvable to try first.
fn sort_candidates(
Expand All @@ -65,3 +56,31 @@ pub trait DependencyProvider<VS: VersionSet, N: PackageName = String> {
match_spec_to_candidates: &Mapping<VersionSetId, OnceCell<Vec<SolvableId>>>,
);
}
/// Defines how merged candidates should be displayed.
pub trait SolvableDisplay<VS: VersionSet, Name: PackageName = String> {
/// A method that is used to display multiple solvables in a user friendly way.
/// For example the conda provider should only display the versions (not build strings etc.)
/// and merges multiple solvables into one line.
fn display_candidates(&self, pool: &Pool<VS, Name>, candidates: &[SolvableId]) -> String;
}

/// Display merged candidates on single line with `|` as separator.
pub struct DefaultSolvableDisplay;

impl<VS: VersionSet, Name: Hash + Eq> SolvableDisplay<VS, Name> for DefaultSolvableDisplay
where
VS::V: Ord,
{
fn display_candidates(
&self,
pool: &Pool<VS, Name>,
merged_candidates: &[SolvableId],
) -> String {
merged_candidates
.iter()
.map(|&id| &pool.resolve_solvable(id).inner)
.sorted()
.map(|s| s.to_string())
.join(" | ")
}
}
44 changes: 28 additions & 16 deletions crates/rattler_libsolv_rs/src/problem.rs
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ use std::collections::{HashMap, HashSet};
use std::fmt;
use std::fmt::{Display, Formatter};
use std::hash::Hash;

use std::rc::Rc;

use itertools::Itertools;
Expand All @@ -16,7 +17,7 @@ use crate::id::{ClauseId, SolvableId, VersionSetId};
use crate::pool::Pool;
use crate::solver::clause::Clause;
use crate::solver::Solver;
use crate::{DependencyProvider, PackageName, VersionSet, VersionTrait};
use crate::{DependencyProvider, PackageName, SolvableDisplay, VersionSet};

/// Represents the cause of the solver being unable to find a solution
#[derive(Debug)]
Expand Down Expand Up @@ -148,12 +149,14 @@ impl Problem {
VS: VersionSet,
N: PackageName + Display,
D: DependencyProvider<VS, N>,
M: SolvableDisplay<VS, N>,
>(
&self,
solver: &'a Solver<VS, N, D>,
) -> DisplayUnsat<'a, VS, N> {
merged_solvable_display: &'a M,
) -> DisplayUnsat<'a, VS, N, M> {
let graph = self.graph(solver);
DisplayUnsat::new(graph, solver.pool())
DisplayUnsat::new(graph, solver.pool(), merged_solvable_display)
}
}

Expand Down Expand Up @@ -368,9 +371,8 @@ impl ProblemGraph {
let mut merged_candidates = HashMap::default();
// TODO: could probably use `sort_candidates` by the dependency provider directly
// but we need to mantain the mapping in `m` which goes from `NodeIndex` to `SolvableId`
for mut m in maybe_merge.into_values() {
for m in maybe_merge.into_values() {
if m.len() > 1 {
m.sort_unstable_by_key(|&(_, id)| pool.resolve_solvable(id).inner.version());
let m = Rc::new(MergedProblemNode {
ids: m.into_iter().map(|(_, snd)| snd).collect(),
});
Expand Down Expand Up @@ -477,16 +479,24 @@ impl ProblemGraph {

/// A struct implementing [`fmt::Display`] that generates a user-friendly representation of a
/// problem graph
pub struct DisplayUnsat<'pool, VS: VersionSet, N: PackageName + Display> {
pub struct DisplayUnsat<'pool, VS: VersionSet, N: PackageName + Display, M: SolvableDisplay<VS, N>>
{
graph: ProblemGraph,
merged_candidates: HashMap<SolvableId, Rc<MergedProblemNode>>,
installable_set: HashSet<NodeIndex>,
missing_set: HashSet<NodeIndex>,
pool: &'pool Pool<VS, N>,
merged_solvable_display: &'pool M,
}

impl<'pool, VS: VersionSet, N: PackageName + Display> DisplayUnsat<'pool, VS, N> {
pub(crate) fn new(graph: ProblemGraph, pool: &'pool Pool<VS, N>) -> Self {
impl<'pool, VS: VersionSet, N: PackageName + Display, M: SolvableDisplay<VS, N>>
DisplayUnsat<'pool, VS, N, M>
{
pub(crate) fn new(
graph: ProblemGraph,
pool: &'pool Pool<VS, N>,
merged_solvable_display: &'pool M,
) -> Self {
let merged_candidates = graph.simplify(pool);
let installable_set = graph.get_installable_set();
let missing_set = graph.get_missing_set();
Expand All @@ -497,6 +507,7 @@ impl<'pool, VS: VersionSet, N: PackageName + Display> DisplayUnsat<'pool, VS, N>
installable_set,
missing_set,
pool,
merged_solvable_display,
}
}

Expand Down Expand Up @@ -627,13 +638,11 @@ impl<'pool, VS: VersionSet, N: PackageName + Display> DisplayUnsat<'pool, VS, N>
let name = self.pool.resolve_package_name(solvable.name);
let version = if let Some(merged) = self.merged_candidates.get(&solvable_id) {
reported.extend(merged.ids.iter().cloned());
merged
.ids
.iter()
.map(|&id| self.pool.resolve_solvable(id).inner.version().to_string())
.join(" | ")
self.merged_solvable_display
.display_candidates(self.pool, &merged.ids)
} else {
solvable.inner.version().to_string()
self.merged_solvable_display
.display_candidates(self.pool, &[solvable_id])
};

let already_installed = graph.edges(candidate).any(|e| {
Expand Down Expand Up @@ -704,7 +713,9 @@ impl<'pool, VS: VersionSet, N: PackageName + Display> DisplayUnsat<'pool, VS, N>
}
}

impl<VS: VersionSet, N: PackageName + Display> fmt::Display for DisplayUnsat<'_, VS, N> {
impl<VS: VersionSet, N: PackageName + Display, M: SolvableDisplay<VS, N>> fmt::Display
for DisplayUnsat<'_, VS, N, M>
{
fn fmt(&self, f: &mut Formatter<'_>) -> fmt::Result {
let (top_level_missing, top_level_conflicts): (Vec<_>, _) = self
.graph
Expand Down Expand Up @@ -741,7 +752,8 @@ impl<VS: VersionSet, N: PackageName + Display> fmt::Display for DisplayUnsat<'_,
f,
"{indent}{} {} is locked, but another version is required as reported above",
locked.name.display(self.pool),
locked.inner.version()
self.merged_solvable_display
.display_candidates(self.pool, &[locked_id])
)?;
}
}
Expand Down
Loading

0 comments on commit 55c6f11

Please sign in to comment.