Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: also install environment when using add #213

Merged
merged 5 commits into from
Jul 14, 2023
Merged
Show file tree
Hide file tree
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
48 changes: 46 additions & 2 deletions src/cli/add.rs
Original file line number Diff line number Diff line change
@@ -1,15 +1,21 @@
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,
virtual_packages::get_minimal_virtual_packages,
};
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;
Expand Down Expand Up @@ -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)]
Expand All @@ -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<MatchSpec>,
spec_type: SpecType,
no_install: bool,
) -> anyhow::Result<()> {
// Split the specs into package name and version specifier
let new_specs = specs
Expand Down Expand Up @@ -160,14 +171,47 @@ 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),
)
.await?;
project.save()?;

if !no_install {
let platform = Platform::current();
tdejager marked this conversation as resolved.
Show resolved Hide resolved
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?;
tdejager marked this conversation as resolved.
Show resolved Hide resolved
}
} else {
eprintln!("{} skipping installation of environment because your platform ({platform}) is not supported by this project.", style("!").yellow().bold())
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Shouldn't this warning also be shown when the get_up_to_date_prefix is run?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

get_up_to_date_prefix also does this but it results in an error.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I explicitly did it this way because I think it makes sense that you can add dependencies even if your platform doesn't support the project. However, actually installing the dependencies (in pixi run for instance) is problematic.

}
}

for spec in added_specs {
eprintln!(
"{}Added {}",
Expand Down
7 changes: 7 additions & 0 deletions tests/common/builders.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down
1 change: 1 addition & 0 deletions tests/common/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -118,6 +118,7 @@ impl PixiControl {
host: false,
specs: vec![spec.into()],
build: false,
no_install: true,
},
}
}
Expand Down
2 changes: 1 addition & 1 deletion tests/install_tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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();
Expand Down
Loading