From 7eaafa1d7db0dd7280bda53920f1117216762fac Mon Sep 17 00:00:00 2001 From: Bas Zalmstra Date: Wed, 6 Mar 2024 10:09:41 +0100 Subject: [PATCH] fix: switching channels (#923) When switching channels we should disregard all locked conda packages. Fixes #922 --- src/lock_file/outdated.rs | 23 ++++++++++++- src/lock_file/update.rs | 8 +++++ src/project/grouped_environment.rs | 8 +++++ tests/common/mod.rs | 8 +++++ tests/common/package_database.rs | 20 +++++++++++ tests/install_tests.rs | 53 +++++++++++++++++++++++++++++- 6 files changed, 118 insertions(+), 2 deletions(-) diff --git a/src/lock_file/outdated.rs b/src/lock_file/outdated.rs index 12220d37f..3de97a503 100644 --- a/src/lock_file/outdated.rs +++ b/src/lock_file/outdated.rs @@ -1,4 +1,5 @@ use super::{verify_environment_satisfiability, verify_platform_satisfiability, PlatformUnsat}; +use crate::lock_file::satisfiability::EnvironmentUnsat; use crate::{consts, project::Environment, project::SolveGroup, Project}; use itertools::Itertools; use rattler_conda_types::Platform; @@ -15,6 +16,10 @@ pub struct OutdatedEnvironments<'p> { /// The pypi environments that are considered out of date with the lock-file. pub pypi: HashMap, HashSet>, + + /// Records the environments for which the lock-file content should also be discarded. This is + /// the case for instance when the order of the channels changed. + pub disregard_locked_content: HashSet>, } impl<'p> OutdatedEnvironments<'p> { @@ -23,9 +28,16 @@ impl<'p> OutdatedEnvironments<'p> { pub fn from_project_and_lock_file(project: &'p Project, lock_file: &LockFile) -> Self { let mut outdated_conda: HashMap<_, HashSet<_>> = HashMap::new(); let mut outdated_pypi: HashMap<_, HashSet<_>> = HashMap::new(); + let mut disregard_locked_content = HashSet::new(); // Find all targets that are not satisfied by the lock-file - find_unsatisfiable_targets(project, lock_file, &mut outdated_conda, &mut outdated_pypi); + find_unsatisfiable_targets( + project, + lock_file, + &mut outdated_conda, + &mut outdated_pypi, + &mut disregard_locked_content, + ); // Extend the outdated targets to include the solve groups let (mut conda_solve_groups_out_of_date, mut pypi_solve_groups_out_of_date) = @@ -70,6 +82,7 @@ impl<'p> OutdatedEnvironments<'p> { Self { conda: outdated_conda, pypi: outdated_pypi, + disregard_locked_content, } } @@ -87,6 +100,7 @@ fn find_unsatisfiable_targets<'p>( lock_file: &LockFile, outdated_conda: &mut HashMap, HashSet>, outdated_pypi: &mut HashMap, HashSet>, + disregard_locked_content: &mut HashSet>, ) { for environment in project.environments() { let platforms = environment.platforms(); @@ -118,6 +132,13 @@ fn find_unsatisfiable_targets<'p>( .or_default() .extend(platforms); + match unsat { + EnvironmentUnsat::ChannelsMismatch => { + // If the channels mismatched we also cannot trust any of the locked content. + disregard_locked_content.insert(environment.clone()); + } + } + continue; } diff --git a/src/lock_file/update.rs b/src/lock_file/update.rs index c11a041f9..df4e4dd65 100644 --- a/src/lock_file/update.rs +++ b/src/lock_file/update.rs @@ -546,6 +546,14 @@ pub async fn ensure_up_to_date_lock_file( let locked_grouped_repodata_records = all_grouped_environments .iter() .filter_map(|group| { + // If any content of the environments in the group are outdated we need to disregard the locked content. + if group + .environments() + .any(|e| outdated.disregard_locked_content.contains(&e)) + { + return None; + } + let records = match group { GroupedEnvironment::Environment(env) => locked_repodata_records.get(env)?.clone(), GroupedEnvironment::Group(group) => { diff --git a/src/project/grouped_environment.rs b/src/project/grouped_environment.rs index 849158ef7..15b0ff81d 100644 --- a/src/project/grouped_environment.rs +++ b/src/project/grouped_environment.rs @@ -53,6 +53,14 @@ impl<'p> From> for GroupedEnvironment<'p> { } impl<'p> GroupedEnvironment<'p> { + /// Returns an iterator over all the environments in the group. + pub fn environments(&self) -> impl Iterator> { + match self { + GroupedEnvironment::Group(group) => Either::Left(group.environments()), + GroupedEnvironment::Environment(env) => Either::Right(std::iter::once(env.clone())), + } + } + /// Constructs a `GroupedEnvironment` from a `GroupedEnvironmentName`. pub fn from_name(project: &'p Project, name: &GroupedEnvironmentName) -> Option { match name { diff --git a/tests/common/mod.rs b/tests/common/mod.rs index 3ed767226..03d670e0f 100644 --- a/tests/common/mod.rs +++ b/tests/common/mod.rs @@ -163,6 +163,14 @@ impl PixiControl { Ok(pixi) } + /// Updates the complete manifest + pub fn update_manifest(&self, manifest: &str) -> miette::Result<()> { + std::fs::write(&self.manifest_path(), manifest) + .into_diagnostic() + .context("failed to write pixi.toml")?; + Ok(()) + } + /// Loads the project manifest and returns it. pub fn project(&self) -> miette::Result { Project::load_or_else_discover(Some(&self.manifest_path())) diff --git a/tests/common/package_database.rs b/tests/common/package_database.rs index 084f2c753..662957d01 100644 --- a/tests/common/package_database.rs +++ b/tests/common/package_database.rs @@ -11,6 +11,19 @@ use rattler_conda_types::{ VersionWithSource, }; use std::{collections::HashSet, path::Path}; +use tempfile::TempDir; +use url::Url; + +pub struct LocalChannel { + dir: TempDir, + db: PackageDatabase, +} + +impl LocalChannel { + pub fn url(&self) -> Url { + Url::from_file_path(self.dir.path()).unwrap() + } +} /// A database of packages #[derive(Default, Clone, Debug)] @@ -81,6 +94,13 @@ impl PackageDatabase { Ok(()) } + /// Converts this database into a local channel which can be referenced by a pixi project. + pub async fn into_channel(self) -> miette::Result { + let dir = TempDir::new().into_diagnostic()?; + self.write_repodata(dir.path()).await?; + Ok(LocalChannel { dir, db: self }) + } + /// Returns all packages for the specified platform. pub fn packages_by_platform( &self, diff --git a/tests/install_tests.rs b/tests/install_tests.rs index 2ddd9bb1a..5180678e7 100644 --- a/tests/install_tests.rs +++ b/tests/install_tests.rs @@ -54,7 +54,6 @@ async fn install_run_python() { /// version `2` is available. This is because `bar` was previously locked to version `1` and it is /// still a valid solution to keep using version `1` of bar. #[tokio::test] -#[serial] async fn test_incremental_lock_file() { let mut package_database = PackageDatabase::default(); @@ -250,3 +249,55 @@ async fn pypi_reinstall_python() { assert!(installed_311.iter().count() > 0); } } + +#[tokio::test] +async fn test_channels_changed() { + // Write a channel with a package `bar` with only one version + let mut package_database_a = PackageDatabase::default(); + package_database_a.add_package(Package::build("bar", "2").finish()); + let channel_a = package_database_a.into_channel().await.unwrap(); + + // Write another channel with a package `bar` with only one version but another one. + let mut package_database_b = PackageDatabase::default(); + package_database_b.add_package(Package::build("bar", "1").finish()); + let channel_b = package_database_b.into_channel().await.unwrap(); + + let platform = Platform::current(); + let pixi = PixiControl::from_manifest(&format!( + r#" + [project] + name = "test-channel-change" + channels = ["{channel_a}"] + platforms = ["{platform}"] + + [dependencies] + bar = "*" + "#, + channel_a = channel_a.url(), + )) + .unwrap(); + + // Get an up-to-date lockfile and verify that bar version 2 was selected from channel `a`. + let lock_file = pixi.up_to_date_lock_file().await.unwrap(); + assert!(lock_file.contains_match_spec(DEFAULT_ENVIRONMENT_NAME, platform, "bar ==2")); + + // Switch the channel around + let platform = Platform::current(); + pixi.update_manifest(&format!( + r#" + [project] + name = "test-channel-change" + channels = ["{channel_b}"] + platforms = ["{platform}"] + + [dependencies] + bar = "*" + "#, + channel_b = channel_b.url() + )) + .unwrap(); + + // Get an up-to-date lockfile and verify that bar version 1 was now selected from channel `b`. + let lock_file = pixi.up_to_date_lock_file().await.unwrap(); + assert!(lock_file.contains_match_spec(DEFAULT_ENVIRONMENT_NAME, platform, "bar ==1")); +}