Skip to content

Commit

Permalink
fix: switching channels (#923)
Browse files Browse the repository at this point in the history
When switching channels we should disregard all locked conda packages.

Fixes #922
  • Loading branch information
baszalmstra authored Mar 6, 2024
1 parent cd4b16c commit 7eaafa1
Show file tree
Hide file tree
Showing 6 changed files with 118 additions and 2 deletions.
23 changes: 22 additions & 1 deletion src/lock_file/outdated.rs
Original file line number Diff line number Diff line change
@@ -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;
Expand All @@ -15,6 +16,10 @@ pub struct OutdatedEnvironments<'p> {

/// The pypi environments that are considered out of date with the lock-file.
pub pypi: HashMap<Environment<'p>, HashSet<Platform>>,

/// 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<Environment<'p>>,
}

impl<'p> OutdatedEnvironments<'p> {
Expand All @@ -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) =
Expand Down Expand Up @@ -70,6 +82,7 @@ impl<'p> OutdatedEnvironments<'p> {
Self {
conda: outdated_conda,
pypi: outdated_pypi,
disregard_locked_content,
}
}

Expand All @@ -87,6 +100,7 @@ fn find_unsatisfiable_targets<'p>(
lock_file: &LockFile,
outdated_conda: &mut HashMap<Environment<'p>, HashSet<Platform>>,
outdated_pypi: &mut HashMap<Environment<'p>, HashSet<Platform>>,
disregard_locked_content: &mut HashSet<Environment<'p>>,
) {
for environment in project.environments() {
let platforms = environment.platforms();
Expand Down Expand Up @@ -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;
}

Expand Down
8 changes: 8 additions & 0 deletions src/lock_file/update.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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) => {
Expand Down
8 changes: 8 additions & 0 deletions src/project/grouped_environment.rs
Original file line number Diff line number Diff line change
Expand Up @@ -53,6 +53,14 @@ impl<'p> From<Environment<'p>> for GroupedEnvironment<'p> {
}

impl<'p> GroupedEnvironment<'p> {
/// Returns an iterator over all the environments in the group.
pub fn environments(&self) -> impl Iterator<Item = Environment<'p>> {
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<Self> {
match name {
Expand Down
8 changes: 8 additions & 0 deletions tests/common/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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> {
Project::load_or_else_discover(Some(&self.manifest_path()))
Expand Down
20 changes: 20 additions & 0 deletions tests/common/package_database.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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)]
Expand Down Expand Up @@ -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<LocalChannel> {
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,
Expand Down
53 changes: 52 additions & 1 deletion tests/install_tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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();

Expand Down Expand Up @@ -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"));
}

0 comments on commit 7eaafa1

Please sign in to comment.