Skip to content
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
2 changes: 1 addition & 1 deletion src/cli/add.rs
Original file line number Diff line number Diff line change
Expand Up @@ -198,7 +198,7 @@ pub async fn execute(args: Args) -> miette::Result<()> {
pypi_deps,
source_specs,
prefix_update_config.no_install,
&lock_file_update_config.lock_file_usage()?,
&lock_file_update_config.lock_file_usage(),
&dependency_config.feature,
&dependency_config.platforms,
args.editable,
Expand Down
12 changes: 4 additions & 8 deletions src/cli/cli_config.rs
Original file line number Diff line number Diff line change
Expand Up @@ -112,16 +112,12 @@ pub struct LockFileUpdateConfig {
}

impl LockFileUpdateConfig {
pub fn lock_file_usage(&self) -> miette::Result<LockFileUsage> {
let usage: LockFileUsage = self
.lock_file_usage
.clone()
.try_into()
.map_err(|e: crate::cli::LockFileUsageError| miette::miette!(e))?;
pub fn lock_file_usage(&self) -> LockFileUsage {
let usage: LockFileUsage = self.lock_file_usage.clone().into();
if self.no_lockfile_update {
Ok(LockFileUsage::Frozen)
LockFileUsage::Frozen
} else {
Ok(usage)
usage
}
}
}
Expand Down
2 changes: 1 addition & 1 deletion src/cli/install.rs
Original file line number Diff line number Diff line change
Expand Up @@ -89,7 +89,7 @@ pub async fn execute(args: Args) -> miette::Result<()> {
&environments,
UpdateMode::Revalidate,
UpdateLockFileOptions {
lock_file_usage: args.lock_file_usage.try_into()?,
lock_file_usage: args.lock_file_usage.into(),
no_install: false,
max_concurrent_solves: workspace.config().max_concurrent_solves(),
},
Expand Down
2 changes: 1 addition & 1 deletion src/cli/list.rs
Original file line number Diff line number Diff line change
Expand Up @@ -184,7 +184,7 @@ pub async fn execute(args: Args) -> miette::Result<()> {

let lock_file = workspace
.update_lock_file(UpdateLockFileOptions {
lock_file_usage: args.lock_file_update_config.lock_file_usage()?,
lock_file_usage: args.lock_file_update_config.lock_file_usage(),
no_install: false,
max_concurrent_solves: workspace.config().max_concurrent_solves(),
})
Expand Down
137 changes: 14 additions & 123 deletions src/cli/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -198,11 +198,11 @@ pub struct LockFileUsageArgs {
struct LockFileUsageArgsRaw {
/// Install the environment as defined in the lockfile, doesn't update
/// lockfile if it isn't up-to-date with the manifest file.
#[clap(long, env = "PIXI_FROZEN", help_heading = consts::CLAP_UPDATE_OPTIONS)]
#[clap(long, env = "PIXI_FROZEN", help_heading = consts::CLAP_UPDATE_OPTIONS, conflicts_with = "locked")]
frozen: bool,
/// Check if lockfile is up-to-date before installing the environment,
/// aborts when lockfile isn't up-to-date with the manifest file.
#[clap(long, env = "PIXI_LOCKED", help_heading = consts::CLAP_UPDATE_OPTIONS)]
#[clap(long, env = "PIXI_LOCKED", help_heading = consts::CLAP_UPDATE_OPTIONS, conflicts_with = "frozen")]
locked: bool,
}

Expand All @@ -216,25 +216,18 @@ impl LockFileUsageArgs {
}
}

// Automatic validation when converting from raw args
impl TryFrom<LockFileUsageArgsRaw> for LockFileUsageArgs {
type Error = LockFileUsageError;

fn try_from(raw: LockFileUsageArgsRaw) -> Result<Self, LockFileUsageError> {
if raw.frozen && raw.locked {
return Err(LockFileUsageError::FrozenAndLocked);
}
Ok(LockFileUsageArgs { inner: raw })
// Automatic conversion from raw args (conflicts handled by clap)
impl From<LockFileUsageArgsRaw> for LockFileUsageArgs {
fn from(raw: LockFileUsageArgsRaw) -> Self {
LockFileUsageArgs { inner: raw }
}
}

// For clap flattening - this provides automatic validation
impl clap::FromArgMatches for LockFileUsageArgs {
fn from_arg_matches(matches: &clap::ArgMatches) -> Result<Self, clap::Error> {
let raw = LockFileUsageArgsRaw::from_arg_matches(matches)?;
raw.try_into().map_err(|e: LockFileUsageError| {
clap::Error::raw(clap::error::ErrorKind::ArgumentConflict, e.to_string())
})
Ok(raw.into())
}

fn update_from_arg_matches(&mut self, matches: &clap::ArgMatches) -> Result<(), clap::Error> {
Expand Down Expand Up @@ -265,17 +258,14 @@ impl From<LockFileUsageArgs> for pixi_core::environment::LockFileUsage {
}
}

impl TryFrom<LockFileUsageConfig> for pixi_core::environment::LockFileUsage {
type Error = LockFileUsageError;

fn try_from(value: LockFileUsageConfig) -> Result<Self, LockFileUsageError> {
value.validate()?;
impl From<LockFileUsageConfig> for pixi_core::environment::LockFileUsage {
fn from(value: LockFileUsageConfig) -> Self {
if value.frozen {
Ok(Self::Frozen)
Self::Frozen
} else if value.locked {
Ok(Self::Locked)
Self::Locked
} else {
Ok(Self::Update)
Self::Update
}
}
}
Expand All @@ -285,24 +275,14 @@ impl TryFrom<LockFileUsageConfig> for pixi_core::environment::LockFileUsage {
pub struct LockFileUsageConfig {
/// Install the environment as defined in the lockfile, doesn't update
/// lockfile if it isn't up-to-date with the manifest file.
#[clap(long, env = "PIXI_FROZEN", help_heading = consts::CLAP_UPDATE_OPTIONS)]
#[clap(long, env = "PIXI_FROZEN", help_heading = consts::CLAP_UPDATE_OPTIONS, conflicts_with = "locked")]
pub frozen: bool,
/// Check if lockfile is up-to-date before installing the environment,
/// aborts when lockfile isn't up-to-date with the manifest file.
#[clap(long, env = "PIXI_LOCKED", help_heading = consts::CLAP_UPDATE_OPTIONS)]
#[clap(long, env = "PIXI_LOCKED", help_heading = consts::CLAP_UPDATE_OPTIONS, conflicts_with = "frozen")]
pub locked: bool,
}

impl LockFileUsageConfig {
/// Validate that the configuration is valid
pub fn validate(&self) -> Result<(), LockFileUsageError> {
if self.frozen && self.locked {
return Err(LockFileUsageError::FrozenAndLocked);
}
Ok(())
}
}

pub async fn execute() -> miette::Result<()> {
let args = Args::parse();

Expand Down Expand Up @@ -583,95 +563,6 @@ mod tests {
use super::*;
use temp_env;

#[test]
fn test_frozen_and_locked_conflict() {
// Test that --frozen and --locked conflict is caught by validation
let config_result = LockFileUsageConfig::try_parse_from(["test", "--frozen", "--locked"]);
assert!(config_result.is_ok(), "Parsing should succeed");
let parsed = config_result.unwrap();
assert!(parsed.frozen, "Expected frozen to be true");
assert!(parsed.locked, "Expected locked to be true");
assert!(
parsed.validate().is_err(),
"Expected validation to fail when both --frozen and --locked are provided"
);
}

#[test]
fn test_lockfile_usage_args_try_from_validation() {
// Valid case
let valid_raw = LockFileUsageArgsRaw {
frozen: true,
locked: false,
};
let result = LockFileUsageArgs::try_from(valid_raw);
assert!(result.is_ok());

// Valid case
let valid_raw = LockFileUsageArgsRaw {
frozen: false,
locked: true,
};
let result = LockFileUsageArgs::try_from(valid_raw);
assert!(result.is_ok());

// Valid case
let valid_raw = LockFileUsageArgsRaw {
frozen: false,
locked: false,
};
let result = LockFileUsageArgs::try_from(valid_raw);
assert!(result.is_ok());

// Invalid case
let invalid_raw = LockFileUsageArgsRaw {
frozen: true,
locked: true,
};
let result = LockFileUsageArgs::try_from(invalid_raw);
assert!(result.is_err());

let error = result.unwrap_err();
assert!(error.to_string().contains("cannot be used together with"));
}

#[test]
fn test_pixi_frozen_true_with_locked_flag_should_fail() {
// PIXI_FROZEN=true with --locked should fail validation
temp_env::with_var("PIXI_FROZEN", Some("true"), || {
let result = LockFileUsageConfig::try_parse_from(["test", "--locked"]);

assert!(
result.is_ok(),
"Parsing should succeed, but validation should fail"
);
let parsed = result.unwrap();
assert!(parsed.frozen, "Expected frozen flag to be true");
assert!(parsed.locked, "Expected locked flag to be true");
assert!(
parsed.validate().is_err(),
"Expected validation to fail when both frozen and locked are true"
);
});
}

#[test]
fn test_pixi_frozen_false_with_locked_flag_should_pass() {
// PIXI_FROZEN=false with --locked should pass validation
temp_env::with_var("PIXI_FROZEN", Some("false"), || {
let result = LockFileUsageConfig::try_parse_from(["test", "--locked"]);

assert!(
result.is_ok(),
"Expected success when PIXI_FROZEN=false and --locked is used"
);
let parsed = result.unwrap();
assert!(parsed.locked, "Expected locked flag to be true");
assert!(!parsed.frozen, "Expected frozen flag to be false");
assert!(parsed.validate().is_ok(), "Expected validation to pass");
});
}

#[test]
fn test_clap_boolean_env_var_behavior() {
// Test PIXI_FROZEN=true
Expand Down
2 changes: 1 addition & 1 deletion src/cli/reinstall.rs
Original file line number Diff line number Diff line change
Expand Up @@ -82,7 +82,7 @@ pub async fn execute(args: Args) -> miette::Result<()> {
&environment,
UpdateMode::Revalidate,
UpdateLockFileOptions {
lock_file_usage: args.lock_file_usage.clone().try_into()?,
lock_file_usage: args.lock_file_usage.clone().into(),
no_install: false,
max_concurrent_solves: workspace.config().max_concurrent_solves(),
},
Expand Down
2 changes: 1 addition & 1 deletion src/cli/remove.rs
Original file line number Diff line number Diff line change
Expand Up @@ -121,7 +121,7 @@ pub async fn execute(args: Args) -> miette::Result<()> {
&workspace.default_environment(),
UpdateMode::Revalidate,
UpdateLockFileOptions {
lock_file_usage: lock_file_update_config.lock_file_usage()?,
lock_file_usage: lock_file_update_config.lock_file_usage(),
no_install: prefix_update_config.no_install,
max_concurrent_solves: workspace.config().max_concurrent_solves(),
},
Expand Down
2 changes: 1 addition & 1 deletion src/cli/run.rs
Original file line number Diff line number Diff line change
Expand Up @@ -127,7 +127,7 @@ pub async fn execute(args: Args) -> miette::Result<()> {
// Ensure that the lock-file is up-to-date.
let lock_file = workspace
.update_lock_file(UpdateLockFileOptions {
lock_file_usage: args.lock_file_update_config.lock_file_usage()?,
lock_file_usage: args.lock_file_update_config.lock_file_usage(),
max_concurrent_solves: workspace.config().max_concurrent_solves(),
..UpdateLockFileOptions::default()
})
Expand Down
2 changes: 1 addition & 1 deletion src/cli/shell.rs
Original file line number Diff line number Diff line change
Expand Up @@ -280,7 +280,7 @@ pub async fn execute(args: Args) -> miette::Result<()> {
&environment,
UpdateMode::QuickValidate,
UpdateLockFileOptions {
lock_file_usage: args.lock_file_update_config.lock_file_usage()?,
lock_file_usage: args.lock_file_update_config.lock_file_usage(),
no_install: args.prefix_update_config.no_install
&& args.lock_file_update_config.no_lockfile_update,
max_concurrent_solves: workspace.config().max_concurrent_solves(),
Expand Down
2 changes: 1 addition & 1 deletion src/cli/shell_hook.rs
Original file line number Diff line number Diff line change
Expand Up @@ -162,7 +162,7 @@ pub async fn execute(args: Args) -> miette::Result<()> {
&environment,
args.prefix_update_config.update_mode(),
UpdateLockFileOptions {
lock_file_usage: args.lock_file_update_config.lock_file_usage()?,
lock_file_usage: args.lock_file_update_config.lock_file_usage(),
no_install: args.prefix_update_config.no_install
&& args.lock_file_update_config.no_lockfile_update,
max_concurrent_solves: workspace.config().max_concurrent_solves(),
Expand Down
2 changes: 1 addition & 1 deletion src/cli/tree.rs
Original file line number Diff line number Diff line change
Expand Up @@ -81,7 +81,7 @@ pub async fn execute(args: Args) -> miette::Result<()> {

let lock_file = workspace
.update_lock_file(UpdateLockFileOptions {
lock_file_usage: args.lock_file_update_config.lock_file_usage()?,
lock_file_usage: args.lock_file_update_config.lock_file_usage(),
no_install: args.lock_file_update_config.no_lockfile_update,
max_concurrent_solves: workspace.config().max_concurrent_solves(),
})
Expand Down
2 changes: 1 addition & 1 deletion src/cli/upgrade.rs
Original file line number Diff line number Diff line change
Expand Up @@ -93,7 +93,7 @@ pub async fn execute(args: Args) -> miette::Result<()> {
pypi_deps,
IndexMap::default(),
args.prefix_update_config.no_install,
&args.lock_file_update_config.lock_file_usage()?,
&args.lock_file_update_config.lock_file_usage(),
&args.specs.feature,
&[],
false,
Expand Down
2 changes: 1 addition & 1 deletion src/cli/workspace/export/conda_explicit_spec.rs
Original file line number Diff line number Diff line change
Expand Up @@ -168,7 +168,7 @@ pub async fn execute(args: Args) -> miette::Result<()> {

let lockfile = workspace
.update_lock_file(UpdateLockFileOptions {
lock_file_usage: args.lock_file_update_config.lock_file_usage()?,
lock_file_usage: args.lock_file_update_config.lock_file_usage(),
no_install: args.lock_file_update_config.no_lockfile_update,
max_concurrent_solves: workspace.config().max_concurrent_solves(),
})
Expand Down
2 changes: 1 addition & 1 deletion tests/integration_rust/common/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -504,7 +504,7 @@ impl PixiControl {
// Ensure the lock-file is up-to-date
let lock_file = project
.update_lock_file(UpdateLockFileOptions {
lock_file_usage: args.lock_file_update_config.lock_file_usage()?,
lock_file_usage: args.lock_file_update_config.lock_file_usage(),
..UpdateLockFileOptions::default()
})
.await?;
Expand Down
2 changes: 1 addition & 1 deletion tests/integration_rust/upgrade_tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,7 @@ async fn pypi_dependency_index_preserved_on_upgrade() {
pypi_deps,
IndexMap::default(),
args.prefix_update_config.no_install,
&args.lock_file_update_config.lock_file_usage().unwrap(),
&args.lock_file_update_config.lock_file_usage(),
&args.specs.feature,
&[],
true,
Expand Down
Loading