Skip to content

Commit

Permalink
Implement resolving missing requests for embedded components
Browse files Browse the repository at this point in the history 
If a package asks for some component `dep:comp1` and `dep` is already in
the solution because it comes embedded in some other package `other`, but
`dep:comp1` is embedded in some component of `other:comp2` and no request
for `other:comp2` is present, then in order to satisfy the requirement for
`dep:comp1`, automatically detect this situation, find what component(s) of
`other` provide the missing components, roll back the state to before
`other` was resolved, and inject a request for those components.

Signed-off-by: J Robert Ray <[email protected]>
  • Loading branch information
@jrray
jrray committed Jun 5, 2023
1 parent 4a78ab7 commit f11c0e8
Show file tree
Hide file tree
Showing 3 changed files with 164 additions and 30 deletions.
29 changes: 29 additions & 0 deletions crates/spk-solve/crates/graph/src/graph.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -335,6 +335,35 @@ impl<'state, 'cmpt> DecisionBuilder<'state, 'cmpt> {
}
}

/// Make this package the next request to be considered.
pub fn reconsider_package_with_additional_components(
self,
request: PkgRequest,
package_with_unsatisfied_components: &PkgName,
counter: Arc<AtomicU64>,
) -> Decision {
let generate_changes = || {
let changes = vec![
Change::StepBack(StepBack::new(
format!(
"Package {package_with_unsatisfied_components} needs additional components from {}",
request.pkg.name
),
self.base,
counter,
)),
Change::RequestPackage(RequestPackage::prioritize(request)),
];

changes
};

Decision {
changes: generate_changes(),
notes: Vec::default(),
}
}

fn requirements_to_changes(
&self,
requirements: &RequirementsList,
Expand Down
126 changes: 119 additions & 7 deletions crates/spk-solve/src/solver.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@
// SPDX-License-Identifier: Apache-2.0
// https://github.com/imageworks/spk
use std::borrow::Cow;
use std::collections::{HashMap, HashSet};
use std::collections::{BTreeSet, HashMap, HashSet};
use std::mem::take;
use std::sync::atomic::{AtomicU64, Ordering};
use std::sync::Arc;
Expand Down Expand Up @@ -455,6 +455,15 @@ impl Solver {
.map_err(Error::ValidationError)
}

/// Default behavior for skipping an incompatible build.
fn skip_build(&mut self, notes: &mut Vec<Note>, spec: &Spec, compat: &Compatibility) {
notes.push(Note::SkipPackageNote(SkipPackageNote::new(
spec.ident().to_any(),
compat.clone(),
)));
self.number_builds_skipped += 1;
}

async fn step_state(
&mut self,
graph: &Arc<tokio::sync::RwLock<Graph>>,
Expand Down Expand Up @@ -541,7 +550,11 @@ impl Solver {
continue;
}

let builds = if !builds.lock().await.is_sorted_build_iterator() {
let builds: Arc<tokio::sync::Mutex<dyn BuildIterator + Send>> = if !builds
.lock()
.await
.is_sorted_build_iterator()
{
// TODO: this could be a HashSet if build key generation
// only looks at the idents in the hashmap.
let builds_with_impossible_requests = if self.impossible_checks.use_in_build_keys {
Expand Down Expand Up @@ -657,12 +670,111 @@ impl Solver {
Arc::clone(&self.number_of_steps_back),
)
}
Compatibility::Incompatible(
IncompatibleReason::EmbeddedComponentsMissing(parent, missing),
) => {
if let Ok((parent_spec, _, state_id)) =
node.state.get_current_resolve(&parent)
{
// This build couldn't be used because it needs
// components from an embedded package that
// have not been provided from the package that
// embeds it. It might be possible to add a
// request for a component from the parent
// package to bring in the needed component.

// Find which component(s) of parent_spec
// embed the missing components.
let mut remaining_missing_components: BTreeSet<_> =
missing.missing.iter().cloned().collect();
let mut components_to_request = BTreeSet::new();
for component in parent_spec.components().iter() {
for embedded_component in
component.embedded_components.iter()
{
if embedded_component.name != missing.package {
continue;
}

let overlapping: Vec<_> = remaining_missing_components
.intersection(&embedded_component.components)
.collect();
if overlapping.is_empty() {
continue;
}

components_to_request.insert(component.name.clone());

remaining_missing_components =
remaining_missing_components
.difference(
&overlapping.into_iter().cloned().collect(),
)
.cloned()
.collect();

if remaining_missing_components.is_empty() {
break;
}
}

if remaining_missing_components.is_empty() {
break;
}
}

if !remaining_missing_components.is_empty() {
// Couldn't find a way to satisfy all
// the missing components.
self.skip_build(&mut notes, &spec, &compat);
continue;
}

// Add a request for the components of the
// parent package that will provide the
// missing components in the embedded
// package, and retry this build.

let graph_lock = graph.read().await;

let target_state_node = graph_lock
.nodes
.get(&state_id.id())
.ok_or_else(|| Error::String("state not found".into()))?
.read()
.await;

Decision::builder(&target_state_node.state)
.reconsider_package_with_additional_components(
{
let mut pkg_request = PkgRequest::new(
parent_spec.ident().to_any().into(),
RequestedBy::PackageBuild(spec.ident().clone()),
);

let existing_requested_components = node
.state
.get_merged_request(parent_spec.ident().name())
.map(|r| r.pkg.components)
.unwrap_or_default();

pkg_request.pkg.components =
existing_requested_components
.union(&components_to_request)
.cloned()
.collect();
pkg_request
},
spec.ident().name(),
Arc::clone(&self.number_of_steps_back),
)
} else {
self.skip_build(&mut notes, &spec, &compat);
continue;
}
}
compat @ Compatibility::Incompatible(_) => {
notes.push(Note::SkipPackageNote(SkipPackageNote::new(
spec.ident().to_any(),
compat.clone(),
)));
self.number_builds_skipped += 1;
self.skip_build(&mut notes, &spec, &compat);
continue;
}
}
Expand Down
39 changes: 16 additions & 23 deletions crates/spk-solve/src/solver_test.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1863,30 +1863,23 @@ async fn test_solver_components_interaction_with_embeds(mut solver: Solver) {
solver.add_request(request!("fake-pkg:comp1"));
solver.add_request(request!("victim"));

run_and_print_resolve_for_tests(&solver)
.await
.expect_err("no solution is possible without also asking for fake-pkg:comp2");
let Ok(solution) = run_and_print_resolve_for_tests(&solver).await else {
panic!("Expected a valid solution");
};

// TODO: This code should pass instead if the solver can figure it out
// needs to add fake-pkg:comp2 to the solution automatically.
//
// let Ok(solution) = run_and_print_resolve_for_tests(&solver).await else {
// panic!("Expected a valid solution");
// };
//
// let resolved = solution
// .get("fake-pkg")
// .unwrap()
// .request
// .pkg
// .components
// .clone();
// let expected = ["comp1", "comp2"]
// .iter()
// .map(|c| Component::parse(c).map_err(|err| err.into()))
// .map(Result::unwrap)
// .collect();
// assert_eq!(resolved, expected);
let resolved = solution
.get("fake-pkg")
.unwrap()
.request
.pkg
.components
.clone();
let expected = ["comp1", "comp2"]
.iter()
.map(|c| Component::parse(c).map_err(|err| err.into()))
.map(Result::unwrap)
.collect();
assert_eq!(resolved, expected);
}

#[rstest]
Expand Down

0 comments on commit f11c0e8

Please sign in to comment.