From fed10dc3fdaa6d3816065b5baaf90a9bcb948c41 Mon Sep 17 00:00:00 2001 From: Ruben Arts Date: Mon, 12 Feb 2024 16:22:05 +0100 Subject: [PATCH 1/8] feature: add --feature to the add cli --- docs/cli.md | 2 + src/cli/add.rs | 99 ++++++++----- src/environment.rs | 19 ++- src/project/manifest/feature.rs | 5 +- src/project/manifest/mod.rs | 135 +++++++++++++++++- ...ject__manifest__tests__add_dependency.snap | 24 ++++ src/project/mod.rs | 18 +++ tests/common/mod.rs | 1 + 8 files changed, 259 insertions(+), 44 deletions(-) create mode 100644 src/project/manifest/snapshots/pixi__project__manifest__tests__add_dependency.snap diff --git a/docs/cli.md b/docs/cli.md index 529c2dba8..54a75b834 100644 --- a/docs/cli.md +++ b/docs/cli.md @@ -58,6 +58,7 @@ It will only add if the package with its version constraint is able to work with - `--no-install`: Don't install the package to the environment, only add the package to the lock-file. - `--no-lockfile-update`: Don't update the lock-file, implies the `--no-install` flag. - `--platform (-p)`: The platform for which the dependency should be added. (Allowed to be used more than once) +- `--feature (-f)`: The feature for which the dependency should be added. ```shell pixi add numpy @@ -70,6 +71,7 @@ pixi add --pypi requests[security] pixi add --platform osx-64 --build clang pixi add --no-install numpy pixi add --no-lockfile-update numpy +pixi add --feature featurex numpy ``` ## `install` diff --git a/src/cli/add.rs b/src/cli/add.rs index ced94b1fd..b9311398b 100644 --- a/src/cli/add.rs +++ b/src/cli/add.rs @@ -7,6 +7,7 @@ use crate::{ use clap::Parser; use itertools::{Either, Itertools}; +use crate::environment::GroupedEnvironment; use indexmap::IndexMap; use miette::{IntoDiagnostic, WrapErr}; use rattler_conda_types::{ @@ -86,6 +87,10 @@ pub struct Args { /// The platform(s) for which the dependency should be added #[arg(long, short)] pub platform: Vec, + + /// The feature for which the dependency should be added + #[arg(long, short)] + pub feature: Option, } impl DependencyType { @@ -126,6 +131,10 @@ pub async fn execute(args: Args) -> miette::Result<()> { .manifest .add_platforms(platforms_to_add.iter(), &FeatureName::Default)?; + let feature_name = args + .feature + .map_or(FeatureName::Default, FeatureName::Named); + match dependency_type { DependencyType::CondaDependency(spec_type) => { let specs = args @@ -137,6 +146,7 @@ pub async fn execute(args: Args) -> miette::Result<()> { .into_diagnostic()?; add_conda_specs_to_project( &mut project, + &feature_name, specs, spec_type, args.no_install, @@ -247,6 +257,7 @@ pub async fn add_pypi_specs_to_project( pub async fn add_conda_specs_to_project( project: &mut Project, + feature_name: &FeatureName, specs: Vec, spec_type: SpecType, no_install: bool, @@ -268,45 +279,62 @@ pub async fn add_conda_specs_to_project( // Determine the best version per platform let mut package_versions = HashMap::>::new(); - let platforms = if specs_platforms.is_empty() { - Either::Left(project.platforms().into_iter()) - } else { - Either::Right(specs_platforms.iter().copied()) - }; + // Get the grouped environments that contain the feature + let grouped_environments: Vec = project + .grouped_environments() + .iter() + .filter(|env| { + env.features() + .map(|feat| &feat.name) + .contains(&feature_name) + }) + .cloned() + .collect(); + + // TODO: show progress of this set of solves + // TODO: Make this parallel + // TODO: Make this more efficient by reusing the solves in the get_up_to_date_prefix + for grouped_environment in grouped_environments { + let platforms = if specs_platforms.is_empty() { + Either::Left(grouped_environment.platforms().into_iter()) + } else { + Either::Right(specs_platforms.iter().copied()) + }; - for platform in platforms { - // Solve the environment with the new specs added - let solved_versions = match determine_best_version( - project, - &new_specs, - spec_type, - &sparse_repo_data, - platform, - ) { - Ok(versions) => versions, - Err(err) => { - return Err(err).wrap_err_with(|| miette::miette!( + for platform in platforms { + // Solve the environment with the new specs added + let solved_versions = match determine_best_version( + &grouped_environment, + &new_specs, + spec_type, + &sparse_repo_data, + platform, + ) { + Ok(versions) => versions, + Err(err) => { + return Err(err).wrap_err_with(|| miette::miette!( "could not determine any available versions for {} on {platform}. Either the package could not be found or version constraints on other dependencies result in a conflict.", new_specs.keys().map(|s| s.as_source()).join(", ") )); - } - }; + } + }; - // Collect all the versions seen. - for (name, version) in solved_versions { - package_versions.entry(name).or_default().insert(version); + // Collect all the versions seen. + for (name, version) in solved_versions { + package_versions.entry(name).or_default().insert(version); + } } } // Update the specs passed on the command line with the best available versions. for (name, spec) in new_specs { - let versions_seen = package_versions - .get(&name) - .cloned() - .expect("a version must have been previously selected"); let updated_spec = if spec.version.is_none() { let mut updated_spec = spec.clone(); - updated_spec.version = determine_version_constraint(&versions_seen); + if let Some(versions_seen) = package_versions.get(&name).cloned() { + updated_spec.version = determine_version_constraint(&versions_seen); + } else { + updated_spec.version = Some(VersionSpec::Any); + } updated_spec } else { spec @@ -315,12 +343,14 @@ pub async fn add_conda_specs_to_project( // Add the dependency to the project if specs_platforms.is_empty() { - project.manifest.add_dependency(&spec, spec_type, None)?; + project + .manifest + .add_dependency(&spec, spec_type, None, feature_name)?; } else { for platform in specs_platforms.iter() { project .manifest - .add_dependency(&spec, spec_type, Some(*platform))?; + .add_dependency(&spec, spec_type, Some(*platform), feature_name)?; } } } @@ -330,6 +360,7 @@ pub async fn add_conda_specs_to_project( LockFileUsage::Update }; + // Update the prefix get_up_to_date_prefix( &project.default_environment(), lock_file_usage, @@ -337,6 +368,7 @@ pub async fn add_conda_specs_to_project( sparse_repo_data, ) .await?; + project.save()?; Ok(()) @@ -344,7 +376,7 @@ pub async fn add_conda_specs_to_project( /// Given several specs determines the highest installable version for them. pub fn determine_best_version( - project: &Project, + environment: &GroupedEnvironment, new_specs: &HashMap, new_specs_type: SpecType, sparse_repo_data: &IndexMap<(Channel, Platform), SparseRepoData>, @@ -353,7 +385,7 @@ pub fn determine_best_version( // Build the combined set of specs while updating the dependencies with the new specs. let dependencies = SpecType::all() .map(|spec_type| { - let mut deps = project.dependencies(Some(spec_type), Some(platform)); + let mut deps = environment.dependencies(Some(spec_type), Some(platform)); if spec_type == new_specs_type { for (new_name, new_spec) in new_specs.iter() { deps.remove(new_name); // Remove any existing specs @@ -369,7 +401,7 @@ pub fn determine_best_version( let package_names = dependencies.names().cloned().collect_vec(); // Get the repodata for the current platform and for NoArch - let platform_sparse_repo_data = project + let platform_sparse_repo_data = environment .channels() .into_iter() .cloned() @@ -393,9 +425,8 @@ pub fn determine_best_version( available_packages: &available_packages, - virtual_packages: project.virtual_packages(platform), + virtual_packages: environment.virtual_packages(platform), - // TODO: Add the information from the current lock file here. locked_packages: vec![], pinned_packages: vec![], diff --git a/src/environment.rs b/src/environment.rs index e3ffd3e94..1f93d2fca 100644 --- a/src/environment.rs +++ b/src/environment.rs @@ -2,7 +2,7 @@ use miette::{Context, IntoDiagnostic}; use crate::lock_file::{resolve_pypi, LockedCondaPackages, LockedPypiPackages}; use crate::project::virtual_packages::get_minimal_virtual_packages; -use crate::project::{Dependencies, SolveGroup}; +use crate::project::{manifest, Dependencies, SolveGroup}; use crate::{ config, consts, install, install_pypi, lock_file, lock_file::{ @@ -1828,12 +1828,29 @@ impl<'p> GroupedEnvironment<'p> { } } + pub fn platforms(&self) -> HashSet { + match self { + GroupedEnvironment::Group(group) => group + .environments() + .flat_map(|env| env.platforms()) + .collect(), + GroupedEnvironment::Environment(env) => env.platforms(), + } + } + pub fn has_pypi_dependencies(&self) -> bool { match self { GroupedEnvironment::Group(group) => group.has_pypi_dependencies(), GroupedEnvironment::Environment(env) => env.has_pypi_dependencies(), } } + + pub fn features(&self) -> Box + 'p> { + match self { + GroupedEnvironment::Group(group) => Box::new(group.features(true)), + GroupedEnvironment::Environment(env) => Box::new(env.features(true)), + } + } } /// Given a list of dependencies, and list of conda repodata records by package name recursively extract all the diff --git a/src/project/manifest/feature.rs b/src/project/manifest/feature.rs index a629326e1..4a1e1d046 100644 --- a/src/project/manifest/feature.rs +++ b/src/project/manifest/feature.rs @@ -16,8 +16,9 @@ use std::collections::HashMap; use std::fmt; /// The name of a feature. This is either a string or default for the default feature. -#[derive(Clone, Debug, Eq, PartialEq, PartialOrd, Ord, Hash, Serialize)] +#[derive(Clone, Debug, Eq, PartialEq, PartialOrd, Ord, Hash, Serialize, Default)] pub enum FeatureName { + #[default] Default, Named(String), } @@ -99,7 +100,7 @@ impl fmt::Display for FeatureName { /// /// Individual features cannot be used directly, instead they are grouped together into /// environments. Environments are then locked and installed. -#[derive(Debug, Clone)] +#[derive(Debug, Clone, Default)] pub struct Feature { /// The name of the feature or `None` if the feature is the default feature. pub name: FeatureName, diff --git a/src/project/manifest/mod.rs b/src/project/manifest/mod.rs index 276b01487..dfcab0806 100644 --- a/src/project/manifest/mod.rs +++ b/src/project/manifest/mod.rs @@ -338,14 +338,11 @@ impl Manifest { spec: &MatchSpec, spec_type: SpecType, platform: Option, + feature_name: &FeatureName, ) -> miette::Result<()> { // Find the table toml table to add the dependency to. - let dependency_table = get_or_insert_toml_table( - &mut self.document, - platform, - &FeatureName::Default, - spec_type.name(), - )?; + let dependency_table = + get_or_insert_toml_table(&mut self.document, platform, feature_name, spec_type.name())?; // Determine the name of the package to add let (Some(name), spec) = spec.clone().into_nameless() else { @@ -356,7 +353,10 @@ impl Manifest { dependency_table.insert(name.as_source(), Item::Value(spec.to_string().into())); // Add the dependency to the manifest as well - self.default_feature_mut() + self.parsed + .features + .entry(feature_name.clone()) + .or_default() .targets .for_opt_target_or_default_mut(platform.map(TargetSelector::from).as_ref()) .dependencies @@ -364,6 +364,14 @@ impl Manifest { .or_default() .insert(name, spec); + // self.default_feature_mut() + // .targets + // .for_opt_target_or_default_mut(platform.map(TargetSelector::from).as_ref()) + // .dependencies + // .entry(spec_type) + // .or_default() + // .insert(name, spec); + Ok(()) } @@ -2351,4 +2359,117 @@ test = "test initial" ); assert_display_snapshot!(manifest.document.to_string()); } + + #[test] + fn test_add_dependency() { + let file_contents = r#" +[project] +name = "foo" +channels = [] +platforms = ["linux-64", "win-64"] + +[dependencies] +foo = "*" + +[feature.test.dependencies] +bar = "*" + "#; + let mut manifest = Manifest::from_str(Path::new(""), file_contents).unwrap(); + manifest + .add_dependency( + &MatchSpec::from_str(" baz >=1.2.3").unwrap(), + SpecType::Run, + None, + &FeatureName::Default, + ) + .unwrap(); + assert_eq!( + manifest + .default_feature() + .targets + .default() + .dependencies + .get(&SpecType::Run) + .unwrap() + .get(&PackageName::from_str("baz").unwrap()) + .unwrap() + .to_string(), + ">=1.2.3".to_string() + ); + manifest + .add_dependency( + &MatchSpec::from_str(" bal >=2.3").unwrap(), + SpecType::Run, + None, + &FeatureName::Named("test".to_string()), + ) + .unwrap(); + + assert_eq!( + manifest + .feature(&FeatureName::Named("test".to_string())) + .unwrap() + .targets + .default() + .dependencies + .get(&SpecType::Run) + .unwrap() + .get(&PackageName::from_str("bal").unwrap()) + .unwrap() + .to_string(), + ">=2.3".to_string() + ); + + manifest + .add_dependency( + &MatchSpec::from_str(" boef >=2.3").unwrap(), + SpecType::Run, + Some(Platform::Linux64), + &FeatureName::Named("extra".to_string()), + ) + .unwrap(); + + assert_eq!( + manifest + .feature(&FeatureName::Named("extra".to_string())) + .unwrap() + .targets + .for_target(&TargetSelector::Platform(Platform::Linux64)) + .unwrap() + .dependencies + .get(&SpecType::Run) + .unwrap() + .get(&PackageName::from_str("boef").unwrap()) + .unwrap() + .to_string(), + ">=2.3".to_string() + ); + + manifest + .add_dependency( + &MatchSpec::from_str(" cmake >=2.3").unwrap(), + SpecType::Build, + Some(Platform::Linux64), + &FeatureName::Named("build".to_string()), + ) + .unwrap(); + + assert_eq!( + manifest + .feature(&FeatureName::Named("build".to_string())) + .unwrap() + .targets + .for_target(&TargetSelector::Platform(Platform::Linux64)) + .unwrap() + .dependencies + .get(&SpecType::Build) + .unwrap() + .get(&PackageName::from_str("cmake").unwrap()) + .unwrap() + .to_string(), + ">=2.3".to_string() + ); + + assert_display_snapshot!(manifest.document.to_string()); + } } diff --git a/src/project/manifest/snapshots/pixi__project__manifest__tests__add_dependency.snap b/src/project/manifest/snapshots/pixi__project__manifest__tests__add_dependency.snap new file mode 100644 index 000000000..740dff8c3 --- /dev/null +++ b/src/project/manifest/snapshots/pixi__project__manifest__tests__add_dependency.snap @@ -0,0 +1,24 @@ +--- +source: src/project/manifest/mod.rs +expression: manifest.document.to_string() +--- + +[project] +name = "foo" +channels = [] +platforms = ["linux-64", "win-64"] + +[dependencies] +foo = "*" +baz = ">=1.2.3" + +[feature.test.dependencies] +bar = "*" +bal = ">=2.3" + +[feature.extra.target.linux-64.dependencies] +boef = ">=2.3" + +[feature.build.target.linux-64.build-dependencies] +cmake = ">=2.3" + diff --git a/src/project/mod.rs b/src/project/mod.rs index b498b23d4..1225f9c29 100644 --- a/src/project/mod.rs +++ b/src/project/mod.rs @@ -31,6 +31,7 @@ use crate::{ use manifest::{EnvironmentName, Manifest, PyPiRequirement, SystemRequirements}; use url::Url; +use crate::environment::GroupedEnvironment; use crate::task::TaskName; pub use dependencies::Dependencies; pub use environment::Environment; @@ -290,6 +291,23 @@ impl Project { }) } + /// Return the grouped environments, which are all solve-groups and the environments that need to be solved. + pub fn grouped_environments(&self) -> Vec { + let mut environments = self + .environments() + .into_iter() + .filter(|env| env.solve_group().is_none()) + .map(GroupedEnvironment::from) + .collect::>(); + let groups: Vec = self + .solve_groups() + .into_iter() + .map(GroupedEnvironment::from) + .collect(); + environments.extend(groups); + environments + } + /// Returns the channels used by this project. /// /// TODO: Remove this function and use the channels from the default environment instead. diff --git a/tests/common/mod.rs b/tests/common/mod.rs index 68faefd10..9f9d1562f 100644 --- a/tests/common/mod.rs +++ b/tests/common/mod.rs @@ -213,6 +213,7 @@ impl PixiControl { no_lockfile_update: false, platform: Default::default(), pypi: false, + feature: None, }, } } From e34d15f3d17f30ab3235c6a94285ce0247ca418a Mon Sep 17 00:00:00 2001 From: Ruben Arts Date: Tue, 13 Feb 2024 09:42:29 +0100 Subject: [PATCH 2/8] feat: add latest version of no env is available. --- src/cli/add.rs | 38 +++++++++++++++++++++++++++++++++++++- 1 file changed, 37 insertions(+), 1 deletion(-) diff --git a/src/cli/add.rs b/src/cli/add.rs index b9311398b..12f9cb409 100644 --- a/src/cli/add.rs +++ b/src/cli/add.rs @@ -333,7 +333,43 @@ pub async fn add_conda_specs_to_project( if let Some(versions_seen) = package_versions.get(&name).cloned() { updated_spec.version = determine_version_constraint(&versions_seen); } else { - updated_spec.version = Some(VersionSpec::Any); + // If we didn't find any versions, we'll just use the latest version we can find in the repodata. + let mut found_records = Vec::new(); + + // Get platforms to search for including NoArch + let platforms = if specs_platforms.is_empty() { + let mut temp = project.platforms().into_iter().collect_vec(); + temp.push(Platform::NoArch); + temp + } else { + let mut temp = specs_platforms.clone(); + temp.push(Platform::NoArch); + temp + }; + + // Search for the package in the all the channels and platforms + for channel in project.channels() { + for platform in &platforms { + let sparse_repo_data = sparse_repo_data.get(&(channel.clone(), *platform)); + if let Some(sparse_repo_data) = sparse_repo_data { + let records = sparse_repo_data.load_records(&name).into_diagnostic()?; + // Add max of every channel and platform + if let Some(max_record) = records.into_iter().max_by_key(|record| { + record.package_record.version.version().clone() + }) { + found_records.push(max_record); + } + }; + } + } + + // Determine the version constraint based on the max of every channel and platform. + updated_spec.version = determine_version_constraint( + &found_records + .iter() + .map(|record| record.package_record.version.version().clone()) + .collect_vec(), + ); } updated_spec } else { From 0f1feb6996ddf2a1478b71b19143452670333226 Mon Sep 17 00:00:00 2001 From: Ruben Arts Date: Thu, 22 Feb 2024 14:40:06 +0100 Subject: [PATCH 3/8] fix: make grouped envs unique --- src/project/manifest/mod.rs | 8 -------- src/project/mod.rs | 3 ++- 2 files changed, 2 insertions(+), 9 deletions(-) diff --git a/src/project/manifest/mod.rs b/src/project/manifest/mod.rs index 13b132a95..0837cca50 100644 --- a/src/project/manifest/mod.rs +++ b/src/project/manifest/mod.rs @@ -367,14 +367,6 @@ impl Manifest { .or_default() .insert(name, spec); - // self.default_feature_mut() - // .targets - // .for_opt_target_or_default_mut(platform.map(TargetSelector::from).as_ref()) - // .dependencies - // .entry(spec_type) - // .or_default() - // .insert(name, spec); - Ok(()) } diff --git a/src/project/mod.rs b/src/project/mod.rs index 8520d99af..2aa9aaad2 100644 --- a/src/project/mod.rs +++ b/src/project/mod.rs @@ -7,6 +7,7 @@ mod solve_group; pub mod virtual_packages; use indexmap::{Equivalent, IndexMap, IndexSet}; +use itertools::Itertools; use miette::{IntoDiagnostic, NamedSource, WrapErr}; use once_cell::sync::OnceCell; use rattler_conda_types::{Channel, GenericVirtualPackage, Platform, Version}; @@ -312,7 +313,7 @@ impl Project { .map(GroupedEnvironment::from) .collect(); environments.extend(groups); - environments + environments.into_iter().unique().collect() } /// Returns the channels used by this project. From 6d9e63b909e47a29453b4f81f4917b63ee1dd6de Mon Sep 17 00:00:00 2001 From: Ruben Arts Date: Thu, 22 Feb 2024 15:06:26 +0100 Subject: [PATCH 4/8] chore: move determine latest into a function --- src/cli/add.rs | 84 +++++++++++++++++++++---------------- src/project/manifest/mod.rs | 2 + 2 files changed, 49 insertions(+), 37 deletions(-) diff --git a/src/cli/add.rs b/src/cli/add.rs index c8b155ddf..e54b160bc 100644 --- a/src/cli/add.rs +++ b/src/cli/add.rs @@ -333,43 +333,8 @@ pub async fn add_conda_specs_to_project( if let Some(versions_seen) = package_versions.get(&name).cloned() { updated_spec.version = determine_version_constraint(&versions_seen); } else { - // If we didn't find any versions, we'll just use the latest version we can find in the repodata. - let mut found_records = Vec::new(); - - // Get platforms to search for including NoArch - let platforms = if specs_platforms.is_empty() { - let mut temp = project.platforms().into_iter().collect_vec(); - temp.push(Platform::NoArch); - temp - } else { - let mut temp = specs_platforms.clone(); - temp.push(Platform::NoArch); - temp - }; - - // Search for the package in the all the channels and platforms - for channel in project.channels() { - for platform in &platforms { - let sparse_repo_data = sparse_repo_data.get(&(channel.clone(), *platform)); - if let Some(sparse_repo_data) = sparse_repo_data { - let records = sparse_repo_data.load_records(&name).into_diagnostic()?; - // Add max of every channel and platform - if let Some(max_record) = records.into_iter().max_by_key(|record| { - record.package_record.version.version().clone() - }) { - found_records.push(max_record); - } - }; - } - } - - // Determine the version constraint based on the max of every channel and platform. - updated_spec.version = determine_version_constraint( - &found_records - .iter() - .map(|record| record.package_record.version.version().clone()) - .collect_vec(), - ); + updated_spec.version = + determine_latest_version(project, specs_platforms, &sparse_repo_data, &name)?; } updated_spec } else { @@ -410,6 +375,51 @@ pub async fn add_conda_specs_to_project( Ok(()) } +fn determine_latest_version( + project: &Project, + platforms: &Vec, + sparse_repo_data: &IndexMap<(Channel, Platform), SparseRepoData>, + name: &PackageName, +) -> miette::Result> { + // If we didn't find any versions, we'll just use the latest version we can find in the repodata. + let mut found_records = Vec::new(); + + // Get platforms to search for including NoArch + let platforms = if platforms.is_empty() { + let mut temp = project.platforms().into_iter().collect_vec(); + temp.push(Platform::NoArch); + temp + } else { + let mut temp = platforms.clone(); + temp.push(Platform::NoArch); + temp + }; + + // Search for the package in the all the channels and platforms + for channel in project.channels() { + for platform in &platforms { + let sparse_repo_data = sparse_repo_data.get(&(channel.clone(), *platform)); + if let Some(sparse_repo_data) = sparse_repo_data { + let records = sparse_repo_data.load_records(name).into_diagnostic()?; + // Add max of every channel and platform + if let Some(max_record) = records + .into_iter() + .max_by_key(|record| record.package_record.version.version().clone()) + { + found_records.push(max_record); + } + }; + } + } + + // Determine the version constraint based on the max of every channel and platform. + Ok(determine_version_constraint( + &found_records + .iter() + .map(|record| record.package_record.version.version().clone()) + .collect_vec(), + )) +} /// Given several specs determines the highest installable version for them. pub fn determine_best_version( environment: &GroupedEnvironment, diff --git a/src/project/manifest/mod.rs b/src/project/manifest/mod.rs index 0837cca50..975b9a705 100644 --- a/src/project/manifest/mod.rs +++ b/src/project/manifest/mod.rs @@ -2550,6 +2550,8 @@ bar = "*" assert_display_snapshot!(manifest.document.to_string()); } + + #[test] fn test_duplicate_dependency() { let contents = format!( r#" From 7617254dca0dad3e04b03597b65e336098f99582 Mon Sep 17 00:00:00 2001 From: Ruben Arts Date: Thu, 22 Feb 2024 15:15:30 +0100 Subject: [PATCH 5/8] misc: seperate concerns --- src/cli/add.rs | 23 +++++++++++++---------- 1 file changed, 13 insertions(+), 10 deletions(-) diff --git a/src/cli/add.rs b/src/cli/add.rs index e54b160bc..f3de26df6 100644 --- a/src/cli/add.rs +++ b/src/cli/add.rs @@ -333,8 +333,12 @@ pub async fn add_conda_specs_to_project( if let Some(versions_seen) = package_versions.get(&name).cloned() { updated_spec.version = determine_version_constraint(&versions_seen); } else { - updated_spec.version = - determine_latest_version(project, specs_platforms, &sparse_repo_data, &name)?; + updated_spec.version = determine_version_constraint(&determine_latest_versions( + project, + specs_platforms, + &sparse_repo_data, + &name, + )?); } updated_spec } else { @@ -375,12 +379,13 @@ pub async fn add_conda_specs_to_project( Ok(()) } -fn determine_latest_version( +/// Get all the latest versions found in the platforms repodata. +fn determine_latest_versions( project: &Project, platforms: &Vec, sparse_repo_data: &IndexMap<(Channel, Platform), SparseRepoData>, name: &PackageName, -) -> miette::Result> { +) -> miette::Result> { // If we didn't find any versions, we'll just use the latest version we can find in the repodata. let mut found_records = Vec::new(); @@ -413,12 +418,10 @@ fn determine_latest_version( } // Determine the version constraint based on the max of every channel and platform. - Ok(determine_version_constraint( - &found_records - .iter() - .map(|record| record.package_record.version.version().clone()) - .collect_vec(), - )) + Ok(found_records + .iter() + .map(|record| record.package_record.version.version().clone()) + .collect_vec()) } /// Given several specs determines the highest installable version for them. pub fn determine_best_version( From 913cbc48b9fc8270eada1b5dc31bd7dc8adefd48 Mon Sep 17 00:00:00 2001 From: Ruben Arts Date: Fri, 23 Feb 2024 10:57:32 +0100 Subject: [PATCH 6/8] Update src/project/grouped_environment.rs Co-authored-by: Bas Zalmstra --- src/project/grouped_environment.rs | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/src/project/grouped_environment.rs b/src/project/grouped_environment.rs index f4219dbef..fcacb27e4 100644 --- a/src/project/grouped_environment.rs +++ b/src/project/grouped_environment.rs @@ -161,10 +161,7 @@ impl<'p> GroupedEnvironment<'p> { /// Returns the features of the group pub fn features( &self, - ) -> Either< - impl Iterator + DoubleEndedIterator + 'p, - impl Iterator + DoubleEndedIterator + 'p, - > { + ) ->impl Iterator + DoubleEndedIterator + 'p { match self { GroupedEnvironment::Group(group) => Either::Left(group.features(true)), GroupedEnvironment::Environment(env) => Either::Right(env.features(true)), From 17873338dec3bcd84af55e3c92cadb13c9d62eae Mon Sep 17 00:00:00 2001 From: Ruben Arts Date: Fri, 23 Feb 2024 10:58:06 +0100 Subject: [PATCH 7/8] Update src/project/mod.rs Co-authored-by: Bas Zalmstra --- src/project/mod.rs | 14 ++++++-------- 1 file changed, 6 insertions(+), 8 deletions(-) diff --git a/src/project/mod.rs b/src/project/mod.rs index 2aa9aaad2..e66e8165d 100644 --- a/src/project/mod.rs +++ b/src/project/mod.rs @@ -301,19 +301,17 @@ impl Project { /// Return the grouped environments, which are all solve-groups and the environments that need to be solved. pub fn grouped_environments(&self) -> Vec { - let mut environments = self + let mut environments = HashSet::new(); + environments.extend(self .environments() .into_iter() .filter(|env| env.solve_group().is_none()) - .map(GroupedEnvironment::from) - .collect::>(); - let groups: Vec = self + .map(GroupedEnvironment::from)) + environments.extend(self .solve_groups() .into_iter() - .map(GroupedEnvironment::from) - .collect(); - environments.extend(groups); - environments.into_iter().unique().collect() + .map(GroupedEnvironment::from)); + environments.into_iter().collect() } /// Returns the channels used by this project. From a3413e673b4aa65e179d815068226d724569377c Mon Sep 17 00:00:00 2001 From: Ruben Arts Date: Fri, 23 Feb 2024 11:35:00 +0100 Subject: [PATCH 8/8] fix: lint --- src/project/grouped_environment.rs | 4 +--- src/project/mod.rs | 21 +++++++++++---------- 2 files changed, 12 insertions(+), 13 deletions(-) diff --git a/src/project/grouped_environment.rs b/src/project/grouped_environment.rs index fcacb27e4..60e69325b 100644 --- a/src/project/grouped_environment.rs +++ b/src/project/grouped_environment.rs @@ -159,9 +159,7 @@ impl<'p> GroupedEnvironment<'p> { } /// Returns the features of the group - pub fn features( - &self, - ) ->impl Iterator + DoubleEndedIterator + 'p { + pub fn features(&self) -> impl Iterator + DoubleEndedIterator + 'p { match self { GroupedEnvironment::Group(group) => Either::Left(group.features(true)), GroupedEnvironment::Environment(env) => Either::Right(env.features(true)), diff --git a/src/project/mod.rs b/src/project/mod.rs index e66e8165d..0bcb99bb3 100644 --- a/src/project/mod.rs +++ b/src/project/mod.rs @@ -7,7 +7,6 @@ mod solve_group; pub mod virtual_packages; use indexmap::{Equivalent, IndexMap, IndexSet}; -use itertools::Itertools; use miette::{IntoDiagnostic, NamedSource, WrapErr}; use once_cell::sync::OnceCell; use rattler_conda_types::{Channel, GenericVirtualPackage, Platform, Version}; @@ -302,15 +301,17 @@ impl Project { /// Return the grouped environments, which are all solve-groups and the environments that need to be solved. pub fn grouped_environments(&self) -> Vec { let mut environments = HashSet::new(); - environments.extend(self - .environments() - .into_iter() - .filter(|env| env.solve_group().is_none()) - .map(GroupedEnvironment::from)) - environments.extend(self - .solve_groups() - .into_iter() - .map(GroupedEnvironment::from)); + environments.extend( + self.environments() + .into_iter() + .filter(|env| env.solve_group().is_none()) + .map(GroupedEnvironment::from), + ); + environments.extend( + self.solve_groups() + .into_iter() + .map(GroupedEnvironment::from), + ); environments.into_iter().collect() }