Skip to content
Open
Show file tree
Hide file tree
Changes from 1 commit
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
41 changes: 41 additions & 0 deletions crates/pixi/tests/integration_rust/add_tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -707,6 +707,47 @@ async fn pinning_dependency() {
assert_eq!(python_spec, r#""==3.13""#);
}

#[tokio::test]
async fn add_existing_dependency_without_version_is_noop() {
setup_tracing();

let mut package_database = MockRepoData::default();
package_database.add_package(Package::build("foobar", "1").finish());
package_database.add_package(Package::build("foobar", "2").finish());
let local_channel = package_database.into_channel().await.unwrap();

let pixi = PixiControl::new().unwrap();
pixi.init().with_channel(local_channel.url()).await.unwrap();

// Add with an explicit version
pixi.add("foobar==1").await.unwrap();

let get_spec = |pixi: &PixiControl| -> String {
pixi.workspace()
.unwrap()
.workspace
.value
.default_feature()
.dependencies(SpecType::Run, None)
.unwrap_or_default()
.get_single("foobar")
.unwrap()
.unwrap()
.clone()
.to_toml_value()
.to_string()
};
assert_eq!(get_spec(&pixi), r#""==1""#);

// Re-add without a version — should be a noop, spec should remain ==1
pixi.add("foobar").await.unwrap();
assert_eq!(get_spec(&pixi), r#""==1""#);

// Re-add with an explicit version — should overwrite
pixi.add("foobar==2").await.unwrap();
assert_eq!(get_spec(&pixi), r#""==2""#);
}

#[tokio::test]
async fn add_dependency_pinning_strategy() {
setup_tracing();
Expand Down
8 changes: 5 additions & 3 deletions crates/pixi/tests/integration_rust/install_tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -635,19 +635,21 @@ async fn minimal_lockfile_update_pypi() {
pep508_rs::Requirement::from_str("click==7.1.2").unwrap()
));

// Widening the click version to allow for the latest version
// Re-adding click without a version is a noop since it's already present.
// Only uvicorn gets updated.
pixi.add_multiple(vec!["uvicorn==0.29.0", "click"])
.set_type(pixi_core::DependencyType::PypiDependency)
.with_install(true)
.await
.unwrap();

// `click` should not be updated to a higher version.
Comment thread
baszalmstra marked this conversation as resolved.
// `click` should remain at its original pinned version since the re-add
// without a version was skipped.
let lock = pixi.lock_file().await.unwrap();
assert!(lock.contains_pep508_requirement(
consts::DEFAULT_ENVIRONMENT_NAME,
Platform::current(),
pep508_rs::Requirement::from_str("click>7.1.2").unwrap()
pep508_rs::Requirement::from_str("click==7.1.2").unwrap()
));
}

Expand Down
2 changes: 2 additions & 0 deletions crates/pixi/tests/integration_rust/upgrade_tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ use indexmap::IndexMap;
use insta::assert_snapshot;
use pixi_cli::upgrade::{Args, parse_specs_for_platform};
use pixi_core::Workspace;
use pixi_manifest::DependencyOverwriteBehavior;
use rattler_conda_types::Platform;
use tempfile::TempDir;
use url::Url;
Expand Down Expand Up @@ -77,6 +78,7 @@ async fn pypi_dependency_index_preserved_on_upgrade() {
&[],
true,
args.dry_run,
DependencyOverwriteBehavior::Overwrite,
)
.await
.unwrap();
Expand Down
4 changes: 2 additions & 2 deletions crates/pixi_api/src/context.rs
Original file line number Diff line number Diff line change
Expand Up @@ -288,7 +288,7 @@ impl<I: Interface> WorkspaceContext<I> {
spec_type: SpecType,
dep_options: DependencyOptions,
git_options: GitOptions,
) -> miette::Result<Option<UpdateDeps>> {
) -> miette::Result<(Option<UpdateDeps>, Vec<String>)> {
Box::pin(crate::workspace::add::add_conda_dep(
self.workspace_mut()?,
specs,
Expand All @@ -304,7 +304,7 @@ impl<I: Interface> WorkspaceContext<I> {
pypi_deps: PypiDeps,
editable: bool,
options: DependencyOptions,
) -> miette::Result<Option<UpdateDeps>> {
) -> miette::Result<(Option<UpdateDeps>, Vec<String>)> {
Box::pin(crate::workspace::add::add_pypi_dep(
self.workspace_mut()?,
pypi_deps,
Expand Down
24 changes: 13 additions & 11 deletions crates/pixi_api/src/workspace/add/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ use pixi_core::{
environment::sanity_check_workspace,
workspace::{PypiDeps, UpdateDeps, WorkspaceMut},
};
use pixi_manifest::{FeatureName, KnownPreviewFeature, SpecType};
use pixi_manifest::{DependencyOverwriteBehavior, FeatureName, KnownPreviewFeature, SpecType};
use pixi_spec::{GitSpec, SourceLocationSpec, Subdirectory};
use rattler_conda_types::{MatchSpec, PackageName};

Expand All @@ -18,7 +18,7 @@ pub async fn add_conda_dep(
spec_type: SpecType,
dep_options: DependencyOptions,
git_options: GitOptions,
) -> miette::Result<Option<UpdateDeps>> {
) -> miette::Result<(Option<UpdateDeps>, Vec<String>)> {
sanity_check_workspace(workspace.workspace()).await?;

// Add the platform if it is not already present
Expand Down Expand Up @@ -80,7 +80,7 @@ pub async fn add_conda_dep(
// TODO: add dry_run logic to add
let dry_run = false;

let update_deps = match Box::pin(workspace.update_dependencies(
let (update_deps, skipped) = match Box::pin(workspace.update_dependencies(
match_specs,
IndexMap::default(),
source_specs,
Expand All @@ -90,29 +90,30 @@ pub async fn add_conda_dep(
&dep_options.platforms,
false,
dry_run,
DependencyOverwriteBehavior::OverwriteIfExplicit,
))
.await
{
Ok(update_deps) => {
Ok(result) => {
// Write the updated manifest
workspace.save().await.into_diagnostic()?;
update_deps
result
}
Err(e) => {
workspace.revert().await.into_diagnostic()?;
return Err(e);
}
};

Ok(update_deps)
Ok((update_deps, skipped))
}

pub async fn add_pypi_dep(
mut workspace: WorkspaceMut,
pypi_deps: PypiDeps,
editable: bool,
options: DependencyOptions,
) -> miette::Result<Option<UpdateDeps>> {
) -> miette::Result<(Option<UpdateDeps>, Vec<String>)> {
sanity_check_workspace(workspace.workspace()).await?;

// Add the platform if it is not already present
Expand All @@ -123,7 +124,7 @@ pub async fn add_pypi_dep(
// TODO: add dry_run logic to add
let dry_run = false;

let update_deps = match Box::pin(workspace.update_dependencies(
let (update_deps, skipped) = match Box::pin(workspace.update_dependencies(
IndexMap::default(),
pypi_deps,
IndexMap::default(),
Expand All @@ -133,19 +134,20 @@ pub async fn add_pypi_dep(
&options.platforms,
editable,
dry_run,
DependencyOverwriteBehavior::OverwriteIfExplicit,
))
.await
{
Ok(update_deps) => {
Ok(result) => {
// Write the updated manifest
workspace.save().await.into_diagnostic()?;
update_deps
result
}
Err(e) => {
workspace.revert().await.into_diagnostic()?;
return Err(e);
}
};

Ok(update_deps)
Ok((update_deps, skipped))
}
18 changes: 15 additions & 3 deletions crates/pixi_cli/src/add.rs
Original file line number Diff line number Diff line change
Expand Up @@ -136,7 +136,7 @@ pub async fn execute(args: Args) -> miette::Result<()> {

let workspace_ctx = WorkspaceContext::new(CliInterface {}, workspace.clone());

let update_deps = match args.dependency_config.dependency_type() {
let (update_deps, skipped) = match args.dependency_config.dependency_type() {
DependencyType::CondaDependency(spec_type) => {
let git_options = GitOptions {
git: args.dependency_config.git.clone(),
Expand Down Expand Up @@ -182,10 +182,22 @@ pub async fn execute(args: Args) -> miette::Result<()> {
}
};

for package in &skipped {
eprintln!(
"{}{} is already a dependency",
console::style(console::Emoji("✔ ", "")).green(),
console::style(package).bold(),
);
eprintln!(
" Run {} to get the newest compatible version",
console::style(format!("pixi upgrade {package}")).bold(),
);
}

if let Some(update_deps) = update_deps {
// Notify the user we succeeded
// Notify the user we succeeded, excluding packages that were skipped
args.dependency_config
.display_success("Added", update_deps.implicit_constraints);
.display_success("Added", update_deps.implicit_constraints, &skipped);
}

Ok(())
Expand Down
9 changes: 9 additions & 0 deletions crates/pixi_cli/src/cli_config.rs
Original file line number Diff line number Diff line change
Expand Up @@ -333,8 +333,17 @@ impl DependencyConfig {
&self,
operation: &str,
implicit_constraints: HashMap<String, String>,
skipped: &[String],
) {
for package in self.specs.clone() {
if skipped.iter().any(|s| {
package == s.as_str()
|| package.strip_prefix(s.as_str()).is_some_and(|rest| {
rest.starts_with(|c: char| !c.is_alphanumeric() && c != '-' && c != '_')
})
}) {
continue;
}

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

What does this do?

@benmoss benmoss May 19, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

This prevents the "✔ Added crane" logs from appearing for packages that we skip. The extra prefix testing is to handle the fact that self.specs is raw strings of MatchSpecs and PyPI requirements. It's kinda hacky though. Let me see if I can make this simpler.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

9ca5329 refactors this to use the CondaDependency/PypiDependency types and filter in add.rs

eprintln!(
"{}{operation} {}{}",
console::style(console::Emoji("✔ ", "")).green(),
Expand Down
2 changes: 1 addition & 1 deletion crates/pixi_cli/src/remove.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<()> {
};

args.dependency_config
.display_success("Removed", Default::default());
.display_success("Removed", Default::default(), &[]);

Ok(())
}
7 changes: 5 additions & 2 deletions crates/pixi_cli/src/upgrade.rs
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ use pixi_core::{
workspace::{MatchSpecs, PypiDeps, WorkspaceMut},
};
use pixi_diff::{LockFileDiff, LockFileJsonDiff};
use pixi_manifest::DependencyOverwriteBehavior;
use pixi_manifest::{FeatureName, SpecType};
use pixi_pypi_spec::PixiPypiSource;
use pixi_spec::PixiSpec;
Expand Down Expand Up @@ -157,7 +158,7 @@ pub async fn execute(args: Args) -> miette::Result<()> {
} = specs;

if (!default_match_specs.is_empty() || !default_pypi_deps.is_empty())
&& let Some(update) = workspace
&& let (Some(update), _) = workspace
.update_dependencies(
default_match_specs,
default_pypi_deps,
Expand All @@ -168,6 +169,7 @@ pub async fn execute(args: Args) -> miette::Result<()> {
&[],
false,
args.dry_run,
DependencyOverwriteBehavior::Overwrite,
)
.await?
{
Expand All @@ -185,7 +187,7 @@ pub async fn execute(args: Args) -> miette::Result<()> {
continue;
}

if let Some(update) = workspace
if let (Some(update), _) = workspace
.update_dependencies(
platform_match_specs,
platform_pypi_deps,
Expand All @@ -196,6 +198,7 @@ pub async fn execute(args: Args) -> miette::Result<()> {
&[platform],
false,
args.dry_run,
DependencyOverwriteBehavior::Overwrite,
)
.await?
{
Expand Down
27 changes: 18 additions & 9 deletions crates/pixi_core/src/workspace/workspace_mut.rs
Original file line number Diff line number Diff line change
Expand Up @@ -250,11 +250,13 @@ impl WorkspaceMut {
platforms: &[Platform],
editable: bool,
dry_run: bool,
) -> Result<Option<UpdateDeps>, miette::Error> {
overwrite_behavior: DependencyOverwriteBehavior,
) -> Result<(Option<UpdateDeps>, Vec<String>), miette::Error> {
let mut conda_specs_to_add_constraints_for = IndexMap::new();
let mut pypi_specs_to_add_constraints_for = IndexMap::new();
let mut conda_packages = HashSet::new();
let mut pypi_packages = HashSet::new();
let mut skipped_packages = Vec::new();
let channel_config = self.workspace().channel_config();
for (name, (spec, spec_type)) in match_specs {
let (_, nameless_spec) = spec.into_nameless();
Expand All @@ -267,14 +269,16 @@ impl WorkspaceMut {
spec_type,
platforms,
feature_name,
DependencyOverwriteBehavior::Overwrite,
overwrite_behavior,
)?;
if added {
if nameless_spec.version.is_none() {
conda_specs_to_add_constraints_for
.insert(name.clone(), (spec_type, nameless_spec));
}
conda_packages.insert(name);
} else {
skipped_packages.push(name.as_normalized().to_string());
}
}

Expand All @@ -287,7 +291,7 @@ impl WorkspaceMut {
spec_type,
platforms,
feature_name,
DependencyOverwriteBehavior::Overwrite,
overwrite_behavior,
)?;
}

Expand All @@ -297,7 +301,7 @@ impl WorkspaceMut {
platforms,
feature_name,
Some(editable),
DependencyOverwriteBehavior::Overwrite,
overwrite_behavior,
location,
)?;
if added {
Expand All @@ -306,6 +310,8 @@ impl WorkspaceMut {
.insert(name.clone(), (spec, pixi_spec, location));
}
pypi_packages.insert(name.as_normalized().clone());
} else {
skipped_packages.push(name.as_normalized().to_string());
}
}

Expand All @@ -317,7 +323,7 @@ impl WorkspaceMut {
}

if *lock_file_update_config != LockFileUsage::Update {
return Ok(None);
return Ok((None, skipped_packages));
}

let original_lock_file = self
Expand Down Expand Up @@ -472,10 +478,13 @@ impl WorkspaceMut {
let lock_file_diff =
LockFileDiff::from_lock_files(&original_lock_file, &updated_lock_file.into_lock_file());

Ok(Some(UpdateDeps {
implicit_constraints,
lock_file_diff,
}))
Ok((
Some(UpdateDeps {
implicit_constraints,
lock_file_diff,
}),
skipped_packages,
))
}

// Take some conda and PyPI deps as Vecs of MatchSpecs and Requirements, and add them
Expand Down
Loading