diff --git a/.github/workflows/rust-compile.yml b/.github/workflows/rust-compile.yml index 22f677b6..34546fc0 100644 --- a/.github/workflows/rust-compile.yml +++ b/.github/workflows/rust-compile.yml @@ -1,6 +1,6 @@ on: push: - branches: ["main"] + branches: [ "main" ] pull_request: name: Rust @@ -103,8 +103,8 @@ jobs: - name: Build run: > - cargo build - --all-targets + cargo build + --all-targets ${{ steps.build-options.outputs.CARGO_BUILD_OPTIONS}} - name: Disable testing the tools crate if cross compiling @@ -116,9 +116,10 @@ jobs: - name: Run tests if: ${{ !matrix.skip-tests }} run: > - cargo test - --workspace - ${{ steps.build-options.outputs.CARGO_BUILD_OPTIONS}} - ${{ steps.test-options.outputs.CARGO_TEST_OPTIONS}} - -- + cargo test + --workspace + ${{ steps.build-options.outputs.CARGO_BUILD_OPTIONS}} + ${{ steps.test-options.outputs.CARGO_TEST_OPTIONS}} + --features resolvo + -- --nocapture diff --git a/crates/rattler_installs_packages/src/lib.rs b/crates/rattler_installs_packages/src/lib.rs index bb582cb9..fcdee6ac 100644 --- a/crates/rattler_installs_packages/src/lib.rs +++ b/crates/rattler_installs_packages/src/lib.rs @@ -32,7 +32,7 @@ mod sdist; mod system_python; #[cfg(feature = "resolvo")] -pub use resolve::{resolve, PinnedPackage}; +pub use resolve::{resolve, PinnedPackage, ResolveOptions, SDistResolution}; pub use file_store::{CacheKey, FileStore}; pub use package_database::PackageDb; diff --git a/crates/rattler_installs_packages/src/resolve.rs b/crates/rattler_installs_packages/src/resolve.rs index e89df187..eaa7c5f0 100644 --- a/crates/rattler_installs_packages/src/resolve.rs +++ b/crates/rattler_installs_packages/src/resolve.rs @@ -11,10 +11,13 @@ use crate::sdist::SDist; use crate::tags::WheelTags; use crate::wheel::Wheel; use crate::{ - ArtifactInfo, ArtifactName, Extra, NormalizedPackageName, PackageDb, PackageName, Requirement, - Version, + Artifact, ArtifactInfo, ArtifactName, Extra, NormalizedPackageName, PackageDb, PackageName, + Requirement, Version, }; use elsa::FrozenMap; +use futures::future::ready; +use futures::{FutureExt, TryFutureExt}; +use itertools::Itertools; use pep440_rs::{Operator, VersionSpecifier, VersionSpecifiers}; use pep508_rs::{MarkerEnvironment, VersionOrUrl}; use resolvo::{ @@ -131,6 +134,8 @@ struct PypiDependencyProvider<'db, 'i> { favored_packages: HashMap>, locked_packages: HashMap>, + + options: &'i ResolveOptions, } impl<'db, 'i> PypiDependencyProvider<'db, 'i> { @@ -142,6 +147,7 @@ impl<'db, 'i> PypiDependencyProvider<'db, 'i> { compatible_tags: Option<&'i WheelTags>, locked_packages: HashMap>, favored_packages: HashMap>, + options: &'i ResolveOptions, ) -> miette::Result { Ok(Self { pool: Pool::new(), @@ -151,6 +157,7 @@ impl<'db, 'i> PypiDependencyProvider<'db, 'i> { cached_artifacts: Default::default(), favored_packages, locked_packages, + options, }) } @@ -180,48 +187,87 @@ impl<'db, 'i> PypiDependencyProvider<'db, 'i> { return Err("it is yanked"); } - // Filter into Sdists and wheels - let mut sdists = artifacts - .iter() - // This filters out sdists and checks if - // the format is supported - // can get rid of this if we decide to support all formats - .filter(|a| { + // This should keep only the wheels + let mut wheels = if self.options.sdist_resolution.allow_wheels() { + let wheels = artifacts + .iter() + .filter(|a| a.is::()) + .cloned() + .collect::>(); + + if !self.options.sdist_resolution.allow_sdists() && wheels.is_empty() { + return Err("there are no wheels available"); + } + + wheels + } else { + vec![] + }; + + // Extract sdists + let mut sdists = if self.options.sdist_resolution.allow_sdists() { + let mut sdists = artifacts + .iter() + .filter(|a| a.is::()) + .cloned() + .collect::>(); + + if wheels.is_empty() && sdists.is_empty() { + if self.options.sdist_resolution.allow_wheels() { + return Err("there are no wheels or sdists"); + } else { + return Err("there are no sdists"); + } + } + + sdists.retain(|a| { a.filename .as_sdist() .is_some_and(|f| f.format.is_supported()) - }) - .cloned() - .collect::>(); + }); - // This should keep only the wheels - let mut wheels = artifacts - .iter() - .filter(|a| a.is::()) - .cloned() - .collect::>(); + if wheels.is_empty() && sdists.is_empty() { + return Err("none of the sdists formats are supported"); + } + + sdists + } else { + vec![] + }; // Filter based on compatibility - if let Some(compatible_tags) = self.compatible_tags { - wheels.retain(|artifact| match &artifact.filename { - ArtifactName::Wheel(wheel_name) => wheel_name - .all_tags_iter() - .any(|t| compatible_tags.is_compatible(&t)), - ArtifactName::SDist(_) => false, - }); + if self.options.sdist_resolution.allow_wheels() { + if let Some(compatible_tags) = self.compatible_tags { + wheels.retain(|artifact| match &artifact.filename { + ArtifactName::Wheel(wheel_name) => wheel_name + .all_tags_iter() + .any(|t| compatible_tags.is_compatible(&t)), + ArtifactName::SDist(_) => false, + }); + + // Sort the artifacts from most compatible to least compatible, this ensures that we + // check the most compatible artifacts for dependencies first. + // this only needs to be done for wheels + wheels.sort_by_cached_key(|a| { + -a.filename + .as_wheel() + .expect("only wheels are considered") + .all_tags_iter() + .filter_map(|tag| compatible_tags.compatibility(&tag)) + .max() + .unwrap_or(0) + }); + } - // Sort the artifacts from most compatible to least compatible, this ensures that we - // check the most compatible artifacts for dependencies first. - // this only needs to be done for wheels - wheels.sort_by_cached_key(|a| { - -a.filename - .as_wheel() - .expect("only wheels are considered") - .all_tags_iter() - .filter_map(|tag| compatible_tags.compatibility(&tag)) - .max() - .unwrap_or(0) - }); + if !self.options.sdist_resolution.allow_sdists() && wheels.is_empty() { + return Err( + "none of the artifacts are compatible with the Python interpreter or glibc version", + ); + } + + if wheels.is_empty() && sdists.is_empty() { + return Err("none of the artifacts are compatible with the Python interpreter or glibc version and there are no supported sdists"); + } } // Append these together @@ -229,13 +275,19 @@ impl<'db, 'i> PypiDependencyProvider<'db, 'i> { let artifacts = wheels; if artifacts.is_empty() { - return Err( - "none of the artifacts are compatible with the Python interpreter or glibc version", - ); + return Err("there are no supported artifacts"); } Ok(artifacts) } + + fn solvable_has_artifact_type(&self, solvable_id: SolvableId) -> bool { + self.cached_artifacts + .get(&solvable_id) + .unwrap_or(&[]) + .iter() + .any(|a| a.is::()) + } } impl<'p> DependencyProvider @@ -251,6 +303,27 @@ impl<'p> DependencyProvider solvables: &mut [SolvableId], ) { solvables.sort_by(|&a, &b| { + // First sort the solvables based on the artifact types we have available for them and + // whether some of them are preferred. If one artifact type is preferred over another + // we sort those versions above the others even if the versions themselves are lower. + if matches!(self.options.sdist_resolution, SDistResolution::PreferWheels) { + let a_has_wheels = self.solvable_has_artifact_type::(a); + let b_has_wheels = self.solvable_has_artifact_type::(b); + match (a_has_wheels, b_has_wheels) { + (true, false) => return Ordering::Less, + (false, true) => return Ordering::Greater, + _ => {} + } + } else if matches!(self.options.sdist_resolution, SDistResolution::PreferSDists) { + let a_has_sdists = self.solvable_has_artifact_type::(a); + let b_has_sdists = self.solvable_has_artifact_type::(b); + match (a_has_sdists, b_has_sdists) { + (true, false) => return Ordering::Less, + (false, true) => return Ordering::Greater, + _ => {} + } + } + let solvable_a = solver.pool().resolve_solvable(a); let solvable_b = solver.pool().resolve_solvable(b); @@ -391,26 +464,27 @@ impl<'p> DependencyProvider return dependencies; } - let metadata = task::block_in_place(|| { + let Some((_, metadata)) = task::block_in_place(|| { // First try getting wheels - let result = Handle::current() - .block_on(self.package_db.get_metadata::(artifacts)) - .unwrap(); - - match result { - None => { - // If nothing is found then try the sdists - let (_, metadata) = Handle::current() - .block_on(self.package_db.get_metadata::(artifacts)) - .unwrap() - .expect("no metadata found for sdist or wheels"); - - tracing::info!("found metadata for sdist: {:?}", metadata); - metadata - } - Some((_, metadata)) => metadata, - } - }); + Handle::current() + .block_on( + self.package_db + .get_metadata::(artifacts) + .and_then(|result| match result { + None => self + .package_db + .get_metadata::(artifacts) + .left_future(), + result => ready(Ok(result)).right_future(), + }), + ) + .unwrap() + }) else { + panic!( + "could not find metadata for any sdist or wheel for {} {}. The following artifacts are available:\n{}", + package_name, package_version, artifacts.iter().format_with("\n", |a, f| f(&format_args!("- {}", a.filename))) + ); + }; // Add constraints that restrict that the extra packages are set to the same version. if let PypiPackageName::Base(package_name) = package_name { @@ -481,7 +555,7 @@ impl<'p> DependencyProvider } /// Represents a single locked down distribution (python package) after calling [`resolve`]. -#[derive(Debug)] +#[derive(Debug, Clone)] pub struct PinnedPackage<'db> { /// The name of the package pub name: NormalizedPackageName, @@ -499,6 +573,124 @@ pub struct PinnedPackage<'db> { pub artifacts: Vec<&'db ArtifactInfo>, } +/// Defines how to handle sdists during resolution. +#[derive(Default, Clone, Copy, Eq, PartialOrd, PartialEq)] +pub enum SDistResolution { + /// Both versions with wheels and/or sdists are allowed to be selected during resolution. But + /// during resolution the metadata from wheels is preferred over sdists. + /// + /// If we have the following scenario: + /// + /// ```txt + /// Version@1 + /// - WheelA + /// - WheelB + /// Version@2 + /// - SDist + /// - WheelA + /// - WheelB + /// Version@3 + /// - SDist + /// ``` + /// + /// Then the Version@3 will be selected because it has the highest version. This option makes no + /// distinction between whether the version has wheels or sdist. + #[default] + Normal, + + /// Allow sdists to be selected during resolution but only if all versions with wheels cannot + /// be selected. This means that even if a higher version is technically available it might not + /// be selected if it only has an available sdist. + /// + /// If we have the following scenario: + /// + /// ```txt + /// Version@1 + /// - SDist + /// - WheelA + /// - WheelB + /// Version@2 + /// - SDist + /// ``` + /// + /// Then the Version@1 will be selected even though the highest version is 2. This is because + /// version 2 has no available wheels. If version 1 would not exist though then version 2 is + /// selected because there are no other versions with a wheel. + PreferWheels, + + /// Allow sdists to be selected during resolution and prefer them over wheels. This means that + /// even if a higher version is available but it only includes wheels it might not be selected. + /// + /// If we have the following scenario: + /// + /// ```txt + /// Version@1 + /// - SDist + /// - WheelA + /// Version@2 + /// - WheelA + /// ``` + /// + /// Then the version@1 will be selected even though the highest version is 2. This is because + /// version 2 has no sdists available. If version 1 would not exist though then version 2 is + /// selected because there are no other versions with an sdist. + PreferSDists, + + /// Don't select sdists during resolution + /// + /// If we have the following scenario: + /// + /// ```txt + /// Version@1 + /// - SDist + /// - WheelA + /// - WheelB + /// Version@2 + /// - SDist + /// ``` + /// + /// Then version 1 will be selected because it has wheels and version 2 does not. If version 1 + /// would not exist there would be no solution because none of the versions have wheels. + OnlyWheels, + + /// Only select sdists during resolution + /// + /// If we have the following scenario: + /// + /// ```txt + /// Version@1 + /// - SDist + /// Version@2 + /// - WheelA + /// ``` + /// + /// Then version 1 will be selected because it has an sdist and version 2 does not. If version 1 + /// would not exist there would be no solution because none of the versions have sdists. + OnlySDists, +} + +impl SDistResolution { + /// Returns true if sdists are allowed to be selected during resolution + fn allow_sdists(&self) -> bool { + !matches!(self, SDistResolution::OnlyWheels) + } + + /// Returns true if sdists are allowed to be selected during resolution + fn allow_wheels(&self) -> bool { + !matches!(self, SDistResolution::OnlySDists) + } +} + +/// Additional options that may influence the solver. In general passing [`Default::default`] to +/// the [`resolve`] function should provide sane defaults, however if you want to fine tune the +/// resolver you can do so via this struct. +#[derive(Default, Clone)] +pub struct ResolveOptions { + /// Defines how to handle sdists during resolution. By default sdists will be treated the same + /// as wheels. + pub sdist_resolution: SDistResolution, +} + /// Resolves an environment that contains the given requirements and all dependencies of those /// requirements. /// @@ -516,6 +708,7 @@ pub async fn resolve<'db>( compatible_tags: Option<&WheelTags>, locked_packages: HashMap>, favored_packages: HashMap>, + options: &ResolveOptions, ) -> miette::Result>> { // Construct a provider let provider = PypiDependencyProvider::new( @@ -524,6 +717,7 @@ pub async fn resolve<'db>( compatible_tags, locked_packages, favored_packages, + options, )?; let pool = &provider.pool; @@ -604,3 +798,6 @@ pub async fn resolve<'db>( Ok(result.into_values().collect()) } + +#[cfg(test)] +mod test {} diff --git a/crates/rattler_installs_packages/tests/resolver.rs b/crates/rattler_installs_packages/tests/resolver.rs new file mode 100644 index 00000000..1522adf9 --- /dev/null +++ b/crates/rattler_installs_packages/tests/resolver.rs @@ -0,0 +1,164 @@ +#![cfg(feature = "resolvo")] + +use pep508_rs::{MarkerEnvironment, Requirement}; +use rattler_installs_packages::{ + resolve, + tags::{WheelTag, WheelTags}, + NormalizedPackageName, PackageDb, PinnedPackage, ResolveOptions, SDistResolution, +}; +use std::{collections::HashMap, path::Path, str::FromStr, sync::OnceLock}; + +#[tokio::test(flavor = "multi_thread")] +async fn no_sdists() { + let error = ResolveBuilder::default() + .with_requirement("sdist") + .with_sdist_resolution(SDistResolution::OnlyWheels) + .resolve() + .await + .unwrap_err(); + + insta::assert_display_snapshot!(error) +} + +/// Tests that the `SDistResolution::PreferWheels` option selects the highest version with a wheel +/// over any version with an sdist. +/// +/// PySDL2 is a package that has newer versions that only have sdists available. +#[tokio::test(flavor = "multi_thread")] +async fn prefer_wheel() { + let result = ResolveBuilder::default() + .with_requirement("pysdl2") + .with_sdist_resolution(SDistResolution::PreferWheels) + .resolve() + .await; + + // Get the pysdl2 package from the resolution + let packages = result.expect("expected a valid solution"); + let pysdl_pkg = packages + .iter() + .find(|p| p.name.as_str() == "pysdl2") + .unwrap(); + + // A version should be selected that has wheels (which is not the latest version!) + assert_eq!(pysdl_pkg.version.to_string(), "0.9.12"); +} + +/// Returns a package database that uses pypi as its index. The cache directory is stored in the +/// `target/` folder to make it easier to share the cache between tests. +/// TODO: Instead of relying on the public mutable pypi index, it would be very nice to have a copy +/// locally that we can run tests against. +fn package_database() -> &'static PackageDb { + static PACKAGE_DB: OnceLock = OnceLock::new(); + PACKAGE_DB.get_or_init(|| { + PackageDb::new( + Default::default(), + &["https://pypi.org/simple/".parse().unwrap()], + Path::new(env!("CARGO_TARGET_TMPDIR")).join("pypi-cache"), + ) + .unwrap() + }) +} + +/// Returns a `MarkerEnvironment` instance for a Windows system. +pub fn win_environment_markers() -> MarkerEnvironment { + MarkerEnvironment { + implementation_name: "cpython".to_string(), + implementation_version: "3.10.4".parse().unwrap(), + os_name: "nt".to_string(), + platform_machine: "AMD64".to_string(), + platform_python_implementation: "CPython".to_string(), + platform_release: "10".to_string(), + platform_system: "Windows".to_string(), + platform_version: "10.0.22635".to_string(), + python_full_version: "3.10.4".parse().unwrap(), + python_version: "3.10".parse().unwrap(), + sys_platform: "win32".to_string(), + } +} + +/// Returns `WheelTags` instance for a Windows system. +pub fn win_compatible_tags() -> WheelTags { + [ + "cp310-cp310-win_amd64", + "cp310-abi3-win_amd64", + "cp310-none-win_amd64", + "cp39-abi3-win_amd64", + "cp38-abi3-win_amd64", + "cp37-abi3-win_amd64", + "cp36-abi3-win_amd64", + "cp35-abi3-win_amd64", + "cp34-abi3-win_amd64", + "cp33-abi3-win_amd64", + "cp32-abi3-win_amd64", + "py310-none-win_amd64", + "py3-none-win_amd64", + "py39-none-win_amd64", + "py38-none-win_amd64", + "py37-none-win_amd64", + "py36-none-win_amd64", + "py35-none-win_amd64", + "py34-none-win_amd64", + "py33-none-win_amd64", + "py32-none-win_amd64", + "py31-none-win_amd64", + "py30-none-win_amd64", + "cp310-none-any", + "py310-none-any", + "py3-none-any", + "py39-none-any", + "py38-none-any", + "py37-none-any", + "py36-none-any", + "py35-none-any", + "py34-none-any", + "py33-none-any", + "py32-none-any", + "py31-none-any", + "py30-none-any", + ] + .iter() + .map(|s| WheelTag::from_str(s).unwrap()) + .collect() +} + +/// A helper struct that makes writing tests easier. This struct allows customizing the a resolve +/// task without having to specify all the parameters required. If a parameter is not specified a +/// sane default is chosen. +#[derive(Default, Clone)] +struct ResolveBuilder { + requirements: Vec, + marker_env: Option, + compatible_tags: Option>, + locked_packages: HashMap>, + pinned_packages: HashMap>, + options: ResolveOptions, +} + +impl ResolveBuilder { + pub fn with_requirement(mut self, req: &str) -> Self { + let req = Requirement::from_str(req).unwrap(); + self.requirements.push(req); + self + } + + pub fn with_sdist_resolution(mut self, sdist_resolution: SDistResolution) -> Self { + self.options.sdist_resolution = sdist_resolution; + self + } + + pub async fn resolve(self) -> Result>, String> { + resolve( + package_database(), + self.requirements.iter(), + &self.marker_env.unwrap_or(win_environment_markers()), + self.compatible_tags + .unwrap_or(Some(win_compatible_tags())) + .as_ref(), + self.locked_packages, + self.pinned_packages, + &self.options, + ) + .await + .map_err(|e| e.to_string()) + } +} diff --git a/crates/rattler_installs_packages/tests/snapshots/resolver__no_sdists.snap b/crates/rattler_installs_packages/tests/snapshots/resolver__no_sdists.snap new file mode 100644 index 00000000..d97eefdb --- /dev/null +++ b/crates/rattler_installs_packages/tests/snapshots/resolver__no_sdists.snap @@ -0,0 +1,8 @@ +--- +source: crates/rattler_installs_packages/tests/resolver.rs +assertion_line: 20 +expression: error +--- +The following packages are incompatible +|-- sdist * cannot be installed because there are no viable options: + |-- sdist 0.0.0 is excluded because there are no wheels available diff --git a/crates/rip_bin/src/main.rs b/crates/rip_bin/src/main.rs index b526c03e..f794f29b 100644 --- a/crates/rip_bin/src/main.rs +++ b/crates/rip_bin/src/main.rs @@ -11,7 +11,9 @@ use tracing_subscriber::{fmt, layer::SubscriberExt, util::SubscriberInitExt, Env use url::Url; use rattler_installs_packages::tags::WheelTags; -use rattler_installs_packages::{normalize_index_url, resolve, Pep508EnvMakers, Requirement}; +use rattler_installs_packages::{ + normalize_index_url, resolve, Pep508EnvMakers, Requirement, ResolveOptions, +}; #[derive(Parser)] #[command(author, version, about, long_about = None)] @@ -26,6 +28,45 @@ struct Args { #[clap(short)] verbose: bool, + + #[clap(flatten)] + sdist_resolution: SDistResolution, +} + +#[derive(Parser)] +#[group(multiple = false)] +struct SDistResolution { + /// Prefer any version with wheels over any version with sdists + #[clap(long)] + prefer_wheels: bool, + + /// Prefer any version with sdists over any version with wheels + #[clap(long)] + prefer_sdists: bool, + + /// Only select versions with wheels, ignore versions with sdists + #[clap(long)] + only_wheels: bool, + + /// Only select versions with sdists, ignore versions with wheels + #[clap(long)] + only_sdists: bool, +} + +impl From for rattler_installs_packages::SDistResolution { + fn from(value: SDistResolution) -> Self { + if value.only_sdists { + rattler_installs_packages::SDistResolution::OnlySDists + } else if value.only_wheels { + rattler_installs_packages::SDistResolution::OnlyWheels + } else if value.prefer_sdists { + rattler_installs_packages::SDistResolution::PreferSDists + } else if value.prefer_wheels { + rattler_installs_packages::SDistResolution::PreferWheels + } else { + rattler_installs_packages::SDistResolution::Normal + } + } } async fn actual_main() -> miette::Result<()> { @@ -86,6 +127,9 @@ async fn actual_main() -> miette::Result<()> { Some(&compatible_tags), HashMap::default(), HashMap::default(), + &ResolveOptions { + sdist_resolution: args.sdist_resolution.into(), + }, ) .await {