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

Open
wants to merge 45 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
45 commits
Select commit Hold shift + click to select a range
0586e74
initial commit for conditional dependencies support
prsabahrami Jan 17, 2025
0383bfc
Add the watched literals for conditional requirements
prsabahrami Jan 21, 2025
2e6d57b
move conditional requirements to a separate struct
prsabahrami Jan 21, 2025
995a2c3
minor fix
prsabahrami Jan 21, 2025
dd636d2
remove deprecated conditional requirement function from resolvo.h
prsabahrami Jan 21, 2025
eb4d253
add a map of conditional clauses to the DependencyProvider struct
prsabahrami Jan 22, 2025
451536e
add the conditional clauses to the DependencyProvider
prsabahrami Jan 22, 2025
a9fdad2
update decide function to iterate over the conditional clauses.
prsabahrami Jan 22, 2025
e62b91c
add new node types and labels for conditional edges int he conflict g…
prsabahrami Jan 22, 2025
db7ac8b
Fix the issues with current tests
prsabahrami Jan 22, 2025
766a1a6
fix cpp bindings and tests
prsabahrami Jan 22, 2025
69042a1
remove extra file
prsabahrami Jan 22, 2025
220fed0
lint fix
prsabahrami Jan 22, 2025
0426ba3
minor fix
prsabahrami Jan 24, 2025
b95b789
Revert "minor fix"
prsabahrami Jan 24, 2025
a63278a
minor fix
prsabahrami Jan 24, 2025
190cd8a
update tests in `tests/solver.rs`
prsabahrami Jan 24, 2025
9323767
remove extra snap
prsabahrami Jan 24, 2025
a13facf
update the tests
prsabahrami Jan 24, 2025
286801b
fix tests and added tracing to conditional operations
prsabahrami Jan 24, 2025
2155d6b
rename `Dependencies::conditional_requirement to `Dependencies::requi…
prsabahrami Jan 24, 2025
eaeaf42
implement conditions in `from_provider_async` function
prsabahrami Jan 24, 2025
43a0b6b
update `fn conditional` in `clause.rs`
prsabahrami Jan 24, 2025
7e0e1c6
Add a map from version sets to variable ids which hold the variables…
prsabahrami Jan 24, 2025
3ddb16c
Add the `version_sets_to_variables` map
prsabahrami Jan 24, 2025
07e13e3
add the parser for conditional reqs
prsabahrami Jan 27, 2025
6b441f9
rn fmt
prsabahrami Jan 27, 2025
6dd7455
run fmt
prsabahrami Jan 27, 2025
057989b
fix bug in the solver
prsabahrami Jan 27, 2025
ed14321
run fmt
prsabahrami Jan 27, 2025
28210b9
add tests and minor fixes in the conditional parser
prsabahrami Jan 28, 2025
f65436b
assert when no conditional candidates where found
prsabahrami Jan 28, 2025
dbb9843
update conditional clause generation
prsabahrami Jan 28, 2025
8cd35a9
use peekable on the iterator in conditional clause generation
prsabahrami Jan 28, 2025
997ae4d
switch from version set id to variable id in in conditional clauses t…
prsabahrami Jan 28, 2025
83770fb
fix clippy issue
prsabahrami Jan 28, 2025
c8687d8
add tests with more versions and to confirm the behaviour of the reso…
prsabahrami Jan 28, 2025
4f297cc
run fmt
prsabahrami Jan 28, 2025
7e1bd18
remove extra snapshot
prsabahrami Jan 28, 2025
bd787d5
Add tests to show the conflict graph
prsabahrami Jan 30, 2025
cd3c9a3
fix the conditionals' text printing
prsabahrami Jan 31, 2025
3d055c8
add the basic setup for having multiple conditionals in a clause
prsabahrami Feb 3, 2025
eb540c0
finish the test api for both conditionals and extras
prsabahrami Feb 3, 2025
0f81524
update the clause to support a set of conditions instead of single co…
prsabahrami Feb 3, 2025
698e88c
change the conditionals type to pair of `VariableId` and `Condition`
prsabahrami Feb 3, 2025
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
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 @@
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 @@
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 @@
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 @@
}

/// 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 @@
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 @@
&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 @@ -354,8 +407,18 @@
ConflictEdge::Conflict(ConflictCause::ForbidMultipleInstances)
| ConflictEdge::Conflict(ConflictCause::Locked(_)) => {
"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 @@ -491,9 +554,10 @@
// Edges grouped by dependency
let dependencies = self
.graph
.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 @@ -537,9 +601,10 @@
// Edges grouped by dependency
let dependencies = self
.graph
.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 @@
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 @@
installable_set,
missing_set,
interner,
variable_map,
}
}

Expand Down Expand Up @@ -1020,6 +1087,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 @@ -1043,8 +1111,17 @@
"{indent}{} is locked, but another version is required as reported above",
self.interner.display_merged_solvables(&[solvable_id]),
)?;
}

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 @@
/// 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 @@
&'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 @@

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 @@

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 @@ -278,8 +278,13 @@
.requirement_to_sorted_candidates
.insert(requirement, sorted_candidates))
}
}

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
Loading