Skip to content

Commit

Permalink
fix: improve the lock_file_usage flags and behavior. (#2078)
Browse files Browse the repository at this point in the history
  • Loading branch information
ruben-arts authored Sep 18, 2024
1 parent 76fc13d commit e5b65ad
Show file tree
Hide file tree
Showing 12 changed files with 94 additions and 57 deletions.
20 changes: 9 additions & 11 deletions crates/pixi_config/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -117,16 +117,21 @@ pub struct ConfigCli {
pypi_keyring_provider: Option<KeyringProvider>,
}

#[derive(Parser, Debug, Default, Clone)]
#[derive(Parser, Debug, Clone, Default)]
pub struct ConfigCliPrompt {
#[clap(flatten)]
config: ConfigCli,

/// Do not change the PS1 variable when starting a prompt.
#[arg(long)]
change_ps1: Option<bool>,
}

impl ConfigCliPrompt {
pub fn merge_with_config(self, config: Config) -> Config {
let mut config = config;
config.change_ps1 = self.change_ps1.or(config.change_ps1);
config
}
}

#[derive(Clone, Default, Debug, Deserialize, Serialize)]
#[serde(deny_unknown_fields, rename_all = "kebab-case")]
pub struct RepodataConfig {
Expand Down Expand Up @@ -439,13 +444,6 @@ impl From<ConfigCli> for Config {
}
}

impl From<ConfigCliPrompt> for Config {
fn from(cli: ConfigCliPrompt) -> Self {
let mut config: Config = cli.config.into();
config.change_ps1 = cli.change_ps1;
config
}
}
#[cfg(feature = "rattler_repodata_gateway")]
impl From<Config> for rattler_repodata_gateway::ChannelConfig {
fn from(config: Config) -> Self {
Expand Down
3 changes: 2 additions & 1 deletion src/cli/add.rs
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ use rattler_conda_types::{MatchSpec, PackageName, Platform, Version};
use rattler_lock::{LockFile, Package};

use super::has_specs::HasSpecs;
use crate::environment::LockFileUsage;
use crate::{
cli::cli_config::{DependencyConfig, PrefixUpdateConfig, ProjectConfig},
environment::verify_prefix_location_unchanged,
Expand Down Expand Up @@ -149,7 +150,7 @@ pub async fn execute(args: Args) -> miette::Result<()> {
}

// If the lock-file should not be updated we only need to save the project.
if prefix_update_config.no_lockfile_update {
if prefix_update_config.lock_file_usage() != LockFileUsage::Update {
project.save()?;

// Notify the user we succeeded.
Expand Down
10 changes: 8 additions & 2 deletions src/cli/cli_config.rs
Original file line number Diff line number Diff line change
Expand Up @@ -83,6 +83,10 @@ pub struct PrefixUpdateConfig {
#[clap(long, conflicts_with = "no_install")]
pub no_lockfile_update: bool,

/// Lock file usage from the CLI
#[clap(flatten)]
pub lock_file_usage: super::LockFileUsageArgs,

/// Don't modify the environment, only modify the lock-file.
#[arg(long)]
pub no_install: bool,
Expand All @@ -91,8 +95,10 @@ pub struct PrefixUpdateConfig {
pub config: ConfigCli,
}
impl PrefixUpdateConfig {
pub(crate) fn lock_file_usage(&self) -> LockFileUsage {
if self.no_lockfile_update {
pub fn lock_file_usage(&self) -> LockFileUsage {
if self.lock_file_usage.locked {
LockFileUsage::Locked
} else if self.lock_file_usage.frozen || self.no_lockfile_update {
LockFileUsage::Frozen
} else {
LockFileUsage::Update
Expand Down
3 changes: 3 additions & 0 deletions src/cli/install.rs
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,10 @@ pub async fn execute(args: Args) -> miette::Result<()> {
let mut installed_envs = Vec::with_capacity(envs.len());
for env in envs {
let environment = project.environment_from_name_or_env_var(Some(env))?;

// Update the prefix by installing all packages
update_prefix(&environment, args.lock_file_usage.into(), false).await?;

installed_envs.push(environment.name().clone());
}

Expand Down
3 changes: 0 additions & 3 deletions src/cli/list.rs
Original file line number Diff line number Diff line change
Expand Up @@ -56,9 +56,6 @@ pub struct Args {
#[clap(flatten)]
pub project_config: ProjectConfig,

#[clap(flatten)]
pub lock_file_usage: super::LockFileUsageArgs,

/// The environment to list packages for. Defaults to the default environment.
#[arg(short, long)]
pub environment: Option<String>,
Expand Down
4 changes: 0 additions & 4 deletions src/cli/project/export/conda_explicit_spec.rs
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,6 @@ use clap::Parser;
use miette::{Context, IntoDiagnostic};

use crate::cli::cli_config::PrefixUpdateConfig;
use crate::cli::LockFileUsageArgs;
use crate::lock_file::UpdateLockFileOptions;
use crate::Project;
use rattler_conda_types::{ExplicitEnvironmentEntry, ExplicitEnvironmentSpec, Platform};
Expand All @@ -32,9 +31,6 @@ pub struct Args {
#[arg(long, default_value = "false")]
pub ignore_pypi_errors: bool,

#[clap(flatten)]
pub lock_file_usage: LockFileUsageArgs,

#[clap(flatten)]
pub prefix_update_config: PrefixUpdateConfig,
}
Expand Down
12 changes: 4 additions & 8 deletions src/cli/run.rs
Original file line number Diff line number Diff line change
Expand Up @@ -7,9 +7,8 @@ use clap::Parser;
use dialoguer::theme::ColorfulTheme;
use itertools::Itertools;
use miette::{Diagnostic, IntoDiagnostic};
use pixi_config::ConfigCli;

use crate::cli::cli_config::ProjectConfig;
use crate::cli::cli_config::{PrefixUpdateConfig, ProjectConfig};
use crate::environment::verify_prefix_location_unchanged;
use crate::lock_file::UpdateLockFileOptions;
use crate::project::errors::UnsupportedPlatformError;
Expand Down Expand Up @@ -37,15 +36,12 @@ pub struct Args {
pub project_config: ProjectConfig,

#[clap(flatten)]
pub lock_file_usage: super::LockFileUsageArgs,
pub prefix_update_config: PrefixUpdateConfig,

/// The environment to run the task in.
#[arg(long, short)]
pub environment: Option<String>,

#[clap(flatten)]
pub config: ConfigCli,

/// Use a clean environment to run the task
///
/// Using this flag will ignore your current shell environment and use bare minimum environment to activate the pixi environment in.
Expand All @@ -58,7 +54,7 @@ pub struct Args {
pub async fn execute(args: Args) -> miette::Result<()> {
// Load the project
let project = Project::load_or_else_discover(args.project_config.manifest_path.as_deref())?
.with_cli_config(args.config);
.with_cli_config(args.prefix_update_config.config.clone());

// Sanity check of prefix location
verify_prefix_location_unchanged(project.default_environment().dir().as_path()).await?;
Expand All @@ -84,7 +80,7 @@ pub async fn execute(args: Args) -> miette::Result<()> {
// Ensure that the lock-file is up-to-date.
let mut lock_file = project
.update_lock_file(UpdateLockFileOptions {
lock_file_usage: args.lock_file_usage.into(),
lock_file_usage: args.prefix_update_config.lock_file_usage(),
..UpdateLockFileOptions::default()
})
.await?;
Expand Down
20 changes: 14 additions & 6 deletions src/cli/shell.rs
Original file line number Diff line number Diff line change
Expand Up @@ -8,9 +8,9 @@ use rattler_shell::{
shell::{CmdExe, PowerShell, Shell, ShellEnum, ShellScript},
};

use crate::cli::cli_config::ProjectConfig;
use crate::cli::cli_config::{PrefixUpdateConfig, ProjectConfig};
use crate::{
activation::CurrentEnvVarBehavior, cli::LockFileUsageArgs, environment::update_prefix,
activation::CurrentEnvVarBehavior, environment::update_prefix,
project::virtual_packages::verify_current_platform_has_required_virtual_packages, prompt,
Project,
};
Expand All @@ -26,14 +26,14 @@ pub struct Args {
project_config: ProjectConfig,

#[clap(flatten)]
lock_file_usage: LockFileUsageArgs,
pub prefix_update_config: PrefixUpdateConfig,

/// The environment to activate in the shell
#[arg(long, short)]
environment: Option<String>,

#[clap(flatten)]
config: ConfigCliPrompt,
prompt_config: ConfigCliPrompt,
}

fn start_powershell(
Expand Down Expand Up @@ -203,8 +203,11 @@ async fn start_nu_shell(
}

pub async fn execute(args: Args) -> miette::Result<()> {
let config = args
.prompt_config
.merge_with_config(args.prefix_update_config.config.clone().into());
let project = Project::load_or_else_discover(args.project_config.manifest_path.as_deref())?
.with_cli_config(args.config);
.with_cli_config(config);
let environment = project.environment_from_name_or_env_var(args.environment)?;

verify_current_platform_has_required_virtual_packages(&environment).into_diagnostic()?;
Expand All @@ -215,7 +218,12 @@ pub async fn execute(args: Args) -> miette::Result<()> {
};

// Make sure environment is up-to-date, default to install, users can avoid this with frozen or locked.
update_prefix(&environment, args.lock_file_usage.into(), false).await?;
update_prefix(
&environment,
args.prefix_update_config.lock_file_usage(),
false,
)
.await?;

// Get the environment variables we need to set activate the environment in the shell.
let env = project
Expand Down
19 changes: 13 additions & 6 deletions src/cli/shell_hook.rs
Original file line number Diff line number Diff line change
Expand Up @@ -10,11 +10,10 @@ use rattler_shell::{
use serde::Serialize;
use serde_json;

use crate::cli::cli_config::ProjectConfig;
use crate::cli::cli_config::{PrefixUpdateConfig, ProjectConfig};
use crate::project::HasProjectRef;
use crate::{
activation::{get_activator, CurrentEnvVarBehavior},
cli::LockFileUsageArgs,
environment::update_prefix,
project::Environment,
Project,
Expand All @@ -35,7 +34,7 @@ pub struct Args {
pub project_config: ProjectConfig,

#[clap(flatten)]
lock_file_usage: LockFileUsageArgs,
pub prefix_update_config: PrefixUpdateConfig,

/// The environment to activate in the script
#[arg(long, short)]
Expand All @@ -46,7 +45,7 @@ pub struct Args {
json: bool,

#[clap(flatten)]
config: ConfigCliPrompt,
prompt_config: ConfigCliPrompt,
}

#[derive(Serialize)]
Expand Down Expand Up @@ -103,11 +102,19 @@ async fn generate_environment_json(environment: &Environment<'_>) -> miette::Res

/// Prints the activation script to the stdout.
pub async fn execute(args: Args) -> miette::Result<()> {
let config = args
.prompt_config
.merge_with_config(args.prefix_update_config.config.clone().into());
let project = Project::load_or_else_discover(args.project_config.manifest_path.as_deref())?
.with_cli_config(args.config);
.with_cli_config(config);
let environment = project.environment_from_name_or_env_var(args.environment)?;

update_prefix(&environment, args.lock_file_usage.into(), false).await?;
update_prefix(
&environment,
args.prefix_update_config.lock_file_usage(),
false,
)
.await?;

let output = match args.json {
true => generate_environment_json(&environment).await?,
Expand Down
44 changes: 32 additions & 12 deletions src/lock_file/update.rs
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ use futures::{future::Either, stream::FuturesUnordered, FutureExt, StreamExt, Tr
use indexmap::IndexSet;
use indicatif::{HumanBytes, ProgressBar, ProgressState};
use itertools::Itertools;
use miette::{IntoDiagnostic, LabeledSpan, MietteDiagnostic, WrapErr};
use miette::{Diagnostic, IntoDiagnostic, LabeledSpan, MietteDiagnostic, WrapErr};
use parking_lot::Mutex;
use pixi_consts::consts;
use pixi_manifest::{EnvironmentName, FeaturesExt, HasFeaturesIter};
Expand All @@ -30,6 +30,7 @@ use rattler_conda_types::{Arch, MatchSpec, Platform, RepoDataRecord};
use rattler_lock::{LockFile, PypiIndexes, PypiPackageData, PypiPackageEnvironmentData};
use rattler_repodata_gateway::{Gateway, RepoData};
use rattler_solve::ChannelPriority;
use thiserror::Error;
use tokio::sync::Semaphore;
use tracing::Instrument;
use url::Url;
Expand Down Expand Up @@ -67,6 +68,14 @@ impl Project {
}
}

#[derive(Debug, Error, Diagnostic)]
enum UpdateError {
#[error("the lockfile is not up-to-date with requested environment: '{}'", .0.fancy_display())]
LockFileMissingEnv(EnvironmentName),
#[error("the lockfile is not up-to-date with the requested platform: '{}'", .0)]
LockFileMissingPlatform(Platform),
}

/// Options to pass to [`Project::update_lock_file`].
#[derive(Default)]
pub struct UpdateLockFileOptions {
Expand Down Expand Up @@ -138,8 +147,12 @@ impl<'p> LockFileDerivedData<'p> {
let (prefix, python_status) = self.conda_prefix(environment).await?;
let repodata_records = self
.repodata_records(environment, platform)
.into_diagnostic()?
.unwrap_or_default();
let pypi_records = self
.pypi_records(environment, platform)
.into_diagnostic()?
.unwrap_or_default();
let pypi_records = self.pypi_records(environment, platform).unwrap_or_default();

// No `uv` support for WASM right now
if platform.arch() == Some(Arch::Wasm32) {
Expand Down Expand Up @@ -171,7 +184,7 @@ impl<'p> LockFileDerivedData<'p> {
&python_status,
&environment.system_requirements(),
&uv_context,
self.pypi_indexes(environment).as_ref(),
self.pypi_indexes(environment).into_diagnostic()?.as_ref(),
env_variables,
self.project.root(),
environment.best_platform(),
Expand All @@ -196,32 +209,38 @@ impl<'p> LockFileDerivedData<'p> {
&self,
environment: &Environment<'p>,
platform: Platform,
) -> Option<Vec<(PypiPackageData, PypiPackageEnvironmentData)>> {
) -> Result<Option<Vec<(PypiPackageData, PypiPackageEnvironmentData)>>, UpdateError> {
let locked_env = self
.lock_file
.environment(environment.name().as_str())
.expect("the lock-file should be up-to-date so it should also include the environment");
locked_env.pypi_packages_for_platform(platform)
.ok_or_else(|| UpdateError::LockFileMissingEnv(environment.name().clone()))?;

Ok(locked_env.pypi_packages_for_platform(platform))
}

fn pypi_indexes(&self, environment: &Environment<'p>) -> Option<PypiIndexes> {
fn pypi_indexes(
&self,
environment: &Environment<'p>,
) -> Result<Option<PypiIndexes>, UpdateError> {
let locked_env = self
.lock_file
.environment(environment.name().as_str())
.expect("the lock-file should be up-to-date so it should also include the environment");
locked_env.pypi_indexes().cloned()
.ok_or_else(|| UpdateError::LockFileMissingEnv(environment.name().clone()))?;
Ok(locked_env.pypi_indexes().cloned())
}

fn repodata_records(
&self,
environment: &Environment<'p>,
platform: Platform,
) -> Option<Vec<RepoDataRecord>> {
) -> Result<Option<Vec<RepoDataRecord>>, UpdateError> {
let locked_env = self
.lock_file
.environment(environment.name().as_str())
.expect("the lock-file should be up-to-date so it should also include the environment");
locked_env.conda_repodata_records_for_platform(platform).expect("since the lock-file is up to date we should be able to extract the repodata records from it")
.ok_or_else(|| UpdateError::LockFileMissingEnv(environment.name().clone()))?;
locked_env
.conda_repodata_records_for_platform(platform)
.map_err(|_| UpdateError::LockFileMissingPlatform(platform))
}

async fn conda_prefix(
Expand Down Expand Up @@ -250,6 +269,7 @@ impl<'p> LockFileDerivedData<'p> {
// Get the locked environment from the lock-file.
let records = self
.repodata_records(environment, platform)
.into_diagnostic()?
.unwrap_or_default();

// Update the prefix with conda packages.
Expand Down
Loading

0 comments on commit e5b65ad

Please sign in to comment.