Skip to content

Commit

Permalink
fix: Use correct dependency location for pixi upgrade (prefix-dev#2472
Browse files Browse the repository at this point in the history 
)

Fixes: prefix-dev#2470

This only changes behavior for pyproject.toml.
- if `tool.pixi.dependencies.python` doesn't exist, don't add it
- leave the dependencies at the location where they were originally
defined

---------

Co-authored-by: Julian Hofer <[email protected]>
Co-authored-by: Hofer-Julian <[email protected]>
  • Loading branch information
@ruben-arts @Hofer-Julian
3 people authored Dec 3, 2024
1 parent 2e8483d commit 7571fa4
Show file tree
Hide file tree
Showing 11 changed files with 320 additions and 102 deletions.
12 changes: 7 additions & 5 deletions crates/pixi_manifest/src/lib.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,7 @@ pub use features_ext::FeaturesExt;
pub use has_features_iter::HasFeaturesIter;
pub use has_manifest_ref::HasManifestRef;
use itertools::Itertools;
pub use manifests::{Manifest, ManifestKind, PackageManifest, WorkspaceManifest};
pub use manifests::{Manifest, ManifestKind, ManifestSource, PackageManifest, WorkspaceManifest};
use miette::Diagnostic;
pub use preview::{KnownPreviewFeature, Preview, PreviewFeature};
pub use pypi::pypi_requirement::PyPiRequirement;
Expand Down Expand Up @@ -76,11 +76,13 @@ pub enum DependencyOverwriteBehavior {
}

pub enum PypiDependencyLocation {
// The [pypi-dependencies] or [tool.pixi.pypi-dependencies] table
Pixi,
// The [project.optional-dependencies] table in a 'pyproject.toml' manifest
/// [pypi-dependencies] in pixi.toml or [tool.pixi.pypi-dependencies] in pyproject.toml
PixiPypiDependencies,
/// [project.dependencies] in pyproject.toml
Dependencies,
/// [project.optional-dependencies] table in pyproject.toml
OptionalDependencies,
// The [dependency-groups] table in a 'pyproject.toml' manifest
/// [dependency-groups] in pyproject.toml
DependencyGroups,
}

Expand Down
123 changes: 101 additions & 22 deletions crates/pixi_manifest/src/manifests/source.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -56,7 +56,14 @@ impl ManifestSource {
}
}

fn manifest(&mut self) -> &mut TomlDocument {
fn manifest_mut(&mut self) -> &mut TomlDocument {
match self {
ManifestSource::PyProjectToml(document) => document,
ManifestSource::PixiToml(document) => document,
}
}

fn manifest(&self) -> &TomlDocument {
match self {
ManifestSource::PyProjectToml(document) => document,
ManifestSource::PixiToml(document) => document,
Expand All @@ -80,8 +87,8 @@ impl ManifestSource {
.with_feature_name(Some(feature_name))
.with_table(table);

self.manifest()
.get_or_insert_toml_array(table_name.to_string().as_str(), array_name)
self.manifest_mut()
.get_or_insert_toml_array_mut(table_name.to_string().as_str(), array_name)
}

fn as_table_mut(&mut self) -> &mut Table {
Expand All @@ -105,7 +112,9 @@ impl ManifestSource {
// arrays
let remove_requirement =
|source: &mut ManifestSource, table, array_name| -> Result<(), TomlError> {
let array = source.manifest().get_toml_array(table, array_name)?;
let array = source
.manifest_mut()
.get_mut_toml_array(table, array_name)?;
if let Some(array) = array {
array.retain(|x| {
let req: pep508_rs::Requirement = x
Expand All @@ -118,7 +127,7 @@ impl ManifestSource {
});
if array.is_empty() {
source
.manifest()
.manifest_mut()
.get_or_insert_nested_table(table)?
.remove(array_name);
}
Expand Down Expand Up @@ -146,7 +155,7 @@ impl ManifestSource {
.with_platform(platform.as_ref())
.with_table(Some(consts::PYPI_DEPENDENCIES));

self.manifest()
self.manifest_mut()
.get_or_insert_nested_table(table_name.to_string().as_str())
.map(|t| t.remove(dep.as_source()))?;
Ok(())
Expand All @@ -169,7 +178,7 @@ impl ManifestSource {
.with_platform(platform.as_ref())
.with_table(Some(spec_type.name()));

self.manifest()
self.manifest_mut()
.get_or_insert_nested_table(table_name.to_string().as_str())
.map(|t| t.remove(dep.as_source()))?;
Ok(())
Expand All @@ -186,21 +195,16 @@ impl ManifestSource {
platform: Option<Platform>,
feature_name: &FeatureName,
) -> Result<(), TomlError> {
// let dependency_table =
// self.get_or_insert_toml_table(platform, feature_name, spec_type.name())?;

let dependency_table = TableName::new()
.with_prefix(self.table_prefix())
.with_platform(platform.as_ref())
.with_feature_name(Some(feature_name))
.with_table(Some(spec_type.name()));

self.manifest()
self.manifest_mut()
.get_or_insert_nested_table(dependency_table.to_string().as_str())
.map(|t| t.insert(name.as_normalized(), Item::Value(spec.to_toml_value())))?;

// dependency_table.insert(name.as_normalized(),
// Item::Value(spec.to_toml_value()));
Ok(())
}

Expand Down Expand Up @@ -234,7 +238,7 @@ impl ManifestSource {
// - When a specific platform is requested, as markers are not supported (https://github.com/prefix-dev/pixi/issues/2149)
// - When an editable install is requested
if matches!(self, ManifestSource::PixiToml(_))
|| matches!(location, Some(PypiDependencyLocation::Pixi))
|| matches!(location, Some(PypiDependencyLocation::PixiPypiDependencies))
|| platform.is_some()
|| editable.is_some_and(|e| e)
{
Expand All @@ -250,7 +254,7 @@ impl ManifestSource {
.with_feature_name(Some(feature_name))
.with_table(Some(consts::PYPI_DEPENDENCIES));

self.manifest()
self.manifest_mut()
.get_or_insert_nested_table(dependency_table.to_string().as_str())?
.insert(
requirement.name.as_ref(),
Expand All @@ -266,12 +270,14 @@ impl ManifestSource {
let add_requirement =
|source: &mut ManifestSource, table, array| -> Result<(), TomlError> {
source
.manifest()
.get_or_insert_toml_array(table, array)?
.manifest_mut()
.get_or_insert_toml_array_mut(table, array)?
.push(requirement.to_string());
Ok(())
};
if feature_name.is_default() {
if feature_name.is_default()
|| matches!(location, Some(PypiDependencyLocation::Dependencies))
{
add_requirement(self, "project", "dependencies")?
} else if matches!(location, Some(PypiDependencyLocation::OptionalDependencies)) {
add_requirement(
Expand All @@ -285,6 +291,79 @@ impl ManifestSource {
Ok(())
}

/// Determines the location of a PyPi dependency within the manifest.
///
/// This method checks various sections of the manifest to locate the specified
/// PyPi dependency. It searches in the following order:
/// 1. `pypi-dependencies` table in the manifest.
/// 2. `project.dependencies` array in the manifest.
/// 3. `project.optional-dependencies` array in the manifest.
/// 4. `dependency-groups` array in the manifest.
///
/// # Arguments
///
/// * `dep` - The name of the PyPi package to locate.
/// * `platform` - An optional platform specification.
/// * `feature_name` - The name of the feature to which the dependency belongs.
///
/// # Returns
///
/// An `Option` containing the `PypiDependencyLocation` if the dependency is found,
/// or `None` if it is not found in any of the checked sections.
pub fn pypi_dependency_location(
&self,
package_name: &PyPiPackageName,
platform: Option<Platform>,
feature_name: &FeatureName,
) -> Option<PypiDependencyLocation> {
// For both 'pyproject.toml' and 'pixi.toml' manifest,
// try and to get `pypi-dependency`
let table_name = TableName::new()
.with_prefix(self.table_prefix())
.with_feature_name(Some(feature_name))
.with_platform(platform.as_ref())
.with_table(Some(consts::PYPI_DEPENDENCIES));

let pypi_dependency_table = self
.manifest()
.get_nested_table(table_name.to_string().as_str())
.ok();

if pypi_dependency_table
.and_then(|table| table.get(package_name.as_source()))
.is_some()
{
return Some(PypiDependencyLocation::PixiPypiDependencies);
}

if self
.manifest()
.get_toml_array("project", "dependencies")
.is_ok()
{
return Some(PypiDependencyLocation::Dependencies);
}
let name = feature_name.to_string();

if self
.manifest()
.get_toml_array("project.optional-dependencies", &name)
.is_ok()
{
return Some(PypiDependencyLocation::OptionalDependencies);
}

if self
.manifest()
.get_toml_array("dependency-groups", &name)
.is_ok()
{
return Some(PypiDependencyLocation::DependencyGroups);
}

None
}

/// Removes a task from the TOML manifest
pub fn remove_task(
&mut self,
Expand All @@ -301,7 +380,7 @@ impl ManifestSource {
.with_feature_name(Some(feature_name))
.with_table(Some("tasks"));

self.manifest()
self.manifest_mut()
.get_or_insert_nested_table(task_table.to_string().as_str())?
.remove(name);

Expand All @@ -323,7 +402,7 @@ impl ManifestSource {
.with_feature_name(Some(feature_name))
.with_table(Some("tasks"));

self.manifest()
self.manifest_mut()
.get_or_insert_nested_table(task_table.to_string().as_str())?
.insert(name, task.into());

Expand Down Expand Up @@ -363,7 +442,7 @@ impl ManifestSource {
.with_table(Some("environments"));

// Get the environment table
self.manifest()
self.manifest_mut()
.get_or_insert_nested_table(env_table.to_string().as_str())?
.insert(&name.into(), item);

Expand All @@ -379,7 +458,7 @@ impl ManifestSource {
.with_table(Some("environments"));

Ok(self
.manifest()
.manifest_mut()
.get_or_insert_nested_table(env_table.to_string().as_str())?
.remove(name)
.is_some())
Expand Down
10 changes: 5 additions & 5 deletions crates/pixi_manifest/src/target.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -237,16 +237,16 @@ impl WorkspaceTarget {
) -> Result<bool, DependencyError> {
if self.has_pypi_dependency(requirement, false) {
match dependency_overwrite_behavior {
DependencyOverwriteBehavior::OverwriteIfExplicit
if requirement.version_or_url.is_none() =>
{
return Ok(false)
DependencyOverwriteBehavior::OverwriteIfExplicit => {
if requirement.version_or_url.is_none() {
return Ok(false);
}
}
DependencyOverwriteBehavior::IgnoreDuplicate => return Ok(false),
DependencyOverwriteBehavior::Error => {
return Err(DependencyError::Duplicate(requirement.name.to_string()));
}
_ => {}
DependencyOverwriteBehavior::Overwrite => {}
}
}

Expand Down
40 changes: 38 additions & 2 deletions crates/pixi_manifest/src/toml/document.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,26 @@ impl TomlDocument {
self.0.entry(key).or_insert(item)
}

/// Retrieve a reference to a target table `table_name`
/// in dotted form (e.g. `table1.table2`) from the root of the document.
pub fn get_nested_table<'a>(
&'a self,
table_name: &str,
) -> Result<&'a dyn TableLike, TomlError> {
let parts: Vec<&str> = table_name.split('.').collect();

let mut current_table = self.0.as_table() as &dyn TableLike;

for part in parts {
current_table = current_table
.get(part)
.ok_or_else(|| TomlError::table_error(part, table_name))?
.as_table_like()
.ok_or_else(|| TomlError::table_error(part, table_name))?;
}
Ok(current_table)
}

/// Retrieve a mutable reference to a target table `table_name`
/// in dotted form (e.g. `table1.table2`) from the root of the document.
/// If the table is not found, it is inserted into the document.
Expand Down Expand Up @@ -108,7 +128,7 @@ impl TomlDocument {
/// in table `table_name` in dotted form (e.g. `table1.table2.array`).
///
/// If the array is not found, it is inserted into the document.
pub fn get_or_insert_toml_array<'a>(
pub fn get_or_insert_toml_array_mut<'a>(
&'a mut self,
table_name: &str,
array_name: &str,
Expand All @@ -124,7 +144,7 @@ impl TomlDocument {
/// in table `table_name` in dotted form (e.g. `table1.table2.array`).
///
/// If the array is not found, returns None.
pub fn get_toml_array<'a>(
pub fn get_mut_toml_array<'a>(
&'a mut self,
table_name: &str,
array_name: &str,
Expand All @@ -135,6 +155,22 @@ impl TomlDocument {
.and_then(|a| a.as_array_mut());
Ok(array)
}

/// Retrieves a reference to a target array `array_name`
/// in table `table_name` in dotted form (e.g. `table1.table2.array`).
///
/// If the array is not found, returns None.
pub fn get_toml_array<'a>(
&'a self,
table_name: &str,
array_name: &str,
) -> Result<Option<&'a Array>, TomlError> {
let array = self
.get_nested_table(table_name)?
.get(array_name)
.and_then(|a| a.as_array());
Ok(array)
}
}

#[cfg(test)]
Expand Down
7 changes: 5 additions & 2 deletions src/cli/add.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -109,7 +109,11 @@ pub async fn execute(args: Args) -> miette::Result<()> {
}
DependencyType::PypiDependency => {
let match_specs = IndexMap::default();
let pypi_deps = dependency_config.pypi_deps(&project)?;
let pypi_deps = dependency_config
.pypi_deps(&project)?
.into_iter()
.map(|(name, req)| (name, (req, None)))
.collect();
(match_specs, pypi_deps)
}
};
Expand All @@ -124,7 +128,6 @@ pub async fn execute(args: Args) -> miette::Result<()> {
&args.dependency_config.feature,
&args.dependency_config.platforms,
args.editable,
&None,
dry_run,
)
.await?;
Expand Down
Loading

0 comments on commit 7571fa4

Please sign in to comment.