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

fix: constraints on virtual packages were ignored #795

Merged
merged 2 commits into from
Jul 29, 2024
Merged
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
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
---
source: crates/rattler_conda_types/src/repo_data/mod.rs
assertion_line: 538
expression: file_urls
---
- channels/dummy/linux-64/issue_717-2.1-bla_1.tar.bz2
Expand All @@ -14,6 +15,7 @@ expression: file_urls
- channels/dummy/linux-64/foo-3.0.2-py36h1af98f8_2.conda
- channels/dummy/linux-64/bors-1.0-bla_1.tar.bz2
- channels/dummy/linux-64/bors-1.1-bla_1.tar.bz2
- channels/dummy/linux-64/foobar-2.1-bla_1.tar.bz2
- channels/dummy/linux-64/baz-2.0-unix_py36h1af98f8_2.tar.bz2
- channels/dummy/linux-64/foobar-2.1-bla_1.tar.bz2
- channels/dummy/linux-64/cuda-version-12.5-hd4f0392_3.conda
- channels/dummy/linux-64/bors-2.0-bla_1.tar.bz2
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
---
source: crates/rattler_conda_types/src/repo_data/mod.rs
assertion_line: 497
expression: json
---
{
Expand Down Expand Up @@ -124,6 +125,23 @@ expression: json
"timestamp": 1715610974,
"version": "2.1"
},
"cuda-version-12.5-hd4f0392_3.conda": {
"build": "hd4f0392_3",
"build_number": 3,
"constrains": [
"__cuda >=12.1"
],
"depends": [],
"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.conda": {
"build": "py36h1af98f8_1",
"build_number": 1,
Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
---
source: crates/rattler_conda_types/src/repo_data/mod.rs
assertion_line: 493
expression: repodata
---
info:
Expand Down Expand Up @@ -112,6 +113,21 @@ packages:
subdir: linux-64
timestamp: 1715610974
version: "2.1"
cuda-version-12.5-hd4f0392_3.conda:
build: hd4f0392_3
build_number: 3
constrains:
- __cuda >=12.1
depends: []
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.conda:
build: py36h1af98f8_1
build_number: 1
Expand Down
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