Skip to content

Commit

Permalink
initial commit for conditional dependencies support
Browse files Browse the repository at this point in the history
  • Loading branch information
prsabahrami committed Jan 17, 2025
1 parent 9a3a233 commit 0586e74
Show file tree
Hide file tree
Showing 8 changed files with 244 additions and 19 deletions.
9 changes: 9 additions & 0 deletions cpp/include/resolvo.h
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,15 @@ inline Requirement requirement_union(VersionSetUnionId id) {
return cbindgen_private::resolvo_requirement_union(id);
}

/**
* Specifies a conditional requirement, where the requirement is only active when the condition is met.
* @param condition The version set that must be satisfied for the requirement to be active.
* @param requirement The version set that must be satisfied when the condition is met.
*/
inline Requirement requirement_conditional(VersionSetId condition, VersionSetId requirement) {
return cbindgen_private::resolvo_requirement_conditional(condition, requirement);
}

/**
* Called to solve a package problem.
*
Expand Down
20 changes: 20 additions & 0 deletions cpp/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -48,13 +48,21 @@ pub enum Requirement {
/// cbindgen:derive-eq
/// cbindgen:derive-neq
Union(VersionSetUnionId),
/// Specifies a conditional requirement, where the requirement is only active when the condition is met.
/// First VersionSetId is the condition, second is the requirement.
/// cbindgen:derive-eq
/// cbindgen:derive-neq
ConditionalRequires(VersionSetId, VersionSetId),
}

impl From<resolvo::Requirement> for crate::Requirement {
fn from(value: resolvo::Requirement) -> Self {
match value {
resolvo::Requirement::Single(id) => Requirement::Single(id.into()),
resolvo::Requirement::Union(id) => Requirement::Union(id.into()),
resolvo::Requirement::ConditionalRequires(condition, requirement) => {
Requirement::ConditionalRequires(condition.into(), requirement.into())
}
}
}
}
Expand All @@ -64,6 +72,9 @@ impl From<crate::Requirement> for resolvo::Requirement {
match value {
Requirement::Single(id) => resolvo::Requirement::Single(id.into()),
Requirement::Union(id) => resolvo::Requirement::Union(id.into()),
Requirement::ConditionalRequires(condition, requirement) => {
resolvo::Requirement::ConditionalRequires(condition.into(), requirement.into())
}
}
}
}
Expand Down Expand Up @@ -539,6 +550,15 @@ pub extern "C" fn resolvo_requirement_union(
Requirement::Union(version_set_union_id)
}

#[no_mangle]
#[allow(unused)]
pub extern "C" fn resolvo_requirement_conditional(
condition: VersionSetId,
requirement: VersionSetId,
) -> Requirement {
Requirement::ConditionalRequires(condition, requirement)
}

#[cfg(test)]
mod tests {
use super::*;
Expand Down
89 changes: 83 additions & 6 deletions src/conflict.rs
Original file line number Diff line number Diff line change
Expand Up @@ -11,14 +11,17 @@ use petgraph::{
Direction,
};

use crate::solver::variable_map::VariableOrigin;
use crate::{
internal::{
arena::ArenaId,
id::{ClauseId, SolvableId, SolvableOrRootId, StringId, VersionSetId},
id::{ClauseId, SolvableId, SolvableOrRootId, StringId, VariableId, VersionSetId},
},
runtime::AsyncRuntime,
solver::{clause::Clause, Solver},
solver::{
clause::Clause,
variable_map::{VariableMap, VariableOrigin},
Solver,
},
DependencyProvider, Interner, Requirement,
};

Expand Down Expand Up @@ -160,6 +163,49 @@ impl Conflict {
ConflictEdge::Conflict(ConflictCause::Constrains(version_set_id)),
);
}
&Clause::Conditional(package_id, condition, then_version_set_id) => {
let solvable = package_id
.as_solvable_or_root(&solver.variable_map)
.expect("only solvables can be excluded");
let package_node = Self::add_node(&mut graph, &mut nodes, solvable);

let candidates = solver.async_runtime.block_on(solver.cache.get_or_cache_sorted_candidates(then_version_set_id)).unwrap_or_else(|_| {

Check warning on line 172 in src/conflict.rs

View workflow job for this annotation

GitHub Actions / Format and Lint

Diff in /home/runner/work/resolvo/resolvo/src/conflict.rs
unreachable!("The version set was used in the solver, so it must have been cached. Therefore cancellation is impossible here and we cannot get an `Err(...)`")
});

if candidates.is_empty() {
tracing::trace!(
"{package_id:?} conditionally requires {then_version_set_id:?}, which has no candidates"
);
graph.add_edge(
package_node,
unresolved_node,
ConflictEdge::ConditionalRequires(then_version_set_id, condition),
);

Check warning on line 184 in src/conflict.rs

View workflow job for this annotation

GitHub Actions / Format and Lint

Diff in /home/runner/work/resolvo/resolvo/src/conflict.rs
} else {
for &candidate_id in candidates {
tracing::trace!("{package_id:?} conditionally requires {candidate_id:?}");

let candidate_node =
Self::add_node(&mut graph, &mut nodes, candidate_id.into());
graph.add_edge(
package_node,
candidate_node,
ConflictEdge::ConditionalRequires(then_version_set_id, condition),
);

Check warning on line 195 in src/conflict.rs

View workflow job for this annotation

GitHub Actions / Format and Lint

Diff in /home/runner/work/resolvo/resolvo/src/conflict.rs
}
}

// Add an edge for the unsatisfied condition if it exists
if let Some(condition_solvable) = condition.as_solvable(&solver.variable_map) {
let condition_node = Self::add_node(&mut graph, &mut nodes, condition_solvable.into());
graph.add_edge(
package_node,
condition_node,
ConflictEdge::Conflict(ConflictCause::UnsatisfiedCondition(condition)),
);
}
}
}
}

Expand Down Expand Up @@ -205,7 +251,7 @@ impl Conflict {
solver: &'a Solver<D, RT>,
) -> DisplayUnsat<'a, D> {
let graph = self.graph(solver);
DisplayUnsat::new(graph, solver.provider())
DisplayUnsat::new(graph, solver.provider(), &solver.variable_map)
}
}

Expand Down Expand Up @@ -239,26 +285,30 @@ impl ConflictNode {
}

/// An edge in the graph representation of a [`Conflict`]
#[derive(Copy, Clone, Hash, Eq, PartialEq, Ord, PartialOrd)]
#[derive(Clone, Copy, Hash, Eq, PartialEq, Ord, PartialOrd)]
pub(crate) enum ConflictEdge {
/// The target node is a candidate for the dependency specified by the
/// [`Requirement`]
Requires(Requirement),
/// The target node is involved in a conflict, caused by `ConflictCause`
Conflict(ConflictCause),
/// The target node is a candidate for a conditional dependency
ConditionalRequires(Requirement, VariableId),
}

impl ConflictEdge {
fn try_requires(self) -> Option<Requirement> {
match self {
ConflictEdge::Requires(match_spec_id) => Some(match_spec_id),
ConflictEdge::Conflict(_) => None,
ConflictEdge::ConditionalRequires(match_spec_id, _) => Some(match_spec_id),
}
}

fn requires(self) -> Requirement {
match self {
ConflictEdge::Requires(match_spec_id) => match_spec_id,
ConflictEdge::ConditionalRequires(match_spec_id, _) => match_spec_id,
ConflictEdge::Conflict(_) => panic!("expected requires edge, found conflict"),
}
}
Expand All @@ -275,6 +325,8 @@ pub(crate) enum ConflictCause {
ForbidMultipleInstances,
/// The node was excluded
Excluded,
/// The condition for a conditional dependency was not satisfied
UnsatisfiedCondition(VariableId),
}

/// Represents a node that has been merged with others
Expand Down Expand Up @@ -307,6 +359,7 @@ impl ConflictGraph {
&self,
f: &mut impl std::io::Write,
interner: &impl Interner,
variable_map: &VariableMap,
simplify: bool,
) -> Result<(), std::io::Error> {
let graph = &self.graph;
Expand Down Expand Up @@ -356,6 +409,16 @@ impl ConflictGraph {
"already installed".to_string()
}

Check warning on line 410 in src/conflict.rs

View workflow job for this annotation

GitHub Actions / Format and Lint

Diff in /home/runner/work/resolvo/resolvo/src/conflict.rs
ConflictEdge::Conflict(ConflictCause::Excluded) => "excluded".to_string(),
ConflictEdge::Conflict(ConflictCause::UnsatisfiedCondition(condition)) => {
let condition_solvable = condition.as_solvable(variable_map)
.expect("condition must be a solvable");
format!("unsatisfied condition: {}", condition_solvable.display(interner))
}
ConflictEdge::ConditionalRequires(requirement, condition) => {
let condition_solvable = condition.as_solvable(variable_map)
.expect("condition must be a solvable");
format!("if {} then {}", condition_solvable.display(interner), requirement.display(interner))
}
};

let target = match target {
Expand Down Expand Up @@ -494,6 +557,7 @@ impl ConflictGraph {
.edges_directed(nx, Direction::Outgoing)

Check warning on line 557 in src/conflict.rs

View workflow job for this annotation

GitHub Actions / Format and Lint

Diff in /home/runner/work/resolvo/resolvo/src/conflict.rs
.map(|e| match e.weight() {
ConflictEdge::Requires(version_set_id) => (version_set_id, e.target()),
ConflictEdge::ConditionalRequires(version_set_id, _) => (version_set_id, e.target()),
ConflictEdge::Conflict(_) => unreachable!(),
})
.chunk_by(|(&version_set_id, _)| version_set_id);
Expand Down Expand Up @@ -540,6 +604,7 @@ impl ConflictGraph {
.edges_directed(nx, Direction::Outgoing)

Check warning on line 604 in src/conflict.rs

View workflow job for this annotation

GitHub Actions / Format and Lint

Diff in /home/runner/work/resolvo/resolvo/src/conflict.rs
.map(|e| match e.weight() {
ConflictEdge::Requires(version_set_id) => (version_set_id, e.target()),
ConflictEdge::ConditionalRequires(version_set_id, _) => (version_set_id, e.target()),
ConflictEdge::Conflict(_) => unreachable!(),
})
.chunk_by(|(&version_set_id, _)| version_set_id);
Expand Down Expand Up @@ -673,10 +738,11 @@ pub struct DisplayUnsat<'i, I: Interner> {
installable_set: HashSet<NodeIndex>,
missing_set: HashSet<NodeIndex>,
interner: &'i I,
variable_map: &'i VariableMap,
}

Check warning on line 742 in src/conflict.rs

View workflow job for this annotation

GitHub Actions / Format and Lint

Diff in /home/runner/work/resolvo/resolvo/src/conflict.rs

impl<'i, I: Interner> DisplayUnsat<'i, I> {
pub(crate) fn new(graph: ConflictGraph, interner: &'i I) -> Self {
pub(crate) fn new(graph: ConflictGraph, interner: &'i I, variable_map: &'i VariableMap) -> Self {
let merged_candidates = graph.simplify(interner);
let installable_set = graph.get_installable_set();
let missing_set = graph.get_missing_set();
Expand All @@ -687,6 +753,7 @@ impl<'i, I: Interner> DisplayUnsat<'i, I> {
installable_set,
missing_set,
interner,
variable_map,
}
}

Expand Down Expand Up @@ -1020,6 +1087,7 @@ impl<'i, I: Interner> fmt::Display for DisplayUnsat<'i, I> {
let conflict = match e.weight() {
ConflictEdge::Requires(_) => continue,
ConflictEdge::Conflict(conflict) => conflict,
ConflictEdge::ConditionalRequires(_, _) => continue,
};

// The only possible conflict at the root level is a Locked conflict
Expand All @@ -1045,6 +1113,15 @@ impl<'i, I: Interner> fmt::Display for DisplayUnsat<'i, I> {
)?;
}

Check warning on line 1114 in src/conflict.rs

View workflow job for this annotation

GitHub Actions / Format and Lint

Diff in /home/runner/work/resolvo/resolvo/src/conflict.rs
ConflictCause::Excluded => continue,
&ConflictCause::UnsatisfiedCondition(condition) => {
let condition_solvable = condition.as_solvable(self.variable_map)
.expect("condition must be a solvable");
writeln!(
f,
"{indent}condition {} is not satisfied",
condition_solvable.display(self.interner),
)?;
}
};
}
}
Expand Down
30 changes: 23 additions & 7 deletions src/requirement.rs
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,9 @@ pub enum Requirement {
/// This variant is typically used for requirements that can be satisfied by two or more
/// version sets belonging to _different_ packages.
Union(VersionSetUnionId),
/// Specifies a conditional requirement, where the requirement is only active when the condition is met.
/// First VersionSetId is the condition, second is the requirement.
ConditionalRequires(VersionSetId, VersionSetId),
}

impl Default for Requirement {
Expand Down Expand Up @@ -46,12 +49,15 @@ impl Requirement {
&'i self,
interner: &'i impl Interner,
) -> impl Iterator<Item = VersionSetId> + 'i {
match *self {
match self {
Requirement::Single(version_set) => {

Check warning on line 53 in src/requirement.rs

View workflow job for this annotation

GitHub Actions / Format and Lint

Diff in /home/runner/work/resolvo/resolvo/src/requirement.rs
itertools::Either::Left(std::iter::once(version_set))
itertools::Either::Left(itertools::Either::Left(std::iter::once(*version_set)))
}
Requirement::Union(version_set_union) => {
itertools::Either::Right(interner.version_sets_in_union(version_set_union))
itertools::Either::Left(itertools::Either::Right(interner.version_sets_in_union(*version_set_union)))
}
Requirement::ConditionalRequires(condition, requirement) => {
itertools::Either::Right(std::iter::once(*condition).chain(std::iter::once(*requirement)))
}
}
}
Expand All @@ -64,18 +70,18 @@ pub(crate) struct DisplayRequirement<'i, I: Interner> {

impl<'i, I: Interner> Display for DisplayRequirement<'i, I> {
fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
match *self.requirement {
match self.requirement {
Requirement::Single(version_set) => write!(
f,
"{} {}",
self.interner
.display_name(self.interner.version_set_name(version_set)),
self.interner.display_version_set(version_set)
.display_name(self.interner.version_set_name(*version_set)),
self.interner.display_version_set(*version_set)
),
Requirement::Union(version_set_union) => {
let formatted_version_sets = self
.interner
.version_sets_in_union(version_set_union)
.version_sets_in_union(*version_set_union)
.format_with(" | ", |version_set, f| {
f(&format_args!(
"{} {}",
Expand All @@ -87,6 +93,16 @@ impl<'i, I: Interner> Display for DisplayRequirement<'i, I> {

write!(f, "{}", formatted_version_sets)
}
Requirement::ConditionalRequires(condition, requirement) => {
write!(
f,
"if {} then {} {}",
self.interner.display_version_set(*condition),
self.interner
.display_name(self.interner.version_set_name(*requirement)),
self.interner.display_version_set(*requirement)
)
}
}
}
}
8 changes: 8 additions & 0 deletions src/snapshot.rs
Original file line number Diff line number Diff line change
Expand Up @@ -243,6 +243,14 @@ impl DependencySnapshot {
.version_set_unions
.insert(version_set_union_id, version_sets);
}
Requirement::ConditionalRequires(condition, requirement) => {
if seen.insert(Element::VersionSet(condition)) {
queue.push_back(Element::VersionSet(condition));
}
if seen.insert(Element::VersionSet(requirement)) {
queue.push_back(Element::VersionSet(requirement));
}
}
}
}
}
Expand Down
5 changes: 5 additions & 0 deletions src/solver/cache.rs
Original file line number Diff line number Diff line change
Expand Up @@ -280,6 +280,11 @@ impl<D: DependencyProvider> SolverCache<D> {
}
}

Check warning on line 281 in src/solver/cache.rs

View workflow job for this annotation

GitHub Actions / Format and Lint

Diff in /home/runner/work/resolvo/resolvo/src/solver/cache.rs
}
Requirement::ConditionalRequires(condition, requirement) => {
let candidates = self.get_or_cache_sorted_candidates_for_version_set(condition).await?;
let sorted_candidates = self.get_or_cache_sorted_candidates_for_version_set(requirement).await?;
Ok(sorted_candidates)
}
}
}

Expand Down
Loading

0 comments on commit 0586e74

Please sign in to comment.