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

fix: improve the lock_file_usage flags and behavior. #2078

Merged
merged 3 commits into from
Sep 18, 2024
Merged
Show file tree
Hide file tree
Changes from all 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
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
Loading