Skip to content

Commit

Permalink
fix: pixi add with more than just package name and version (#1704)
Browse files Browse the repository at this point in the history
Fixes #1697 
We broke `pixi add --pypi pytest[dev]` as it didn't add the [dev]
anymore. Now that is fixed.

---------

Co-authored-by: Tim de Jager <tdejager89@gmail.com>
  • Loading branch information
ruben-arts and tdejager authored Aug 1, 2024
1 parent d0d103c commit bd1424e
Show file tree
Hide file tree
Showing 3 changed files with 53 additions and 19 deletions.
2 changes: 1 addition & 1 deletion crates/pixi_manifest/src/target.rs
Original file line number Diff line number Diff line change
Expand Up @@ -227,7 +227,7 @@ impl Target {
) {
// TODO: add proper error handling for this
let mut pypi_requirement = PyPiRequirement::try_from(requirement.clone())
.expect("could not convert pep508 requirm");
.expect("could not convert pep508 requirement");
if let Some(editable) = editable {
pypi_requirement.set_editable(editable);
}
Expand Down
30 changes: 13 additions & 17 deletions src/cli/add.rs
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ use itertools::Itertools;
use pep440_rs::VersionSpecifiers;
use pep508_rs::{Requirement, VersionOrUrl::VersionSpecifier};
use pixi_manifest::{pypi::PyPiPackageName, DependencyOverwriteBehavior, FeatureName, SpecType};
use rattler_conda_types::{MatchSpec, NamelessMatchSpec, PackageName, Platform, Version};
use rattler_conda_types::{MatchSpec, PackageName, Platform, Version};
use rattler_lock::{LockFile, Package};

use super::has_specs::HasSpecs;
Expand Down Expand Up @@ -397,7 +397,7 @@ fn update_pypi_specs_from_lock_file(
let pinning_strategy = project.config().pinning_strategy.unwrap_or_default();

// Determine the versions of the packages in the lock-file
for (name, _) in pypi_specs_to_add_constraints_for {
for (name, req) in pypi_specs_to_add_constraints_for {
let version_constraint = pinning_strategy.determine_version_constraint(
pypi_records
.iter()
Expand All @@ -416,14 +416,12 @@ fn update_pypi_specs_from_lock_file(
version_constraint.and_then(|spec| VersionSpecifiers::from_str(&spec.to_string()).ok());
if let Some(version_spec) = version_spec {
implicit_constraints.insert(name.as_source().to_string(), version_spec.to_string());
let req = Requirement {
version_or_url: Some(VersionSpecifier(version_spec)),
..req
};
project.manifest.add_pypi_dependency(
&Requirement {
name: name.as_normalized().clone(),
extras: vec![],
version_or_url: Some(VersionSpecifier(version_spec)),
marker: None,
origin: None,
},
&req,
platforms,
feature_name,
Some(editable),
Expand Down Expand Up @@ -463,7 +461,7 @@ fn update_conda_specs_from_lock_file(

let pinning_strategy = project.config().pinning_strategy.unwrap_or_default();
let channel_config = project.channel_config();
for (name, (spec_type, _)) in conda_specs_to_add_constraints_for {
for (name, (spec_type, spec)) in conda_specs_to_add_constraints_for {
let version_constraint = pinning_strategy.determine_version_constraint(
conda_records.iter().filter_map(|record| {
if record.package_record.name == name {
Expand All @@ -477,14 +475,12 @@ fn update_conda_specs_from_lock_file(
if let Some(version_constraint) = version_constraint {
implicit_constraints
.insert(name.as_source().to_string(), version_constraint.to_string());
let spec = MatchSpec {
version: Some(version_constraint),
..spec
};
project.manifest.add_dependency(
&MatchSpec::from_nameless(
NamelessMatchSpec {
version: Some(version_constraint),
..NamelessMatchSpec::default()
},
Some(name),
),
&spec,
spec_type,
platforms,
feature_name,
Expand Down
40 changes: 39 additions & 1 deletion tests/add_tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4,13 +4,15 @@ use crate::common::builders::HasDependencyConfig;
use crate::common::package_database::{Package, PackageDatabase};
use crate::common::LockFileExt;
use crate::common::PixiControl;
use pixi::{DependencyType, HasFeatures};
use pixi::{DependencyType, HasFeatures, Project};
use pixi_consts::consts;
use pixi_manifest::pypi::PyPiPackageName;
use pixi_manifest::SpecType;
use rattler_conda_types::{PackageName, Platform};
use serial_test::serial;
use std::str::FromStr;
use tempfile::TempDir;
use uv_normalize::ExtraName;

/// Test add functionality for different types of packages.
/// Run, dev, build
Expand Down Expand Up @@ -75,6 +77,29 @@ async fn add_functionality() {
));
}

/// Test adding a package with a specific channel
#[tokio::test]
async fn add_with_channel() {
let pixi = PixiControl::new().unwrap();

pixi.init().no_fast_prefix_overwrite(true).await.unwrap();

pixi.add("conda-forge::py_rattler")
.without_lockfile_update()
.await
.unwrap();

let project = Project::from_path(pixi.manifest_path().as_path()).unwrap();
let mut specs = project
.default_environment()
.dependencies(Some(SpecType::Run), Some(Platform::current()))
.into_specs();

let (name, spec) = specs.next().unwrap();
assert_eq!(name, PackageName::try_from("py_rattler").unwrap());
assert_eq!(spec.channel.unwrap().name(), "conda-forge");
}

/// Test that we get the union of all packages in the lockfile for the run, build and host
#[tokio::test]
async fn add_functionality_union() {
Expand Down Expand Up @@ -240,6 +265,19 @@ async fn add_pypi_functionality() {
.await
.unwrap();

// Read project from file and check if the dev extras are added.
let project = Project::from_path(pixi.manifest_path().as_path()).unwrap();
project
.default_environment()
.pypi_dependencies(None)
.into_specs()
.for_each(|(name, spec)| {
if name == PyPiPackageName::from_str("pytest").unwrap() {
assert_eq!(spec.extras(), &[ExtraName::from_str("dev").unwrap()]);
}
});

// Test all the added packages are in the lock file
let lock = pixi.lock_file().await.unwrap();
assert!(lock.contains_pypi_package(
consts::DEFAULT_ENVIRONMENT_NAME,
Expand Down

0 comments on commit bd1424e

Please sign in to comment.