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: add support for conditional dependencies #101

Draft
wants to merge 5 commits into
base: main
Choose a base branch
from
Draft
Show file tree
Hide file tree
Changes from all 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
37 changes: 33 additions & 4 deletions cpp/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,17 @@ impl From<SolvableId> for resolvo::SolvableId {
}
}

/// 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
#[repr(C)]
#[derive(Copy, Clone)]
pub struct ConditionalRequirement {
pub condition: Option<VersionSetId>,
pub requirement: Requirement,
}

/// Specifies the dependency of a solvable on a set of version sets.
/// cbindgen:derive-eq
/// cbindgen:derive-neq
Expand Down Expand Up @@ -68,6 +79,24 @@ impl From<crate::Requirement> for resolvo::Requirement {
}
}

impl From<resolvo::ConditionalRequirement> for ConditionalRequirement {
fn from(value: resolvo::ConditionalRequirement) -> Self {
Self {
condition: value.condition.map(|id| id.into()),
requirement: value.requirement.into(),
}
}
}

impl From<ConditionalRequirement> for resolvo::ConditionalRequirement {
fn from(value: ConditionalRequirement) -> Self {
Self {
condition: value.condition.map(|id| id.into()),
requirement: value.requirement.into(),
}
}
}

/// A unique identifier for a version set union. A version set union describes
/// the union (logical OR) of a non-empty set of version sets belonging to
/// more than one package.
Expand Down Expand Up @@ -162,7 +191,7 @@ pub struct Dependencies {
/// A pointer to the first element of a list of requirements. Requirements
/// defines which packages should be installed alongside the depending
/// package and the constraints applied to the package.
pub requirements: Vector<Requirement>,
pub conditional_requirements: Vector<ConditionalRequirement>,

/// Defines additional constraints on packages that may or may not be part
/// of the solution. Different from `requirements`, packages in this set
Expand Down Expand Up @@ -459,8 +488,8 @@ impl<'d> resolvo::DependencyProvider for &'d DependencyProvider {
};

resolvo::Dependencies::Known(KnownDependencies {
requirements: dependencies
.requirements
conditional_requirements: dependencies
.conditional_requirements
.into_iter()
.map(Into::into)
.collect(),
Expand All @@ -475,7 +504,7 @@ impl<'d> resolvo::DependencyProvider for &'d DependencyProvider {

#[repr(C)]
pub struct Problem<'a> {
pub requirements: Slice<'a, Requirement>,
pub requirements: Slice<'a, ConditionalRequirement>,
pub constraints: Slice<'a, VersionSetId>,
pub soft_requirements: Slice<'a, SolvableId>,
}
Expand Down
93 changes: 89 additions & 4 deletions src/conflict.rs
Original file line number Diff line number Diff line change
Expand Up @@ -11,14 +11,13 @@
Direction,
};

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

Expand Down Expand Up @@ -160,6 +159,53 @@
ConflictEdge::Conflict(ConflictCause::Constrains(version_set_id)),
);
}
&Clause::Conditional(package_id, condition, 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(version_set_id)).unwrap_or_else(|_| {
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 {version_set_id:?}, which has no candidates"
);
graph.add_edge(
package_node,
unresolved_node,
ConflictEdge::ConditionalRequires(condition, version_set_id),
);
} 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(condition, version_set_id),
);
}
}

// TODO: Add an edge for the unsatisfied condition if it exists
// // 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.into())),
// );
// }
}
}
}

Expand Down Expand Up @@ -239,26 +285,32 @@
}

/// 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(VersionSetId, Requirement),
}

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

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

Check failure on line 331 in src/conflict.rs

View workflow job for this annotation

GitHub Actions / Format and Lint

variant `UnsatisfiedCondition` is never constructed
}

/// Represents a node that has been merged with others
Expand Down Expand Up @@ -356,6 +410,18 @@
"already installed".to_string()
}
ConflictEdge::Conflict(ConflictCause::Excluded) => "excluded".to_string(),
ConflictEdge::Conflict(ConflictCause::UnsatisfiedCondition(condition)) => {

Check failure on line 413 in src/conflict.rs

View workflow job for this annotation

GitHub Actions / Format and Lint

unused variable: `condition`
// let condition_solvable = condition.as_solvable(&solver.variable_map)
// .expect("condition must be a solvable");
// format!("unsatisfied condition: {}", condition_solvable.display(interner))
todo!()
}
ConflictEdge::ConditionalRequires(requirement, condition) => {

Check failure on line 419 in src/conflict.rs

View workflow job for this annotation

GitHub Actions / Format and Lint

unused variable: `requirement`

Check failure on line 419 in src/conflict.rs

View workflow job for this annotation

GitHub Actions / Format and Lint

unused variable: `condition`
// let condition_solvable = condition.as_solvable(&solver.variable_map)
// .expect("condition must be a solvable");
// format!("if {} then {}", condition_solvable.display(interner), requirement.display(interner))
todo!()
}
};

let target = match target {
Expand Down Expand Up @@ -493,9 +559,12 @@
.graph
.edges_directed(nx, Direction::Outgoing)
.map(|e| match e.weight() {
ConflictEdge::Requires(version_set_id) => (version_set_id, e.target()),
ConflictEdge::Requires(req) => (req, e.target()),
ConflictEdge::ConditionalRequires(_, req) => (req, e.target()),
ConflictEdge::Conflict(_) => unreachable!(),
})
.collect::<Vec<_>>()
.into_iter()
.chunk_by(|(&version_set_id, _)| version_set_id);

for (_, mut deps) in &dependencies {
Expand Down Expand Up @@ -540,8 +609,13 @@
.edges_directed(nx, Direction::Outgoing)
.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!(),
})
.collect::<Vec<_>>()
.into_iter()
.chunk_by(|(&version_set_id, _)| version_set_id);

// Missing if at least one dependency is missing
Expand Down Expand Up @@ -1020,6 +1094,7 @@
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 +1120,16 @@
)?;
}
ConflictCause::Excluded => continue,
&ConflictCause::UnsatisfiedCondition(condition) => {

Check failure on line 1123 in src/conflict.rs

View workflow job for this annotation

GitHub Actions / Format and Lint

unused variable: `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),
// )?;
todo!()
}
};
}
}
Expand Down
4 changes: 2 additions & 2 deletions src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@ pub use internal::{
mapping::Mapping,
};
use itertools::Itertools;
pub use requirement::Requirement;
pub use requirement::{ConditionalRequirement, Requirement};
pub use solver::{Problem, Solver, SolverCache, UnsolvableOrCancelled};

/// An object that is used by the solver to query certain properties of
Expand Down Expand Up @@ -206,7 +206,7 @@ pub struct KnownDependencies {
feature = "serde",
serde(default, skip_serializing_if = "Vec::is_empty")
)]
pub requirements: Vec<Requirement>,
pub conditional_requirements: Vec<ConditionalRequirement>,

/// Defines additional constraints on packages that may or may not be part
/// of the solution. Different from `requirements`, packages in this set
Expand Down
51 changes: 51 additions & 0 deletions src/requirement.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,57 @@ use crate::{Interner, VersionSetId, VersionSetUnionId};
use itertools::Itertools;
use std::fmt::Display;

/// Specifies a conditional requirement, where the requirement is only active when the condition is met.
#[derive(Clone, Copy, Debug, Hash, Eq, PartialEq, Ord, PartialOrd)]
#[cfg_attr(feature = "serde", derive(serde::Serialize, serde::Deserialize))]
pub struct ConditionalRequirement {
/// The condition that must be met for the requirement to be active.
pub condition: Option<VersionSetId>,
/// The requirement that is only active when the condition is met.
pub requirement: Requirement,
}

impl ConditionalRequirement {
/// Creates a new conditional requirement.
pub fn new(condition: VersionSetId, requirement: Requirement) -> Self {
Self {
condition: Some(condition),
requirement,
}
}
/// Returns the version sets that satisfy the requirement.
pub fn requirement_version_sets<'i>(
&'i self,
interner: &'i impl Interner,
) -> impl Iterator<Item = VersionSetId> + 'i {
self.requirement.version_sets(interner)
}

/// Returns the version sets that satisfy the requirement, along with the condition that must be met.
pub fn version_sets_with_condition<'i>(
&'i self,
interner: &'i impl Interner,
) -> impl Iterator<Item = (VersionSetId, Option<VersionSetId>)> + 'i {
self.requirement
.version_sets(interner)
.map(move |vs| (vs, self.condition))
}

/// Returns the condition and requirement.
pub fn into_condition_and_requirement(self) -> (Option<VersionSetId>, Requirement) {
(self.condition, self.requirement)
}
}

impl From<Requirement> for ConditionalRequirement {
fn from(value: Requirement) -> Self {
Self {
condition: None,
requirement: value,
}
}
}

/// Specifies the dependency of a solvable on a set of version sets.
#[derive(Clone, Copy, Debug, Hash, Eq, PartialEq, Ord, PartialOrd)]
#[cfg_attr(feature = "serde", derive(serde::Serialize, serde::Deserialize))]
Expand Down
4 changes: 3 additions & 1 deletion src/snapshot.rs
Original file line number Diff line number Diff line change
Expand Up @@ -220,7 +220,9 @@ impl DependencySnapshot {
}
}

for &requirement in deps.requirements.iter() {
for &req in deps.conditional_requirements.iter() {
let (_, requirement) = req.into_condition_and_requirement(); // TODO: condition

match requirement {
Requirement::Single(version_set) => {
if seen.insert(Element::VersionSet(version_set)) {
Expand Down
Loading
Loading