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: pixi add with more than just package name and version #1704

Merged
merged 6 commits into from
Aug 1, 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
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
Loading