Skip to content

Commit

Permalink
fix: constraints on virtual packages were ignored
Browse files Browse the repository at this point in the history
  • Loading branch information
baszalmstra committed Jul 25, 2024
1 parent 809d3c4 commit 3374345
Show file tree
Hide file tree
Showing 7 changed files with 116 additions and 26 deletions.
13 changes: 12 additions & 1 deletion crates/rattler_solve/src/libsolv_c/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ pub use input::cache_repodata;
use input::{add_repodata_records, add_solv_file, add_virtual_packages};
pub use libc_byte_slice::LibcByteSlice;
use output::get_required_packages;
use rattler_conda_types::RepoDataRecord;
use rattler_conda_types::{MatchSpec, NamelessMatchSpec, RepoDataRecord};
use wrapper::{
flags::SolverFlag,
pool::{Pool, Verbosity},
Expand Down Expand Up @@ -239,6 +239,17 @@ impl super::SolverImpl for Solver {
goal.install(id, true);
}

// Add virtual packages to the queue. We want to install these as part of the
// solution as well. This ensures that if a package only has a constraint on a
// virtual package, the virtual package is installed.
for virtual_package in task.virtual_packages {
let id = pool.intern_matchspec(&MatchSpec::from_nameless(
NamelessMatchSpec::default(),
Some(virtual_package.name),
));
goal.install(id, false);
}

// Construct a solver and solve the problems in the queue
let mut solver = pool.create_solver();
solver.set_flag(SolverFlag::allow_uninstall(), true);
Expand Down
14 changes: 8 additions & 6 deletions crates/rattler_solve/src/libsolv_c/output.rs
Original file line number Diff line number Diff line change
Expand Up @@ -35,8 +35,11 @@ pub fn get_required_packages(
let transaction_type = transaction.transaction_type(id);

// Retrieve the repodata record corresponding to this solvable
let (repo_index, solvable_index) =
get_solvable_indexes(pool, repo_mapping, solvable_index_id, id);
let Some((repo_index, solvable_index)) =
get_solvable_indexes(pool, repo_mapping, solvable_index_id, id)
else {
continue;
};
let repodata_record = repodata_records[repo_index][solvable_index];

match transaction_type as u32 {
Expand All @@ -61,15 +64,14 @@ fn get_solvable_indexes(
repo_mapping: &HashMap<RepoId, usize>,
solvable_index_id: StringId,
id: SolvableId,
) -> (usize, usize) {
) -> Option<(usize, usize)> {
let solvable = id.resolve_raw(pool);
let solvable_index =
solvable::lookup_num(solvable.as_ptr(), solvable_index_id).unwrap() as usize;
let solvable_index = solvable::lookup_num(solvable.as_ptr(), solvable_index_id)? as usize;

// Safe because there are no active mutable borrows of any solvable at this stage
let repo_id = RepoId::from_ffi_solvable(unsafe { solvable.as_ref() });

let repo_index = repo_mapping[&repo_id];

(repo_index, solvable_index)
Some((repo_index, solvable_index))
}
31 changes: 19 additions & 12 deletions crates/rattler_solve/src/resolvo/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -609,17 +609,24 @@ impl super::SolverImpl for Solver {
)?;

// Construct the requirements that the solver needs to satisfy.
let root_requirements = task
.specs
.iter()
.map(|spec| {
let (name, nameless_spec) = spec.clone().into_nameless();
let name = name.expect("cannot use matchspec without a name");
let name_id = provider.pool.intern_package_name(name.as_normalized());
provider
.pool
.intern_version_set(name_id, nameless_spec.into())
})
let virtual_package_requirements = task.virtual_packages.iter().map(|spec| {
let name_id = provider.pool.intern_package_name(spec.name.as_normalized());
provider
.pool
.intern_version_set(name_id, NamelessMatchSpec::default().into())
});

let root_requirements = task.specs.iter().map(|spec| {
let (name, nameless_spec) = spec.clone().into_nameless();
let name = name.expect("cannot use matchspec without a name");
let name_id = provider.pool.intern_package_name(name.as_normalized());
provider
.pool
.intern_version_set(name_id, nameless_spec.into())
});

let all_requirements = virtual_package_requirements
.chain(root_requirements)
.collect();

let root_constraints = task
Expand All @@ -635,7 +642,7 @@ impl super::SolverImpl for Solver {

// Construct a solver and solve the problems in the queue
let mut solver = LibSolvRsSolver::new(provider);
let solvables = solver.solve(root_requirements, root_constraints).map_err(
let solvables = solver.solve(all_requirements, root_constraints).map_err(
|unsolvable_or_cancelled| {
match unsolvable_or_cancelled {
UnsolvableOrCancelled::Unsolvable(problem) => {
Expand Down
52 changes: 45 additions & 7 deletions crates/rattler_solve/tests/backends.rs
Original file line number Diff line number Diff line change
Expand Up @@ -202,6 +202,7 @@ fn read_conda_forge_sparse_repo_data() -> &'static SparseRepoData {
macro_rules! solver_backend_tests {
($T:path) => {
use chrono::{DateTime, Utc};
use itertools::Itertools;

#[test]
fn test_solve_quetz() {
Expand Down Expand Up @@ -561,6 +562,41 @@ macro_rules! solver_backend_tests {
assert_eq!(operations[0].file_name, "bors-1.0-bla_1.tar.bz2");
assert_eq!(operations[1].file_name, "foobar-2.1-bla_1.tar.bz2");
}

#[test]
fn test_virtual_package_constrains() {
// This tests that a package that has a constrains on a virtual package is
// properly restricted.
let result = solve::<$T>(
dummy_channel_json_path(),
SimpleSolveTask {
specs: &["cuda-version"],
virtual_packages: vec![GenericVirtualPackage {
name: "__cuda".parse().unwrap(),
version: Version::from_str("1").unwrap(),
build_string: "0".to_string(),
}],
..SimpleSolveTask::default()
},
);

let output = match result {
Ok(pkgs) => pkgs
.iter()
.format_with("\n", |pkg, f| {
f(&format_args!(
"{}={}={}",
pkg.package_record.name.as_normalized(),
pkg.package_record.version.as_str(),
&pkg.package_record.build
))
})
.to_string(),
Err(e) => e.to_string(),
};

insta::assert_snapshot!(output);
}
};
}

Expand Down Expand Up @@ -661,16 +697,17 @@ mod libsolv_c {

#[cfg(feature = "resolvo")]
mod resolvo {
use super::{
dummy_channel_json_path, installed_package, solve, solve_real_world, FromStr,
GenericVirtualPackage, SimpleSolveTask, SolveError, Version,
};
use rattler_conda_types::{
MatchSpec, PackageRecord, ParseStrictness, RepoDataRecord, VersionWithSource,
};
use rattler_solve::{SolveStrategy, SolverImpl, SolverTask};
use url::Url;

use super::{
dummy_channel_json_path, installed_package, solve, solve_real_world, FromStr,
GenericVirtualPackage, SimpleSolveTask, SolveError, Version,
};

solver_backend_tests!(rattler_solve::resolvo::Solver);

#[test]
Expand Down Expand Up @@ -805,7 +842,8 @@ mod resolvo {
);
}

/// Try to solve a package with a direct url, and then try to do it again without having it in the repodata.
/// Try to solve a package with a direct url, and then try to do it again
/// without having it in the repodata.
#[test]
fn test_solve_on_url() {
let url_str =
Expand All @@ -817,8 +855,8 @@ mod resolvo {

// Create RepoData with only the package from the url, so the solver can find it
let package_record = PackageRecord::new(
// // Only defining the name, version and url is enough for the solver to find the package
// direct_url: Some(url.clone()),
// // Only defining the name, version and url is enough for the solver to find the
// package direct_url: Some(url.clone()),
"_libgcc_mutex".parse().unwrap(),
VersionWithSource::from_str("0.1").unwrap(),
"0".to_string(),
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
---
source: crates/rattler_solve/tests/backends.rs
assertion_line: 614
expression: output
---
Cannot solve the request because of: package cuda-version-12.5-hd4f0392_3 has constraint __cuda >=12.1 conflicting with __cuda-1
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
---
source: crates/rattler_solve/tests/backends.rs
assertion_line: 711
expression: output
---
Cannot solve the request because of: The following packages are incompatible
├─ __cuda * can be installed with any of the following options:
│ └─ __cuda 1
└─ cuda-version * cannot be installed because there are no viable options:
└─ cuda-version 12.5 would constrain
└─ __cuda >=12.1 , which conflicts with any installable versions previously reported
15 changes: 15 additions & 0 deletions test-data/channels/dummy/linux-64/repodata.json
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,21 @@
"base_url": "../linux-64"
},
"packages": {
"cuda-version-12.5-hd4f0392_3.conda": {
"build": "hd4f0392_3",
"build_number": 3,
"depends": [],
"constrains": ["__cuda >=12.1"],
"license": "LicenseRef-NVIDIA-End-User-License-Agreement",
"license_family": "LicenseRef-NVIDIA-End-User-License-Agreement",
"md5": "6ae1a563a4aa61e55e8ae8260f0d021b",
"name": "cuda-version",
"sha256": "e45a5d14909296abd0784a073da9ee5c420fa58671fbc999f8a9ec898cf3486b",
"size": 21151,
"subdir": "noarch",
"timestamp": 1716314536803,
"version": "12.5"
},
"foo-3.0.2-py36h1af98f8_1.tar.bz2": {
"build": "py36h1af98f8_1",
"build_number": 1,
Expand Down

0 comments on commit 3374345

Please sign in to comment.