From fb48e0eb8b1b547b9b8365efec58f4fbca490f08 Mon Sep 17 00:00:00 2001 From: Bas Zalmstra Date: Thu, 13 Jul 2023 15:11:05 +0200 Subject: [PATCH 1/4] feat: also install environment when using add --- src/cli/add.rs | 48 ++++++++++++++++++++++++++++++++++++++-- tests/common/builders.rs | 7 ++++++ tests/common/mod.rs | 1 + tests/install_tests.rs | 2 +- 4 files changed, 55 insertions(+), 3 deletions(-) diff --git a/src/cli/add.rs b/src/cli/add.rs index ca67292dc..2c56bf646 100644 --- a/src/cli/add.rs +++ b/src/cli/add.rs @@ -1,3 +1,6 @@ +use crate::environment::{execute_transaction, get_required_packages}; +use crate::prefix::Prefix; +use crate::progress::await_in_progress; use crate::{ environment::{load_lock_file, update_lock_file}, project::Project, @@ -5,11 +8,14 @@ use crate::{ }; use anyhow::Context; use clap::Parser; +use console::style; use indexmap::IndexMap; use itertools::Itertools; +use rattler::install::Transaction; use rattler_conda_types::{ version_spec::VersionOperator, MatchSpec, NamelessMatchSpec, Platform, Version, VersionSpec, }; +use rattler_networking::AuthenticatedClient; use rattler_repodata_gateway::sparse::SparseRepoData; use rattler_solve::{libsolv_c, SolverImpl}; use std::collections::HashMap; @@ -48,6 +54,10 @@ pub struct Args { /// This is a build dependency #[arg(long, conflicts_with = "host")] pub build: bool, + + /// Don't install the package to the environment, only add the package to the lock-file. + #[arg(long)] + pub no_install: bool, } #[derive(Debug, Copy, Clone)] @@ -72,13 +82,14 @@ impl SpecType { pub async fn execute(args: Args) -> anyhow::Result<()> { let mut project = Project::load_or_else_discover(args.manifest_path.as_deref())?; let spec_type = SpecType::from_args(&args); - add_specs_to_project(&mut project, args.specs, spec_type).await + add_specs_to_project(&mut project, args.specs, spec_type, args.no_install).await } pub async fn add_specs_to_project( project: &mut Project, specs: Vec, spec_type: SpecType, + no_install: bool, ) -> anyhow::Result<()> { // Split the specs into package name and version specifier let new_specs = specs @@ -160,7 +171,7 @@ pub async fn add_specs_to_project( } // Update the lock file and write to disk - update_lock_file( + let lock_file = update_lock_file( project, load_lock_file(project).await?, Some(sparse_repo_data), @@ -168,6 +179,39 @@ pub async fn add_specs_to_project( .await?; project.save()?; + if !no_install { + let platform = Platform::current(); + if project.platforms().contains(&platform) { + // Get the currently installed packages + let prefix = Prefix::new(project.root().join(".pixi/env"))?; + let installed_packages = prefix.find_installed_packages(None).await?; + + // Construct a transaction to bring the environment up to date with the lock-file content + let transaction = Transaction::from_current_and_desired( + installed_packages, + get_required_packages(&lock_file, platform)?, + platform, + )?; + + // Execute the transaction if there is work to do + if !transaction.operations.is_empty() { + // Execute the operations that are returned by the solver. + await_in_progress( + "updating environment", + execute_transaction( + transaction, + prefix.root().to_path_buf(), + rattler::default_cache_dir()?, + AuthenticatedClient::default(), + ), + ) + .await?; + } + } else { + eprintln!("{} skipping installation of environment because your platform ({platform}) is not supported by this project.", style("!").yellow().bold()) + } + } + for spec in added_specs { eprintln!( "{}Added {}", diff --git a/tests/common/builders.rs b/tests/common/builders.rs index ac03ad642..5cfc52a01 100644 --- a/tests/common/builders.rs +++ b/tests/common/builders.rs @@ -92,6 +92,13 @@ impl AddBuilder { } self } + + /// Set whether or not to also install the environment. By default the environment is NOT + /// installed to reduce test times. + pub fn with_install(mut self, install: bool) -> Self { + self.args.no_install = !install; + self + } } impl IntoFuture for AddBuilder { diff --git a/tests/common/mod.rs b/tests/common/mod.rs index 197421143..4baffe6c1 100644 --- a/tests/common/mod.rs +++ b/tests/common/mod.rs @@ -118,6 +118,7 @@ impl PixiControl { host: false, specs: vec![spec.into()], build: false, + no_install: true }, } } diff --git a/tests/install_tests.rs b/tests/install_tests.rs index 46ced883d..31af5d27b 100644 --- a/tests/install_tests.rs +++ b/tests/install_tests.rs @@ -13,7 +13,7 @@ use tempfile::TempDir; async fn install_run_python() { let pixi = PixiControl::new().unwrap(); pixi.init().await.unwrap(); - pixi.add("python==3.11.0").await.unwrap(); + pixi.add("python==3.11.0").with_install(true).await.unwrap(); // Check if lock has python version let lock = pixi.lock_file().await.unwrap(); From 2c596027eaac413013a4308bc504e7032fae886a Mon Sep 17 00:00:00 2001 From: Bas Zalmstra Date: Thu, 13 Jul 2023 15:13:08 +0200 Subject: [PATCH 2/4] fix: fmt --- tests/common/mod.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/common/mod.rs b/tests/common/mod.rs index 4baffe6c1..bfe17a88d 100644 --- a/tests/common/mod.rs +++ b/tests/common/mod.rs @@ -118,7 +118,7 @@ impl PixiControl { host: false, specs: vec![spec.into()], build: false, - no_install: true + no_install: true, }, } } From 7583047d5ae1055f24c2172c2c013f3df3a7b6e4 Mon Sep 17 00:00:00 2001 From: Bas Zalmstra Date: Thu, 13 Jul 2023 16:20:52 +0200 Subject: [PATCH 3/4] fix: merge issues --- src/cli/add.rs | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/src/cli/add.rs b/src/cli/add.rs index cd1e5b685..958451a02 100644 --- a/src/cli/add.rs +++ b/src/cli/add.rs @@ -10,8 +10,8 @@ use clap::Parser; use console::style; use indexmap::IndexMap; use itertools::Itertools; -use rattler::install::Transaction; use miette::{IntoDiagnostic, WrapErr}; +use rattler::install::Transaction; use rattler_conda_types::{ version_spec::VersionOperator, MatchSpec, NamelessMatchSpec, Platform, Version, VersionSpec, }; @@ -191,7 +191,8 @@ pub async fn add_specs_to_project( installed_packages, get_required_packages(&lock_file, platform)?, platform, - )?; + ) + .into_diagnostic()?; // Execute the transaction if there is work to do if !transaction.operations.is_empty() { @@ -201,7 +202,9 @@ pub async fn add_specs_to_project( execute_transaction( transaction, prefix.root().to_path_buf(), - rattler::default_cache_dir()?, + rattler::default_cache_dir().map_err(|_| { + miette::miette!("could not determine default cache directory") + })?, AuthenticatedClient::default(), ), ) From 3674d45ca67e8989f855ea086db5be5d20bd9171 Mon Sep 17 00:00:00 2001 From: Bas Zalmstra Date: Thu, 13 Jul 2023 16:38:42 +0200 Subject: [PATCH 4/4] fix: code review --- src/cli/add.rs | 31 +++---------------------------- src/environment.rs | 26 ++++++++++++++++++++++---- 2 files changed, 25 insertions(+), 32 deletions(-) diff --git a/src/cli/add.rs b/src/cli/add.rs index 958451a02..1fde21b17 100644 --- a/src/cli/add.rs +++ b/src/cli/add.rs @@ -1,6 +1,5 @@ -use crate::environment::{execute_transaction, get_required_packages}; +use crate::environment::update_prefix; use crate::prefix::Prefix; -use crate::progress::await_in_progress; use crate::{ environment::{load_lock_file, update_lock_file}, project::Project, @@ -11,11 +10,9 @@ use console::style; use indexmap::IndexMap; use itertools::Itertools; use miette::{IntoDiagnostic, WrapErr}; -use rattler::install::Transaction; use rattler_conda_types::{ version_spec::VersionOperator, MatchSpec, NamelessMatchSpec, Platform, Version, VersionSpec, }; -use rattler_networking::AuthenticatedClient; use rattler_repodata_gateway::sparse::SparseRepoData; use rattler_solve::{libsolv_rs, SolverImpl}; use std::collections::HashMap; @@ -186,30 +183,8 @@ pub async fn add_specs_to_project( let prefix = Prefix::new(project.root().join(".pixi/env"))?; let installed_packages = prefix.find_installed_packages(None).await?; - // Construct a transaction to bring the environment up to date with the lock-file content - let transaction = Transaction::from_current_and_desired( - installed_packages, - get_required_packages(&lock_file, platform)?, - platform, - ) - .into_diagnostic()?; - - // Execute the transaction if there is work to do - if !transaction.operations.is_empty() { - // Execute the operations that are returned by the solver. - await_in_progress( - "updating environment", - execute_transaction( - transaction, - prefix.root().to_path_buf(), - rattler::default_cache_dir().map_err(|_| { - miette::miette!("could not determine default cache directory") - })?, - AuthenticatedClient::default(), - ), - ) - .await?; - } + // Update the prefix + update_prefix(&prefix, installed_packages, &lock_file, platform).await?; } else { eprintln!("{} skipping installation of environment because your platform ({platform}) is not supported by this project.", style("!").yellow().bold()) } diff --git a/src/environment.rs b/src/environment.rs index 80f101669..43f70cba3 100644 --- a/src/environment.rs +++ b/src/environment.rs @@ -68,10 +68,29 @@ pub async fn get_up_to_date_prefix(project: &Project) -> miette::Result lock_file }; + // Update the environment + update_prefix( + &prefix, + installed_packages_future.await.into_diagnostic()??, + &lock_file, + platform, + ) + .await?; + + Ok(prefix) +} + +/// Updates the environment to contain the packages from the specified lock-file +pub async fn update_prefix( + prefix: &Prefix, + installed_packages: Vec, + lock_file: &CondaLock, + platform: Platform, +) -> miette::Result<()> { // Construct a transaction to bring the environment up to date with the lock-file content let transaction = Transaction::from_current_and_desired( - installed_packages_future.await.into_diagnostic()??, - get_required_packages(&lock_file, platform)?, + installed_packages, + get_required_packages(lock_file, platform)?, platform, ) .into_diagnostic()?; @@ -91,8 +110,7 @@ pub async fn get_up_to_date_prefix(project: &Project) -> miette::Result ) .await?; } - - Ok(prefix) + Ok(()) } /// Loads the lockfile for the specified project or returns a dummy one if none could be found.