From 9427c56facfe38aebbe8a0912520efa090b84f89 Mon Sep 17 00:00:00 2001 From: Julian Hofer Date: Fri, 3 Jul 2026 12:24:17 +0000 Subject: [PATCH 01/17] feat: accept inline package definitions in package dependency tables Parse `package = {...}` inline definitions in `[package.run-dependencies]`, `[package.host-dependencies]`, `[package.build-dependencies]` and `[package.extra-dependencies.*]` (including their `if(...)` conditional sub-tables), mirroring the workspace-level dependency tables: - `InheritablePackageMap` peels the inline definition off each entry before spec parsing. The surrounding spec must be a `git`, `path` or `url` source, and combining a definition with `workspace = true` is rejected pointing at `[workspace.dependencies]`. - `PackageTarget` gains a name-keyed `inline_packages` map, drained from all dependency tables of the target (one definition per name), converted through the same `TomlPackage::into_manifest` path as workspace-level definitions and fingerprinted the same way (recipe now shared via `InlinePackageManifest::from_named_manifest`). `run-constraints` rejects inline definitions; constraints are binary-only. - The stable hash of `PackageTarget` folds in `(name, content hash)` pairs; empty maps are skipped so existing hashes are unchanged. - Inline definitions now nest: a definition's own dependency tables are parsed by the same code, so a manifest-less source dependency of another inline definition can itself be defined inline. Definitions parsed here are not yet honored by the solve; that plumbing follows in later commits. --- crates/pixi_manifest/src/target.rs | 35 +++ crates/pixi_manifest/src/toml/package.rs | 29 +- .../pixi_manifest/src/toml/package_target.rs | 274 ++++++++++++++++-- crates/pixi_manifest/src/toml/target.rs | 31 +- .../src/utils/inheritable_package_map.rs | 69 ++++- crates/pixi_manifest/src/utils/package_map.rs | 31 +- 6 files changed, 413 insertions(+), 56 deletions(-) diff --git a/crates/pixi_manifest/src/target.rs b/crates/pixi_manifest/src/target.rs index 114a21e374..5e550e9d15 100644 --- a/crates/pixi_manifest/src/target.rs +++ b/crates/pixi_manifest/src/target.rs @@ -90,6 +90,26 @@ pub struct InlinePackageManifest { pub content_hash: InlineContentHash, } +impl InlinePackageManifest { + /// Fingerprint the assembled manifest so editing the inline definition + /// invalidates the content-addressed build caches it feeds. The dependency + /// name is folded in so two identical inline tables declared under + /// different names stay distinct. + pub fn from_named_manifest(name: &PackageName, manifest: PackageManifest) -> Self { + use xxhash_rust::xxh3::Xxh3; + let content_hash = { + let mut hasher = Xxh3::new(); + name.as_normalized().hash(&mut hasher); + manifest.hash(&mut hasher); + InlineContentHash(hasher.finish()) + }; + Self { + manifest, + content_hash, + } + } +} + /// A package target describes the dependencies for a specific platform. #[derive(Default, Debug, Clone)] pub struct PackageTarget { @@ -98,6 +118,11 @@ pub struct PackageTarget { /// Extra groups declared by the package for this target. pub extra_dependencies: IndexMap>, + + /// Inline package definitions attached to source dependencies in this + /// target. Keyed by dependency name; the matching source spec lives in + /// [`Self::dependencies`] or [`Self::extra_dependencies`]. + pub inline_packages: IndexMap, } impl Hash for PackageTarget { @@ -111,6 +136,7 @@ impl Hash for PackageTarget { let Self { dependencies, extra_dependencies, + inline_packages, } = self; let collect = |spec_type: SpecType| -> Vec<(&PackageName, &PixiSpec)> { dependencies @@ -127,11 +153,20 @@ impl Hash for PackageTarget { .iter() .map(|(group, dependencies)| (group, dependencies.iter_specs().collect())) .collect(); + // The content hash is a faithful fingerprint of an inline definition, so + // `(name, content hash)` pairs cover the nested manifests without + // re-walking them. `inline_packages` is an `IndexMap`; its declaration + // order is stable. + let inline: Vec<(&PackageName, u64)> = inline_packages + .iter() + .map(|(name, inline)| (name, inline.content_hash.as_u64())) + .collect(); StableHashBuilder::new() .field("build_dependencies", &build) .field("extra_dependencies", &extra) .field("host_dependencies", &host) + .field("inline_packages", &inline) .field("run_dependencies", &run) .finish(state); } diff --git a/crates/pixi_manifest/src/toml/package.rs b/crates/pixi_manifest/src/toml/package.rs index e50244683b..e60c8f48f8 100644 --- a/crates/pixi_manifest/src/toml/package.rs +++ b/crates/pixi_manifest/src/toml/package.rs @@ -343,6 +343,11 @@ impl TomlPackage { ) -> Result, TomlError> { let mut warnings = Vec::new(); + // Inline package definitions attached to this package's dependency + // specs inherit the same workspace package properties this package + // does. Capture them before the fields are consumed below. + let inline_workspace_properties = workspace.clone(); + // Re-base workspace dependency path specs against this member's // directory. The pool itself stores them relative to the workspace root. let workspace_dependencies = rebase_workspace_path_specs( @@ -399,14 +404,23 @@ impl TomlPackage { } // Unconditional entries form the default target. - let default_package_target = TomlPackageTarget { + let WithWarnings { + value: default_package_target, + warnings: mut default_target_warnings, + } = TomlPackageTarget { run_dependencies: run_unconditional, run_constraints: constraints_unconditional, host_dependencies: host_unconditional, build_dependencies: build_unconditional, extra_dependencies: extra_unconditional, } - .into_package_target(preview, &workspace_dependencies)?; + .into_package_target( + preview, + &workspace_dependencies, + &inline_workspace_properties, + root_directory, + )?; + warnings.append(&mut default_target_warnings); // Fold the conditional sub-tables into one `TomlPackageTarget` per // distinct expression, merging across the dependency sections. @@ -481,7 +495,16 @@ impl TomlPackage { let mut conditional_dependencies: IndexMap = IndexMap::new(); for (expression, toml_target) in conditional_targets { - let target = toml_target.into_package_target(preview, &workspace_dependencies)?; + let WithWarnings { + value: target, + warnings: mut target_warnings, + } = toml_target.into_package_target( + preview, + &workspace_dependencies, + &inline_workspace_properties, + root_directory, + )?; + warnings.append(&mut target_warnings); conditional_dependencies.insert(expression, target); } diff --git a/crates/pixi_manifest/src/toml/package_target.rs b/crates/pixi_manifest/src/toml/package_target.rs index 3d599ba46f..12d2b82f36 100644 --- a/crates/pixi_manifest/src/toml/package_target.rs +++ b/crates/pixi_manifest/src/toml/package_target.rs @@ -1,3 +1,5 @@ +use std::path::Path; + use indexmap::IndexMap; use pixi_build_types::ExtraGroupName; use pixi_spec::TomlSpec; @@ -6,10 +8,12 @@ use rattler_conda_types::PackageName; use toml_span::{DeserError, Value, de_helpers::TableHelper}; use crate::{ - KnownPreviewFeature, Preview, SpecType, TomlError, + InlinePackageManifest, KnownPreviewFeature, Preview, SpecType, TomlError, Warning, + WithWarnings, error::GenericError, target::PackageTarget, toml::target::combine_target_dependencies, + toml::{PackageDefaults, TomlPackage, WorkspacePackageProperties}, utils::{PixiSpanned, inheritable_package_map::InheritablePackageMap}, }; @@ -45,12 +49,81 @@ impl<'de> toml_span::Deserialize<'de> for TomlPackageTarget { } impl TomlPackageTarget { + /// Converts this target into a [`PackageTarget`]. + /// + /// `workspace_properties` and `root_directory` are used to convert any + /// inline package definitions attached to the dependency specs; the + /// definitions inherit the consuming workspace's package properties, so + /// `{ workspace = true }` fields resolve as they would for an on-disk + /// `[package]`. pub fn into_package_target( self, preview: &Preview, workspace_dependencies: &IndexMap, - ) -> Result { + workspace_properties: &WorkspacePackageProperties, + root_directory: &Path, + ) -> Result, TomlError> { let pixi_build_enabled = preview.is_enabled(KnownPreviewFeature::PixiBuild); + let mut warnings: Vec = Vec::new(); + + let TomlPackageTarget { + mut run_dependencies, + run_constraints, + mut host_dependencies, + mut build_dependencies, + mut extra_dependencies, + } = self; + + // Constraints only apply to packages resolved from channels; an inline + // package definition (which describes how to build a source dependency) + // is meaningless there. + if let Some(run_constraints) = &run_constraints + && let Some((_, package)) = run_constraints.value.inline_packages.first() + { + return Err(TomlError::Generic( + GenericError::new( + "inline package definitions are not allowed in `run-constraints`", + ) + .with_opt_span(package.span.clone()) + .with_span_label("inline package definition specified here") + .with_help( + "constraints only apply to packages resolved from channels, not source packages", + ), + )); + } + + // Peel inline package definitions off each dependency table, leaving + // the source specs to flow into the regular dependency map. A package + // name may carry at most one inline definition across the tables of + // this target. + let mut inline_toml: IndexMap> = IndexMap::new(); + { + let mut drain_inline = |map: &mut InheritablePackageMap| -> Result<(), TomlError> { + for (name, package) in std::mem::take(&mut map.inline_packages) { + if inline_toml.insert(name.clone(), package).is_some() { + return Err(TomlError::Generic(GenericError::new(format!( + "the package '{}' has more than one inline definition", + name.as_source() + )))); + } + } + Ok(()) + }; + + for table in [ + &mut run_dependencies, + &mut host_dependencies, + &mut build_dependencies, + ] + .into_iter() + .flatten() + { + drain_inline(&mut table.value)?; + } + for table in extra_dependencies.values_mut() { + drain_inline(&mut table.value)?; + } + } let resolve = |entry: Option>| -> Result< Option>, @@ -68,8 +141,7 @@ impl TomlPackageTarget { .transpose() }; - let extra_dependencies = self - .extra_dependencies + let extra_dependencies = extra_dependencies .into_iter() .map(|(name, dependencies)| { let PixiSpanned { value: name, span } = name; @@ -91,18 +163,42 @@ impl TomlPackageTarget { }) .collect::>()?; - Ok(PackageTarget { + // Convert the inline package definitions into full package manifests. + // Their build source is taken from the surrounding dependency spec, so + // the converted manifests carry no `build.source` of their own. Package + // defaults stay empty: an inline definition describes a dependency, not + // the consuming project. + let mut inline_packages: IndexMap = IndexMap::new(); + for (name, package) in inline_toml { + let WithWarnings { + value: manifest, + warnings: mut package_warnings, + } = package.value.into_manifest( + workspace_properties.clone(), + PackageDefaults::default(), + preview, + root_directory, + )?; + warnings.append(&mut package_warnings); + + let inline = InlinePackageManifest::from_named_manifest(&name, manifest); + inline_packages.insert(name, inline); + } + + Ok(WithWarnings::from(PackageTarget { dependencies: combine_target_dependencies( [ - (SpecType::Run, resolve(self.run_dependencies)?), - (SpecType::Host, resolve(self.host_dependencies)?), - (SpecType::Build, resolve(self.build_dependencies)?), - (SpecType::RunConstraints, resolve(self.run_constraints)?), + (SpecType::Run, resolve(run_dependencies)?), + (SpecType::Host, resolve(host_dependencies)?), + (SpecType::Build, resolve(build_dependencies)?), + (SpecType::RunConstraints, resolve(run_constraints)?), ], pixi_build_enabled, )?, extra_dependencies, + inline_packages, }) + .with_warnings(warnings)) } } @@ -117,6 +213,20 @@ mod test { use super::*; use crate::toml::FromTomlStr; + fn into_package_target( + target: TomlPackageTarget, + preview: &Preview, + ) -> Result { + target + .into_package_target( + preview, + &IndexMap::new(), + &WorkspacePackageProperties::default(), + Path::new(""), + ) + .map(|with_warnings| with_warnings.value) + } + #[test] fn test_package_target_all_dependency_types() { // All four dependency tables on a package target must end up in the @@ -135,10 +245,11 @@ mod test { constrained = ">=4.0" "#; - let package_target = TomlPackageTarget::from_toml_str(input) - .unwrap() - .into_package_target(&Preview::default(), &IndexMap::new()) - .unwrap(); + let package_target = into_package_target( + TomlPackageTarget::from_toml_str(input).unwrap(), + &Preview::default(), + ) + .unwrap(); let lookup = |spec_type: SpecType, name: &str| -> String { package_target @@ -178,14 +289,143 @@ mod test { [extra-dependencies.Invalid] gtest = "*" "#; - let err = TomlPackageTarget::from_toml_str(input) - .unwrap() - .into_package_target(&Preview::default(), &IndexMap::new()) - .unwrap_err(); + let err = into_package_target( + TomlPackageTarget::from_toml_str(input).unwrap(), + &Preview::default(), + ) + .unwrap_err(); let message = err.to_string(); assert!( message.contains("extra") && message.contains("invalid character"), "unexpected error: {message}" ); } + + #[test] + fn test_inline_package_in_run_dependencies() { + // An inline package definition on a run dependency is peeled off into + // the target's inline map; the source spec stays in the dependency + // table. + let preview = Preview::from_iter([KnownPreviewFeature::PixiBuild]); + let input = r#" + [run-dependencies] + rust-package = { git = "https://github.com/user/repo.git", package.build = { backend = { name = "pixi-build-rust", version = "1.0" } } } + "#; + + let target = + into_package_target(TomlPackageTarget::from_toml_str(input).unwrap(), &preview) + .unwrap(); + + let name = PackageName::from_str("rust-package").unwrap(); + let spec = target + .dependencies + .get(&SpecType::Run) + .and_then(|d| d.get(&name)) + .and_then(|s| s.iter().next()) + .expect("spec retained"); + assert!(spec.is_source(), "the spec should remain a source spec"); + + let inline = target + .inline_packages + .get(&name) + .expect("inline package captured"); + assert_eq!( + inline.manifest.build.backend.name.as_normalized(), + "pixi-build-rust" + ); + } + + #[test] + fn test_inline_package_in_extra_dependencies() { + // Extra dependency groups accept source specs, so they accept inline + // definitions too. + let preview = Preview::from_iter([KnownPreviewFeature::PixiBuild]); + let input = r#" + [extra-dependencies.test] + rust-package = { git = "https://github.com/user/repo.git", package.build = { backend = { name = "pixi-build-rust", version = "1.0" } } } + "#; + + let target = + into_package_target(TomlPackageTarget::from_toml_str(input).unwrap(), &preview) + .unwrap(); + + let name = PackageName::from_str("rust-package").unwrap(); + assert!( + target.inline_packages.contains_key(&name), + "inline definition from an extra group must be captured" + ); + } + + #[test] + fn test_inline_package_rejected_in_run_constraints() { + // Constraints are binary-only; an inline definition there is an error. + let input = r#" + [run-constraints] + rust-package = { git = "https://github.com/user/repo.git", package.build = { backend = { name = "pixi-build-rust", version = "1.0" } } } + "#; + + let err = into_package_target( + TomlPackageTarget::from_toml_str(input).unwrap(), + &Preview::from_iter([KnownPreviewFeature::PixiBuild]), + ) + .unwrap_err(); + assert!( + err.to_string().contains("run-constraints"), + "unexpected error: {err}" + ); + } + + #[test] + fn test_inline_package_duplicate_across_tables() { + // The same dependency name may carry at most one inline definition per + // target, mirroring the workspace-level rule. + let preview = Preview::from_iter([KnownPreviewFeature::PixiBuild]); + let input = r#" + [run-dependencies] + rust-package = { git = "https://github.com/user/repo.git", package.build = { backend = { name = "pixi-build-rust", version = "1.0" } } } + + [build-dependencies] + rust-package = { git = "https://github.com/user/repo.git", package.build = { backend = { name = "pixi-build-rust", version = "1.0" } } } + "#; + + let err = into_package_target(TomlPackageTarget::from_toml_str(input).unwrap(), &preview) + .unwrap_err(); + assert!( + err.to_string().contains("more than one inline definition"), + "unexpected error: {err}" + ); + } + + #[test] + fn test_inline_package_with_workspace_marker_rejected() { + // `workspace = true` and an inline definition split ownership between + // the pool and the use site; the combination is rejected at parse time. + let input = r#" + [run-dependencies] + rust-package = { workspace = true, package.build = { backend = { name = "pixi-build-rust", version = "1.0" } } } + "#; + + let err = TomlPackageTarget::from_toml_str(input).unwrap_err(); + let rendered = format_parse_error(input, err); + assert!( + rendered.contains("[workspace.dependencies]"), + "unexpected error: {rendered}" + ); + } + + #[test] + fn test_inline_package_requires_source_location() { + // An inline definition without a source location is meaningless. + let input = r#" + [run-dependencies] + rust-package = { version = "1.0", package.build = { backend = { name = "pixi-build-rust", version = "1.0" } } } + "#; + + let err = TomlPackageTarget::from_toml_str(input).unwrap_err(); + let rendered = format_parse_error(input, err); + assert!( + rendered.contains("requires a `git`, `path` or `url` source"), + "unexpected error: {rendered}" + ); + } } diff --git a/crates/pixi_manifest/src/toml/target.rs b/crates/pixi_manifest/src/toml/target.rs index 4ac51f3349..479a262a84 100644 --- a/crates/pixi_manifest/src/toml/target.rs +++ b/crates/pixi_manifest/src/toml/target.rs @@ -1,19 +1,14 @@ -use std::{ - collections::HashMap, - hash::{Hash, Hasher}, - path::Path, -}; +use std::{collections::HashMap, path::Path}; use indexmap::IndexMap; use pixi_spec::{PixiSpec, TomlLocationSpec}; use pixi_spec_containers::DependencyMap; use pixi_toml::{TomlHashMap, TomlIndexMap}; use toml_span::{DeserError, Value, de_helpers::TableHelper}; -use xxhash_rust::xxh3::Xxh3; use crate::{ - Activation, InlineContentHash, InlinePackageManifest, KnownPreviewFeature, SpecType, - TargetSelector, Task, TaskName, TomlError, Warning, WithWarnings, WorkspaceTarget, + Activation, InlinePackageManifest, KnownPreviewFeature, SpecType, TargetSelector, Task, + TaskName, TomlError, Warning, WithWarnings, WorkspaceTarget, error::GenericError, toml::{ PackageDefaults, TomlPackage, WorkspacePackageProperties, preview::TomlPreview, @@ -134,24 +129,8 @@ impl TomlTarget { )?; warnings.append(&mut package_warnings); - // Fingerprint the assembled manifest so editing the inline - // definition invalidates the content-addressed build caches it - // feeds. The dependency name is folded in so two identical inline - // tables declared under different names stay distinct. - let content_hash = { - let mut hasher = Xxh3::new(); - name.as_normalized().hash(&mut hasher); - manifest.hash(&mut hasher); - InlineContentHash(hasher.finish()) - }; - - inline_packages.insert( - name, - InlinePackageManifest { - manifest, - content_hash, - }, - ); + let inline = InlinePackageManifest::from_named_manifest(&name, manifest); + inline_packages.insert(name, inline); } // Convert dev dependencies from TomlLocationSpec to SourceLocationSpec diff --git a/crates/pixi_manifest/src/utils/inheritable_package_map.rs b/crates/pixi_manifest/src/utils/inheritable_package_map.rs index c63b0694fd..a3cab81d3c 100644 --- a/crates/pixi_manifest/src/utils/inheritable_package_map.rs +++ b/crates/pixi_manifest/src/utils/inheritable_package_map.rs @@ -15,7 +15,11 @@ use crate::{ TomlError, error::GenericError, target::{key_looks_conditional, parse_if_expression}, - utils::package_map::UniquePackageMap, + toml::TomlPackage, + utils::{ + PixiSpanned, + package_map::{UniquePackageMap, peel_inline_package}, + }, }; /// Entry in a `[package.*-dependencies]` table that may inherit from @@ -41,6 +45,11 @@ pub struct InheritablePackageMap { pub specs: IndexMap, pub name_spans: IndexMap>, pub value_spans: IndexMap>, + + /// Inline `package` tables keyed by dependency name. Each is the parsed + /// `package = { ... }` sub-table of the matching source spec. Callers + /// drain these before [`Self::resolve`], which only materializes `specs`. + pub inline_packages: IndexMap>, } impl InheritablePackageMap { @@ -50,11 +59,18 @@ impl InheritablePackageMap { /// Resolve every entry against the pool. Source specs require the /// `pixi-build` preview, matching the non-inheritable tables. + /// + /// Inline package definitions must be drained off `self.inline_packages` + /// before calling this; only the specs are materialized here. pub fn resolve( self, workspace_deps: &IndexMap, is_pixi_build_enabled: bool, ) -> Result { + debug_assert!( + self.inline_packages.is_empty(), + "inline package definitions must be drained before resolve()" + ); let mut out = UniquePackageMap::default(); for (name, spec) in self.specs { let name_span = self.name_spans.get(&name).cloned(); @@ -187,6 +203,35 @@ impl<'de> toml_span::Deserialize<'de> for InheritablePackageMap { }; let entry_span = entry_value.span; + + // An inline package definition describes a dependency the consumer + // fully controls; combining it with `workspace = true` would split + // that control between the pool and the use site. Reject the + // combination before peeling so the error names both keys. + if entry_value.as_table().is_some_and(|table| { + table.contains_key("package") && table.contains_key("workspace") + }) { + errors.errors.push(toml_span::Error { + kind: toml_span::ErrorKind::Custom( + "an inline package definition cannot be combined with `workspace = true`; declare the inline definition in `[workspace.dependencies]` instead" + .into(), + ), + span: entry_span, + line_info: None, + }); + continue; + } + + // Peel off an inline package definition before spec parsing, which + // rejects unknown keys. + let inline_package = match peel_inline_package(&mut entry_value) { + Ok(inline) => inline, + Err(e) => { + errors.merge(e); + None + } + }; + let spec = match parse_inheritable_entry(&mut entry_value) { Ok(spec) => Some(spec), Err(e) => { @@ -195,6 +240,23 @@ impl<'de> toml_span::Deserialize<'de> for InheritablePackageMap { } }; + // An inline package definition describes how to build source code, + // so the surrounding spec must point at a git, path or url source. + if inline_package.is_some() + && let Some(InheritableSpec::Direct(spec)) = spec.as_ref() + && !spec.is_source() + { + errors.errors.push(toml_span::Error { + kind: toml_span::ErrorKind::Custom( + "an inline package definition requires a `git`, `path` or `url` source location" + .into(), + ), + span: entry_span, + line_info: None, + }); + continue; + } + if let (Some(name), Some(spec)) = (name, spec) { result.specs.insert(name.clone(), spec); result @@ -202,7 +264,10 @@ impl<'de> toml_span::Deserialize<'de> for InheritablePackageMap { .insert(name.clone(), key.span.start..key.span.end); result .value_spans - .insert(name, entry_span.start..entry_span.end); + .insert(name.clone(), entry_span.start..entry_span.end); + if let Some(package) = inline_package { + result.inline_packages.insert(name, package); + } } } diff --git a/crates/pixi_manifest/src/utils/package_map.rs b/crates/pixi_manifest/src/utils/package_map.rs index 80afca3b96..e3b3df8195 100644 --- a/crates/pixi_manifest/src/utils/package_map.rs +++ b/crates/pixi_manifest/src/utils/package_map.rs @@ -327,7 +327,7 @@ fn deserialize_dependency_table<'de>( /// Returns `None` otherwise. Rejects an explicit `package.name` (the name comes /// from the dependency key) and a `package.build.source` (the source comes from /// the surrounding spec). -fn peel_inline_package<'de>( +pub(crate) fn peel_inline_package<'de>( value: &mut Value<'de>, ) -> Result>, DeserError> { if value @@ -490,18 +490,33 @@ mod test { ); } - /// Inline definitions do not nest: an inline package's own dependency tables - /// are parsed as plain [`UniquePackageMap`]s, which reject a `package` key. - /// So an inline definition placed inside another inline definition's - /// `run-dependencies` is an error, not a recursively-built package. + /// Inline definitions nest: an inline definition is just a package + /// manifest without a file, so its own dependency tables accept further + /// inline definitions. The nested definition is captured on the inner + /// package's dependency table. #[test] - fn test_inline_package_does_not_nest() { + fn test_inline_package_nests() { let input = r#" outer = { git = "https://x/y.git", package = { build = { backend = { name = "b", version = "1.0" } }, run-dependencies = { inner = { git = "https://x/z.git", package.build = { backend = { name = "b", version = "1.0" } } } } } } "#; + let table = + DependencyTable::from_toml_str(input).expect("a nested inline definition must parse"); + let outer = table + .inline_packages + .get(&PackageName::from_str("outer").unwrap()) + .expect("outer inline captured"); + let run_dependencies = outer + .value + .run_dependencies + .as_ref() + .expect("outer inline has run-dependencies"); assert!( - DependencyTable::from_toml_str(input).is_err(), - "a nested inline definition must be rejected, not built recursively" + run_dependencies + .value + .unconditional + .inline_packages + .contains_key(&PackageName::from_str("inner").unwrap()), + "the nested inline definition must be captured on the inner table" ); } } From 2426577bfc9bffada8b561350caa84da57ebdc2e Mon Sep 17 00:00:00 2001 From: Julian Hofer Date: Fri, 3 Jul 2026 12:32:15 +0000 Subject: [PATCH 02/17] feat: carry inline package definitions on the [workspace.dependencies] pool MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Pool entries may attach an inline package definition to a source spec. A `{ workspace = true }` package dependency inherits the definition together with the spec — a manifest-less source location without its definition would fail backend discovery at build time. - `WorkspaceDependencyMap` peels `package = {...}` off each entry and requires a `git`, `path` or `url` source location next to it. - Definitions convert once at workspace assembly (inheriting the workspace's external package properties, like the workspace-level dependency-table definitions do) and are stored on `Workspace::dependency_inline_packages` / `WorkspacePackageProperties::dependency_inline_packages`. - `into_package_target` adopts the pool definition for inherited entries. The same pool entry inherited in several tables is fine (identical content); clashing with a direct definition of the same name errors as a duplicate. - Backend inheritance from a definition-carrying pool entry keeps erroring via the existing 'build backends cannot be defined as source dependencies' rejection (such entries are always source specs). --- .../pixi_manifest/src/manifests/workspace.rs | 1 + crates/pixi_manifest/src/toml/manifest.rs | 1 + crates/pixi_manifest/src/toml/package.rs | 6 + .../pixi_manifest/src/toml/package_target.rs | 215 ++++++++++++++++-- crates/pixi_manifest/src/toml/workspace.rs | 109 ++++++++- crates/pixi_manifest/src/workspace.rs | 6 + 6 files changed, 305 insertions(+), 33 deletions(-) diff --git a/crates/pixi_manifest/src/manifests/workspace.rs b/crates/pixi_manifest/src/manifests/workspace.rs index 86983b28be..d58f3e2adb 100644 --- a/crates/pixi_manifest/src/manifests/workspace.rs +++ b/crates/pixi_manifest/src/manifests/workspace.rs @@ -271,6 +271,7 @@ impl WorkspaceManifest { documentation: self.workspace.documentation.clone(), homepage: self.workspace.homepage.clone(), dependencies: self.workspace.dependencies.clone(), + dependency_inline_packages: self.workspace.dependency_inline_packages.clone(), workspace_root: Some(self.workspace.root_directory.clone()), } } diff --git a/crates/pixi_manifest/src/toml/manifest.rs b/crates/pixi_manifest/src/toml/manifest.rs index cfcbc53b55..f74b2f6adb 100644 --- a/crates/pixi_manifest/src/toml/manifest.rs +++ b/crates/pixi_manifest/src/toml/manifest.rs @@ -179,6 +179,7 @@ impl TomlManifest { repository: external.repository.clone(), documentation: external.documentation.clone(), dependencies: IndexMap::new(), + dependency_inline_packages: IndexMap::new(), workspace_root: Some(root_directory.to_path_buf()), }; diff --git a/crates/pixi_manifest/src/toml/package.rs b/crates/pixi_manifest/src/toml/package.rs index e60c8f48f8..ff9968365e 100644 --- a/crates/pixi_manifest/src/toml/package.rs +++ b/crates/pixi_manifest/src/toml/package.rs @@ -241,6 +241,11 @@ pub struct WorkspacePackageProperties { /// `[workspace.dependencies]` pool; paths are relative to `workspace_root`. pub dependencies: IndexMap, + /// Inline package definitions attached to pool entries, keyed by + /// dependency name. Inherited together with the matching spec by + /// `{ workspace = true }` package dependencies. + pub dependency_inline_packages: IndexMap, + /// Absolute directory of the workspace manifest. Used to re-base /// `dependencies` path specs against the member's directory. pub workspace_root: Option, @@ -260,6 +265,7 @@ impl From for WorkspacePackageProperties { repository: value.repository, documentation: value.documentation, dependencies: IndexMap::new(), + dependency_inline_packages: IndexMap::new(), workspace_root: None, } } diff --git a/crates/pixi_manifest/src/toml/package_target.rs b/crates/pixi_manifest/src/toml/package_target.rs index 12d2b82f36..556bb46022 100644 --- a/crates/pixi_manifest/src/toml/package_target.rs +++ b/crates/pixi_manifest/src/toml/package_target.rs @@ -14,7 +14,10 @@ use crate::{ target::PackageTarget, toml::target::combine_target_dependencies, toml::{PackageDefaults, TomlPackage, WorkspacePackageProperties}, - utils::{PixiSpanned, inheritable_package_map::InheritablePackageMap}, + utils::{ + PixiSpanned, + inheritable_package_map::{InheritablePackageMap, InheritableSpec}, + }, }; #[derive(Debug, Default)] @@ -125,6 +128,71 @@ impl TomlPackageTarget { } } + // Convert the inline package definitions into full package manifests. + // Their build source is taken from the surrounding dependency spec, so + // the converted manifests carry no `build.source` of their own. Package + // defaults stay empty: an inline definition describes a dependency, not + // the consuming project. + let mut inline_packages: IndexMap = IndexMap::new(); + for (name, package) in inline_toml { + let WithWarnings { + value: manifest, + warnings: mut package_warnings, + } = package.value.into_manifest( + workspace_properties.clone(), + PackageDefaults::default(), + preview, + root_directory, + )?; + warnings.append(&mut package_warnings); + + let inline = InlinePackageManifest::from_named_manifest(&name, manifest); + inline_packages.insert(name, inline); + } + + // `{ workspace = true }` entries inherit the pool's inline definition + // together with the spec: a manifest-less source location without its + // definition would fail discovery at build time. The same pool entry + // inherited in several tables is fine (identical content); clashing + // with a direct definition of the same name is not. + { + let mut adopt = |map: &InheritablePackageMap| -> Result<(), TomlError> { + for (name, spec) in &map.specs { + if !matches!(spec, InheritableSpec::Inherited { .. }) { + continue; + } + let Some(pool_inline) = + workspace_properties.dependency_inline_packages.get(name) + else { + continue; + }; + match inline_packages.entry(name.clone()) { + indexmap::map::Entry::Occupied(existing) => { + if existing.get().content_hash != pool_inline.content_hash { + return Err(TomlError::Generic(GenericError::new(format!( + "the package '{}' has more than one inline definition", + name.as_source() + )))); + } + } + indexmap::map::Entry::Vacant(entry) => { + entry.insert(pool_inline.clone()); + } + } + } + Ok(()) + }; + for table in [&run_dependencies, &host_dependencies, &build_dependencies] + .into_iter() + .flatten() + { + adopt(&table.value)?; + } + for table in extra_dependencies.values() { + adopt(&table.value)?; + } + } + let resolve = |entry: Option>| -> Result< Option>, TomlError, @@ -163,28 +231,6 @@ impl TomlPackageTarget { }) .collect::>()?; - // Convert the inline package definitions into full package manifests. - // Their build source is taken from the surrounding dependency spec, so - // the converted manifests carry no `build.source` of their own. Package - // defaults stay empty: an inline definition describes a dependency, not - // the consuming project. - let mut inline_packages: IndexMap = IndexMap::new(); - for (name, package) in inline_toml { - let WithWarnings { - value: manifest, - warnings: mut package_warnings, - } = package.value.into_manifest( - workspace_properties.clone(), - PackageDefaults::default(), - preview, - root_directory, - )?; - warnings.append(&mut package_warnings); - - let inline = InlinePackageManifest::from_named_manifest(&name, manifest); - inline_packages.insert(name, inline); - } - Ok(WithWarnings::from(PackageTarget { dependencies: combine_target_dependencies( [ @@ -227,6 +273,129 @@ mod test { .map(|with_warnings| with_warnings.value) } + /// Build workspace properties whose pool declares a single git source + /// dependency `name` carrying an inline definition. + fn pool_properties_with_inline(name: &str) -> WorkspacePackageProperties { + use crate::toml::TomlWorkspace; + let doc = format!( + r#" + name = "ws" + channels = [] + platforms = [] + preview = ["pixi-build"] + + [dependencies] + {name} = {{ git = "https://github.com/user/repo.git", package.build = {{ backend = {{ name = "pixi-build-rust", version = "1.0" }} }} }} + "# + ); + let workspace = TomlWorkspace::from_toml_str(&doc) + .expect("workspace parses") + .into_workspace(Default::default(), Path::new("")) + .expect("workspace converts") + .value; + WorkspacePackageProperties { + dependencies: workspace.dependencies, + dependency_inline_packages: workspace.dependency_inline_packages, + ..Default::default() + } + } + + #[test] + fn test_workspace_marker_inherits_pool_inline_definition() { + // `{ workspace = true }` inherits the pool entry's inline definition + // together with the source spec. + let preview = Preview::from_iter([KnownPreviewFeature::PixiBuild]); + let properties = pool_properties_with_inline("rust-package"); + let input = r#" + [run-dependencies] + rust-package = { workspace = true } + "#; + + let target = TomlPackageTarget::from_toml_str(input) + .unwrap() + .into_package_target( + &preview, + &properties.dependencies.clone(), + &properties, + Path::new(""), + ) + .unwrap() + .value; + + let name = PackageName::from_str("rust-package").unwrap(); + let inline = target + .inline_packages + .get(&name) + .expect("pool inline definition inherited"); + assert_eq!( + inline.manifest.build.backend.name.as_normalized(), + "pixi-build-rust" + ); + let spec = target + .dependencies + .get(&SpecType::Run) + .and_then(|d| d.get(&name)) + .and_then(|s| s.iter().next()) + .expect("spec inherited"); + assert!(spec.is_source(), "the inherited spec is a source spec"); + } + + #[test] + fn test_workspace_marker_inherited_twice_is_not_a_conflict() { + // The same pool definition inherited in two tables has identical + // content and must not trip the duplicate check. + let preview = Preview::from_iter([KnownPreviewFeature::PixiBuild]); + let properties = pool_properties_with_inline("rust-package"); + let input = r#" + [run-dependencies] + rust-package = { workspace = true } + + [host-dependencies] + rust-package = { workspace = true } + "#; + + let target = TomlPackageTarget::from_toml_str(input) + .unwrap() + .into_package_target( + &preview, + &properties.dependencies.clone(), + &properties, + Path::new(""), + ) + .unwrap() + .value; + assert_eq!(target.inline_packages.len(), 1); + } + + #[test] + fn test_direct_definition_conflicts_with_inherited_pool_definition() { + // A direct inline definition in one table and a pool-inherited one in + // another for the same name disagree about the package's content. + let preview = Preview::from_iter([KnownPreviewFeature::PixiBuild]); + let properties = pool_properties_with_inline("rust-package"); + let input = r#" + [run-dependencies] + rust-package = { workspace = true } + + [build-dependencies] + rust-package = { git = "https://github.com/user/other.git", package.build = { backend = { name = "pixi-build-cmake", version = "1.0" } } } + "#; + + let err = TomlPackageTarget::from_toml_str(input) + .unwrap() + .into_package_target( + &preview, + &properties.dependencies.clone(), + &properties, + Path::new(""), + ) + .unwrap_err(); + assert!( + err.to_string().contains("more than one inline definition"), + "unexpected error: {err}" + ); + } + #[test] fn test_package_target_all_dependency_types() { // All four dependency tables on a package target must end up in the diff --git a/crates/pixi_manifest/src/toml/workspace.rs b/crates/pixi_manifest/src/toml/workspace.rs index 982c26cf9d..d648383b6e 100644 --- a/crates/pixi_manifest/src/toml/workspace.rs +++ b/crates/pixi_manifest/src/toml/workspace.rs @@ -12,14 +12,15 @@ use toml_span::{DeserError, Span, Spanned, Value, de_helpers::TableHelper, value use url::Url; use crate::{ - KnownPreviewFeature, PixiPlatform, PrioritizedChannel, S3Options, TargetSelector, Targets, - TomlError, WithWarnings, Workspace, + InlinePackageManifest, KnownPreviewFeature, PixiPlatform, PrioritizedChannel, S3Options, + TargetSelector, Targets, TomlError, WithWarnings, Workspace, error::GenericError, pypi::pypi_options::PypiOptions, toml::{ + PackageDefaults, TomlPackage, WorkspacePackageProperties, manifest::ExternalWorkspaceProperties, platform::TomlPixiPlatform, preview::TomlPreview, }, - utils::PixiSpanned, + utils::{PixiSpanned, package_map::peel_inline_package}, workspace::{BuildVariantSource, ChannelPriority, CondaPypiMap, SolveStrategy}, }; @@ -27,9 +28,15 @@ use crate::{ /// Unlike `UniquePackageMap` (which materializes `PixiSpec`), this keeps the /// flat `TomlSpec` form so member overrides can be layered without a round /// trip back through `into_spec`. -#[derive(Debug, Default, Clone)] +#[derive(Debug, Default)] pub struct WorkspaceDependencyMap { pub specs: IndexMap, + + /// Inline `package` tables keyed by dependency name. Each is the parsed + /// `package = { ... }` sub-table of the matching source spec. Entries that + /// carry one are inherited together with their spec by + /// `{ workspace = true }` package dependencies. + pub inline_packages: IndexMap>, } impl<'de> toml_span::Deserialize<'de> for WorkspaceDependencyMap { @@ -43,6 +50,7 @@ impl<'de> toml_span::Deserialize<'de> for WorkspaceDependencyMap { let mut errors = DeserError { errors: vec![] }; let mut specs: IndexMap = IndexMap::new(); + let mut inline_packages: IndexMap> = IndexMap::new(); let mut seen: IndexMap = IndexMap::new(); for (key, mut entry) in table { let name = match PackageName::from_str(&key.name) { @@ -70,28 +78,70 @@ impl<'de> toml_span::Deserialize<'de> for WorkspaceDependencyMap { continue; } }; + + let entry_span = entry.span; + + // Peel off an inline package definition before spec parsing, which + // rejects unknown keys. + let inline_package = match peel_inline_package(&mut entry) { + Ok(inline) => inline, + Err(e) => { + errors.merge(e); + None + } + }; + match TomlSpec::deserialize_from_value(&mut entry) { Ok(spec) => { - specs.insert(name, spec); + // An inline package definition describes how to build + // source code, so the surrounding spec must point at a + // git, path or url source. + if inline_package.is_some() && !toml_spec_has_source_location(&spec) { + errors.errors.push(toml_span::Error { + kind: toml_span::ErrorKind::Custom( + "an inline package definition requires a `git`, `path` or `url` source location" + .into(), + ), + span: entry_span, + line_info: None, + }); + continue; + } + specs.insert(name.clone(), spec); + if let Some(package) = inline_package { + inline_packages.insert(name, package); + } } Err(e) => errors.merge(e), } } if errors.errors.is_empty() { - Ok(Self { specs }) + Ok(Self { + specs, + inline_packages, + }) } else { Err(errors) } } } +/// Returns true when the spec carries any source location (`path`, `git` or +/// `url`). Used to validate that an inline package definition sits on a +/// source spec. +fn toml_spec_has_source_location(spec: &TomlSpec) -> bool { + spec.location + .as_ref() + .is_some_and(|loc| loc.path.is_some() || loc.git.is_some() || loc.url.is_some()) +} + #[derive(Debug, Clone)] pub struct TomlWorkspaceTarget { build_variants: Option>>, } /// The TOML representation of the `[[workspace]]` section in a pixi manifest. -#[derive(Debug, Clone)] +#[derive(Debug)] pub struct TomlWorkspace { // In TOML the workspace name can be empty. It is a required field though, but this is enforced // when converting the TOML model to the actual manifest. When using a PyProject we want to use @@ -199,7 +249,7 @@ impl TomlWorkspace { // Source specs gated on pixi-build. Path specs are left // workspace-relative; members re-base them at inheritance time. - let dependencies = if let Some(deps) = self.dependencies { + let (dependencies, dependency_inline_packages) = if let Some(deps) = self.dependencies { let pixi_build_enabled = preview.is_enabled(KnownPreviewFeature::PixiBuild); let specs = deps.value.specs; if !pixi_build_enabled @@ -215,9 +265,47 @@ impl TomlWorkspace { .with_opt_span(deps.span.clone()) .into()); } - specs + + // Convert inline package definitions attached to pool entries. Like + // definitions on the workspace dependency tables, they inherit the + // workspace's external package properties; the workspace manifest + // itself is not built yet. + let mut inline_packages = IndexMap::new(); + if !deps.value.inline_packages.is_empty() { + let inline_properties = WorkspacePackageProperties { + name: external.name.clone(), + version: external.version.clone(), + description: external.description.clone(), + authors: external.authors.clone(), + license: external.license.clone(), + license_file: external.license_file.clone(), + readme: external.readme.clone(), + homepage: external.homepage.clone(), + repository: external.repository.clone(), + documentation: external.documentation.clone(), + dependencies: IndexMap::new(), + dependency_inline_packages: IndexMap::new(), + workspace_root: Some(root_directory.to_path_buf()), + }; + for (name, package) in deps.value.inline_packages { + let WithWarnings { + value: manifest, + warnings: mut package_warnings, + } = package.value.into_manifest( + inline_properties.clone(), + PackageDefaults::default(), + &preview, + root_directory, + )?; + warnings.append(&mut package_warnings); + let inline = InlinePackageManifest::from_named_manifest(&name, manifest); + inline_packages.insert(name, inline); + } + } + + (specs, inline_packages) } else { - IndexMap::new() + (IndexMap::new(), IndexMap::new()) }; Ok(WithWarnings::from(Workspace { @@ -256,6 +344,7 @@ impl TomlWorkspace { exclude_newer_package_overrides: IndexMap::default(), pypi_exclude_newer_package_overrides: IndexMap::default(), dependencies, + dependency_inline_packages, root_directory: root_directory.to_path_buf(), must_migrate: false, use_platform_composition: false, diff --git a/crates/pixi_manifest/src/workspace.rs b/crates/pixi_manifest/src/workspace.rs index ce0ad4a8b7..755d03bc93 100644 --- a/crates/pixi_manifest/src/workspace.rs +++ b/crates/pixi_manifest/src/workspace.rs @@ -15,6 +15,7 @@ use toml_span::{DeserError, Value}; use url::Url; use super::pypi::pypi_options::PypiOptions; +use crate::InlinePackageManifest; use crate::{ PixiPlatform, PixiPlatformName, PrioritizedChannel, S3Options, TargetSelector, Targets, preview::Preview, @@ -108,6 +109,11 @@ pub struct Workspace { /// `root_directory`; members re-base them at inheritance time. pub dependencies: IndexMap, + /// Inline package definitions attached to pool entries, keyed by + /// dependency name. Inherited together with the matching spec by + /// `{ workspace = true }` package dependencies. + pub dependency_inline_packages: IndexMap, + /// Absolute directory of the workspace manifest. Used to re-base relative /// path specs in `dependencies` for members in other directories. pub root_directory: PathBuf, From b50eb88299b588e4dacfc8a6c47809273f2aaf68 Mon Sep 17 00:00:00 2001 From: Julian Hofer Date: Fri, 3 Jul 2026 12:46:02 +0000 Subject: [PATCH 03/17] feat: honor package-level inline definitions throughout the solve Inline package definitions declared in a package's dependency tables now take effect for every source package pixi resolves, uniformly for run-, host-, build- and extra-dependencies: - `PackageManifest::combined_inline_packages` merges the definitions across the default and conditional targets. Which conditional target applies is decided by the backend, so matching is by name alone; parsing rejects a name carrying different definitions in different targets. - `DiscoveredBackend` carries the merged map (plus the workspace manifest that gives definitions their channel context) off backend discovery. The map is not serialized: it feeds only the in-process pipeline, never the metadata disk cache. Discovery runs on cache hits too, so `BuildBackendMetadata` and `SourceOutputs` expose the map on both paths. - `assemble_source_record` passes the package's map into its nested build/host environment solves; the existing seed-attachment logic matches definitions by name against the backend-reported deps. - The transitive walk in `walk_and_resolve` attaches the parent package's definition when pushing a child source dependency (run deps and extras flow through the same loop). The seen-set now records the inline content hash and the pushing parent: a direct dependency of the environment keeps overriding package-level definitions (seed-first, as before), while two package-level parents disagreeing about the same (package, source location) fail the solve with a `ConflictingInlineDefinitions` error naming both parents. - `ResolveSourcePackageKey` now returns `ResolvedSourcePackage` (records + the package's inline map) so the walk can do the attachment; cache identity of children is unchanged since the child key already hashes its inline definition. --- Cargo.lock | 1 + crates/pixi_build_discovery/Cargo.toml | 1 + crates/pixi_build_discovery/src/discovery.rs | 53 ++++++++++++- crates/pixi_cli/src/publish.rs | 2 +- .../src/build_backend_metadata/mod.rs | 13 ++- crates/pixi_command_dispatcher/src/errors.rs | 17 ++++ .../src/inline_package.rs | 33 +++++++- .../src/keys/resolve_source_package.rs | 28 ++++++- .../src/keys/resolve_source_record.rs | 20 +++-- .../src/keys/solve_pixi_environment.rs | 79 +++++++++++++++---- .../src/keys/source_metadata.rs | 4 + .../lock_file/satisfiability/legacy/key.rs | 1 + crates/pixi_manifest/src/manifests/package.rs | 23 +++++- crates/pixi_manifest/src/toml/package.rs | 31 ++++++++ 14 files changed, 276 insertions(+), 30 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 229927c575..78811e219e 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -6305,6 +6305,7 @@ name = "pixi_build_discovery" version = "0.1.0" dependencies = [ "dunce", + "indexmap 2.14.0", "insta", "itertools 0.15.0", "miette 7.6.0", diff --git a/crates/pixi_build_discovery/Cargo.toml b/crates/pixi_build_discovery/Cargo.toml index 3c1434332f..15a905cdd9 100644 --- a/crates/pixi_build_discovery/Cargo.toml +++ b/crates/pixi_build_discovery/Cargo.toml @@ -11,6 +11,7 @@ version = "0.1.0" [dependencies] dunce = { workspace = true } +indexmap = { workspace = true } itertools = { workspace = true } miette = { workspace = true } ordermap = { workspace = true } diff --git a/crates/pixi_build_discovery/src/discovery.rs b/crates/pixi_build_discovery/src/discovery.rs index 014b5b19a6..9dc96eab71 100644 --- a/crates/pixi_build_discovery/src/discovery.rs +++ b/crates/pixi_build_discovery/src/discovery.rs @@ -1,8 +1,10 @@ use std::{ ffi::OsStr, path::{Path, PathBuf}, + sync::Arc, }; +use indexmap::IndexMap; use itertools::Itertools; use miette::Diagnostic; use ordermap::OrderMap; @@ -13,12 +15,13 @@ use pixi_consts::consts::{ KNOWN_MANIFEST_FILES, RATTLER_BUILD_DIRS, RATTLER_BUILD_FILE_NAMES, ROS_BACKEND_FILE_NAMES, }; use pixi_manifest::{ - DiscoveryStart, ExplicitManifestError, PackageManifest, PrioritizedChannel, WithProvenance, - WorkspaceDiscoverer, WorkspaceDiscoveryError, WorkspaceManifest, + DiscoveryStart, ExplicitManifestError, InlinePackageManifest, PackageManifest, + PrioritizedChannel, WithProvenance, WorkspaceDiscoverer, WorkspaceDiscoveryError, + WorkspaceManifest, }; use pixi_spec::{SourceLocationSpec, SpecConversionError}; use pixi_spec_containers::DependencyMap; -use rattler_conda_types::ChannelConfig; +use rattler_conda_types::{ChannelConfig, PackageName}; use thiserror::Error; use crate::{ @@ -37,6 +40,28 @@ pub struct DiscoveredBackend { /// The parameters used to initialize the backend. pub init_params: BackendInitializationParams, + + /// Inline package definitions declared by the discovered package's + /// dependency tables, if any. These describe how to build *dependencies* + /// of the discovered package that carry no manifest of their own; the + /// consumer matches them by name against the dependencies the backend + /// reports. Not serialized: the parsed manifests only matter to the + /// in-process build pipeline. + #[cfg_attr(feature = "serde", serde(skip))] + pub inline_packages: Option>, +} + +/// Inline package definitions found on a discovered package, together with the +/// workspace manifest that provides their context (channels, workspace root) +/// when a backend is built for them. +#[derive(Debug, Clone)] +pub struct DiscoveredInlinePackages { + /// The definitions, keyed by dependency name and merged across the + /// package's targets. + pub packages: IndexMap, + + /// The workspace manifest the definitions were declared under. + pub workspace: Arc, } /// The parameters used to initialize a build backend @@ -227,6 +252,7 @@ impl DiscoveredBackend { configuration: None, target_configuration: None, }, + inline_packages: None, }) } @@ -295,6 +321,25 @@ impl DiscoveredBackend { // Construct the project model from the manifest let project_model = to_project_model_v1(package_manifest, channel_config)?; + // Collect the inline package definitions the package declares for its + // own dependencies. They stay pixi-side (the project model carries only + // the source specs) and are matched by name against the dependencies + // the backend reports. The workspace manifest travels along so a + // backend can later be built for each definition. + let inline_packages = { + let merged: IndexMap = package_manifest + .combined_inline_packages() + .into_iter() + .map(|(name, inline)| (name, inline.clone())) + .collect(); + (!merged.is_empty()).then(|| { + Arc::new(DiscoveredInlinePackages { + packages: merged, + workspace: Arc::new(workspace_manifest.clone()), + }) + }) + }; + // Determine the build system requirements. let build_system = package_manifest.build.clone(); let requirement = ( @@ -359,6 +404,7 @@ impl DiscoveredBackend { }), target_configuration, }, + inline_packages, }) } @@ -453,6 +499,7 @@ impl DiscoveredBackend { configuration: None, target_configuration: None, }, + inline_packages: None, }) } } diff --git a/crates/pixi_cli/src/publish.rs b/crates/pixi_cli/src/publish.rs index 358f30188f..d1354721b8 100644 --- a/crates/pixi_cli/src/publish.rs +++ b/crates/pixi_cli/src/publish.rs @@ -731,7 +731,7 @@ pub async fn execute(args: Args) -> miette::Result<()> { .await .map_err_into_dispatcher(std::convert::identity) .into_diagnostic()?; - resolved_records.extend(records.iter().cloned()); + resolved_records.extend(records.records.iter().cloned()); } // `--clean` nukes the per-package artifact + workspace caches so diff --git a/crates/pixi_command_dispatcher/src/build_backend_metadata/mod.rs b/crates/pixi_command_dispatcher/src/build_backend_metadata/mod.rs index fdb90775c1..8593e619c0 100644 --- a/crates/pixi_command_dispatcher/src/build_backend_metadata/mod.rs +++ b/crates/pixi_command_dispatcher/src/build_backend_metadata/mod.rs @@ -4,7 +4,7 @@ use once_cell::sync::Lazy; use pixi_build_types::procedures::conda_outputs::CondaOutputsParams; use pixi_record::{CanonicalSourceLocation, PinnedBuildSourceSpec, PinnedSourceSpec, VariantValue}; use pixi_spec::{ResolvedExcludeNewer, SourceAnchor, SourceLocationSpec}; -use rattler_conda_types::ChannelUrl; +use rattler_conda_types::{ChannelUrl, PackageName}; use std::time::SystemTime; use std::{ collections::{BTreeMap, HashSet}, @@ -233,6 +233,11 @@ pub struct BuildBackendMetadata { /// The cache key string that was used to store/look up this metadata. pub cache_key: CacheKeyString, + /// Inline package definitions this package declares for its own + /// dependencies, keyed by dependency name. Produced by backend discovery + /// (which runs on cache hits too), never by the metadata cache itself. + pub inline_packages: Arc>, + /// The metadata that was acquired from the build backend. pub metadata: CacheEntry, @@ -911,6 +916,9 @@ impl BuildBackendMetadataInner { Ok(CacheProbe::Hit(BuildBackendMetadata { source: manifest_source_location, cache_key: cache_key.key(), + inline_packages: crate::inline_package::inline_packages_from_backend( + &checkouts.discovered_backend, + ), metadata: fresh, skip_cache, })) @@ -1082,6 +1090,9 @@ impl BuildBackendMetadataInner { Ok(BuildBackendMetadata { source: manifest_source_location, cache_key: cache_key.key(), + inline_packages: crate::inline_package::inline_packages_from_backend( + &checkouts.discovered_backend, + ), metadata, skip_cache, }) diff --git a/crates/pixi_command_dispatcher/src/errors.rs b/crates/pixi_command_dispatcher/src/errors.rs index 5559e0f025..b034f5cb1f 100644 --- a/crates/pixi_command_dispatcher/src/errors.rs +++ b/crates/pixi_command_dispatcher/src/errors.rs @@ -258,6 +258,23 @@ pub enum SolvePixiEnvironmentError { #[error(transparent)] #[diagnostic(transparent)] SourceMetadata(SourceMetadataError), + + /// Two packages in the same solve declare different inline package + /// definitions (or one declares a definition and the other none) for the + /// same dependency at the same source location. There is no priority + /// order between arbitrary packages, so the ambiguity is an error. + #[error( + "the packages '{first_parent}' and '{second_parent}' declare conflicting inline definitions for '{package}' at '{source_location}'" + )] + #[diagnostic(help( + "Declare '{package}' with one inline definition in the workspace's dependency tables; the workspace-level definition overrides package-level ones." + ))] + ConflictingInlineDefinitions { + package: Box, + source_location: Box, + first_parent: Box, + second_parent: Box, + }, } impl From for SolvePixiEnvironmentError { diff --git a/crates/pixi_command_dispatcher/src/inline_package.rs b/crates/pixi_command_dispatcher/src/inline_package.rs index 13deff2cce..facdebc4bf 100644 --- a/crates/pixi_command_dispatcher/src/inline_package.rs +++ b/crates/pixi_command_dispatcher/src/inline_package.rs @@ -15,6 +15,7 @@ //! whenever the definition is edited. use std::{ + collections::BTreeMap, hash::{Hash, Hasher}, path::Path, sync::Arc, @@ -24,7 +25,7 @@ use pixi_build_discovery::{DiscoveredBackend, DiscoveryError}; use pixi_compute_engine::ComputeCtx; use pixi_manifest::{InlineContentHash, PackageManifest, WorkspaceManifest}; use pixi_spec::SpecConversionError; -use rattler_conda_types::ChannelConfig; +use rattler_conda_types::{ChannelConfig, PackageName}; use crate::{discovered_backend::DiscoveredBackendKey, injected_config::ChannelConfigKey}; @@ -102,6 +103,36 @@ pub(crate) fn serialize_optional_content_hash( } } +/// Convert the inline package definitions carried on a discovered backend into +/// dispatcher [`InlinePackage`]s, keyed by dependency name. +/// +/// These are the definitions the discovered package declares for its *own* +/// dependencies; the caller matches them by name against the dependencies the +/// backend reports (nested build/host env seeds and transitive run deps). +pub(crate) fn inline_packages_from_backend( + backend: &DiscoveredBackend, +) -> Arc> { + let Some(discovered) = backend.inline_packages.as_ref() else { + return Arc::new(BTreeMap::new()); + }; + Arc::new( + discovered + .packages + .iter() + .map(|(name, inline)| { + ( + name.clone(), + InlinePackage { + manifest: Arc::new(inline.manifest.clone()), + workspace: discovered.workspace.clone(), + content_hash: inline.content_hash, + }, + ) + }) + .collect(), + ) +} + /// Discover the build backend for a checked-out source, honoring an inline /// package definition. /// diff --git a/crates/pixi_command_dispatcher/src/keys/resolve_source_package.rs b/crates/pixi_command_dispatcher/src/keys/resolve_source_package.rs index b8f15924d2..87fef3aedf 100644 --- a/crates/pixi_command_dispatcher/src/keys/resolve_source_package.rs +++ b/crates/pixi_command_dispatcher/src/keys/resolve_source_package.rs @@ -55,6 +55,19 @@ pub struct ResolveSourcePackageSpec { pub installed_source_hints: PtrArc, } +/// The assembled records of one source package plus the inline package +/// definitions that package declares for its own dependencies. +#[derive(Debug)] +pub struct ResolvedSourcePackage { + /// One assembled record per variant output. + pub records: Vec>, + + /// Inline definitions this package declares for its dependencies, keyed + /// by dependency name. The enclosing environment walk applies them to the + /// transitive source dependencies discovered on `records`. + pub inline_packages: Arc>, +} + /// Compute-engine Key returning every variant's assembled /// `SourceRecord` for one source package. /// @@ -70,7 +83,7 @@ impl ResolveSourcePackageKey { } impl Key for ResolveSourcePackageKey { - type Value = Result>>, SourceRecordError>; + type Value = Result, SourceRecordError>; #[instrument( skip_all, @@ -156,7 +169,7 @@ async fn resolve_source_package_inner( spec: Arc, own_pin: Option, manifest_pin_override: Option, -) -> Result>>, SourceRecordError> { +) -> Result, SourceRecordError> { // SMK only needs this package's pin as a checkout override; // the full pin map flows through assembly for recursion. let outputs = ctx @@ -181,6 +194,11 @@ async fn resolve_source_package_inner( // Fold the inline definition's content hash into each assembled record's // identifier so editing the inline table changes the lock entry. let inline_content_hash = spec.inline.as_ref().map(|inline| inline.content_hash); + // Inline definitions this package declares for its own dependencies, + // matched by name against the deps the backend reported. They feed the + // nested build/host solves here and the caller's transitive walk. + let inline_packages = Arc::clone(&outputs.inline_packages); + let assemble_inline_packages = Arc::clone(&inline_packages); let mapper = ComputeCtx::declare_join_closure( async move |bctx: &mut ComputeCtx, output: CondaOutput| { assemble_source_record( @@ -191,6 +209,7 @@ async fn resolve_source_package_inner( &env_ref, &source_hints, inline_content_hash, + &assemble_inline_packages, ) .await }, @@ -199,7 +218,10 @@ async fn resolve_source_package_inner( .try_compute_join(outputs.outputs.iter().cloned(), mapper) .await?; - Ok(Arc::new(records)) + Ok(Arc::new(ResolvedSourcePackage { + records, + inline_packages, + })) } /// Map a `SourceMetadataError` into `SourceRecordError`. diff --git a/crates/pixi_command_dispatcher/src/keys/resolve_source_record.rs b/crates/pixi_command_dispatcher/src/keys/resolve_source_record.rs index c05a040752..e6471360d8 100644 --- a/crates/pixi_command_dispatcher/src/keys/resolve_source_record.rs +++ b/crates/pixi_command_dispatcher/src/keys/resolve_source_record.rs @@ -24,8 +24,8 @@ use rattler_conda_types::{PackageName, PackageRecord, package::RunExportsJson}; use rattler_solve::SolveStrategy; use crate::{ - BuildBackendMetadataSpec, DerivedEnvKind, EnvironmentRef, InstalledSourceHints, PtrArc, - SourceRecordError, SourceRecordReporterSpec, + BuildBackendMetadataSpec, DerivedEnvKind, EnvironmentRef, InlinePackage, InstalledSourceHints, + PtrArc, SourceRecordError, SourceRecordReporterSpec, build::{Dependencies, PinnedSourceCodeLocation, PixiRunExports, convert_extra_dependencies}, compute_data::{HasGateway, HasSourceRecordReporter}, cycle::CycleEnvironment, @@ -48,6 +48,7 @@ use pixi_manifest::InlineContentHash; /// in the subtree remain visible. /// /// Reports progress via `Arc` set on the engine `DataStore`, if any. +#[allow(clippy::too_many_arguments)] pub(super) async fn assemble_source_record( ctx: &mut ComputeCtx, source: &PinnedSourceCodeLocation, @@ -56,6 +57,7 @@ pub(super) async fn assemble_source_record( env_ref: &EnvironmentRef, installed_source_hints: &PtrArc, inline_content_hash: Option, + inline_packages: &Arc>, ) -> Result, SourceRecordError> { // Reporter lifecycle for this variant's source-record assembly. // Build a `SourceRecordReporterSpec` from the data flowing through here so @@ -101,6 +103,7 @@ pub(super) async fn assemble_source_record( env_ref, installed_source_hints, inline_content_hash, + inline_packages, ); match active_id { Some(id) => id.scope_active(work).await, @@ -108,7 +111,7 @@ pub(super) async fn assemble_source_record( } } -#[allow(clippy::result_large_err)] +#[allow(clippy::result_large_err, clippy::too_many_arguments)] async fn assemble_source_record_inner( ctx: &mut ComputeCtx, source: &PinnedSourceCodeLocation, @@ -117,6 +120,7 @@ async fn assemble_source_record_inner( env_ref: &EnvironmentRef, installed_source_hints: &PtrArc, inline_content_hash: Option, + inline_packages: &Arc>, ) -> Result, SourceRecordError> { let source_location = SourceLocationSpec::from(source.manifest_source().clone()); let source_anchor = SourceAnchor::from(source_location.clone()); @@ -157,6 +161,7 @@ async fn assemble_source_record_inner( build_dependencies.clone(), Arc::clone(&installed_build_packages), installed_source_hints, + inline_packages, ) .await?; @@ -201,6 +206,7 @@ async fn assemble_source_record_inner( host_dependencies.clone(), Arc::clone(&installed_host_packages), installed_source_hints, + inline_packages, ) .await?; @@ -485,6 +491,7 @@ async fn nested_solve( dependencies: Dependencies, installed: Arc<[UnresolvedPixiRecord]>, installed_source_hints: &PtrArc, + inline_packages: &Arc>, ) -> Result, SourceRecordError> { if dependencies.dependencies.is_empty() { return Ok(vec![]); @@ -512,9 +519,10 @@ async fn nested_solve( strategy: SolveStrategy::default(), preferred_build_source: Arc::clone(preferred_build_source), env_ref: env_ref.derived(pkg_name.clone(), kind), - // A nested build/host env solves binary/source build deps; inline - // definitions apply only to the consumer's direct dependencies. - inline_packages: Default::default(), + // The package's own inline definitions apply to the source deps of its + // build/host envs; the seed-attachment inside the nested solve matches + // them by name. + inline_packages: Arc::clone(inline_packages), }; // Wrap the nested SolvePixiEnvironmentKey call in a cycle diff --git a/crates/pixi_command_dispatcher/src/keys/solve_pixi_environment.rs b/crates/pixi_command_dispatcher/src/keys/solve_pixi_environment.rs index 5845428dae..6d370314e7 100644 --- a/crates/pixi_command_dispatcher/src/keys/solve_pixi_environment.rs +++ b/crates/pixi_command_dispatcher/src/keys/solve_pixi_environment.rs @@ -9,7 +9,7 @@ //! private `resolve_source_record` module. use std::{ - collections::{BTreeMap, HashMap, HashSet}, + collections::{BTreeMap, HashMap}, hash::{Hash, Hasher}, mem, sync::Arc, @@ -477,7 +477,7 @@ struct SourceSeed { /// the seeds. Binary match specs are NOT collected here; they're /// derived downstream inside [`SolveCondaKey`] from the same /// assembled records (see `derive_fetch_specs_from_source_repodata`). -#[allow(clippy::mutable_key_type)] +#[allow(clippy::mutable_key_type, clippy::result_large_err)] async fn walk_and_resolve( ctx: &mut ComputeCtx, seeds: Vec, @@ -486,7 +486,7 @@ async fn walk_and_resolve( installed_source_hints: &PtrArc, ) -> Result>, SolvePixiEnvironmentError> { let mut all_records: Vec> = Vec::new(); - let mut seen_sources: HashSet<(PackageName, SourceLocationSpec)> = HashSet::new(); + let mut seen_sources: HashMap<(PackageName, SourceLocationSpec), SeenSource> = HashMap::new(); // Fallback cycle frame used when a pushed RSP's guard fires but // the engine's cycle ring carried no RSP frames (e.g. the cycle @@ -505,13 +505,48 @@ async fn walk_and_resolve( // cannot preserve across completed peers. let push = |p: &mut ParallelBuilder<'_>, pending: &mut FuturesUnordered<_>, - seen: &mut HashSet<(PackageName, SourceLocationSpec)>, + seen: &mut HashMap<(PackageName, SourceLocationSpec), SeenSource>, name: PackageName, location: SourceLocationSpec, parent: Option, - inline: Option| { - if !seen.insert((name.clone(), location.clone())) { - return; + inline: Option| + -> Result<(), SolvePixiEnvironmentError> { + match seen.entry((name.clone(), location.clone())) { + std::collections::hash_map::Entry::Occupied(existing) => { + let existing = existing.get(); + // A direct dependency of this environment is the consumer's + // own declaration; it overrides package-level definitions the + // same way a workspace-level inline definition overrides an + // on-disk manifest. + if existing.parent.is_none() { + return Ok(()); + } + // Two package-level parents must agree: there is no priority + // order between arbitrary packages, so a mismatch (including + // definition-vs-none) is ambiguous. + let inline_hash = inline.as_ref().map(|inline| inline.content_hash); + if existing.inline == inline_hash { + return Ok(()); + } + let describe = |parent: &Option| -> Box { + parent + .as_ref() + .map(|p| p.as_source().into()) + .unwrap_or_else(|| "".into()) + }; + return Err(SolvePixiEnvironmentError::ConflictingInlineDefinitions { + package: name.as_source().into(), + source_location: location.to_string().into(), + first_parent: describe(&existing.parent), + second_parent: describe(&parent), + }); + } + std::collections::hash_map::Entry::Vacant(entry) => { + entry.insert(SeenSource { + inline: inline.as_ref().map(|inline| inline.content_hash), + parent: parent.clone(), + }); + } } let key = ResolveSourcePackageKey::new(ResolveSourcePackageSpec { package: name.clone(), @@ -533,7 +568,7 @@ async fn walk_and_resolve( .with_cycle_guard(async |cctx| cctx.compute(&key).await) .await; match guarded { - Ok(Ok(records)) => Ok((name, parent, records)), + Ok(Ok(resolved)) => Ok((name, parent, resolved)), Ok(Err(e)) => Err(SolvePixiEnvironmentError::from( crate::SourceMetadataError::SourceRecord(e), )), @@ -565,6 +600,7 @@ async fn walk_and_resolve( } } })); + Ok(()) }; for SourceSeed { @@ -581,11 +617,12 @@ async fn walk_and_resolve( source.location, None, inline, - ); + )?; } while let Some(result) = pending.next().await { - let (parent_pkg, _parent_of_parent, records) = result?; + let (parent_pkg, _parent_of_parent, resolved) = result?; + let records = &resolved.records; // Walk the assembled records' `.depends` (post run-exports) // for source refs and queue newly-seen pairs, tagging each @@ -611,6 +648,11 @@ async fn walk_and_resolve( }; if let Some(source_location) = record.sources().get(child_name.as_normalized()) { let resolved_location = anchor.resolve_location(source_location.clone()); + // The parent package's own inline definitions apply to + // its dependencies, matched by name. This covers run + // deps and extras alike; both flow through `.depends` / + // `.extra_depends` above. + let child_inline = resolved.inline_packages.get(&child_name).cloned(); push( &mut p, &mut pending, @@ -618,10 +660,8 @@ async fn walk_and_resolve( child_name, resolved_location, Some(parent_pkg.clone()), - // Transitive source deps carry their own on-disk - // manifest; inline definitions never apply to them. - None, - ); + child_inline, + )?; } } } @@ -632,6 +672,17 @@ async fn walk_and_resolve( Ok(all_records) } +/// Bookkeeping for one `(package, source location)` pair the walk has already +/// scheduled: which inline definition it was resolved with and who pushed it. +struct SeenSource { + /// Content hash of the inline definition attached to the scheduled + /// resolution, if any. + inline: Option, + /// `None` when pushed as a direct dependency of this environment (seed); + /// `Some(pkg)` when discovered through `pkg`'s assembled record. + parent: Option, +} + /// Pin + metadata for every dev source, concurrently. async fn process_dev_sources( ctx: &mut ComputeCtx, diff --git a/crates/pixi_command_dispatcher/src/keys/source_metadata.rs b/crates/pixi_command_dispatcher/src/keys/source_metadata.rs index 7846f7c932..f490a9721e 100644 --- a/crates/pixi_command_dispatcher/src/keys/source_metadata.rs +++ b/crates/pixi_command_dispatcher/src/keys/source_metadata.rs @@ -109,6 +109,9 @@ pub struct SourceOutputs { /// Outputs for `package` in backend order. Each carries declared /// build/host/run deps; none are resolved yet. pub outputs: Vec, + /// Inline package definitions this package declares for its own + /// dependencies, keyed by dependency name. + pub inline_packages: Arc>, } /// Compute-engine Key for "outputs of this package from this source". @@ -213,6 +216,7 @@ impl Key for SourceMetadataKey { Ok(Arc::new(SourceOutputs { source: build_backend_metadata.source.clone(), outputs: matching, + inline_packages: Arc::clone(&build_backend_metadata.inline_packages), })) } } diff --git a/crates/pixi_core/src/lock_file/satisfiability/legacy/key.rs b/crates/pixi_core/src/lock_file/satisfiability/legacy/key.rs index d714eb22cf..d49a01d85d 100644 --- a/crates/pixi_core/src/lock_file/satisfiability/legacy/key.rs +++ b/crates/pixi_core/src/lock_file/satisfiability/legacy/key.rs @@ -170,6 +170,7 @@ impl Key for LegacySourceEnvKey { .await?; let matched = variants + .records .iter() .find(|sr| variants_equivalent(&self.variants, &sr.variants)) .ok_or_else(|| LegacySourceEnvError::NoMatchingVariant { diff --git a/crates/pixi_manifest/src/manifests/package.rs b/crates/pixi_manifest/src/manifests/package.rs index 3cb66af745..af69a2566e 100644 --- a/crates/pixi_manifest/src/manifests/package.rs +++ b/crates/pixi_manifest/src/manifests/package.rs @@ -2,8 +2,9 @@ use std::hash::{Hash, Hasher}; use indexmap::IndexMap; use pixi_build_types::ConditionalExpression; +use rattler_conda_types::PackageName; -use crate::target::PackageTarget; +use crate::target::{InlinePackageManifest, PackageTarget}; use crate::{PackageBuild, package::Package}; /// Holds the parsed content of the package part of a pixi manifest. This @@ -27,6 +28,26 @@ pub struct PackageManifest { pub conditional_dependencies: IndexMap, } +impl PackageManifest { + /// Returns the inline package definitions declared by this package's + /// dependency tables, merged across the default and conditional targets. + /// + /// Which conditional targets apply is decided by the build backend, so the + /// merge is name-keyed and target-agnostic; parsing rejects the ambiguous + /// case of a name carrying *different* definitions in different targets. + pub fn combined_inline_packages(&self) -> IndexMap { + let mut merged: IndexMap = IndexMap::new(); + for target in + std::iter::once(&self.dependencies).chain(self.conditional_dependencies.values()) + { + for (name, inline) in &target.inline_packages { + merged.entry(name.clone()).or_insert(inline); + } + } + merged + } +} + impl Hash for PackageManifest { fn hash(&self, state: &mut H) { self.package.hash(state); diff --git a/crates/pixi_manifest/src/toml/package.rs b/crates/pixi_manifest/src/toml/package.rs index ff9968365e..f62a2c9b32 100644 --- a/crates/pixi_manifest/src/toml/package.rs +++ b/crates/pixi_manifest/src/toml/package.rs @@ -514,6 +514,37 @@ impl TomlPackage { conditional_dependencies.insert(expression, target); } + // Which conditional target applies is decided by the build backend, + // not by pixi, so inline definitions are matched to backend-reported + // dependencies by name alone. A name carrying *different* definitions + // in different targets is therefore ambiguous; reject it. + { + let mut seen: IndexMap<&PackageName, crate::InlineContentHash> = IndexMap::new(); + for target in + std::iter::once(&default_package_target).chain(conditional_dependencies.values()) + { + for (name, inline) in &target.inline_packages { + match seen.entry(name) { + indexmap::map::Entry::Occupied(existing) => { + if *existing.get() != inline.content_hash { + return Err(GenericError::new(format!( + "the package '{}' has conflicting inline definitions across conditional dependency tables", + name.as_source() + )) + .with_help( + "Inline definitions are matched to dependencies by name; declare one definition per package name.", + ) + .into()); + } + } + indexmap::map::Entry::Vacant(entry) => { + entry.insert(inline.content_hash); + } + } + } + } + } + if let Some(WorkspaceInheritableField::Value(Spanned { value: license, span, From adc91d79dcc6322deea279a8c0c242a6b07a43ec Mon Sep 17 00:00:00 2001 From: Julian Hofer Date: Fri, 3 Jul 2026 12:57:39 +0000 Subject: [PATCH 04/17] feat: honor package-level inline definitions at build and verify time MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The solve resolves records with package-level inline definitions; the build/install pipeline and lock-file verification need the same definitions to reconstruct backends without an on-disk manifest: - `SourceBuildKey` discovers its record's backend once (in-memory cached, no backend spawn) and uses the resulting map to (a) attach definitions to nested source-dependency builds (previously hardcoded `inline: None`) and (b) pass them into the build/host prefix installs, which recurse through the same env-install path. - The env install (`install_pixi_environment`) extends the caller-supplied (consumer-level, higher-precedence) definitions with package-level ones collected from each source record's manifest. Inline-defined records have no on-disk manifest, so collection iterates until no new definitions turn up, resolving one link of an A→B→C definition chain per round. - `verify_partial_source_record_against_backend` falls back to the workspace's own `[package]` definitions when the environment-level lookup misses. Definitions declared by transitive packages' manifests are not found there; the fresh metadata query then fails discovery and forces a re-lock, which resolves them through the regular walk. --- .../src/install_pixi/ext.rs | 56 ++++++++++++++++++- .../src/keys/source_build.rs | 53 ++++++++++++++---- .../lock_file/satisfiability/source_record.rs | 26 +++++++-- 3 files changed, 118 insertions(+), 17 deletions(-) diff --git a/crates/pixi_command_dispatcher/src/install_pixi/ext.rs b/crates/pixi_command_dispatcher/src/install_pixi/ext.rs index e0776d9176..35afc5f829 100644 --- a/crates/pixi_command_dispatcher/src/install_pixi/ext.rs +++ b/crates/pixi_command_dispatcher/src/install_pixi/ext.rs @@ -153,7 +153,61 @@ async fn install_inner( }; // Inline package definitions for the source records in this // install, looked up per record by name when building from source. - let inline_packages = Arc::new(std::mem::take(&mut spec.inline_packages)); + // The caller-supplied (consumer-level) definitions take precedence; + // they are extended with package-level ones below. + let mut combined_inline = std::mem::take(&mut spec.inline_packages); + + // Each source record's manifest may declare inline definitions for other + // source records of this same install (its own dependencies). Collect + // them by discovering each record's backend; the discovery is in-memory + // cached and spawns no backend. A record that is itself inline-defined + // has no on-disk manifest, so its discovery needs its definition first — + // iterate until no new definitions turn up (a chain A→B→C resolves one + // link per round). Checkout/discovery failures are skipped here; the + // real error resurfaces with proper context in the build below. + { + use pixi_compute_sources::SourceCheckoutExt; + let mut discovered: std::collections::HashSet = + std::collections::HashSet::new(); + loop { + let mut progressed = false; + for record in &source_records { + if discovered.contains(record.name()) { + continue; + } + let inline = combined_inline.get(record.name()).cloned(); + let Ok(checkout) = ctx + .checkout_pinned_source(record.manifest_source.clone()) + .await + else { + continue; + }; + let Ok(backend) = crate::inline_package::discover_backend( + ctx, + checkout.path.as_std_path(), + inline.as_ref(), + ) + .await + else { + continue; + }; + discovered.insert(record.name().clone()); + progressed = true; + for (name, inline) in + crate::inline_package::inline_packages_from_backend(&backend).iter() + { + combined_inline + .entry(name.clone()) + .or_insert_with(|| inline.clone()); + } + } + if !progressed { + break; + } + } + } + + let inline_packages = Arc::new(combined_inline); let mapper = { let shared = shared.clone(); let inline_packages = inline_packages.clone(); diff --git a/crates/pixi_command_dispatcher/src/keys/source_build.rs b/crates/pixi_command_dispatcher/src/keys/source_build.rs index b76ff8f5fc..959c3e14c5 100644 --- a/crates/pixi_command_dispatcher/src/keys/source_build.rs +++ b/crates/pixi_command_dispatcher/src/keys/source_build.rs @@ -140,16 +140,29 @@ async fn compute_inner( ctx: &mut ComputeCtx, spec: Arc, ) -> Result { - // sha256s are collected in a stable (build, host) order so the - // artifact cache key stays deterministic across buckets. - let (build_source_dep_sha256s, host_source_dep_sha256s) = - recurse_source_deps(ctx, &spec).await?; - let manifest_source = spec.record.manifest_source.clone(); let manifest_checkout = ctx .checkout_pinned_source(manifest_source.clone()) .await .map_err(SourceBuildError::SourceCheckout)?; + + // Inline package definitions this package declares for its own + // dependencies. They are needed to build nested source deps and to + // install the build/host environments below; the discovery behind this is + // in-memory cached, no backend is spawned. + let dep_inline_packages = crate::inline_package::discover_backend( + ctx, + manifest_checkout.path.as_std_path(), + spec.inline.as_ref(), + ) + .await + .map(|backend| crate::inline_package::inline_packages_from_backend(&backend)) + .map_err(SourceBuildError::Discovery)?; + + // sha256s are collected in a stable (build, host) order so the + // artifact cache key stays deterministic across buckets. + let (build_source_dep_sha256s, host_source_dep_sha256s) = + recurse_source_deps(ctx, &spec, &dep_inline_packages).await?; let build_source_checkout = match spec.record.build_source.as_ref() { Some(pinned) => ctx .checkout_pinned_source(pinned.pinned().clone()) @@ -292,6 +305,7 @@ async fn compute_inner( InstallTarget::Build, directories.build_prefix.clone(), spec.record.build_packages.clone(), + &dep_inline_packages, ) .await }, @@ -302,6 +316,7 @@ async fn compute_inner( InstallTarget::Host, directories.host_prefix.clone(), spec.record.host_packages.clone(), + &dep_inline_packages, ) .await }, @@ -482,6 +497,9 @@ async fn compute_inner( async fn recurse_source_deps( ctx: &mut ComputeCtx, spec: &Arc, + dep_inline_packages: &Arc< + std::collections::BTreeMap, + >, ) -> Result<(Vec, Vec), SourceBuildError> { // build_packages run on the build platform. The nested build's // HOST platform is therefore the outer's BUILD platform. @@ -490,6 +508,7 @@ async fn recurse_source_deps( spec.clone(), spec.record.build_packages.clone(), spec.build_environment.to_build_from_build(), + dep_inline_packages, ) .await?; // host_packages target the outer host platform. The nested build's @@ -499,6 +518,7 @@ async fn recurse_source_deps( spec.clone(), spec.record.host_packages.clone(), spec.build_environment.clone(), + dep_inline_packages, ) .await?; Ok((build, host)) @@ -510,6 +530,9 @@ async fn build_source_deps( spec: Arc, packages: Vec, nested_build_environment: BuildEnvironment, + dep_inline_packages: &Arc< + std::collections::BTreeMap, + >, ) -> Result, SourceBuildError> { let sources: Vec> = packages .into_iter() @@ -524,9 +547,13 @@ async fn build_source_deps( let mapper = { let spec = spec.clone(); let build_env = nested_build_environment; + let dep_inline_packages = Arc::clone(dep_inline_packages); async move |sub_ctx: &mut ComputeCtx, src: Arc| -> Result { + // A nested source dependency may be defined inline by this + // package's own dependency tables; match by name. + let inline = dep_inline_packages.get(src.name()).cloned(); let nested_spec = SourceBuildSpec { record: src, channels: spec.channels.clone(), @@ -543,9 +570,7 @@ async fn build_source_deps( // Nested source deps are unpacked into the parent's // prefix immediately; use the cheapest compression. package_format: Some(CondaPackageFormat::fast()), - // A nested source dependency carries its own on-disk manifest; - // inline definitions apply only to the consumer's direct deps. - inline: None, + inline, }; let result = sub_ctx.compute(&SourceBuildKey::new(nested_spec)).await?; Ok(result.artifact_sha256) @@ -637,6 +662,9 @@ async fn install_prefix( target: InstallTarget, prefix_path: PathBuf, packages: Vec, + dep_inline_packages: &Arc< + std::collections::BTreeMap, + >, ) -> Result< ( Vec, @@ -673,9 +701,12 @@ async fn install_prefix( channels: spec.channels.clone(), variant_configuration: spec.variant_configuration.clone(), variant_files: spec.variant_files.clone(), - // Build/host environments install pre-built packages; no inline - // definitions apply here. - inline_packages: Default::default(), + // Source packages in the build/host environment may be defined + // inline by this package's own dependency tables. + inline_packages: dep_inline_packages + .iter() + .map(|(name, inline)| (name.clone(), inline.clone())) + .collect(), }; let result = ctx .install_pixi_environment(install_spec) diff --git a/crates/pixi_core/src/lock_file/satisfiability/source_record.rs b/crates/pixi_core/src/lock_file/satisfiability/source_record.rs index 383a74c5ea..1dfa310259 100644 --- a/crates/pixi_core/src/lock_file/satisfiability/source_record.rs +++ b/crates/pixi_core/src/lock_file/satisfiability/source_record.rs @@ -140,15 +140,31 @@ pub(super) async fn verify_partial_source_record_against_backend( // Look up this package's inline definition from the current // manifest, if any. It is needed both to re-query metadata without an // on-disk manifest and so an edit to the inline table re-derives different - // outputs (forcing a re-lock). - let pixi_platform = pixi_manifest::HasWorkspaceManifest::workspace_manifest(ctx.environment) - .workspace - .platform_by_name(&ctx.platform); + // outputs (forcing a re-lock). Workspace-level definitions take + // precedence; the workspace's own `[package]` dependency tables are the + // fallback. A definition declared by a *transitive* package's manifest is + // not found here: the fresh metadata query below then fails discovery and + // forces a re-lock, which resolves it through the regular walk. + let workspace_manifest = + pixi_manifest::HasWorkspaceManifest::workspace_manifest(ctx.environment); + let pixi_platform = workspace_manifest.workspace.platform_by_name(&ctx.platform); let inline = crate::workspace::grouped_environment::GroupedEnvironment::from(ctx.environment.clone()) .combined_inline_packages(pixi_platform) .get(&pkg_name) - .cloned(); + .cloned() + .or_else(|| { + let package = ctx.environment.workspace().package.as_ref()?; + let inline = package + .value + .combined_inline_packages() + .swap_remove(&pkg_name)?; + Some(pixi_command_dispatcher::InlinePackage { + manifest: Arc::new(inline.manifest.clone()), + workspace: Arc::new(workspace_manifest.clone()), + content_hash: inline.content_hash, + }) + }); // Query fresh backend metadata for the source's manifest checkout. let backend_metadata = ctx From a2842481849b7ef50769336cd8b54ab0dfa47f9b Mon Sep 17 00:00:00 2001 From: Julian Hofer Date: Fri, 3 Jul 2026 13:05:10 +0000 Subject: [PATCH 05/17] docs: document and test inline definitions in package dependency tables --- docs/build/inline_packages.md | 52 ++++- .../inline_package_run_dep/.gitattributes | 2 + .../inline_package_run_dep/.gitignore | 3 + .../package_a/pixi.toml | 27 +++ .../package_a/pyproject.toml | 13 ++ .../package_a/src/package_a/__init__.py | 6 + .../package_b/pixi.toml | 10 + .../package_b/recipe.yaml | 17 ++ .../inline_package_run_dep/pixi.toml | 13 ++ .../pixi_build/test_inline_packages.py | 210 ++++++++++++++++++ 10 files changed, 350 insertions(+), 3 deletions(-) create mode 100644 tests/data/pixi-build/inline_package_run_dep/.gitattributes create mode 100644 tests/data/pixi-build/inline_package_run_dep/.gitignore create mode 100644 tests/data/pixi-build/inline_package_run_dep/package_a/pixi.toml create mode 100644 tests/data/pixi-build/inline_package_run_dep/package_a/pyproject.toml create mode 100644 tests/data/pixi-build/inline_package_run_dep/package_a/src/package_a/__init__.py create mode 100644 tests/data/pixi-build/inline_package_run_dep/package_b/pixi.toml create mode 100644 tests/data/pixi-build/inline_package_run_dep/package_b/recipe.yaml create mode 100644 tests/data/pixi-build/inline_package_run_dep/pixi.toml diff --git a/docs/build/inline_packages.md b/docs/build/inline_packages.md index 799ce98244..dc2dcb5659 100644 --- a/docs/build/inline_packages.md +++ b/docs/build/inline_packages.md @@ -56,6 +56,52 @@ my-lib = { git = "https://github.com/user/repo.git", package = { version = "1.2. ## Where inline definitions are allowed -Inline definitions are accepted wherever a source dependency is, namely -`[dependencies]`, and their `[feature.*]` and `[target.*]` variants. -They are not allowed in `[package.build-dependencies]`, `[package.host-dependencies]` and `[package.run-dependencies]`. \ No newline at end of file +Inline definitions are accepted wherever a source dependency is: + +- `[dependencies]`, `[host-dependencies]` and `[build-dependencies]`, and their + `[feature.*]` and `[target.*]` variants, +- the package dependency tables `[package.run-dependencies]`, + `[package.host-dependencies]`, `[package.build-dependencies]` and + `[package.extra-dependencies.*]`, including their `if(...)` conditional + sub-tables, +- the `[workspace.dependencies]` pool (see below). + +This works in *any* package manifest Pixi builds — not just your workspace's +own `[package]` section, but also the manifests of `path`, `git` or `url` +source dependencies. Inline definitions also nest: a definition's own +dependency tables may declare further inline definitions, so a chain of +manifest-less repositories can be described from one place. + +They are not accepted in `[constraints]` or `[package.run-constraints]`; +constraints only apply to packages resolved from channels. + +## Inheriting a definition through the workspace pool + +An entry in `[workspace.dependencies]` may carry an inline definition. A +package dependency that opts in with `{ workspace = true }` inherits the +source location *and* the definition together, so the package is declared +once and used by every member: + +```toml title="pixi.toml" +[workspace.dependencies] +rust-package = { git = "https://github.com/user/repo.git", package.build.backend.name = "pixi-build-rust" } + +[package.run-dependencies] +rust-package = { workspace = true } +``` + +Combining `workspace = true` with a `package` table at the use site is an +error; declare the definition on the pool entry instead. + +## Conflicting definitions + +Inline definitions are matched to dependencies by package name. Within one +dependency table set, a name may carry at most one definition. When the same +package (at the same source location) is reached through several declarers +during a solve: + +- a definition in *your* manifest's dependency tables overrides whatever a + transitive package declares, just like it overrides an on-disk manifest; +- two transitive packages that disagree about the definition are an error — + there is no priority order between arbitrary packages. Resolve it by + declaring the dependency (with one definition) at the workspace level. \ No newline at end of file diff --git a/tests/data/pixi-build/inline_package_run_dep/.gitattributes b/tests/data/pixi-build/inline_package_run_dep/.gitattributes new file mode 100644 index 0000000000..887a2c18f0 --- /dev/null +++ b/tests/data/pixi-build/inline_package_run_dep/.gitattributes @@ -0,0 +1,2 @@ +# SCM syntax highlighting & preventing 3-way merges +pixi.lock merge=binary linguist-language=YAML linguist-generated=true diff --git a/tests/data/pixi-build/inline_package_run_dep/.gitignore b/tests/data/pixi-build/inline_package_run_dep/.gitignore new file mode 100644 index 0000000000..096b5eb543 --- /dev/null +++ b/tests/data/pixi-build/inline_package_run_dep/.gitignore @@ -0,0 +1,3 @@ +# pixi environments +.pixi +*.egg-info diff --git a/tests/data/pixi-build/inline_package_run_dep/package_a/pixi.toml b/tests/data/pixi-build/inline_package_run_dep/package_a/pixi.toml new file mode 100644 index 0000000000..63afeabded --- /dev/null +++ b/tests/data/pixi-build/inline_package_run_dep/package_a/pixi.toml @@ -0,0 +1,27 @@ +[package] +name = "package_a" +version = "0.1.0" + +[package.build.backend] +channels = [ + "https://prefix.dev/pixi-build-backends", + "https://prefix.dev/conda-forge", +] +name = "pixi-build-python" +version = "*" + +[package.host-dependencies] +hatchling = "*" + +# package_b ships only a recipe.yaml and no manifest of its own; the inline +# definition below describes how to build it. +[package.run-dependencies.package_b] +path = "../package_b" + +[package.run-dependencies.package_b.package.build.backend] +channels = [ + "https://prefix.dev/pixi-build-backends", + "https://prefix.dev/conda-forge", +] +name = "pixi-build-rattler-build" +version = "*" diff --git a/tests/data/pixi-build/inline_package_run_dep/package_a/pyproject.toml b/tests/data/pixi-build/inline_package_run_dep/package_a/pyproject.toml new file mode 100644 index 0000000000..9fbce2a03c --- /dev/null +++ b/tests/data/pixi-build/inline_package_run_dep/package_a/pyproject.toml @@ -0,0 +1,13 @@ +[project] +authors = [{ name = "Julian Hofer", email = "julianhofer@gnome.org" }] +dependencies = [] +name = "package_a" +requires-python = ">= 3.11" +version = "0.1.0" + +[build-system] +build-backend = "hatchling.build" +requires = ["hatchling"] + +[project.scripts] +package-a = "package_a:main" diff --git a/tests/data/pixi-build/inline_package_run_dep/package_a/src/package_a/__init__.py b/tests/data/pixi-build/inline_package_run_dep/package_a/src/package_a/__init__.py new file mode 100644 index 0000000000..b7e0fc0b81 --- /dev/null +++ b/tests/data/pixi-build/inline_package_run_dep/package_a/src/package_a/__init__.py @@ -0,0 +1,6 @@ +import subprocess + + +def main() -> None: + print("Pixi Build is number 1") + subprocess.run("package-b", check=True, shell=True) diff --git a/tests/data/pixi-build/inline_package_run_dep/package_b/pixi.toml b/tests/data/pixi-build/inline_package_run_dep/package_b/pixi.toml new file mode 100644 index 0000000000..cf8e17d419 --- /dev/null +++ b/tests/data/pixi-build/inline_package_run_dep/package_b/pixi.toml @@ -0,0 +1,10 @@ +# This on-disk manifest names a backend that cannot exist. The consumer +# (package_a) describes this package with an inline definition instead, which +# suppresses on-disk discovery; resolving this manifest's backend would fail. +[package] +name = "package_b" +version = "0.1.0" + +[package.build.backend] +name = "ondiskbogusbackend" +version = "*" diff --git a/tests/data/pixi-build/inline_package_run_dep/package_b/recipe.yaml b/tests/data/pixi-build/inline_package_run_dep/package_b/recipe.yaml new file mode 100644 index 0000000000..e6d570fc41 --- /dev/null +++ b/tests/data/pixi-build/inline_package_run_dep/package_b/recipe.yaml @@ -0,0 +1,17 @@ +package: + name: package_b + version: 0.1.0 + +build: + number: 0 + script: + - if: win + then: + - if not exist %PREFIX%\bin mkdir %PREFIX%\bin + - echo @echo off > %PREFIX%\bin\package-b.bat + - echo echo hello from package-b >> %PREFIX%\bin\package-b.bat + else: + - mkdir -p $PREFIX/bin + - echo "#!/usr/bin/env bash" > $PREFIX/bin/package-b + - echo "echo hello from package-b" >> $PREFIX/bin/package-b + - chmod +x $PREFIX/bin/package-b diff --git a/tests/data/pixi-build/inline_package_run_dep/pixi.toml b/tests/data/pixi-build/inline_package_run_dep/pixi.toml new file mode 100644 index 0000000000..82459fc1d2 --- /dev/null +++ b/tests/data/pixi-build/inline_package_run_dep/pixi.toml @@ -0,0 +1,13 @@ +[workspace] +authors = ["Julian Hofer "] +channels = ["https://prefix.dev/conda-forge"] +exclude-newer = "7d" +name = "inline-package-run-dep" +platforms = ["linux-64", "osx-arm64", "win-64"] +preview = ["pixi-build"] +version = "0.1.0" + +[tasks] + +[dependencies] +package_a = { path = "package_a" } diff --git a/tests/integration_python/pixi_build/test_inline_packages.py b/tests/integration_python/pixi_build/test_inline_packages.py index 4abe7f006a..848c15cfdd 100644 --- a/tests/integration_python/pixi_build/test_inline_packages.py +++ b/tests/integration_python/pixi_build/test_inline_packages.py @@ -31,6 +31,7 @@ CONDA_FORGE_CHANNEL, CURRENT_PLATFORM, ExitCode, + copytree_with_local_backend, git_test_repo, verify_cli_command, ) @@ -431,3 +432,212 @@ def test_lower_priority_inline_does_not_leak_onto_plain_dependency( stderr_contains="highbogusbackend", stderr_excludes="lowbogusbackend", ) + + +def write_package_consumer_manifest( + manifest_path: Path, + package_tables: dict, +) -> None: + """Write a workspace pixi.toml whose own `[package]` carries `package_tables`. + + The workspace depends on its own package (`path = "."`) so the package's + dependency tables participate in the default environment. + """ + manifest: dict = { + "workspace": { + "channels": [CONDA_FORGE_CHANNEL], + "platforms": [CURRENT_PLATFORM], + "name": "consumer", + "version": "0.1.0", + "preview": ["pixi-build"], + }, + "dependencies": {"consumer": {"path": "."}}, + "package": { + "build": {"backend": {"name": "pixi-build-rattler-build", "version": "*"}}, + **package_tables, + }, + } + manifest_path.write_text(tomli_w.dumps(manifest)) + + +def test_inline_definition_accepted_in_package_run_dependencies( + pixi: Path, tmp_pixi_workspace: Path +) -> None: + """`[package.run-dependencies]` accepts an inline definition. + + `workspace environment list` only loads and converts the manifest (no solve + or build), so this is a fast, offline guard that the package dependency + tables parse the `package = {...}` form instead of rejecting it as an + unknown key. + """ + write_recipe_source(tmp_pixi_workspace / "pkg") + manifest = tmp_pixi_workspace / "pixi.toml" + write_package_consumer_manifest( + manifest, + { + "run-dependencies": { + "simple-package": { + "path": "pkg", + "package": { + "build": {"backend": {"name": "pixi-build-rattler-build", "version": "*"}} + }, + } + } + }, + ) + + verify_cli_command( + [pixi, "workspace", "environment", "list", "--manifest-path", manifest], + expected_exit_code=ExitCode.SUCCESS, + ) + + +def test_inline_definition_rejected_in_package_run_constraints( + pixi: Path, tmp_pixi_workspace: Path +) -> None: + """Constraints are binary-only; an inline definition there must error.""" + write_recipe_source(tmp_pixi_workspace / "pkg") + manifest = tmp_pixi_workspace / "pixi.toml" + write_package_consumer_manifest( + manifest, + { + "run-constraints": { + "simple-package": { + "path": "pkg", + "package": { + "build": {"backend": {"name": "pixi-build-rattler-build", "version": "*"}} + }, + } + } + }, + ) + + verify_cli_command( + [pixi, "workspace", "environment", "list", "--manifest-path", manifest], + expected_exit_code=ExitCode.FAILURE, + stderr_contains="run-constraints", + ) + + +def test_pool_inline_definition_inherited_by_package_dependency( + pixi: Path, tmp_pixi_workspace: Path +) -> None: + """A `[workspace.dependencies]` entry carrying an inline definition is + inherited — location and definition together — by a package dependency + declared with `{ workspace = true }`. + + Loading the manifest is enough to prove the inheritance resolves; a + missing pool entry or a dropped definition would fail conversion. + """ + write_recipe_source(tmp_pixi_workspace / "pkg") + manifest = tmp_pixi_workspace / "pixi.toml" + manifest.write_text( + tomli_w.dumps( + { + "workspace": { + "channels": [CONDA_FORGE_CHANNEL], + "platforms": [CURRENT_PLATFORM], + "name": "consumer", + "version": "0.1.0", + "preview": ["pixi-build"], + "dependencies": { + "simple-package": { + "path": "pkg", + "package": { + "build": { + "backend": { + "name": "pixi-build-rattler-build", + "version": "*", + } + } + }, + } + }, + }, + "dependencies": {"consumer": {"path": "."}}, + "package": { + "build": {"backend": {"name": "pixi-build-rattler-build", "version": "*"}}, + "run-dependencies": {"simple-package": {"workspace": True}}, + }, + } + ) + ) + + verify_cli_command( + [pixi, "workspace", "environment", "list", "--manifest-path", manifest], + expected_exit_code=ExitCode.SUCCESS, + ) + + +def test_workspace_marker_combined_with_inline_definition_rejected( + pixi: Path, tmp_pixi_workspace: Path +) -> None: + """`{ workspace = true, package = {...} }` splits ownership of the + definition between the pool and the use site and is rejected, pointing at + `[workspace.dependencies]`.""" + write_recipe_source(tmp_pixi_workspace / "pkg") + manifest = tmp_pixi_workspace / "pixi.toml" + write_package_consumer_manifest( + manifest, + { + "run-dependencies": { + "simple-package": { + "workspace": True, + "package": { + "build": {"backend": {"name": "pixi-build-rattler-build", "version": "*"}} + }, + } + } + }, + ) + + verify_cli_command( + [pixi, "workspace", "environment", "list", "--manifest-path", manifest], + expected_exit_code=ExitCode.FAILURE, + stderr_contains="[workspace.dependencies]", + ) + + +@pytest.mark.slow +def test_inline_in_package_run_dependencies_builds( + pixi: Path, build_data: Path, tmp_pixi_workspace: Path +) -> None: + """End-to-end: a package's run dependency defined inline builds and runs. + + Mirrors `test_recursive_source_run_dependencies`, except `package_b`'s + on-disk `pixi.toml` names a backend that cannot exist, and `package_a` + describes the package with an inline definition (naming the real + rattler-build backend) in `[package.run-dependencies]`. The build can only + succeed if the inline definition reaches discovery and suppresses the + on-disk manifest; if package-level definitions were dropped, resolution + would fail on the bogus backend. + """ + project = "inline_package_run_dep" + test_data = build_data.joinpath(project) + + copytree_with_local_backend(test_data, tmp_pixi_workspace, dirs_exist_ok=True) + manifest_path = tmp_pixi_workspace.joinpath("pixi.toml") + + verify_cli_command( + [ + pixi, + "install", + "-v", + "--manifest-path", + manifest_path, + ], + ) + + # package_b is an inline-defined run dependency of package_a; check that + # it is properly built and installed. + verify_cli_command( + [ + pixi, + "run", + "-v", + "--manifest-path", + manifest_path, + "package-b", + ], + stdout_contains="hello from package-b", + ) From 2a36d9f84aea7bfdf0dfb1e03623ca3b5dfcd3af Mon Sep 17 00:00:00 2001 From: Julian Hofer Date: Mon, 6 Jul 2026 15:21:26 +0000 Subject: [PATCH 06/17] fix: resolve nested workspace markers against the real pool and skip file checks in inline definitions --- crates/pixi_manifest/src/toml/manifest.rs | 248 +++++++++++++++++- crates/pixi_manifest/src/toml/package.rs | 106 +++++++- .../pixi_manifest/src/toml/package_target.rs | 143 +++++++++- crates/pixi_manifest/src/toml/target.rs | 2 +- crates/pixi_manifest/src/toml/workspace.rs | 139 ++++++---- 5 files changed, 565 insertions(+), 73 deletions(-) diff --git a/crates/pixi_manifest/src/toml/manifest.rs b/crates/pixi_manifest/src/toml/manifest.rs index f74b2f6adb..ed8b4a16e2 100644 --- a/crates/pixi_manifest/src/toml/manifest.rs +++ b/crates/pixi_manifest/src/toml/manifest.rs @@ -155,13 +155,40 @@ impl TomlManifest { package_defaults: PackageDefaults, root_directory: &Path, ) -> Result<(WorkspaceManifest, Option, Vec), TomlError> { - let workspace = self + let mut workspace = self .workspace .ok_or_else(|| TomlError::MissingField("project/workspace".into(), None))?; - let preview = &workspace.value.preview; + let preview = &workspace.value.preview.clone(); let pixi_build_enabled = preview.is_enabled(KnownPreviewFeature::PixiBuild); + // Get the name from the [package] section if it's missing from the workspace. + let project_name = self + .package + .as_ref() + .and_then(|p| p.value.name.as_ref()) + .and_then(|field| field.clone().value()); + + // Convert the `[workspace.dependencies]` pool up front: inline package + // definitions in the dependency tables below resolve + // `{ workspace = true }` markers in their own dependency tables against + // it. The converted pool is attached to the workspace after its own + // conversion below. + let full_preview = workspace.value.preview.clone().into_preview().value; + let pool_external = ExternalWorkspaceProperties { + name: project_name.clone().or(external.name.clone()), + ..external.clone() + }; + let WithWarnings { + value: (pool_dependencies, pool_inline_packages), + warnings: mut pool_warnings, + } = super::workspace::convert_dependency_pool( + workspace.value.dependencies.take(), + &pool_external, + &full_preview, + root_directory, + )?; + // Inline package definitions declared on dependencies are converted into // full package manifests while building the targets below, so they must // inherit the same workspace package properties an on-disk `[package]` @@ -178,8 +205,8 @@ impl TomlManifest { homepage: external.homepage.clone(), repository: external.repository.clone(), documentation: external.documentation.clone(), - dependencies: IndexMap::new(), - dependency_inline_packages: IndexMap::new(), + dependencies: pool_dependencies.clone(), + dependency_inline_packages: pool_inline_packages.clone(), workspace_root: Some(root_directory.to_path_buf()), }; @@ -464,13 +491,6 @@ impl TomlManifest { )); } - // Get the name from the [package] section if it's missing from the workspace. - let project_name = self - .package - .as_ref() - .and_then(|p| p.value.name.as_ref()) - .and_then(|field| field.clone().value()); - let WithWarnings { warnings: mut workspace_warnings, value: mut workspace, @@ -482,6 +502,13 @@ impl TomlManifest { root_directory, )?; warnings.append(&mut workspace_warnings); + warnings.append(&mut pool_warnings); + + // The pool was taken out and converted before the dependency tables, so + // `into_workspace` saw an empty `[workspace.dependencies]`; attach the + // converted pool here. + workspace.dependencies = pool_dependencies; + workspace.dependency_inline_packages = pool_inline_packages; workspace.exclude_newer_package_overrides = self .exclude_newer .map(PixiSpanned::into_inner) @@ -2301,4 +2328,203 @@ mod test { "# ); } + + /// Parses a workspace manifest, panicking on failure. + fn parse_workspace(source: &str) -> WorkspaceManifest { + let manifest = ::from_toml_str(source).expect("parse toml"); + manifest + .into_workspace_manifest( + ExternalWorkspaceProperties::default(), + PackageDefaults::default(), + Path::new(""), + ) + .expect("convert to workspace manifest") + .0 + } + + #[test] + fn test_inline_definition_accepted_in_feature_dependencies() { + // Feature dependency tables peel inline definitions the same way the + // default `[dependencies]` table does. + let ws = parse_workspace( + r#" + [workspace] + channels = [] + platforms = ['linux-64'] + preview = ["pixi-build"] + + [feature.extra.dependencies] + tool-c = { path = "pkg", package.build = { backend = { name = "pixi-build-rattler-build", version = "*" } } } + + [environments] + e = ["extra"] + "#, + ); + let name = rattler_conda_types::PackageName::new_unchecked("tool-c"); + let feature = ws + .features + .get(&crate::FeatureName::from("extra")) + .expect("feature exists"); + assert!( + feature + .targets + .default() + .inline_packages + .contains_key(&name), + "inline definition captured on the feature target" + ); + } + + #[test] + fn test_inline_definition_accepted_in_target_dependencies() { + // `[target..dependencies]` tables accept inline definitions. + let ws = parse_workspace( + r#" + [workspace] + channels = [] + platforms = ['linux-64'] + preview = ["pixi-build"] + + [target.linux-64.dependencies] + tool-c = { path = "pkg", package.build = { backend = { name = "pixi-build-rattler-build", version = "*" } } } + "#, + ); + let name = rattler_conda_types::PackageName::new_unchecked("tool-c"); + let platform = crate::PixiPlatform::from(rattler_conda_types::Platform::Linux64); + let inline = ws.default_feature().inline_packages(Some(&platform)); + assert!( + inline.contains_key(&name), + "inline definition captured on the platform target" + ); + } + + #[test] + fn test_inline_definition_requires_pixi_build_preview() { + // Without the pixi-build preview, source dependencies (and with them + // inline definitions) are rejected. + let manifest = ::from_toml_str( + r#" + [workspace] + channels = [] + platforms = ['linux-64'] + + [dependencies] + tool-c = { path = "pkg", package.build = { backend = { name = "pixi-build-rattler-build", version = "*" } } } + "#, + ) + .expect("parse toml"); + let err = manifest + .into_workspace_manifest( + ExternalWorkspaceProperties::default(), + PackageDefaults::default(), + Path::new(""), + ) + .expect_err("source dependency must require the preview"); + assert!( + err.to_string().contains("pixi-build"), + "unexpected error: {err}" + ); + } + + #[test] + fn test_nested_workspace_marker_resolves_against_pool_in_feature_inline() { + // A `{ workspace = true }` marker inside an inline definition declared + // in a feature dependency table resolves against the real + // `[workspace.dependencies]` pool. + let ws = parse_workspace( + r#" + [workspace] + channels = [] + platforms = ['linux-64'] + preview = ["pixi-build"] + + [workspace.dependencies] + tool-c = { path = "c_pkg", package.build = { backend = { name = "pixi-build-rattler-build", version = "*" } } } + + [dependencies] + lib-b = { path = "b_pkg", package = { build = { backend = { name = "pixi-build-rattler-build", version = "*" } }, run-dependencies = { tool-c = { workspace = true } } } } + "#, + ); + let lib_b = rattler_conda_types::PackageName::new_unchecked("lib-b"); + let tool_c = rattler_conda_types::PackageName::new_unchecked("tool-c"); + let inline = ws + .default_feature() + .targets + .default() + .inline_packages + .get(&lib_b) + .expect("lib-b inline definition captured"); + assert!( + inline + .manifest + .combined_inline_packages() + .contains_key(&tool_c), + "the nested marker must inherit the pool entry's definition" + ); + } + + #[test] + fn test_nested_workspace_marker_resolves_against_pool_in_package_inline() { + // The same nested marker, but for an inline definition declared in the + // workspace's own `[package.run-dependencies]`. + let (_ws, pkg) = parse_workspace_and_package( + r#" + [workspace] + name = "consumer" + version = "0.1.0" + channels = [] + platforms = ['linux-64'] + preview = ["pixi-build"] + + [workspace.dependencies] + tool-c = { path = "c_pkg", package.build = { backend = { name = "pixi-build-rattler-build", version = "*" } } } + + [package] + build = { backend = { name = "pixi-build-rattler-build", version = "*" } } + + [package.run-dependencies] + lib-b = { path = "b_pkg", package = { build = { backend = { name = "pixi-build-rattler-build", version = "*" } }, run-dependencies = { tool-c = { workspace = true } } } } + "#, + ); + let lib_b = rattler_conda_types::PackageName::new_unchecked("lib-b"); + let tool_c = rattler_conda_types::PackageName::new_unchecked("tool-c"); + let inline = pkg + .combined_inline_packages() + .swap_remove(&lib_b) + .expect("lib-b inline definition captured"); + assert!( + inline + .manifest + .combined_inline_packages() + .contains_key(&tool_c), + "the nested marker must inherit the pool entry's definition" + ); + } + + #[test] + fn test_inline_definition_skips_license_file_validation() { + // `license-file` in an inline definition refers to the dependency's + // source tree; it must not be validated against the consuming + // manifest's directory. + let root = tempfile::tempdir().expect("create tempdir"); + let manifest = ::from_toml_str( + r#" + [workspace] + channels = [] + platforms = ['linux-64'] + preview = ["pixi-build"] + + [dependencies] + tool-c = { path = "pkg", package = { license-file = "LICENSE", build = { backend = { name = "pixi-build-rattler-build", version = "*" } } } } + "#, + ) + .expect("parse toml"); + manifest + .into_workspace_manifest( + ExternalWorkspaceProperties::default(), + PackageDefaults::default(), + root.path(), + ) + .expect("license-file in an inline definition must not be checked against the consumer directory"); + } } diff --git a/crates/pixi_manifest/src/toml/package.rs b/crates/pixi_manifest/src/toml/package.rs index f62a2c9b32..d66f0fe778 100644 --- a/crates/pixi_manifest/src/toml/package.rs +++ b/crates/pixi_manifest/src/toml/package.rs @@ -346,6 +346,33 @@ impl TomlPackage { package_defaults: PackageDefaults, preview: &Preview, root_directory: &Path, + ) -> Result, TomlError> { + self.into_manifest_impl(workspace, package_defaults, preview, root_directory, false) + } + + /// Converts an inline package definition attached to a dependency spec. + /// + /// Identical to [`Self::into_manifest`] except that file references such + /// as `license-file` and `readme` are not checked for existence: they + /// refer to the dependency's source tree, which is generally not present + /// next to the consuming manifest. + pub fn into_inline_manifest( + self, + workspace: WorkspacePackageProperties, + package_defaults: PackageDefaults, + preview: &Preview, + root_directory: &Path, + ) -> Result, TomlError> { + self.into_manifest_impl(workspace, package_defaults, preview, root_directory, true) + } + + fn into_manifest_impl( + self, + workspace: WorkspacePackageProperties, + package_defaults: PackageDefaults, + preview: &Preview, + root_directory: &Path, + inline: bool, ) -> Result, TomlError> { let mut warnings = Vec::new(); @@ -600,10 +627,15 @@ impl TomlPackage { } // Determine the directory to use for file validation based on build.source: + // - Inline definitions describe the dependency's source tree, which is + // not required to exist next to the consuming manifest, so validation + // is skipped // - If build.source is a git or URL source, pass None to skip validation (files are remote) // - If build.source is a path source, resolve the path and validate against that directory // - If build.source is not set, validate against the manifest directory - let file_validation_dir: Option = + let file_validation_dir: Option = if inline { + None + } else { match (&build_result.value.source, root_directory) { // Git or URL source: skip validation (files are in remote location) (Some(pixi_spec::SourceLocationSpec::Git(_)), _) @@ -615,7 +647,8 @@ impl TomlPackage { // No source: use the manifest directory (None, root_dir) => Some(root_dir.to_path_buf()), // No root directory provided: skip validation - }; + } + }; let license_file = check_resolved_file( file_validation_dir.as_deref(), @@ -1999,4 +2032,73 @@ mod test { }); spec } + + #[test] + fn test_conditional_package_dependencies_accept_inline_definition() { + // `if(...)` sub-tables of package dependency tables peel inline + // definitions like their unconditional counterparts. + let input = r#" + name = "pkg" + version = "1.0" + + [build] + backend = { name = "bla", version = "1.0" } + + [run-dependencies."if(unix)"] + tool-c = { path = "c_pkg", package.build = { backend = { name = "pixi-build-rattler-build", version = "*" } } } + "#; + + let manifest = TomlPackage::from_toml_str(input) + .and_then(|w| { + w.into_manifest( + WorkspacePackageProperties::default(), + PackageDefaults::default(), + &Preview::from_iter([KnownPreviewFeature::PixiBuild]), + Path::new(""), + ) + }) + .expect("expected manifest to parse") + .value; + + let conditional = manifest + .conditional_dependencies + .get(&ConditionalExpression::new("unix")) + .expect("conditional target should exist"); + let name = rattler_conda_types::PackageName::new_unchecked("tool-c"); + assert!( + conditional.inline_packages.contains_key(&name), + "inline definition captured on the conditional target" + ); + } + + #[test] + fn test_conditional_run_constraints_reject_inline_definition() { + // The run-constraints rejection also applies inside `if(...)` + // sub-tables, which flow through a separate `TomlPackageTarget`. + let input = r#" + name = "pkg" + version = "1.0" + + [build] + backend = { name = "bla", version = "1.0" } + + [run-constraints."if(unix)"] + tool-c = { path = "c_pkg", package.build = { backend = { name = "pixi-build-rattler-build", version = "*" } } } + "#; + + let err = TomlPackage::from_toml_str(input) + .and_then(|w| { + w.into_manifest( + WorkspacePackageProperties::default(), + PackageDefaults::default(), + &Preview::from_iter([KnownPreviewFeature::PixiBuild]), + Path::new(""), + ) + }) + .expect_err("inline definitions are not allowed in run-constraints"); + assert!( + err.to_string().contains("run-constraints"), + "unexpected error: {err}" + ); + } } diff --git a/crates/pixi_manifest/src/toml/package_target.rs b/crates/pixi_manifest/src/toml/package_target.rs index 556bb46022..9c1521d55b 100644 --- a/crates/pixi_manifest/src/toml/package_target.rs +++ b/crates/pixi_manifest/src/toml/package_target.rs @@ -138,7 +138,7 @@ impl TomlPackageTarget { let WithWarnings { value: manifest, warnings: mut package_warnings, - } = package.value.into_manifest( + } = package.value.into_inline_manifest( workspace_properties.clone(), PackageDefaults::default(), preview, @@ -582,6 +582,147 @@ mod test { ); } + #[test] + fn test_inline_package_in_host_and_build_dependencies() { + // The host and build dependency tables peel inline definitions like + // run-dependencies does. + let preview = Preview::from_iter([KnownPreviewFeature::PixiBuild]); + let input = r#" + [host-dependencies] + host-tool = { path = "host_pkg", package.build = { backend = { name = "pixi-build-rattler-build", version = "*" } } } + + [build-dependencies] + build-tool = { path = "build_pkg", package.build = { backend = { name = "pixi-build-rattler-build", version = "*" } } } + "#; + + let target = + into_package_target(TomlPackageTarget::from_toml_str(input).unwrap(), &preview) + .unwrap(); + for name in ["host-tool", "build-tool"] { + let name = PackageName::from_str(name).unwrap(); + assert!( + target.inline_packages.contains_key(&name), + "inline definition of '{}' captured", + name.as_source() + ); + } + } + + /// Build workspace properties whose pool declares a single binary + /// dependency `name`. + fn pool_properties_with_binary(name: &str) -> WorkspacePackageProperties { + use crate::toml::TomlWorkspace; + let doc = format!( + r#" + name = "ws" + channels = [] + platforms = [] + + [dependencies] + {name} = ">=1" + "# + ); + let workspace = TomlWorkspace::from_toml_str(&doc) + .expect("workspace parses") + .into_workspace(Default::default(), Path::new("")) + .expect("workspace converts") + .value; + WorkspacePackageProperties { + dependencies: workspace.dependencies, + dependency_inline_packages: workspace.dependency_inline_packages, + ..Default::default() + } + } + + #[test] + fn test_workspace_marker_on_binary_pool_entry() { + // A plain binary pool entry inherited with `{ workspace = true }` + // stays a binary dependency without any inline definition. + let properties = pool_properties_with_binary("zlib"); + let input = r#" + [run-dependencies] + zlib = { workspace = true } + "#; + + let target = TomlPackageTarget::from_toml_str(input) + .unwrap() + .into_package_target( + &Preview::default(), + &properties.dependencies.clone(), + &properties, + Path::new(""), + ) + .unwrap() + .value; + + let name = PackageName::from_str("zlib").unwrap(); + assert!( + target.inline_packages.is_empty(), + "no inline definition must be inherited from a binary pool entry" + ); + let spec = target + .dependencies + .get(&SpecType::Run) + .and_then(|d| d.get(&name)) + .and_then(|s| s.iter().next()) + .expect("spec inherited"); + assert!(spec.is_binary(), "the inherited spec stays binary"); + } + + #[test] + fn test_workspace_marker_on_source_pool_entry_without_definition() { + // A source pool entry without an inline definition is inherited as a + // plain source dependency; discovery will use the on-disk manifest. + use crate::toml::TomlWorkspace; + let doc = r#" + name = "ws" + channels = [] + platforms = [] + preview = ["pixi-build"] + + [dependencies] + tool-c = { path = "c_pkg" } + "#; + let workspace = TomlWorkspace::from_toml_str(doc) + .expect("workspace parses") + .into_workspace(Default::default(), Path::new("")) + .expect("workspace converts") + .value; + let properties = WorkspacePackageProperties { + dependencies: workspace.dependencies, + dependency_inline_packages: workspace.dependency_inline_packages, + ..Default::default() + }; + + let input = r#" + [run-dependencies] + tool-c = { workspace = true } + "#; + let target = TomlPackageTarget::from_toml_str(input) + .unwrap() + .into_package_target( + &Preview::from_iter([KnownPreviewFeature::PixiBuild]), + &properties.dependencies.clone(), + &properties, + Path::new(""), + ) + .unwrap() + .value; + + let name = PackageName::from_str("tool-c").unwrap(); + assert!( + target.inline_packages.is_empty(), + "no inline definition to inherit" + ); + let spec = target + .dependencies + .get(&SpecType::Run) + .and_then(|d| d.get(&name)) + .and_then(|s| s.iter().next()) + .expect("spec inherited"); + assert!(spec.is_source(), "the inherited spec stays a source spec"); + } + #[test] fn test_inline_package_requires_source_location() { // An inline definition without a source location is meaningless. diff --git a/crates/pixi_manifest/src/toml/target.rs b/crates/pixi_manifest/src/toml/target.rs index 479a262a84..a53151d2b9 100644 --- a/crates/pixi_manifest/src/toml/target.rs +++ b/crates/pixi_manifest/src/toml/target.rs @@ -121,7 +121,7 @@ impl TomlTarget { let WithWarnings { value: manifest, warnings: mut package_warnings, - } = package.value.into_manifest( + } = package.value.into_inline_manifest( workspace_package_properties.clone(), PackageDefaults::default(), &full_preview, diff --git a/crates/pixi_manifest/src/toml/workspace.rs b/crates/pixi_manifest/src/toml/workspace.rs index d648383b6e..a7c9b0f635 100644 --- a/crates/pixi_manifest/src/toml/workspace.rs +++ b/crates/pixi_manifest/src/toml/workspace.rs @@ -249,64 +249,11 @@ impl TomlWorkspace { // Source specs gated on pixi-build. Path specs are left // workspace-relative; members re-base them at inheritance time. - let (dependencies, dependency_inline_packages) = if let Some(deps) = self.dependencies { - let pixi_build_enabled = preview.is_enabled(KnownPreviewFeature::PixiBuild); - let specs = deps.value.specs; - if !pixi_build_enabled - && let Some((name, _)) = specs.iter().find(|(_, s)| toml_spec_is_source(s)) - { - return Err(GenericError::new( - "conda source dependencies are not allowed without enabling the 'pixi-build' preview feature", - ) - .with_help( - "Add `preview = [\"pixi-build\"]` to the `workspace` table of your manifest", - ) - .with_span_label(format!("source dependency `{}`", name.as_source())) - .with_opt_span(deps.span.clone()) - .into()); - } - - // Convert inline package definitions attached to pool entries. Like - // definitions on the workspace dependency tables, they inherit the - // workspace's external package properties; the workspace manifest - // itself is not built yet. - let mut inline_packages = IndexMap::new(); - if !deps.value.inline_packages.is_empty() { - let inline_properties = WorkspacePackageProperties { - name: external.name.clone(), - version: external.version.clone(), - description: external.description.clone(), - authors: external.authors.clone(), - license: external.license.clone(), - license_file: external.license_file.clone(), - readme: external.readme.clone(), - homepage: external.homepage.clone(), - repository: external.repository.clone(), - documentation: external.documentation.clone(), - dependencies: IndexMap::new(), - dependency_inline_packages: IndexMap::new(), - workspace_root: Some(root_directory.to_path_buf()), - }; - for (name, package) in deps.value.inline_packages { - let WithWarnings { - value: manifest, - warnings: mut package_warnings, - } = package.value.into_manifest( - inline_properties.clone(), - PackageDefaults::default(), - &preview, - root_directory, - )?; - warnings.append(&mut package_warnings); - let inline = InlinePackageManifest::from_named_manifest(&name, manifest); - inline_packages.insert(name, inline); - } - } - - (specs, inline_packages) - } else { - (IndexMap::new(), IndexMap::new()) - }; + let WithWarnings { + value: (dependencies, dependency_inline_packages), + warnings: mut pool_warnings, + } = convert_dependency_pool(self.dependencies, &external, &preview, root_directory)?; + warnings.append(&mut pool_warnings); Ok(WithWarnings::from(Workspace { name: self.name.or(external.name), @@ -353,6 +300,82 @@ impl TomlWorkspace { } } +/// The converted `[workspace.dependencies]` pool: the flat spec map plus the +/// inline package manifests attached to pool entries. +pub(crate) type ConvertedDependencyPool = ( + IndexMap, + IndexMap, +); + +/// Converts the raw `[workspace.dependencies]` pool into the flat spec map +/// and the inline package manifests attached to pool entries. +/// +/// Source specs are gated on the `pixi-build` preview. Inline definitions +/// inherit the workspace's external package properties; the workspace +/// manifest itself is not built yet, so `{ workspace = true }` markers inside +/// a pool entry's own definition cannot reference the pool. +pub(crate) fn convert_dependency_pool( + dependencies: Option>, + external: &ExternalWorkspaceProperties, + preview: &crate::Preview, + root_directory: &Path, +) -> Result, TomlError> { + let Some(deps) = dependencies else { + return Ok(WithWarnings::from((IndexMap::new(), IndexMap::new()))); + }; + let mut warnings = Vec::new(); + let pixi_build_enabled = preview.is_enabled(KnownPreviewFeature::PixiBuild); + let specs = deps.value.specs; + if !pixi_build_enabled + && let Some((name, _)) = specs.iter().find(|(_, s)| toml_spec_is_source(s)) + { + return Err(GenericError::new( + "conda source dependencies are not allowed without enabling the 'pixi-build' preview feature", + ) + .with_help( + "Add `preview = [\"pixi-build\"]` to the `workspace` table of your manifest", + ) + .with_span_label(format!("source dependency `{}`", name.as_source())) + .with_opt_span(deps.span.clone()) + .into()); + } + + let mut inline_packages = IndexMap::new(); + if !deps.value.inline_packages.is_empty() { + let inline_properties = WorkspacePackageProperties { + name: external.name.clone(), + version: external.version.clone(), + description: external.description.clone(), + authors: external.authors.clone(), + license: external.license.clone(), + license_file: external.license_file.clone(), + readme: external.readme.clone(), + homepage: external.homepage.clone(), + repository: external.repository.clone(), + documentation: external.documentation.clone(), + dependencies: IndexMap::new(), + dependency_inline_packages: IndexMap::new(), + workspace_root: Some(root_directory.to_path_buf()), + }; + for (name, package) in deps.value.inline_packages { + let WithWarnings { + value: manifest, + warnings: mut package_warnings, + } = package.value.into_inline_manifest( + inline_properties.clone(), + PackageDefaults::default(), + preview, + root_directory, + )?; + warnings.append(&mut package_warnings); + let inline = InlinePackageManifest::from_named_manifest(&name, manifest); + inline_packages.insert(name, inline); + } + } + + Ok(WithWarnings::from((specs, inline_packages)).with_warnings(warnings)) +} + /// Returns true when the spec carries a source-style location (`path` or /// `git`). Used to gate workspace dep entries on the `pixi-build` preview. fn toml_spec_is_source(spec: &TomlSpec) -> bool { From 22a207595ce37daa9c984794f1645ee9c814a3af Mon Sep 17 00:00:00 2001 From: Julian Hofer Date: Mon, 6 Jul 2026 15:21:26 +0000 Subject: [PATCH 07/17] fix: keep the solve's seed-first inline definition choice at install time --- .../tests/integration_rust/install_tests.rs | 1 + .../src/install_pixi/ext.rs | 52 +++++++++++++++---- .../src/install_pixi/mod.rs | 11 ++++ .../src/keys/source_build.rs | 4 ++ .../tests/integration/main.rs | 1 + .../pixi_core/src/environment/conda_prefix.rs | 17 ++++++ crates/pixi_global/src/project/mod.rs | 1 + 7 files changed, 77 insertions(+), 10 deletions(-) diff --git a/crates/pixi/tests/integration_rust/install_tests.rs b/crates/pixi/tests/integration_rust/install_tests.rs index f99ca657c0..24aabdb20a 100644 --- a/crates/pixi/tests/integration_rust/install_tests.rs +++ b/crates/pixi/tests/integration_rust/install_tests.rs @@ -1471,6 +1471,7 @@ async fn test_multiple_prefix_update() { None, command_dispatcher, Default::default(), + Default::default(), ); let pixi_records = Vec::from([ diff --git a/crates/pixi_command_dispatcher/src/install_pixi/ext.rs b/crates/pixi_command_dispatcher/src/install_pixi/ext.rs index 35afc5f829..182b427f38 100644 --- a/crates/pixi_command_dispatcher/src/install_pixi/ext.rs +++ b/crates/pixi_command_dispatcher/src/install_pixi/ext.rs @@ -3,7 +3,11 @@ //! [`SourceBuildKey`], then (b) running the //! rattler prefix installer over the resulting binary set. -use std::{collections::HashMap, sync::Arc, time::Duration}; +use std::{ + collections::{BTreeMap, HashMap}, + sync::Arc, + time::Duration, +}; /// How often to warn while blocked on a peer's install lock. const INSTALL_LOCK_PROGRESS_INTERVAL: Duration = Duration::from_secs(30); @@ -16,6 +20,7 @@ use rattler_conda_types::{PackageName, Platform, RepoDataRecord}; use crate::BuildProfile; use crate::CommandDispatcherError; use crate::CondaPackageFormat; +use crate::InlinePackage; use crate::cache::markers::{SourceBuildArtifactsDir, SourceBuildWorkspacesDir}; use crate::compute_data::{ HasAllowExecuteLinkScripts, HasAllowLinkOptions, HasIoConcurrencySemaphore, HasPackageCache, @@ -151,11 +156,29 @@ async fn install_inner( variant_configuration: spec.variant_configuration.clone(), variant_files: spec.variant_files.clone(), }; - // Inline package definitions for the source records in this - // install, looked up per record by name when building from source. - // The caller-supplied (consumer-level) definitions take precedence; - // they are extended with package-level ones below. - let mut combined_inline = std::mem::take(&mut spec.inline_packages); + // Inline package definitions for the source records in this install, + // looked up per record by name when building from source. The + // caller-supplied (consumer-level) definitions take precedence; they are + // complemented with package-level ones below. A package-level definition + // never applies to a direct source dependency of the environment: the + // solve's seed-first rule resolved such a record with the consumer's own + // declaration (or none at all), and the build must match that decision. + let consumer_inline = std::mem::take(&mut spec.inline_packages); + let direct_source_dependencies = std::mem::take(&mut spec.direct_source_dependencies); + let mut package_inline: BTreeMap = BTreeMap::new(); + + let select_inline = |consumer: &HashMap, + package: &BTreeMap, + name: &PackageName| + -> Option { + consumer.get(name).cloned().or_else(|| { + if direct_source_dependencies.contains(name) { + None + } else { + package.get(name).cloned() + } + }) + }; // Each source record's manifest may declare inline definitions for other // source records of this same install (its own dependencies). Collect @@ -175,7 +198,7 @@ async fn install_inner( if discovered.contains(record.name()) { continue; } - let inline = combined_inline.get(record.name()).cloned(); + let inline = select_inline(&consumer_inline, &package_inline, record.name()); let Ok(checkout) = ctx .checkout_pinned_source(record.manifest_source.clone()) .await @@ -196,7 +219,7 @@ async fn install_inner( for (name, inline) in crate::inline_package::inline_packages_from_backend(&backend).iter() { - combined_inline + package_inline .entry(name.clone()) .or_insert_with(|| inline.clone()); } @@ -207,7 +230,15 @@ async fn install_inner( } } - let inline_packages = Arc::new(combined_inline); + let inline_packages: Arc> = Arc::new( + source_records + .iter() + .filter_map(|record| { + select_inline(&consumer_inline, &package_inline, record.name()) + .map(|inline| (record.name().clone(), inline)) + }) + .collect(), + ); let mapper = { let shared = shared.clone(); let inline_packages = inline_packages.clone(); @@ -219,6 +250,7 @@ async fn install_inner( > { let name = source.name().clone(); let manifest_source = source.manifest_source.clone(); + let inline = inline_packages.get(&name).cloned(); let build_spec = SourceBuildSpec { record: source, channels: shared.channels.clone(), @@ -237,7 +269,7 @@ async fn install_inner( // Source packages built during `pixi install` are unpacked // immediately, so use the cheapest compression. package_format: Some(CondaPackageFormat::fast()), - inline: inline_packages.get(&name).cloned(), + inline, }; // A failed cross-build is usually the platform mismatch, not the // recipe. Compare the real machine: installs set build_platform to the target. diff --git a/crates/pixi_command_dispatcher/src/install_pixi/mod.rs b/crates/pixi_command_dispatcher/src/install_pixi/mod.rs index 8bb9dc806e..8ee056e796 100644 --- a/crates/pixi_command_dispatcher/src/install_pixi/mod.rs +++ b/crates/pixi_command_dispatcher/src/install_pixi/mod.rs @@ -63,6 +63,16 @@ pub struct InstallPixiEnvironmentSpec { /// discovering one on disk. Empty when no inline definitions are in scope. #[serde(skip)] pub inline_packages: HashMap, + + /// Names the environment declares as direct source dependencies. A + /// package-level inline definition discovered from another record's + /// manifest never applies to these: the solve resolved them with the + /// consumer's own declaration (seed-first), and the build must match that + /// decision. Leave empty when no such distinction exists (e.g. nested + /// build environments, where `inline_packages` already carries exactly + /// the applicable definitions). + #[serde(skip)] + pub direct_source_dependencies: HashSet, } pub struct InstallPixiEnvironmentResult { @@ -104,6 +114,7 @@ impl InstallPixiEnvironmentSpec { variant_configuration: None, variant_files: None, inline_packages: HashMap::new(), + direct_source_dependencies: HashSet::new(), } } } diff --git a/crates/pixi_command_dispatcher/src/keys/source_build.rs b/crates/pixi_command_dispatcher/src/keys/source_build.rs index 959c3e14c5..8a1213ad84 100644 --- a/crates/pixi_command_dispatcher/src/keys/source_build.rs +++ b/crates/pixi_command_dispatcher/src/keys/source_build.rs @@ -707,6 +707,10 @@ async fn install_prefix( .iter() .map(|(name, inline)| (name.clone(), inline.clone())) .collect(), + // `inline_packages` above carries exactly the applicable + // definitions for this nested environment, so no seed-based + // suppression is needed. + direct_source_dependencies: Default::default(), }; let result = ctx .install_pixi_environment(install_spec) diff --git a/crates/pixi_command_dispatcher/tests/integration/main.rs b/crates/pixi_command_dispatcher/tests/integration/main.rs index eecbb9394b..f652d647fa 100644 --- a/crates/pixi_command_dispatcher/tests/integration/main.rs +++ b/crates/pixi_command_dispatcher/tests/integration/main.rs @@ -254,6 +254,7 @@ pub async fn simple_test() { variant_configuration: None, variant_files: None, inline_packages: Default::default(), + direct_source_dependencies: Default::default(), }) .await .unwrap(); diff --git a/crates/pixi_core/src/environment/conda_prefix.rs b/crates/pixi_core/src/environment/conda_prefix.rs index e060eae5f7..2048527c5f 100644 --- a/crates/pixi_core/src/environment/conda_prefix.rs +++ b/crates/pixi_core/src/environment/conda_prefix.rs @@ -87,6 +87,10 @@ pub struct CondaPrefixUpdaterInner { /// build source packages without an on-disk manifest. pub inline_packages: HashMap, + /// Names the environment declares as direct source dependencies; a + /// package-level inline definition never applies to these (seed-first). + pub direct_source_dependencies: HashSet, + /// A flag that indicates if the prefix was created. created: AsyncOnceCell, } @@ -119,6 +123,13 @@ impl CondaPrefixUpdaterBuilder<'_> { .combined_inline_packages(Some(&self.platform)) .into_iter() .collect(); + let direct_source_dependencies = self + .group + .combined_dependencies(Some(&self.platform)) + .iter_specs() + .filter(|(_, spec)| spec.is_source()) + .map(|(name, _)| name.clone()) + .collect(); Ok(CondaPrefixUpdater::new( channels, @@ -130,6 +141,7 @@ impl CondaPrefixUpdaterBuilder<'_> { exclude_newer, self.command_dispatcher, inline_packages, + direct_source_dependencies, )) } } @@ -167,6 +179,7 @@ impl CondaPrefixUpdater { exclude_newer: Option, command_dispatcher: CommandDispatcher, inline_packages: HashMap, + direct_source_dependencies: HashSet, ) -> Self { Self { inner: Arc::new(CondaPrefixUpdaterInner { @@ -179,6 +192,7 @@ impl CondaPrefixUpdater { exclude_newer, command_dispatcher, inline_packages, + direct_source_dependencies, created: Default::default(), }), } @@ -213,6 +227,7 @@ impl CondaPrefixUpdater { reinstall_packages, ignore_packages, self.inner.inline_packages.clone(), + self.inner.direct_source_dependencies.clone(), ) .await?; @@ -246,6 +261,7 @@ pub async fn update_prefix_conda( reinstall_packages: Option>, ignore_packages: Option>, inline_packages: HashMap, + direct_source_dependencies: HashSet, ) -> miette::Result { // Try to increase the rlimit to a sensible value for installation. try_increase_rlimit_to_sensible(); @@ -285,6 +301,7 @@ pub async fn update_prefix_conda( variant_configuration: Some(variant_configuration), variant_files: Some(variant_files), inline_packages, + direct_source_dependencies, }) .await?; diff --git a/crates/pixi_global/src/project/mod.rs b/crates/pixi_global/src/project/mod.rs index 50ded265c2..da6208ea9d 100644 --- a/crates/pixi_global/src/project/mod.rs +++ b/crates/pixi_global/src/project/mod.rs @@ -712,6 +712,7 @@ impl Project { variant_configuration: None, variant_files: None, inline_packages: Default::default(), + direct_source_dependencies: Default::default(), }) .await?; From f9d6f21006f96b4e1cd4a025c1a3ac17f3d3550a Mon Sep 17 00:00:00 2001 From: Julian Hofer Date: Mon, 6 Jul 2026 15:21:26 +0000 Subject: [PATCH 08/17] fix: verify transitively declared inline definitions during satisfiability --- .../src/lock_file/satisfiability/platform.rs | 280 +++++++++++++----- .../lock_file/satisfiability/source_record.rs | 60 ++-- 2 files changed, 234 insertions(+), 106 deletions(-) diff --git a/crates/pixi_core/src/lock_file/satisfiability/platform.rs b/crates/pixi_core/src/lock_file/satisfiability/platform.rs index f6edeb099f..dd1fe15d54 100644 --- a/crates/pixi_core/src/lock_file/satisfiability/platform.rs +++ b/crates/pixi_core/src/lock_file/satisfiability/platform.rs @@ -1,6 +1,6 @@ use std::{ borrow::Cow, - collections::{HashMap, HashSet}, + collections::{BTreeMap, HashMap, HashSet}, path::{Path, PathBuf}, str::FromStr, sync::Arc, @@ -13,7 +13,7 @@ use once_cell::sync::OnceCell; use pixi_command_dispatcher::{ BuildBackendMetadataSpec, CommandDispatcher, CommandDispatcherError, CommandDispatcherErrorResultExt, ComputeResultExt, DevSourceMetadataSpec, EnvironmentRef, - SourceCheckoutExt, WorkspaceEnvRef, executor::CancellationAwareFutures, + InlinePackage, SourceCheckoutExt, WorkspaceEnvRef, executor::CancellationAwareFutures, }; use pixi_config::Config; use pixi_install_pypi::UnresolvedPypiRecord; @@ -253,82 +253,220 @@ pub async fn verify_platform_satisfiability( // // Inline package definitions declared in the current manifest, used to // detect edits against records whose sources are otherwise immutable. + // Environment-level definitions apply to the environment's own (direct) + // source dependencies, mirroring the solve's seed-first rule; the + // workspace's own `[package]` dependency tables and re-queried declaring + // packages provide the definitions for records reached transitively. + let pixi_platform = ctx + .environment + .workspace_manifest() + .workspace + .platform_by_name(&ctx.platform); let inline_packages = crate::workspace::grouped_environment::GroupedEnvironment::from(ctx.environment.clone()) - .combined_inline_packages( - ctx.environment - .workspace_manifest() - .workspace - .platform_by_name(&ctx.platform), - ); - let mut resolve_futures = CancellationAwareFutures::new(ctx.command_dispatcher.executor()); - for (index, record) in unresolved_records.into_iter().enumerate() { - let platform_setup = &platform_setup; - let inline_packages = &inline_packages; - resolve_futures.push(async move { - let resolved = match record { - UnresolvedPixiRecord::Binary(record) => PixiRecord::Binary(record), - UnresolvedPixiRecord::Source(record) => { - let needs_backend_check = - record.data.is_partial() || record.has_mutable_source(); - if needs_backend_check { - // Partial records carry no version/build material in - // the lock file, so they must be resolved from the - // backend. Mutable sources (path-based, or with a - // path-based build source) must also re-evaluate via - // the backend because the manifest can change without - // any lock file-visible signal -- there is no - // content-pinned identifier we can use to detect - // edits to e.g. host-dependencies. Skipping the - // backend here would silently accept stale lock files. - let resolved = verify_partial_source_record_against_backend( - ctx, - platform_setup, - &record, - ) - .await?; - PixiRecord::Source(resolved) - } else { - // Fully immutable + full record: the source is - // content-pinned (git commit / url+sha), so the - // backend cannot tell us anything we can't already - // read off the locked record. Trust the locked - // metadata as-is and avoid contacting the backend - // (which would otherwise require it to be available - // just to pass satisfiability). - // - // An inline package definition lives in the consuming - // manifest, though, and can change without any - // lock-file-visible signal. Its content hash is folded - // into the record's identifier hash at solve time, so - // recomputing the hash with the definition currently - // in the manifest detects edits. - verify_immutable_record_identity( - &record, - inline_packages - .get(record.name()) - .map(|inline| inline.content_hash.as_u64()), - ) - .map_err(CommandDispatcherError::Failed)?; - let full_record = - Arc::unwrap_or_clone(record).try_map_data(|data| match data { - SourceRecordData::Full(data) => Ok(data), - SourceRecordData::Partial(p) => Err(p), - }); - match full_record { - Ok(full) => PixiRecord::Source(Arc::new(full)), - Err(_) => { - unreachable!("guarded by `data.is_partial()` check above") - } - } + .combined_inline_packages(pixi_platform); + + // Names the environment itself declares as source dependencies. These are + // the solve's seeds: their locked identifier hash folds the + // environment-level definition (or none), never a package-level one. + let direct_source_names: HashSet = ctx + .environment + .combined_dependencies(pixi_platform) + .iter_specs() + .filter(|(_, spec)| spec.is_source()) + .map(|(name, _)| name.clone()) + .collect(); + + // Inline definitions declared by the workspace's own `[package]` + // dependency tables. The workspace package is a declaring parent like any + // other, but its manifest is already in memory. + let workspace_package_inline: BTreeMap = ctx + .environment + .workspace() + .package + .as_ref() + .map(|package| { + let workspace_manifest = Arc::new(ctx.environment.workspace_manifest().clone()); + package + .value + .combined_inline_packages() + .into_iter() + .map(|(name, inline)| { + ( + name, + InlinePackage { + manifest: Arc::new(inline.manifest.clone()), + workspace: workspace_manifest.clone(), + content_hash: inline.content_hash, + }, + ) + }) + .collect() + }) + .unwrap_or_default(); + + // Resolve records in declarer-aware waves. A mutable or partial source + // record needs a fresh backend query, and that query needs the record's + // inline definition. Environment-level and workspace `[package]` + // definitions are known up front; a definition declared by another + // package's manifest only becomes known once that (mutable) declarer has + // been re-queried, so such records are deferred to a later wave. + let mut pending: Vec<(usize, UnresolvedPixiRecord)> = + unresolved_records.into_iter().enumerate().collect(); + let mut indexed_records: Vec<(usize, PixiRecord)> = Vec::with_capacity(pending.len()); + // Definitions declared by re-queried records for their own dependencies. + let mut declared_inline: BTreeMap = BTreeMap::new(); + // Immutable records reached transitively; their identity check needs the + // declarers' definitions, so it runs after all waves completed. + let mut deferred_identity_checks: Vec> = Vec::new(); + let mut final_round = false; + + while !pending.is_empty() { + let mut wave: Vec<( + usize, + Arc, + Option, + )> = Vec::new(); + let mut deferred: Vec<(usize, UnresolvedPixiRecord)> = Vec::new(); + for (index, record) in std::mem::take(&mut pending) { + let record = match record { + UnresolvedPixiRecord::Binary(record) => { + indexed_records.push((index, PixiRecord::Binary(record))); + continue; + } + UnresolvedPixiRecord::Source(record) => record, + }; + let is_seed = direct_source_names.contains(record.name()); + let needs_backend_check = record.data.is_partial() || record.has_mutable_source(); + if !needs_backend_check { + // Fully immutable + full record: the source is content-pinned + // (git commit / url+sha), so the backend cannot tell us + // anything we can't already read off the locked record. Trust + // the locked metadata as-is and avoid contacting the backend + // (which would otherwise require it to be available just to + // pass satisfiability). + // + // An inline package definition lives in a consuming manifest, + // though, and can change without any lock-file-visible + // signal. Its content hash is folded into the record's + // identifier hash at solve time, so recomputing the hash with + // the definition that applies today detects edits. For a + // direct dependency that is the environment-level definition; + // for a transitive record the definition comes from a + // declaring parent, checked after all declarers were queried. + if is_seed { + verify_immutable_record_identity( + &record, + inline_packages + .get(record.name()) + .map(|inline| inline.content_hash.as_u64()), + ) + .map_err(CommandDispatcherError::Failed)?; + } else { + deferred_identity_checks.push(record.clone()); + } + let full_record = Arc::unwrap_or_clone(record).try_map_data(|data| match data { + SourceRecordData::Full(data) => Ok(data), + SourceRecordData::Partial(p) => Err(p), + }); + match full_record { + Ok(full) => indexed_records.push((index, PixiRecord::Source(Arc::new(full)))), + Err(_) => { + unreachable!("guarded by `data.is_partial()` check above") } } + continue; + } + + // Partial records carry no version/build material in the lock + // file, so they must be resolved from the backend. Mutable + // sources (path-based, or with a path-based build source) must + // also re-evaluate via the backend because the manifest can + // change without any lock file-visible signal. Skipping the + // backend here would silently accept stale lock files. + let inline = if is_seed { + inline_packages.get(record.name()).cloned() + } else { + workspace_package_inline + .get(record.name()) + .or_else(|| declared_inline.get(record.name())) + .cloned() }; - Ok::<_, CommandDispatcherError>>((index, resolved)) - }); + if inline.is_none() && !is_seed && !final_round { + // The definition, if any, is declared by another package that + // has not been re-queried yet. + deferred.push((index, UnresolvedPixiRecord::Source(record))); + continue; + } + + wave.push((index, record, inline)); + } + + if wave.is_empty() { + if deferred.is_empty() { + break; + } + // No deferred record gained a definition this round: the + // remaining ones are plain source dependencies, or their declarer + // is immutable (and thus cannot have changed its definition). + // Query them without a definition. + final_round = true; + pending = deferred; + continue; + } + + let mut resolve_futures = CancellationAwareFutures::new(ctx.command_dispatcher.executor()); + for (index, record, inline) in wave { + let platform_setup = &platform_setup; + resolve_futures.push(async move { + // No identity-hash comparison here: the identifier hash of a + // mutable record is not reproducible after a lock file round + // trip (path pins are normalized). The backend comparison + // inside the call runs with the definition that applies + // today, so any edit that changes the backend's reported + // outputs is detected. + let (resolved, declared) = verify_partial_source_record_against_backend( + ctx, + platform_setup, + &record, + inline, + ) + .await?; + Ok::<_, CommandDispatcherError>>((index, resolved, declared)) + }); + } + type WaveResult = ( + usize, + Arc, + Arc>, + ); + let wave_results: Vec = resolve_futures.try_collect().await?; + for (index, resolved, declared) in wave_results { + indexed_records.push((index, PixiRecord::Source(resolved))); + for (name, inline) in declared.iter() { + declared_inline + .entry(name.clone()) + .or_insert_with(|| inline.clone()); + } + } + pending = deferred; + } + + // Identity checks for immutable records reached transitively: the + // definition folded into the locked identifier hash must match what a + // declaring parent provides today. When no re-queried parent declares the + // record (e.g. every parent is itself content-pinned), the stored hash is + // trusted: an immutable parent cannot have changed its definition. + for record in deferred_identity_checks { + if let Some(inline) = workspace_package_inline + .get(record.name()) + .or_else(|| declared_inline.get(record.name())) + { + verify_immutable_record_identity(&record, Some(inline.content_hash.as_u64())) + .map_err(CommandDispatcherError::Failed)?; + } } - let mut indexed_records: Vec<(usize, PixiRecord)> = resolve_futures.try_collect().await?; indexed_records.sort_by_key(|(index, _)| *index); let resolved_records: Vec = indexed_records .into_iter() diff --git a/crates/pixi_core/src/lock_file/satisfiability/source_record.rs b/crates/pixi_core/src/lock_file/satisfiability/source_record.rs index 1dfa310259..29aea1006e 100644 --- a/crates/pixi_core/src/lock_file/satisfiability/source_record.rs +++ b/crates/pixi_core/src/lock_file/satisfiability/source_record.rs @@ -124,7 +124,16 @@ pub(super) fn verify_immutable_record_identity( /// the matching output, then return a freshly-assembled full source /// record built from the backend output. /// -/// Returns the freshly-resolved record on success. Returns a specific +/// `inline` is the inline package definition that currently applies to this +/// record, as derived by the caller from the manifest (environment-level or +/// workspace `[package]` tables) or from a declaring parent's fresh metadata. +/// It is needed both to re-query metadata without an on-disk manifest and so +/// an edit to the inline table re-derives different outputs (forcing a +/// re-lock). +/// +/// Returns the freshly-resolved record together with the inline package +/// definitions the record's manifest declares for its own dependencies (so +/// the caller can verify those dependencies in turn). Returns a specific /// [`PlatformUnsat`] variant on the first mismatch so the caller can /// surface a useful diagnostic and trigger a re-lock that carries the /// locked build/host packages forward as solver hints. @@ -132,40 +141,18 @@ pub(super) async fn verify_partial_source_record_against_backend( ctx: &VerifySatisfiabilityContext<'_>, platform_setup: &crate::lock_file::platform_setup::PlatformSetup, record: &pixi_record::UnresolvedSourceRecord, -) -> Result, CommandDispatcherError>> { + inline: Option, +) -> Result< + ( + Arc, + Arc>, + ), + CommandDispatcherError>, +> { use pixi_command_dispatcher::BuildBackendMetadataSpec; let pkg_name = record.name().clone(); - // Look up this package's inline definition from the current - // manifest, if any. It is needed both to re-query metadata without an - // on-disk manifest and so an edit to the inline table re-derives different - // outputs (forcing a re-lock). Workspace-level definitions take - // precedence; the workspace's own `[package]` dependency tables are the - // fallback. A definition declared by a *transitive* package's manifest is - // not found here: the fresh metadata query below then fails discovery and - // forces a re-lock, which resolves it through the regular walk. - let workspace_manifest = - pixi_manifest::HasWorkspaceManifest::workspace_manifest(ctx.environment); - let pixi_platform = workspace_manifest.workspace.platform_by_name(&ctx.platform); - let inline = - crate::workspace::grouped_environment::GroupedEnvironment::from(ctx.environment.clone()) - .combined_inline_packages(pixi_platform) - .get(&pkg_name) - .cloned() - .or_else(|| { - let package = ctx.environment.workspace().package.as_ref()?; - let inline = package - .value - .combined_inline_packages() - .swap_remove(&pkg_name)?; - Some(pixi_command_dispatcher::InlinePackage { - manifest: Arc::new(inline.manifest.clone()), - workspace: Arc::new(workspace_manifest.clone()), - content_hash: inline.content_hash, - }) - }); - // Query fresh backend metadata for the source's manifest checkout. let backend_metadata = ctx .command_dispatcher @@ -272,10 +259,13 @@ pub(super) async fn verify_partial_source_record_against_backend( // downstream verification observes the same env the solver // previously chose. The pinned source / build_source come from the // locked record so paths and commits don't drift. - Ok(Arc::new(build_full_source_record_from_output( - record, - matching_output, - ))) + Ok(( + Arc::new(build_full_source_record_from_output( + record, + matching_output, + )), + backend_metadata.inline_packages.clone(), + )) } /// Reassemble what the locked source record's `depends` (and From 9d5c6a55fcf78182c6c4491c0504e83eb4cd13ac Mon Sep 17 00:00:00 2001 From: Julian Hofer Date: Mon, 6 Jul 2026 15:21:26 +0000 Subject: [PATCH 09/17] test: extend integration coverage for inline package definitions --- .../test_inline_packages_extended.py | 829 ++++++++++++++++++ 1 file changed, 829 insertions(+) create mode 100644 tests/integration_python/pixi_build/test_inline_packages_extended.py diff --git a/tests/integration_python/pixi_build/test_inline_packages_extended.py b/tests/integration_python/pixi_build/test_inline_packages_extended.py new file mode 100644 index 0000000000..5b09b17186 --- /dev/null +++ b/tests/integration_python/pixi_build/test_inline_packages_extended.py @@ -0,0 +1,829 @@ +"""Extended integration tests for inline package definitions. + +This suite exercises inline package definitions end to end: full builds +through the local rattler-build backend, workspace pool inheritance, nesting +with relative path anchoring, transitive conflict rules, seed-first override +at install time, and lock-file stability across re-verification. + +Parse acceptance/rejection rules are covered by unit tests in the +`pixi_manifest` crate; only behaviour that needs the real build pipeline +lives here. + +Run it with:: + + pixi run test-specific-test-debug test_inline_packages_extended + +Most tests run `pixi install` against recipes without any dependencies, so +they only need the workspace-built backends from +`PIXI_BUILD_BACKEND_OVERRIDE`; `test_extra_dependencies_inline_reaches_lock` +and `test_package_level_inline_edit_invalidates_lock` additionally resolve +packages from conda-forge. +""" + +from pathlib import Path +from typing import Any + +import pytest +import tomli_w +import yaml + +from .common import ( + CONDA_FORGE_CHANNEL, + CURRENT_PLATFORM, + ExitCode, + git_test_repo, + verify_cli_command, +) + +RATTLER_BACKEND: dict[str, str] = {"name": "pixi-build-rattler-build", "version": "*"} +BOGUS_BACKEND: dict[str, str] = {"name": "definitely-not-a-real-backend", "version": "*"} + + +def workspace_table(**extra: Any) -> dict[str, Any]: + return { + "channels": [CONDA_FORGE_CHANNEL], + "platforms": [CURRENT_PLATFORM], + "preview": ["pixi-build"], + **extra, + } + + +def inline_def( + backend: dict[str, str] | None = None, + run_dependencies: dict[str, Any] | None = None, + **extra: Any, +) -> dict[str, Any]: + """A minimal inline package definition table.""" + table: dict[str, Any] = {"build": {"backend": backend or RATTLER_BACKEND}, **extra} + if run_dependencies is not None: + table["run-dependencies"] = run_dependencies + return table + + +def script_recipe( + name: str, + version: str = "1.0.0", + extra_unix: list[str] | None = None, + extra_win: list[str] | None = None, + run: list[str] | None = None, + host: list[str] | None = None, + build: list[str] | None = None, +) -> str: + """A recipe.yaml that installs `bin/` printing `hello from `. + + `extra_unix`/`extra_win` lines run before the install step, so tests can + assert on the build environment (e.g. that a host dependency is present). + + With the rattler-build backend the recipe's `requirements` declare the + dependencies; the manifest's `[package.*-dependencies]` tables only map + dependency names to source locations (and inline definitions). `run`, + `host` and `build` list the requirement names for the respective section. + """ + unix = (extra_unix or []) + [ + "mkdir -p $PREFIX/bin", + f'echo "#!/usr/bin/env bash" > $PREFIX/bin/{name}', + f'echo "echo hello from {name}" >> $PREFIX/bin/{name}', + f"chmod +x $PREFIX/bin/{name}", + ] + win = (extra_win or []) + [ + 'if not exist "%PREFIX%\\bin" mkdir "%PREFIX%\\bin"', + f"echo @echo off > %PREFIX%\\bin\\{name}.bat", + f"echo echo hello from {name} >> %PREFIX%\\bin\\{name}.bat", + ] + recipe: dict[str, Any] = { + "package": {"name": name, "version": version}, + "build": { + "number": 0, + "script": [{"if": "win", "then": win, "else": unix}], + }, + } + requirements = { + key: value for key, value in (("build", build), ("host", host), ("run", run)) if value + } + if requirements: + recipe["requirements"] = requirements + return yaml.dump(recipe, sort_keys=False) + + +def write_recipe_source(directory: Path, name: str, **kwargs: Any) -> Path: + """Write a manifest-less source package: a bare recipe.yaml.""" + directory.mkdir(parents=True, exist_ok=True) + directory.joinpath("recipe.yaml").write_text(script_recipe(name, **kwargs)) + return directory + + +def write_manifest(path: Path, manifest: dict[str, Any]) -> Path: + path.write_text(tomli_w.dumps(manifest)) + return path + + +def load_workspace( + pixi: Path, + manifest: Path, + expected_exit_code: ExitCode = ExitCode.SUCCESS, + **kwargs: Any, +) -> None: + """Loading + converting the manifest is enough for parse-level checks.""" + verify_cli_command( + [pixi, "workspace", "environment", "list", "--manifest-path", manifest], + expected_exit_code=expected_exit_code, + **kwargs, + ) + + +def pixi_install(pixi: Path, manifest: Path, *args: str, **kwargs: Any) -> None: + verify_cli_command([pixi, "install", "-v", "--manifest-path", manifest, *args], **kwargs) + + +def pixi_run(pixi: Path, manifest: Path, task: str, **kwargs: Any) -> None: + verify_cli_command([pixi, "run", "--manifest-path", manifest, task], **kwargs) + + +# --------------------------------------------------------------------------- +# Parse-level tests +# +# Parse acceptance/rejection rules (which tables allow inline definitions, +# forbidden fields, duplicates, pool inheritance, preview gating) are covered +# by unit tests in `pixi_manifest`. Only behaviour that needs the real CLI +# stays here. +# --------------------------------------------------------------------------- + + +def package_consumer_manifest(package_tables: dict[str, Any]) -> dict[str, Any]: + """A workspace whose own `[package]` carries `package_tables` and depends + on itself so the package participates in the default environment.""" + return { + "workspace": workspace_table(name="consumer", version="0.1.0"), + "dependencies": {"consumer": {"path": "."}}, + "package": {"build": {"backend": RATTLER_BACKEND}, **package_tables}, + } + + +def test_accepted_in_pyproject_package_tables(pixi: Path, tmp_pixi_workspace: Path) -> None: + """Inline definitions in `[tool.pixi.package.run-dependencies]` of a + pyproject.toml manifest parse like their pixi.toml counterpart.""" + write_recipe_source(tmp_pixi_workspace / "pkg", "tool-c") + manifest = tmp_pixi_workspace / "pyproject.toml" + manifest.write_text( + tomli_w.dumps( + { + "project": {"name": "consumer", "version": "0.1.0"}, + "tool": { + "pixi": { + "workspace": workspace_table(), + "dependencies": {"consumer": {"path": "."}}, + "package": { + "build": {"backend": RATTLER_BACKEND}, + "run-dependencies": { + "tool-c": {"path": "pkg", "package": inline_def()} + }, + }, + } + }, + } + ) + ) + load_workspace(pixi, manifest) + + +# --------------------------------------------------------------------------- +# Build-level tests: happy paths +# --------------------------------------------------------------------------- + + +@pytest.mark.slow +def test_e2e_workspace_dependency_inline_builds(pixi: Path, tmp_pixi_workspace: Path) -> None: + """A manifest-less path source described inline in `[dependencies]` builds + and its binary runs.""" + write_recipe_source(tmp_pixi_workspace / "pkg", "tool-c") + manifest = write_manifest( + tmp_pixi_workspace / "pixi.toml", + { + "workspace": workspace_table(), + "dependencies": {"tool-c": {"path": "pkg", "package": inline_def()}}, + }, + ) + pixi_install(pixi, manifest) + pixi_run(pixi, manifest, "tool-c", stdout_contains="hello from tool-c") + + +@pytest.mark.slow +def test_e2e_recipe_file_path_inline_builds(pixi: Path, tmp_pixi_workspace: Path) -> None: + """A path source pointing directly at a recipe.yaml file (not a directory) + builds with an inline definition.""" + write_recipe_source(tmp_pixi_workspace / "pkg", "tool-c") + manifest = write_manifest( + tmp_pixi_workspace / "pixi.toml", + { + "workspace": workspace_table(), + "dependencies": {"tool-c": {"path": "pkg/recipe.yaml", "package": inline_def()}}, + }, + ) + pixi_install(pixi, manifest) + pixi_run(pixi, manifest, "tool-c", stdout_contains="hello from tool-c") + + +@pytest.mark.slow +def test_e2e_git_source_inline_builds(pixi: Path, tmp_pixi_workspace: Path) -> None: + """A manifest-less git source described inline builds and runs.""" + source = write_recipe_source(tmp_pixi_workspace / "src", "tool-c") + repo_url = git_test_repo(source, "tool-c-repo", tmp_pixi_workspace) + manifest = write_manifest( + tmp_pixi_workspace / "pixi.toml", + { + "workspace": workspace_table(), + "dependencies": {"tool-c": {"git": repo_url, "package": inline_def()}}, + }, + ) + pixi_install(pixi, manifest) + pixi_run(pixi, manifest, "tool-c", stdout_contains="hello from tool-c") + + +@pytest.mark.slow +def test_e2e_target_specific_inline_builds(pixi: Path, tmp_pixi_workspace: Path) -> None: + """An inline definition under `[target..dependencies]` builds.""" + write_recipe_source(tmp_pixi_workspace / "pkg", "tool-c") + manifest = write_manifest( + tmp_pixi_workspace / "pixi.toml", + { + "workspace": workspace_table(), + "target": { + CURRENT_PLATFORM: { + "dependencies": {"tool-c": {"path": "pkg", "package": inline_def()}} + } + }, + }, + ) + pixi_install(pixi, manifest) + pixi_run(pixi, manifest, "tool-c", stdout_contains="hello from tool-c") + + +@pytest.mark.slow +def test_e2e_nested_inline_definitions_build(pixi: Path, tmp_pixi_workspace: Path) -> None: + """Inline definitions nest: lib-b is declared inline, and its definition + declares tool-c inline in its own run-dependencies. + + The path of tool-c (`../c_pkg`) is written inside lib-b's definition, so + it must resolve relative to lib-b's source directory (`sub/b_pkg`), not + relative to the consuming manifest. Both packages live under `sub/` while + the consumer manifest is at the workspace root, so wrong anchoring cannot + accidentally resolve.""" + write_recipe_source(tmp_pixi_workspace / "sub" / "b_pkg", "lib-b", run=["tool-c"]) + write_recipe_source(tmp_pixi_workspace / "sub" / "c_pkg", "tool-c") + manifest = write_manifest( + tmp_pixi_workspace / "pixi.toml", + { + "workspace": workspace_table(), + "dependencies": { + "lib-b": { + "path": "sub/b_pkg", + "package": inline_def( + run_dependencies={"tool-c": {"path": "../c_pkg", "package": inline_def()}} + ), + } + }, + }, + ) + pixi_install(pixi, manifest) + pixi_run(pixi, manifest, "lib-b", stdout_contains="hello from lib-b") + pixi_run(pixi, manifest, "tool-c", stdout_contains="hello from tool-c") + + +@pytest.mark.slow +def test_e2e_package_host_dependency_inline_builds(pixi: Path, tmp_pixi_workspace: Path) -> None: + """An inline definition in `[package.host-dependencies]` is built and + installed into the host environment of the consuming package's build. + + The consumer's build script fails unless the host prefix contains the + binary installed by tool-c, so a passing install proves the host dep was + built from its inline definition and injected.""" + write_recipe_source(tmp_pixi_workspace / "c_pkg", "tool-c") + tmp_pixi_workspace.joinpath("recipe.yaml").write_text( + script_recipe( + "consumer", + host=["tool-c"], + extra_unix=['test -x "$PREFIX/bin/tool-c"'], + extra_win=['if not exist "%PREFIX%\\bin\\tool-c.bat" exit 1'], + ) + ) + manifest = write_manifest( + tmp_pixi_workspace / "pixi.toml", + package_consumer_manifest( + {"host-dependencies": {"tool-c": {"path": "c_pkg", "package": inline_def()}}} + ), + ) + pixi_install(pixi, manifest) + pixi_run(pixi, manifest, "consumer", stdout_contains="hello from consumer") + + +@pytest.mark.slow +def test_e2e_package_build_dependency_inline_builds(pixi: Path, tmp_pixi_workspace: Path) -> None: + """An inline definition in `[package.build-dependencies]` lands in the + build environment of the consuming package's build.""" + write_recipe_source(tmp_pixi_workspace / "c_pkg", "tool-c") + tmp_pixi_workspace.joinpath("recipe.yaml").write_text( + script_recipe( + "consumer", + build=["tool-c"], + extra_unix=['test -x "$BUILD_PREFIX/bin/tool-c"'], + extra_win=['if not exist "%BUILD_PREFIX%\\bin\\tool-c.bat" exit 1'], + ) + ) + manifest = write_manifest( + tmp_pixi_workspace / "pixi.toml", + package_consumer_manifest( + {"build-dependencies": {"tool-c": {"path": "c_pkg", "package": inline_def()}}} + ), + ) + pixi_install(pixi, manifest) + pixi_run(pixi, manifest, "consumer", stdout_contains="hello from consumer") + + +@pytest.mark.slow +def test_e2e_pool_inheritance_builds(pixi: Path, tmp_pixi_workspace: Path) -> None: + """A pool entry carrying an inline definition, inherited with + `{ workspace = true }` in `[package.run-dependencies]`, builds end to end.""" + write_recipe_source(tmp_pixi_workspace / "c_pkg", "tool-c") + tmp_pixi_workspace.joinpath("recipe.yaml").write_text(script_recipe("consumer", run=["tool-c"])) + manifest = write_manifest( + tmp_pixi_workspace / "pixi.toml", + { + "workspace": workspace_table( + name="consumer", + version="0.1.0", + dependencies={"tool-c": {"path": "c_pkg", "package": inline_def()}}, + ), + "dependencies": {"consumer": {"path": "."}}, + "package": { + "build": {"backend": RATTLER_BACKEND}, + "run-dependencies": {"tool-c": {"workspace": True}}, + }, + }, + ) + pixi_install(pixi, manifest) + pixi_run(pixi, manifest, "tool-c", stdout_contains="hello from tool-c") + + +@pytest.mark.slow +def test_e2e_conditional_package_dependency_builds(pixi: Path, tmp_pixi_workspace: Path) -> None: + """An inline definition inside an `if(...)` conditional package dependency + table builds when the condition matches the platform.""" + condition = "if(win)" if CURRENT_PLATFORM.startswith("win") else "if(unix)" + write_recipe_source(tmp_pixi_workspace / "c_pkg", "tool-c") + tmp_pixi_workspace.joinpath("recipe.yaml").write_text(script_recipe("consumer", run=["tool-c"])) + manifest = write_manifest( + tmp_pixi_workspace / "pixi.toml", + package_consumer_manifest( + { + "run-dependencies": { + condition: {"tool-c": {"path": "c_pkg", "package": inline_def()}} + } + } + ), + ) + pixi_install(pixi, manifest) + pixi_run(pixi, manifest, "tool-c", stdout_contains="hello from tool-c") + + +@pytest.mark.slow +def test_extra_dependencies_inline_reaches_lock(pixi: Path, tmp_pixi_workspace: Path) -> None: + """An inline definition in `[package.extra-dependencies.]` resolves + into the lock file when the consumer requests the extra. + + The python backend is used because the rattler-build backend derives its + requirements from the recipe and does not report manifest extras.""" + write_recipe_source(tmp_pixi_workspace / "c_pkg", "tool-c") + tmp_pixi_workspace.joinpath("pyproject.toml").write_text( + tomli_w.dumps( + { + "project": {"name": "consumer", "version": "0.1.0"}, + "build-system": { + "requires": ["hatchling"], + "build-backend": "hatchling.build", + }, + } + ) + ) + manifest = write_manifest( + tmp_pixi_workspace / "pixi.toml", + { + "workspace": workspace_table(name="consumer", version="0.1.0"), + "dependencies": {"consumer": {"path": ".", "extras": ["dev"]}}, + "package": { + "build": {"backend": {"name": "pixi-build-python", "version": "*"}}, + "host-dependencies": {"hatchling": "*"}, + "extra-dependencies": { + "dev": {"tool-c": {"path": "c_pkg", "package": inline_def()}} + }, + }, + }, + ) + verify_cli_command([pixi, "lock", "--manifest-path", manifest]) + lock_text = tmp_pixi_workspace.joinpath("pixi.lock").read_text() + assert "tool-c" in lock_text, ( + "the inline-defined extra dependency never resolved into the lock file" + ) + + +# --------------------------------------------------------------------------- +# Build-level tests: conflict semantics +# --------------------------------------------------------------------------- + + +def write_declaring_package( + directory: Path, + name: str, + dependency_tables: dict[str, Any], +) -> Path: + """An on-disk source package (rattler-build recipe plus pixi.toml with a + `[package]` section) that declares `dependency_tables`. + + The recipe's `requirements` mirror the dependency tables so the backend + reports the dependencies; the manifest tables carry the source locations + and inline definitions.""" + table_to_section = { + "run-dependencies": "run", + "host-dependencies": "host", + "build-dependencies": "build", + } + requirements: dict[str, list[str]] = {} + for table, entries in dependency_tables.items(): + section = table_to_section.get(table) + if section is not None: + requirements[section] = list(entries) + directory.mkdir(parents=True, exist_ok=True) + directory.joinpath("recipe.yaml").write_text( + script_recipe( + name, + run=requirements.get("run"), + host=requirements.get("host"), + build=requirements.get("build"), + ) + ) + write_manifest( + directory / "pixi.toml", + { + "package": { + "name": name, + "version": "1.0.0", + "build": {"backend": RATTLER_BACKEND}, + **dependency_tables, + } + }, + ) + return directory + + +@pytest.mark.slow +def test_transitive_conflicting_definitions_error(pixi: Path, tmp_pixi_workspace: Path) -> None: + """Two packages that disagree about the inline definition for the same + `(package, source location)` fail the solve, naming both parents.""" + write_recipe_source(tmp_pixi_workspace / "c_pkg", "tool-c") + write_declaring_package( + tmp_pixi_workspace / "a_pkg", + "pkg-a", + { + "run-dependencies": { + "tool-c": {"path": "../c_pkg", "package": inline_def(version="1.0.0")} + } + }, + ) + write_declaring_package( + tmp_pixi_workspace / "b_pkg", + "pkg-b", + { + "run-dependencies": { + "tool-c": {"path": "../c_pkg", "package": inline_def(version="2.0.0")} + } + }, + ) + manifest = write_manifest( + tmp_pixi_workspace / "pixi.toml", + { + "workspace": workspace_table(), + "dependencies": { + "pkg-a": {"path": "a_pkg"}, + "pkg-b": {"path": "b_pkg"}, + }, + }, + ) + pixi_install( + pixi, + manifest, + expected_exit_code=ExitCode.FAILURE, + stderr_contains=["conflicting inline definitions", "pkg-a", "pkg-b"], + ) + + +@pytest.mark.slow +def test_transitive_same_definition_ok(pixi: Path, tmp_pixi_workspace: Path) -> None: + """Two packages declaring the identical definition for the same + `(package, source location)` deduplicate instead of conflicting.""" + write_recipe_source(tmp_pixi_workspace / "c_pkg", "tool-c") + tables = { + "run-dependencies": {"tool-c": {"path": "../c_pkg", "package": inline_def(version="1.0.0")}} + } + write_declaring_package(tmp_pixi_workspace / "a_pkg", "pkg-a", tables) + write_declaring_package(tmp_pixi_workspace / "b_pkg", "pkg-b", tables) + manifest = write_manifest( + tmp_pixi_workspace / "pixi.toml", + { + "workspace": workspace_table(), + "dependencies": { + "pkg-a": {"path": "a_pkg"}, + "pkg-b": {"path": "b_pkg"}, + }, + }, + ) + pixi_install(pixi, manifest) + pixi_run(pixi, manifest, "tool-c", stdout_contains="hello from tool-c") + + +@pytest.mark.slow +def test_transitive_definition_vs_plain_conflict(pixi: Path, tmp_pixi_workspace: Path) -> None: + """A package declaring a definition and another declaring the same + dependency plain (relying on the on-disk manifest) is ambiguous and fails.""" + c_pkg = write_recipe_source(tmp_pixi_workspace / "c_pkg", "tool-c") + write_manifest( + c_pkg / "pixi.toml", + { + "package": { + "name": "tool-c", + "version": "1.0.0", + "build": {"backend": RATTLER_BACKEND}, + } + }, + ) + write_declaring_package( + tmp_pixi_workspace / "a_pkg", + "pkg-a", + {"run-dependencies": {"tool-c": {"path": "../c_pkg", "package": inline_def()}}}, + ) + write_declaring_package( + tmp_pixi_workspace / "b_pkg", + "pkg-b", + {"run-dependencies": {"tool-c": {"path": "../c_pkg"}}}, + ) + manifest = write_manifest( + tmp_pixi_workspace / "pixi.toml", + { + "workspace": workspace_table(), + "dependencies": { + "pkg-a": {"path": "a_pkg"}, + "pkg-b": {"path": "b_pkg"}, + }, + }, + ) + pixi_install( + pixi, + manifest, + expected_exit_code=ExitCode.FAILURE, + stderr_contains="conflicting inline definitions", + ) + + +@pytest.mark.slow +def test_direct_dependency_overrides_transitive_definition( + pixi: Path, tmp_pixi_workspace: Path +) -> None: + """A direct dependency of the environment overrides package-level + definitions: pkg-a's bogus definition for tool-c must lose against the + workspace's own plain declaration of tool-c (on-disk manifest).""" + c_pkg = write_recipe_source(tmp_pixi_workspace / "c_pkg", "tool-c") + write_manifest( + c_pkg / "pixi.toml", + { + "package": { + "name": "tool-c", + "version": "1.0.0", + "build": {"backend": RATTLER_BACKEND}, + } + }, + ) + write_declaring_package( + tmp_pixi_workspace / "a_pkg", + "pkg-a", + { + "run-dependencies": { + "tool-c": {"path": "../c_pkg", "package": inline_def(backend=BOGUS_BACKEND)} + } + }, + ) + manifest = write_manifest( + tmp_pixi_workspace / "pixi.toml", + { + "workspace": workspace_table(), + "dependencies": { + "pkg-a": {"path": "a_pkg"}, + "tool-c": {"path": "c_pkg"}, + }, + }, + ) + pixi_install(pixi, manifest) + pixi_run(pixi, manifest, "tool-c", stdout_contains="hello from tool-c") + + +@pytest.mark.slow +def test_two_environments_resolve_definitions_independently( + pixi: Path, tmp_pixi_workspace: Path +) -> None: + """Each environment resolves its own inline definition for the same + `(package, source location)`: two features carrying different definitions + in different environments must not be treated as a conflict (the + transitive conflict rule applies within one environment's walk).""" + write_recipe_source(tmp_pixi_workspace / "c_pkg", "tool-c") + manifest = write_manifest( + tmp_pixi_workspace / "pixi.toml", + { + "workspace": workspace_table(), + "feature": { + "one": { + "dependencies": { + "tool-c": {"path": "c_pkg", "package": inline_def(version="1.0.0")} + } + }, + "two": { + "dependencies": { + "tool-c": {"path": "c_pkg", "package": inline_def(version="2.0.0")} + } + }, + }, + "environments": {"env-one": ["one"], "env-two": ["two"]}, + }, + ) + pixi_install(pixi, manifest, "--environment", "env-one") + pixi_install(pixi, manifest, "--environment", "env-two") + verify_cli_command( + [pixi, "run", "--manifest-path", manifest, "--environment", "env-one", "tool-c"], + stdout_contains="hello from tool-c", + ) + + +# --------------------------------------------------------------------------- +# Build-level tests: lock-file behaviour +# --------------------------------------------------------------------------- + + +@pytest.mark.slow +def test_lock_stable_with_transitive_inline_definition( + pixi: Path, tmp_pixi_workspace: Path +) -> None: + """A lock file produced with a package-level (transitive) inline + definition must stay satisfied on the next runs; re-verification must not + flip-flop into a re-lock loop.""" + write_recipe_source(tmp_pixi_workspace / "c_pkg", "tool-c") + write_declaring_package( + tmp_pixi_workspace / "a_pkg", + "pkg-a", + {"run-dependencies": {"tool-c": {"path": "../c_pkg", "package": inline_def()}}}, + ) + manifest = write_manifest( + tmp_pixi_workspace / "pixi.toml", + { + "workspace": workspace_table(), + "dependencies": {"pkg-a": {"path": "a_pkg"}}, + }, + ) + verify_cli_command([pixi, "lock", "--manifest-path", manifest]) + verify_cli_command( + [pixi, "lock", "--manifest-path", manifest], + stderr_contains="already up-to-date", + ) + pixi_install(pixi, manifest) + verify_cli_command([pixi, "lock", "--check", "--manifest-path", manifest]) + + +@pytest.mark.slow +def test_package_level_inline_edit_invalidates_lock(pixi: Path, tmp_pixi_workspace: Path) -> None: + """Editing an inline definition declared in a *source dependency's* own + manifest (a package-level declarer) must invalidate the lock file. + + tool-c uses the python backend, whose reported metadata derives from the + inline definition, so adding a run dependency there is an observable + change. Re-verification must pick up the declarer's edited definition + instead of trusting the stale locked record.""" + c_pkg = tmp_pixi_workspace / "c_pkg" + c_pkg.mkdir(parents=True, exist_ok=True) + c_pkg.joinpath("pyproject.toml").write_text( + tomli_w.dumps( + { + "project": {"name": "tool-c", "version": "0.1.0"}, + "build-system": { + "requires": ["setuptools"], + "build-backend": "setuptools.build_meta", + }, + } + ) + ) + a_pkg = tmp_pixi_workspace / "a_pkg" + + def declare(run_dependencies: dict[str, str] | None) -> None: + package: dict[str, Any] = { + "version": "0.1.0", + "build": {"backend": {"name": "pixi-build-python", "version": "*"}}, + "host-dependencies": {"setuptools": "*"}, + } + if run_dependencies: + package["run-dependencies"] = run_dependencies + write_declaring_package( + a_pkg, + "pkg-a", + {"run-dependencies": {"tool-c": {"path": "../c_pkg", "package": package}}}, + ) + + declare(None) + manifest = write_manifest( + tmp_pixi_workspace / "pixi.toml", + { + "workspace": workspace_table(), + "dependencies": {"pkg-a": {"path": "a_pkg"}}, + }, + ) + lock_file = tmp_pixi_workspace / "pixi.lock" + verify_cli_command([pixi, "lock", "--manifest-path", manifest]) + assert "rich" not in lock_file.read_text() + + declare({"rich": "*"}) + verify_cli_command([pixi, "lock", "--manifest-path", manifest]) + assert "rich" in lock_file.read_text(), ( + "editing the package-level inline definition did not invalidate the lock file" + ) + verify_cli_command( + [pixi, "lock", "--manifest-path", manifest], + stderr_contains="already up-to-date", + ) + + +@pytest.mark.slow +def test_git_transitive_inline_lock_stable(pixi: Path, tmp_pixi_workspace: Path) -> None: + """A git source whose inline definition is declared by a transitive parent + must not re-lock on every run. + + The definition's content hash is folded into the locked identifier hash. + Verification must reproduce that hash from the parent's definition instead + of comparing against the (empty) environment-level definitions, which + would flag the record as changed forever.""" + source = write_recipe_source(tmp_pixi_workspace / "c_src", "tool-c") + repo_url = git_test_repo(source, "tool-c-repo", tmp_pixi_workspace) + write_declaring_package( + tmp_pixi_workspace / "a_pkg", + "pkg-a", + {"run-dependencies": {"tool-c": {"git": repo_url, "package": inline_def()}}}, + ) + manifest = write_manifest( + tmp_pixi_workspace / "pixi.toml", + { + "workspace": workspace_table(), + "dependencies": {"pkg-a": {"path": "a_pkg"}}, + }, + ) + verify_cli_command([pixi, "lock", "--manifest-path", manifest]) + verify_cli_command( + [pixi, "lock", "--manifest-path", manifest], + stderr_contains="already up-to-date", + ) + + +@pytest.mark.slow +def test_git_package_table_inline_lock_stable(pixi: Path, tmp_pixi_workspace: Path) -> None: + """A git source whose inline definition lives in the workspace's own + `[package.run-dependencies]` must not re-lock on every run.""" + source = write_recipe_source(tmp_pixi_workspace / "c_src", "tool-c") + repo_url = git_test_repo(source, "tool-c-repo", tmp_pixi_workspace) + tmp_pixi_workspace.joinpath("recipe.yaml").write_text(script_recipe("consumer", run=["tool-c"])) + manifest = write_manifest( + tmp_pixi_workspace / "pixi.toml", + package_consumer_manifest( + {"run-dependencies": {"tool-c": {"git": repo_url, "package": inline_def()}}} + ), + ) + verify_cli_command([pixi, "lock", "--manifest-path", manifest]) + verify_cli_command( + [pixi, "lock", "--manifest-path", manifest], + stderr_contains="already up-to-date", + ) + + +# --------------------------------------------------------------------------- +# Build-level tests: diagnostics +# --------------------------------------------------------------------------- + + +@pytest.mark.slow +def test_recipe_name_mismatch_fails_comprehensibly(pixi: Path, tmp_pixi_workspace: Path) -> None: + """The dependency key names the package; a recipe producing a different + name must fail with an error rather than silently misresolving.""" + write_recipe_source(tmp_pixi_workspace / "pkg", "other-name") + manifest = write_manifest( + tmp_pixi_workspace / "pixi.toml", + { + "workspace": workspace_table(), + "dependencies": {"tool-c": {"path": "pkg", "package": inline_def()}}, + }, + ) + pixi_install( + pixi, + manifest, + expected_exit_code=ExitCode.FAILURE, + stderr_contains="tool-c", + ) From 8fd68d3881ffb48a0a09986ee86252c90e1af477 Mon Sep 17 00:00:00 2001 From: Julian Hofer Date: Tue, 7 Jul 2026 14:10:23 +0000 Subject: [PATCH 10/17] fix: resolve nested workspace markers in dependency pool inline definitions --- crates/pixi_manifest/src/error.rs | 2 +- .../pixi_manifest/src/toml/build_backend.rs | 6 +-- crates/pixi_manifest/src/toml/build_target.rs | 2 +- crates/pixi_manifest/src/toml/manifest.rs | 33 ++++++++++++++++ crates/pixi_manifest/src/toml/package.rs | 2 +- .../pixi_manifest/src/toml/package_target.rs | 2 +- crates/pixi_manifest/src/toml/workspace.rs | 39 ++++++++++++++++--- .../src/utils/inheritable_package_map.rs | 8 ++-- .../pixi_manifest/src/warning/deprecation.rs | 2 +- crates/pixi_manifest/src/warning/mod.rs | 4 +- 10 files changed, 81 insertions(+), 19 deletions(-) diff --git a/crates/pixi_manifest/src/error.rs b/crates/pixi_manifest/src/error.rs index ade904bd0e..21da8ff419 100644 --- a/crates/pixi_manifest/src/error.rs +++ b/crates/pixi_manifest/src/error.rs @@ -36,7 +36,7 @@ pub enum RequirementConversionError { InvalidVersion(#[from] ParseVersionSpecError), } -#[derive(Default, Debug)] +#[derive(Default, Debug, Clone)] pub struct GenericError { pub message: Cow<'static, str>, pub span: Option>, diff --git a/crates/pixi_manifest/src/toml/build_backend.rs b/crates/pixi_manifest/src/toml/build_backend.rs index b35a16aad7..937273a201 100644 --- a/crates/pixi_manifest/src/toml/build_backend.rs +++ b/crates/pixi_manifest/src/toml/build_backend.rs @@ -20,7 +20,7 @@ use crate::{ warning::Deprecation, }; -#[derive(Debug)] +#[derive(Debug, Clone)] pub struct TomlPackageBuild { pub backend: PixiSpanned, pub channels: Option>>, @@ -36,7 +36,7 @@ pub struct TomlPackageBuild { pub secrets: BTreeSet, } -#[derive(Debug)] +#[derive(Debug, Clone)] pub struct TomlBuildBackend { pub name: PixiSpanned, pub spec: BackendSpec, @@ -45,7 +45,7 @@ pub struct TomlBuildBackend { } /// Backend spec, direct or inherited from `[workspace.dependencies]`. -#[derive(Debug)] +#[derive(Debug, Clone)] pub enum BackendSpec { Direct(TomlSpec), Inherited { diff --git a/crates/pixi_manifest/src/toml/build_target.rs b/crates/pixi_manifest/src/toml/build_target.rs index 263a8567c7..4e16372c2e 100644 --- a/crates/pixi_manifest/src/toml/build_target.rs +++ b/crates/pixi_manifest/src/toml/build_target.rs @@ -3,7 +3,7 @@ use toml_span::{DeserError, Value, de_helpers::TableHelper}; use crate::toml::build_backend::convert_toml_to_serde; use crate::warning::Deprecation; -#[derive(Debug)] +#[derive(Debug, Clone)] pub struct TomlPackageBuildTarget { pub config: Option, pub warnings: Vec, diff --git a/crates/pixi_manifest/src/toml/manifest.rs b/crates/pixi_manifest/src/toml/manifest.rs index ed8b4a16e2..2c0390ba90 100644 --- a/crates/pixi_manifest/src/toml/manifest.rs +++ b/crates/pixi_manifest/src/toml/manifest.rs @@ -2501,6 +2501,39 @@ mod test { ); } + #[test] + fn test_nested_workspace_marker_resolves_against_pool_in_pool_inline() { + // The same nested marker, but inside an inline definition attached to + // a `[workspace.dependencies]` pool entry itself: it resolves against + // the other pool entries and inherits their inline definitions. + let ws = parse_workspace( + r#" + [workspace] + channels = [] + platforms = ['linux-64'] + preview = ["pixi-build"] + + [workspace.dependencies] + tool-c = { path = "c_pkg", package.build = { backend = { name = "pixi-build-rattler-build", version = "*" } } } + lib-b = { path = "b_pkg", package = { build = { backend = { name = "pixi-build-rattler-build", version = "*" } }, run-dependencies = { tool-c = { workspace = true } } } } + "#, + ); + let lib_b = rattler_conda_types::PackageName::new_unchecked("lib-b"); + let tool_c = rattler_conda_types::PackageName::new_unchecked("tool-c"); + let inline = ws + .workspace + .dependency_inline_packages + .get(&lib_b) + .expect("lib-b inline definition captured on the pool"); + assert!( + inline + .manifest + .combined_inline_packages() + .contains_key(&tool_c), + "the nested marker must inherit the sibling pool entry's definition" + ); + } + #[test] fn test_inline_definition_skips_license_file_validation() { // `license-file` in an inline definition refers to the dependency's diff --git a/crates/pixi_manifest/src/toml/package.rs b/crates/pixi_manifest/src/toml/package.rs index d66f0fe778..4e272fbf9f 100644 --- a/crates/pixi_manifest/src/toml/package.rs +++ b/crates/pixi_manifest/src/toml/package.rs @@ -126,7 +126,7 @@ where /// In TOML some of the fields can be empty even though they are required in the /// data model (e.g. `name`, `version`). This is allowed because some of the /// fields might be derived from other sections of the TOML. -#[derive(Debug)] +#[derive(Debug, Clone)] pub struct TomlPackage { // Fields that can be inherited from workspace or specified directly pub name: Option>, diff --git a/crates/pixi_manifest/src/toml/package_target.rs b/crates/pixi_manifest/src/toml/package_target.rs index 9c1521d55b..811427fbba 100644 --- a/crates/pixi_manifest/src/toml/package_target.rs +++ b/crates/pixi_manifest/src/toml/package_target.rs @@ -20,7 +20,7 @@ use crate::{ }, }; -#[derive(Debug, Default)] +#[derive(Debug, Default, Clone)] pub struct TomlPackageTarget { pub run_dependencies: Option>, pub run_constraints: Option>, diff --git a/crates/pixi_manifest/src/toml/workspace.rs b/crates/pixi_manifest/src/toml/workspace.rs index a7c9b0f635..d8720503dc 100644 --- a/crates/pixi_manifest/src/toml/workspace.rs +++ b/crates/pixi_manifest/src/toml/workspace.rs @@ -311,9 +311,10 @@ pub(crate) type ConvertedDependencyPool = ( /// and the inline package manifests attached to pool entries. /// /// Source specs are gated on the `pixi-build` preview. Inline definitions -/// inherit the workspace's external package properties; the workspace -/// manifest itself is not built yet, so `{ workspace = true }` markers inside -/// a pool entry's own definition cannot reference the pool. +/// inherit the workspace's external package properties. `{ workspace = true }` +/// markers inside a pool entry's own definition resolve against the other pool +/// entries and inherit their inline definitions; the definitions are converted +/// in two passes so the order of the pool entries does not matter. pub(crate) fn convert_dependency_pool( dependencies: Option>, external: &ExternalWorkspaceProperties, @@ -342,7 +343,7 @@ pub(crate) fn convert_dependency_pool( let mut inline_packages = IndexMap::new(); if !deps.value.inline_packages.is_empty() { - let inline_properties = WorkspacePackageProperties { + let base_properties = WorkspacePackageProperties { name: external.name.clone(), version: external.version.clone(), description: external.description.clone(), @@ -353,10 +354,38 @@ pub(crate) fn convert_dependency_pool( homepage: external.homepage.clone(), repository: external.repository.clone(), documentation: external.documentation.clone(), - dependencies: IndexMap::new(), + dependencies: specs.clone(), dependency_inline_packages: IndexMap::new(), workspace_root: Some(root_directory.to_path_buf()), }; + + // First pass: convert each definition against the bare pool so + // `{ workspace = true }` markers resolve their specs; the sibling + // entries' definitions are not converted yet. Warnings are collected + // from the second pass only. + let mut sibling_inline = IndexMap::new(); + for (name, package) in &deps.value.inline_packages { + let manifest = package + .value + .clone() + .into_inline_manifest( + base_properties.clone(), + PackageDefaults::default(), + preview, + root_directory, + )? + .value; + let inline = InlinePackageManifest::from_named_manifest(name, manifest); + sibling_inline.insert(name.clone(), inline); + } + + // Second pass: re-convert with the sibling definitions available so + // markers inherit a pool entry's definition together with its spec, + // like markers in feature and package dependency tables do. + let inline_properties = WorkspacePackageProperties { + dependency_inline_packages: sibling_inline, + ..base_properties + }; for (name, package) in deps.value.inline_packages { let WithWarnings { value: manifest, diff --git a/crates/pixi_manifest/src/utils/inheritable_package_map.rs b/crates/pixi_manifest/src/utils/inheritable_package_map.rs index a3cab81d3c..123c6fe91f 100644 --- a/crates/pixi_manifest/src/utils/inheritable_package_map.rs +++ b/crates/pixi_manifest/src/utils/inheritable_package_map.rs @@ -24,7 +24,7 @@ use crate::{ /// Entry in a `[package.*-dependencies]` table that may inherit from /// `[workspace.dependencies]`. -#[derive(Debug)] +#[derive(Debug, Clone)] pub enum InheritableSpec { Direct(PixiSpec), /// Inherited from the workspace pool. `overrides.version` is always `None`. @@ -40,7 +40,7 @@ pub enum InheritableSpec { /// Dependency map that may declare workspace inheritance. Resolve against the /// pool to obtain a regular [`UniquePackageMap`]. -#[derive(Default, Debug)] +#[derive(Default, Debug, Clone)] pub struct InheritablePackageMap { pub specs: IndexMap, pub name_spans: IndexMap>, @@ -280,7 +280,7 @@ impl<'de> toml_span::Deserialize<'de> for InheritablePackageMap { } /// A single `if()` sub-table inside a package dependency table. -#[derive(Debug)] +#[derive(Debug, Clone)] pub struct ConditionalSpecs { /// The bare inner expression, without the `if(...)` wrapper. pub expression: ConditionalExpression, @@ -298,7 +298,7 @@ pub struct ConditionalSpecs { /// Keys are routed by `key_looks_conditional`: /// a key containing `(` must be a well-formed `if()` or it is /// reported as an error. -#[derive(Default, Debug)] +#[derive(Default, Debug, Clone)] pub struct ConditionalInheritablePackageMap { pub unconditional: InheritablePackageMap, pub conditional: Vec, diff --git a/crates/pixi_manifest/src/warning/deprecation.rs b/crates/pixi_manifest/src/warning/deprecation.rs index b0d139d683..0bfb6e35e6 100644 --- a/crates/pixi_manifest/src/warning/deprecation.rs +++ b/crates/pixi_manifest/src/warning/deprecation.rs @@ -5,7 +5,7 @@ use thiserror::Error; use toml_span::Span; /// A deprecation message for a field. -#[derive(Debug, Error)] +#[derive(Debug, Clone, Error)] #[error("{message}")] pub struct Deprecation { pub message: Cow<'static, str>, diff --git a/crates/pixi_manifest/src/warning/mod.rs b/crates/pixi_manifest/src/warning/mod.rs index 63e53a3424..4b93adee19 100644 --- a/crates/pixi_manifest/src/warning/mod.rs +++ b/crates/pixi_manifest/src/warning/mod.rs @@ -8,7 +8,7 @@ use thiserror::Error; use crate::{error::GenericError, utils::WithSourceCode}; -#[derive(Debug, Error, Diagnostic)] +#[derive(Debug, Clone, Error, Diagnostic)] pub enum Warning { #[error(transparent)] #[diagnostic(transparent)] @@ -48,7 +48,7 @@ impl From for WithWarnings { pub type WarningWithSource = WithSourceCode>>; -#[derive(Debug, Error)] +#[derive(Debug, Clone, Error)] #[error("{}", error.message)] pub struct GenericWarning { error: GenericError, From a408bdaca350f8fc54e7fb7c94b991431ace5a26 Mon Sep 17 00:00:00 2001 From: Julian Hofer Date: Tue, 7 Jul 2026 14:23:24 +0000 Subject: [PATCH 11/17] fix: detect removed inline definitions during satisfiability --- .../src/lock_file/satisfiability/platform.rs | 45 ++++++++++++++++--- .../test_inline_packages_extended.py | 45 +++++++++++++++++++ 2 files changed, 84 insertions(+), 6 deletions(-) diff --git a/crates/pixi_core/src/lock_file/satisfiability/platform.rs b/crates/pixi_core/src/lock_file/satisfiability/platform.rs index dd1fe15d54..beb0ed2505 100644 --- a/crates/pixi_core/src/lock_file/satisfiability/platform.rs +++ b/crates/pixi_core/src/lock_file/satisfiability/platform.rs @@ -319,6 +319,11 @@ pub async fn verify_platform_satisfiability( // Immutable records reached transitively; their identity check needs the // declarers' definitions, so it runs after all waves completed. let mut deferred_identity_checks: Vec> = Vec::new(); + // Dependency names of source records that are not re-queried (immutable + + // full). Such a record's manifest may declare an inline definition for a + // dependency without any lock-file-visible signal, so a missing definition + // for one of these names is inconclusive. + let mut unqueried_declarer_deps: HashSet = HashSet::new(); let mut final_round = false; while !pending.is_empty() { @@ -365,6 +370,23 @@ pub async fn verify_platform_satisfiability( } else { deferred_identity_checks.push(record.clone()); } + if let SourceRecordData::Full(data) = &record.data { + let depends = data + .package_record + .depends + .iter() + .chain(data.package_record.extra_depends.values().flatten()); + for spec in depends { + if let Ok(spec) = MatchSpec::from_str( + spec.as_str(), + ParseMatchSpecOptions::lenient() + .with_repodata_revision(RepodataRevision::V3), + ) && let Some(name) = spec.name.as_exact() + { + unqueried_declarer_deps.insert(name.clone()); + } + } + } let full_record = Arc::unwrap_or_clone(record).try_map_data(|data| match data { SourceRecordData::Full(data) => Ok(data), SourceRecordData::Partial(p) => Err(p), @@ -454,16 +476,27 @@ pub async fn verify_platform_satisfiability( // Identity checks for immutable records reached transitively: the // definition folded into the locked identifier hash must match what a - // declaring parent provides today. When no re-queried parent declares the - // record (e.g. every parent is itself content-pinned), the stored hash is - // trusted: an immutable parent cannot have changed its definition. + // declaring parent provides today. When no declarer provides one, the + // outcome depends on who could: a parent that was not re-queried + // (content-pinned) may declare a definition invisibly and cannot have + // changed it, so the stored hash is trusted. If every parent was + // re-queried, the definition was removed (or never existed), so the hash + // is recomputed without one to force a re-lock on removal. for record in deferred_identity_checks { - if let Some(inline) = workspace_package_inline + match workspace_package_inline .get(record.name()) .or_else(|| declared_inline.get(record.name())) { - verify_immutable_record_identity(&record, Some(inline.content_hash.as_u64())) - .map_err(CommandDispatcherError::Failed)?; + Some(inline) => { + verify_immutable_record_identity(&record, Some(inline.content_hash.as_u64())) + .map_err(CommandDispatcherError::Failed)?; + } + None => { + if !unqueried_declarer_deps.contains(record.name()) { + verify_immutable_record_identity(&record, None) + .map_err(CommandDispatcherError::Failed)?; + } + } } } diff --git a/tests/integration_python/pixi_build/test_inline_packages_extended.py b/tests/integration_python/pixi_build/test_inline_packages_extended.py index 5b09b17186..e95badeaf5 100644 --- a/tests/integration_python/pixi_build/test_inline_packages_extended.py +++ b/tests/integration_python/pixi_build/test_inline_packages_extended.py @@ -784,6 +784,51 @@ def test_git_transitive_inline_lock_stable(pixi: Path, tmp_pixi_workspace: Path) ) +@pytest.mark.slow +def test_package_level_inline_removal_invalidates_lock( + pixi: Path, tmp_pixi_workspace: Path +) -> None: + """Removing the inline definition from a declaring parent's manifest must + invalidate the lock file. + + The definition's content hash is folded into the locked identifier hash + of the (immutable) git record. Once the only declarer stops providing a + definition, recomputing the hash without one must flag the record as + changed instead of silently trusting the stale lock.""" + source = write_recipe_source(tmp_pixi_workspace / "c_src", "tool-c") + repo_url = git_test_repo(source, "tool-c-repo", tmp_pixi_workspace) + a_pkg = tmp_pixi_workspace / "a_pkg" + + def declare(with_definition: bool) -> None: + spec: dict[str, Any] = {"git": repo_url} + if with_definition: + spec["package"] = inline_def() + write_declaring_package(a_pkg, "pkg-a", {"run-dependencies": {"tool-c": spec}}) + + declare(with_definition=True) + manifest = write_manifest( + tmp_pixi_workspace / "pixi.toml", + { + "workspace": workspace_table(), + "dependencies": {"pkg-a": {"path": "a_pkg"}}, + }, + ) + verify_cli_command([pixi, "lock", "--manifest-path", manifest]) + + declare(with_definition=False) + # The record's version and build do not change, so the visible diff is + # empty; the re-written identifier hash is what marks the update. + verify_cli_command( + [pixi, "lock", "--manifest-path", manifest], + stderr_contains="Updated lock file", + ) + # Re-locking without the definition converges again. + verify_cli_command( + [pixi, "lock", "--manifest-path", manifest], + stderr_contains="already up-to-date", + ) + + @pytest.mark.slow def test_git_package_table_inline_lock_stable(pixi: Path, tmp_pixi_workspace: Path) -> None: """A git source whose inline definition lives in the workspace's own From 277a0b684e5b66615a4422f53e91caaa137fd60b Mon Sep 17 00:00:00 2001 From: Julian Hofer Date: Tue, 7 Jul 2026 14:35:29 +0000 Subject: [PATCH 12/17] fix: classify satisfiability seeds at solve-group scope --- .../src/lock_file/satisfiability/platform.rs | 19 ++--- .../test_inline_packages_extended.py | 74 +++++++++++++++++++ 2 files changed, 84 insertions(+), 9 deletions(-) diff --git a/crates/pixi_core/src/lock_file/satisfiability/platform.rs b/crates/pixi_core/src/lock_file/satisfiability/platform.rs index beb0ed2505..78f0eada31 100644 --- a/crates/pixi_core/src/lock_file/satisfiability/platform.rs +++ b/crates/pixi_core/src/lock_file/satisfiability/platform.rs @@ -262,15 +262,16 @@ pub async fn verify_platform_satisfiability( .workspace_manifest() .workspace .platform_by_name(&ctx.platform); - let inline_packages = - crate::workspace::grouped_environment::GroupedEnvironment::from(ctx.environment.clone()) - .combined_inline_packages(pixi_platform); - - // Names the environment itself declares as source dependencies. These are - // the solve's seeds: their locked identifier hash folds the - // environment-level definition (or none), never a package-level one. - let direct_source_names: HashSet = ctx - .environment + let grouped_environment = + crate::workspace::grouped_environment::GroupedEnvironment::from(ctx.environment.clone()); + let inline_packages = grouped_environment.combined_inline_packages(pixi_platform); + + // Names the solve-group declares as source dependencies. These are the + // solve's seeds: their locked identifier hash folds the group-level + // definition (or none), never a package-level one. The solve seeds at + // solve-group scope, so a source dependency declared by a sibling + // environment of the group is a seed here too. + let direct_source_names: HashSet = grouped_environment .combined_dependencies(pixi_platform) .iter_specs() .filter(|(_, spec)| spec.is_source()) diff --git a/tests/integration_python/pixi_build/test_inline_packages_extended.py b/tests/integration_python/pixi_build/test_inline_packages_extended.py index e95badeaf5..26329dbaef 100644 --- a/tests/integration_python/pixi_build/test_inline_packages_extended.py +++ b/tests/integration_python/pixi_build/test_inline_packages_extended.py @@ -693,6 +693,80 @@ def test_lock_stable_with_transitive_inline_definition( verify_cli_command([pixi, "lock", "--check", "--manifest-path", manifest]) +@pytest.mark.slow +def test_lock_stable_with_inline_definition_behind_plain_dependency( + pixi: Path, tmp_pixi_workspace: Path +) -> None: + """A definition declared two hops down (seed -> plain path dependency -> + inline-defined package) must reach its package during re-verification. + + The declarer (pkg-b) is itself a plain dependency that never receives a + definition; verification must still query it before the package it + declares (tool-c), instead of querying both without a definition once no + progress is made.""" + write_recipe_source(tmp_pixi_workspace / "c_pkg", "tool-c") + write_declaring_package( + tmp_pixi_workspace / "b_pkg", + "pkg-b", + {"run-dependencies": {"tool-c": {"path": "../c_pkg", "package": inline_def()}}}, + ) + write_declaring_package( + tmp_pixi_workspace / "a_pkg", + "pkg-a", + {"run-dependencies": {"pkg-b": {"path": "../b_pkg"}}}, + ) + manifest = write_manifest( + tmp_pixi_workspace / "pixi.toml", + { + "workspace": workspace_table(), + "dependencies": {"pkg-a": {"path": "a_pkg"}}, + }, + ) + verify_cli_command([pixi, "lock", "--manifest-path", manifest]) + verify_cli_command( + [pixi, "lock", "--manifest-path", manifest], + stderr_contains="already up-to-date", + ) + + +@pytest.mark.slow +def test_solve_group_inline_definition_lock_stable(pixi: Path, tmp_pixi_workspace: Path) -> None: + """A source dependency declared with an inline definition by one + solve-group member must verify against the group-level definition in a + sibling environment that only reaches it transitively. + + The solve seeds at solve-group scope, so the group-level definition is + folded into the locked identifier hash even for the sibling. Verification + classifying seeds per environment would check the record against a + package-level declarer (none here) instead and re-lock forever.""" + source = write_recipe_source(tmp_pixi_workspace / "c_src", "tool-c") + repo_url = git_test_repo(source, "tool-c-repo", tmp_pixi_workspace) + write_declaring_package( + tmp_pixi_workspace / "a_pkg", + "pkg-a", + {"run-dependencies": {"tool-c": {"git": repo_url}}}, + ) + manifest = write_manifest( + tmp_pixi_workspace / "pixi.toml", + { + "workspace": workspace_table(), + "dependencies": {"pkg-a": {"path": "a_pkg"}}, + "feature": { + "tools": {"dependencies": {"tool-c": {"git": repo_url, "package": inline_def()}}} + }, + "environments": { + "default": {"features": [], "solve-group": "grp"}, + "tools": {"features": ["tools"], "solve-group": "grp"}, + }, + }, + ) + verify_cli_command([pixi, "lock", "--manifest-path", manifest]) + verify_cli_command( + [pixi, "lock", "--manifest-path", manifest], + stderr_contains="already up-to-date", + ) + + @pytest.mark.slow def test_package_level_inline_edit_invalidates_lock(pixi: Path, tmp_pixi_workspace: Path) -> None: """Editing an inline definition declared in a *source dependency's* own From ba6dd8c061c1f8b67365e6cf45a8643f39958ee3 Mon Sep 17 00:00:00 2001 From: Julian Hofer Date: Tue, 7 Jul 2026 14:40:48 +0000 Subject: [PATCH 13/17] fix: query declaring parents before their packages when satisfiability stalls --- .../src/lock_file/satisfiability/platform.rs | 66 ++++++++++++------- .../test_inline_packages_extended.py | 16 ++++- 2 files changed, 58 insertions(+), 24 deletions(-) diff --git a/crates/pixi_core/src/lock_file/satisfiability/platform.rs b/crates/pixi_core/src/lock_file/satisfiability/platform.rs index 78f0eada31..a04810cd35 100644 --- a/crates/pixi_core/src/lock_file/satisfiability/platform.rs +++ b/crates/pixi_core/src/lock_file/satisfiability/platform.rs @@ -320,12 +320,14 @@ pub async fn verify_platform_satisfiability( // Immutable records reached transitively; their identity check needs the // declarers' definitions, so it runs after all waves completed. let mut deferred_identity_checks: Vec> = Vec::new(); - // Dependency names of source records that are not re-queried (immutable + + // Source dependency names of records that are not re-queried (immutable + // full). Such a record's manifest may declare an inline definition for a // dependency without any lock-file-visible signal, so a missing definition // for one of these names is inconclusive. let mut unqueried_declarer_deps: HashSet = HashSet::new(); - let mut final_round = false; + // Records allowed to be queried without an inline definition once no + // remaining potential declarer can provide one. + let mut force_no_inline: HashSet = HashSet::new(); while !pending.is_empty() { let mut wave: Vec<( @@ -371,23 +373,15 @@ pub async fn verify_platform_satisfiability( } else { deferred_identity_checks.push(record.clone()); } - if let SourceRecordData::Full(data) = &record.data { - let depends = data - .package_record - .depends - .iter() - .chain(data.package_record.extra_depends.values().flatten()); - for spec in depends { - if let Ok(spec) = MatchSpec::from_str( - spec.as_str(), - ParseMatchSpecOptions::lenient() - .with_repodata_revision(RepodataRevision::V3), - ) && let Some(name) = spec.name.as_exact() - { - unqueried_declarer_deps.insert(name.clone()); - } - } - } + // A definition can only be declared for one of the record's + // source dependencies; their names are what a missing + // definition has to be weighed against later. + unqueried_declarer_deps.extend( + record + .sources() + .keys() + .map(|name| PackageName::new_unchecked(name.clone())), + ); let full_record = Arc::unwrap_or_clone(record).try_map_data(|data| match data { SourceRecordData::Full(data) => Ok(data), SourceRecordData::Partial(p) => Err(p), @@ -415,7 +409,7 @@ pub async fn verify_platform_satisfiability( .or_else(|| declared_inline.get(record.name())) .cloned() }; - if inline.is_none() && !is_seed && !final_round { + if inline.is_none() && !is_seed && !force_no_inline.contains(record.name()) { // The definition, if any, is declared by another package that // has not been re-queried yet. deferred.push((index, UnresolvedPixiRecord::Source(record))); @@ -432,8 +426,36 @@ pub async fn verify_platform_satisfiability( // No deferred record gained a definition this round: the // remaining ones are plain source dependencies, or their declarer // is immutable (and thus cannot have changed its definition). - // Query them without a definition. - final_round = true; + // Query them without a definition - but declaring parents first. + // A plain record that never receives a definition itself may + // still declare one for its own source dependencies, so a record + // that another deferred record depends on stays deferred until + // that potential declarer was queried. + let deferred_records: Vec<_> = deferred + .iter() + .filter_map(|(_, record)| match record { + UnresolvedPixiRecord::Source(record) => Some(record), + UnresolvedPixiRecord::Binary(_) => None, + }) + .collect(); + let depended_on: HashSet = deferred_records + .iter() + .flat_map(|record| record.sources().keys()) + .map(|name| PackageName::new_unchecked(name.clone())) + .collect(); + force_no_inline = deferred_records + .iter() + .map(|record| record.name().clone()) + .filter(|name| !depended_on.contains(name)) + .collect(); + if force_no_inline.is_empty() { + // A dependency cycle among the remaining records: no query + // order can help, so query them all without a definition. + force_no_inline = deferred_records + .iter() + .map(|record| record.name().clone()) + .collect(); + } pending = deferred; continue; } diff --git a/tests/integration_python/pixi_build/test_inline_packages_extended.py b/tests/integration_python/pixi_build/test_inline_packages_extended.py index 26329dbaef..926a3608a6 100644 --- a/tests/integration_python/pixi_build/test_inline_packages_extended.py +++ b/tests/integration_python/pixi_build/test_inline_packages_extended.py @@ -703,8 +703,20 @@ def test_lock_stable_with_inline_definition_behind_plain_dependency( The declarer (pkg-b) is itself a plain dependency that never receives a definition; verification must still query it before the package it declares (tool-c), instead of querying both without a definition once no - progress is made.""" - write_recipe_source(tmp_pixi_workspace / "c_pkg", "tool-c") + progress is made. tool-c's on-disk manifest names a bogus backend, so a + query that drops the definition fails observably.""" + c_pkg = write_recipe_source(tmp_pixi_workspace / "c_pkg", "tool-c") + c_pkg.joinpath("pixi.toml").write_text( + tomli_w.dumps( + { + "package": { + "name": "tool-c", + "version": "0.1.0", + "build": {"backend": BOGUS_BACKEND}, + } + } + ) + ) write_declaring_package( tmp_pixi_workspace / "b_pkg", "pkg-b", From e0a88946f7e808f6bc126348e3e86c6cc9b0e0a7 Mon Sep 17 00:00:00 2001 From: Julian Hofer Date: Tue, 7 Jul 2026 14:45:54 +0000 Subject: [PATCH 14/17] fix: match inline definitions by name and source location during satisfiability --- .../src/lock_file/satisfiability/platform.rs | 181 +++++++++++++----- .../test_inline_packages_extended.py | 41 ++++ 2 files changed, 172 insertions(+), 50 deletions(-) diff --git a/crates/pixi_core/src/lock_file/satisfiability/platform.rs b/crates/pixi_core/src/lock_file/satisfiability/platform.rs index a04810cd35..aa9ee444e9 100644 --- a/crates/pixi_core/src/lock_file/satisfiability/platform.rs +++ b/crates/pixi_core/src/lock_file/satisfiability/platform.rs @@ -23,7 +23,9 @@ use pixi_manifest::{ use pixi_record::{ DevSourceRecord, LockFileResolver, PixiRecord, SourceRecordData, UnresolvedPixiRecord, }; -use pixi_spec::{PixiSpec, SourceAnchor, SourceLocationSpec, SourceSpec, SpecConversionError}; +use pixi_spec::{ + MatchspecFields, PixiSpec, SourceAnchor, SourceLocationSpec, SourceSpec, SpecConversionError, +}; use pixi_uv_context::UvResolutionContext; use pixi_uv_conversions::{ as_uv_req, pep508_requirement_to_uv_requirement, to_normalize, to_uv_specifiers, to_uv_version, @@ -280,31 +282,56 @@ pub async fn verify_platform_satisfiability( // Inline definitions declared by the workspace's own `[package]` // dependency tables. The workspace package is a declaring parent like any - // other, but its manifest is already in memory. - let workspace_package_inline: BTreeMap = ctx - .environment - .workspace() - .package - .as_ref() - .map(|package| { - let workspace_manifest = Arc::new(ctx.environment.workspace_manifest().clone()); - package - .value - .combined_inline_packages() - .into_iter() - .map(|(name, inline)| { - ( - name, - InlinePackage { - manifest: Arc::new(inline.manifest.clone()), - workspace: workspace_manifest.clone(), - content_hash: inline.content_hash, - }, - ) - }) - .collect() - }) - .unwrap_or_default(); + // other, but its manifest is already in memory. Each definition is + // paired with the source location its dependency spec points at: a + // declaration only applies to the record at that location, never to a + // same-named record from elsewhere. + let workspace_package_inline: BTreeMap> = + ctx.environment + .workspace() + .package + .as_ref() + .map(|package| { + let workspace_manifest = Arc::new(ctx.environment.workspace_manifest().clone()); + let manifest = &package.value; + let mut declared: BTreeMap> = + BTreeMap::new(); + for target in std::iter::once(&manifest.dependencies) + .chain(manifest.conditional_dependencies.values()) + { + for (name, inline) in &target.inline_packages { + // The matching source spec lives in the same target's + // dependency tables. + let location = target + .dependencies + .values() + .chain(target.extra_dependencies.values()) + .filter_map(|map| map.get(name)) + .flatten() + .find_map(|spec| match spec.clone().into_source_or_binary() { + Either::Left(source) => Some(source.location), + Either::Right(_) => None, + }); + let Some(location) = location else { + continue; + }; + let entries = declared.entry(name.clone()).or_default(); + if entries.iter().any(|(existing, _)| existing == &location) { + continue; + } + entries.push(( + location, + InlinePackage { + manifest: Arc::new(inline.manifest.clone()), + workspace: workspace_manifest.clone(), + content_hash: inline.content_hash, + }, + )); + } + } + declared + }) + .unwrap_or_default(); // Resolve records in declarer-aware waves. A mutable or partial source // record needs a fresh backend query, and that query needs the record's @@ -315,16 +342,21 @@ pub async fn verify_platform_satisfiability( let mut pending: Vec<(usize, UnresolvedPixiRecord)> = unresolved_records.into_iter().enumerate().collect(); let mut indexed_records: Vec<(usize, PixiRecord)> = Vec::with_capacity(pending.len()); - // Definitions declared by re-queried records for their own dependencies. - let mut declared_inline: BTreeMap = BTreeMap::new(); + // Definitions declared by re-queried records for their own dependencies, + // keyed by name and paired with the resolved source location the declarer + // maps the dependency to, mirroring the solve's `(name, location)` walk + // keys. + let mut declared_inline: BTreeMap> = + BTreeMap::new(); // Immutable records reached transitively; their identity check needs the // declarers' definitions, so it runs after all waves completed. let mut deferred_identity_checks: Vec> = Vec::new(); - // Source dependency names of records that are not re-queried (immutable + + // Source dependencies of records that are not re-queried (immutable + // full). Such a record's manifest may declare an inline definition for a // dependency without any lock-file-visible signal, so a missing definition - // for one of these names is inconclusive. - let mut unqueried_declarer_deps: HashSet = HashSet::new(); + // for one of these is inconclusive. + let mut unqueried_declarer_deps: BTreeMap> = + BTreeMap::new(); // Records allowed to be queried without an inline definition once no // remaining potential declarer can provide one. let mut force_no_inline: HashSet = HashSet::new(); @@ -374,14 +406,16 @@ pub async fn verify_platform_satisfiability( deferred_identity_checks.push(record.clone()); } // A definition can only be declared for one of the record's - // source dependencies; their names are what a missing - // definition has to be weighed against later. - unqueried_declarer_deps.extend( - record - .sources() - .keys() - .map(|name| PackageName::new_unchecked(name.clone())), - ); + // source dependencies; they are what a missing definition has + // to be weighed against later. + let anchor = + SourceAnchor::from(SourceLocationSpec::from(record.manifest_source.clone())); + for (name, location) in record.sources() { + unqueried_declarer_deps + .entry(PackageName::new_unchecked(name.clone())) + .or_default() + .push(anchor.resolve_location(location.clone())); + } let full_record = Arc::unwrap_or_clone(record).try_map_data(|data| match data { SourceRecordData::Full(data) => Ok(data), SourceRecordData::Partial(p) => Err(p), @@ -404,9 +438,8 @@ pub async fn verify_platform_satisfiability( let inline = if is_seed { inline_packages.get(record.name()).cloned() } else { - workspace_package_inline - .get(record.name()) - .or_else(|| declared_inline.get(record.name())) + find_declared_inline(&workspace_package_inline, &record) + .or_else(|| find_declared_inline(&declared_inline, &record)) .cloned() }; if inline.is_none() && !is_seed && !force_no_inline.contains(record.name()) { @@ -487,12 +520,24 @@ pub async fn verify_platform_satisfiability( ); let wave_results: Vec = resolve_futures.try_collect().await?; for (index, resolved, declared) in wave_results { - indexed_records.push((index, PixiRecord::Source(resolved))); + // A declaration only applies to the record at the location the + // declarer's spec resolves to, mirroring the solve's walk. A + // definition without a matching source entry belongs to a nested + // build/host environment and never applies to this environment's + // records. + let anchor = + SourceAnchor::from(SourceLocationSpec::from(resolved.manifest_source.clone())); for (name, inline) in declared.iter() { - declared_inline - .entry(name.clone()) - .or_insert_with(|| inline.clone()); + let Some(location) = resolved.sources().get(name.as_normalized()) else { + continue; + }; + let location = anchor.resolve_location(location.clone()); + let entries = declared_inline.entry(name.clone()).or_default(); + if !entries.iter().any(|(existing, _)| existing == &location) { + entries.push((location, inline.clone())); + } } + indexed_records.push((index, PixiRecord::Source(resolved))); } pending = deferred; } @@ -506,16 +551,23 @@ pub async fn verify_platform_satisfiability( // re-queried, the definition was removed (or never existed), so the hash // is recomputed without one to force a re-lock on removal. for record in deferred_identity_checks { - match workspace_package_inline - .get(record.name()) - .or_else(|| declared_inline.get(record.name())) + match find_declared_inline(&workspace_package_inline, &record) + .or_else(|| find_declared_inline(&declared_inline, &record)) { Some(inline) => { verify_immutable_record_identity(&record, Some(inline.content_hash.as_u64())) .map_err(CommandDispatcherError::Failed)?; } None => { - if !unqueried_declarer_deps.contains(record.name()) { + let declared_by_unqueried = + unqueried_declarer_deps + .get(record.name()) + .is_some_and(|locations| { + locations + .iter() + .any(|location| record_matches_location(&record, location)) + }); + if !declared_by_unqueried { verify_immutable_record_identity(&record, None) .map_err(CommandDispatcherError::Failed)?; } @@ -622,6 +674,35 @@ pub async fn verify_platform_satisfiability( package_verification_future.await } +/// Whether the record's pinned manifest source satisfies the given source +/// location, i.e. whether a declaration at that location refers to this +/// record. +fn record_matches_location( + record: &pixi_record::UnresolvedSourceRecord, + location: &SourceLocationSpec, +) -> bool { + record + .manifest_source + .satisfies(&SourceSpec { + location: location.clone(), + matchspec: MatchspecFields::default(), + }) + .is_ok() +} + +/// Looks up the inline definition declared for the record's name at a source +/// location that refers to the record. A definition declared for a same-named +/// package at another location never applies. +fn find_declared_inline<'a>( + declared: &'a BTreeMap>, + record: &pixi_record::UnresolvedSourceRecord, +) -> Option<&'a InlinePackage> { + declared + .get(record.name())? + .iter() + .find_map(|(location, inline)| record_matches_location(record, location).then_some(inline)) +} + /// Where a pypi requirement came from. The `index` semantics of a /// [`uv_distribution_types::Requirement`] depend on this: a `None` index from /// the manifest means the user did not pin a per-package index, while a diff --git a/tests/integration_python/pixi_build/test_inline_packages_extended.py b/tests/integration_python/pixi_build/test_inline_packages_extended.py index 926a3608a6..a078b97c18 100644 --- a/tests/integration_python/pixi_build/test_inline_packages_extended.py +++ b/tests/integration_python/pixi_build/test_inline_packages_extended.py @@ -840,6 +840,47 @@ def declare(run_dependencies: dict[str, str] | None) -> None: ) +@pytest.mark.slow +def test_same_name_other_location_definition_not_misapplied( + pixi: Path, tmp_pixi_workspace: Path +) -> None: + """An inline definition declared for a package name at one source location + must not be applied to a record of the same name at another location. + + The workspace's own `[package.host-dependencies]` declares tool-c at a + path location with an inline definition; the runtime environment contains + a different tool-c from a git location, reached through pkg-a without a + definition. Applying the path declaration's definition to the git record + changes its identity hash and re-locks forever.""" + source = write_recipe_source(tmp_pixi_workspace / "c_src", "tool-c") + repo_url = git_test_repo(source, "tool-c-repo", tmp_pixi_workspace) + write_recipe_source(tmp_pixi_workspace / "c_decl", "tool-c") + write_declaring_package( + tmp_pixi_workspace / "a_pkg", + "pkg-a", + {"run-dependencies": {"tool-c": {"git": repo_url}}}, + ) + tmp_pixi_workspace.joinpath("recipe.yaml").write_text( + script_recipe("consumer", host=["tool-c"]) + ) + manifest = write_manifest( + tmp_pixi_workspace / "pixi.toml", + { + "workspace": workspace_table(name="consumer", version="0.1.0"), + "dependencies": {"consumer": {"path": "."}, "pkg-a": {"path": "a_pkg"}}, + "package": { + "build": {"backend": RATTLER_BACKEND}, + "host-dependencies": {"tool-c": {"path": "c_decl", "package": inline_def()}}, + }, + }, + ) + verify_cli_command([pixi, "lock", "--manifest-path", manifest]) + verify_cli_command( + [pixi, "lock", "--manifest-path", manifest], + stderr_contains="already up-to-date", + ) + + @pytest.mark.slow def test_git_transitive_inline_lock_stable(pixi: Path, tmp_pixi_workspace: Path) -> None: """A git source whose inline definition is declared by a transitive parent From 7448c63eca623e21ccae614923bd8938563f9e28 Mon Sep 17 00:00:00 2001 From: Julian Hofer Date: Tue, 7 Jul 2026 14:51:33 +0000 Subject: [PATCH 15/17] fix: keep the nested solve's seed choice when installing build and host environments --- .../src/install_pixi/mod.rs | 5 +- .../src/keys/source_build.rs | 21 +++- .../test_inline_packages_extended.py | 101 ++++++++++++++++++ 3 files changed, 119 insertions(+), 8 deletions(-) diff --git a/crates/pixi_command_dispatcher/src/install_pixi/mod.rs b/crates/pixi_command_dispatcher/src/install_pixi/mod.rs index 8ee056e796..cd8866c0bb 100644 --- a/crates/pixi_command_dispatcher/src/install_pixi/mod.rs +++ b/crates/pixi_command_dispatcher/src/install_pixi/mod.rs @@ -68,9 +68,8 @@ pub struct InstallPixiEnvironmentSpec { /// package-level inline definition discovered from another record's /// manifest never applies to these: the solve resolved them with the /// consumer's own declaration (seed-first), and the build must match that - /// decision. Leave empty when no such distinction exists (e.g. nested - /// build environments, where `inline_packages` already carries exactly - /// the applicable definitions). + /// decision. For nested build/host environments these are the + /// backend-declared dependencies of the environment. #[serde(skip)] pub direct_source_dependencies: HashSet, } diff --git a/crates/pixi_command_dispatcher/src/keys/source_build.rs b/crates/pixi_command_dispatcher/src/keys/source_build.rs index 8a1213ad84..9b100f83ee 100644 --- a/crates/pixi_command_dispatcher/src/keys/source_build.rs +++ b/crates/pixi_command_dispatcher/src/keys/source_build.rs @@ -11,7 +11,7 @@ use derive_more::Display; use futures::{SinkExt, channel::mpsc::unbounded}; use pixi_build_types::procedures::{ conda_build_v1::CondaPackageFormat, - conda_outputs::{CondaOutput, CondaOutputsParams}, + conda_outputs::{CondaOutput, CondaOutputDependencies, CondaOutputsParams}, }; use pixi_compute_engine::{ComputeCtx, Key}; use pixi_record::{PixiRecord, UnresolvedPixiRecord, UnresolvedSourceRecord, VariantValue}; @@ -306,6 +306,7 @@ async fn compute_inner( directories.build_prefix.clone(), spec.record.build_packages.clone(), &dep_inline_packages, + output.build_dependencies.as_ref(), ) .await }, @@ -317,6 +318,7 @@ async fn compute_inner( directories.host_prefix.clone(), spec.record.host_packages.clone(), &dep_inline_packages, + output.host_dependencies.as_ref(), ) .await }, @@ -665,6 +667,7 @@ async fn install_prefix( dep_inline_packages: &Arc< std::collections::BTreeMap, >, + direct_dependencies: Option<&CondaOutputDependencies>, ) -> Result< ( Vec, @@ -707,10 +710,18 @@ async fn install_prefix( .iter() .map(|(name, inline)| (name.clone(), inline.clone())) .collect(), - // `inline_packages` above carries exactly the applicable - // definitions for this nested environment, so no seed-based - // suppression is needed. - direct_source_dependencies: Default::default(), + // The backend-declared dependencies of this environment are the + // nested solve's seeds: it resolved them with this package's own + // declaration (or none), so a definition harvested from a sibling + // record's manifest never applies to them. + direct_source_dependencies: direct_dependencies + .map(|deps| { + deps.depends + .iter() + .map(|dep| dep.name.clone().into()) + .collect() + }) + .unwrap_or_default(), }; let result = ctx .install_pixi_environment(install_spec) diff --git a/tests/integration_python/pixi_build/test_inline_packages_extended.py b/tests/integration_python/pixi_build/test_inline_packages_extended.py index a078b97c18..c02de56162 100644 --- a/tests/integration_python/pixi_build/test_inline_packages_extended.py +++ b/tests/integration_python/pixi_build/test_inline_packages_extended.py @@ -976,6 +976,107 @@ def test_git_package_table_inline_lock_stable(pixi: Path, tmp_pixi_workspace: Pa ) +@pytest.mark.slow +def test_nested_definition_harvested_regardless_of_record_order( + pixi: Path, tmp_pixi_workspace: Path +) -> None: + """A definition nested inside another record's inline manifest must be + harvested at install time even when the inline-defined record is + processed before its declarer. + + zzz-parent declares aaa-mid inline, and that definition declares bbb-leaf + inline. aaa-mid sorts before zzz-parent, so a harvest that commits + aaa-mid on first discovery reads its decoy on-disk manifest (bogus + backend, no nested definition) and never learns bbb-leaf's definition; + bbb-leaf's own decoy manifest then fails the build.""" + + def write_decoy_manifest(directory: Path, name: str) -> None: + directory.joinpath("pixi.toml").write_text( + tomli_w.dumps( + { + "package": { + "name": name, + "version": "0.1.0", + "build": {"backend": BOGUS_BACKEND}, + } + } + ) + ) + + mid = write_recipe_source(tmp_pixi_workspace / "mid_pkg", "aaa-mid", run=["bbb-leaf"]) + write_decoy_manifest(mid, "aaa-mid") + leaf = write_recipe_source(tmp_pixi_workspace / "leaf_pkg", "bbb-leaf") + write_decoy_manifest(leaf, "bbb-leaf") + write_declaring_package( + tmp_pixi_workspace / "parent_pkg", + "zzz-parent", + { + "run-dependencies": { + "aaa-mid": { + "path": "../mid_pkg", + "package": inline_def( + run_dependencies={ + "bbb-leaf": {"path": "../leaf_pkg", "package": inline_def()} + } + ), + } + } + }, + ) + manifest = write_manifest( + tmp_pixi_workspace / "pixi.toml", + { + "workspace": workspace_table(), + "dependencies": {"zzz-parent": {"path": "parent_pkg"}}, + }, + ) + pixi_install(pixi, manifest) + pixi_run(pixi, manifest, "bbb-leaf", stdout_contains="hello from bbb-leaf") + + +@pytest.mark.slow +def test_nested_env_keeps_seed_choice_over_sibling_definition( + pixi: Path, tmp_pixi_workspace: Path +) -> None: + """A sibling's inline definition must not override the nested solve's + seed choice when a build environment is installed. + + The workspace package declares build dependencies tool-x (plain, with an + on-disk recipe) and pkg-y, whose own manifest declares tool-x with a + bogus-backend inline definition. The nested solve seeds tool-x with the + package's plain declaration (seed-first), so the build must use the + on-disk recipe; picking up pkg-y's definition instead fails on the bogus + backend.""" + write_recipe_source(tmp_pixi_workspace / "x_pkg", "tool-x") + write_declaring_package( + tmp_pixi_workspace / "y_pkg", + "pkg-y", + { + "run-dependencies": { + "tool-x": {"path": "../x_pkg", "package": inline_def(BOGUS_BACKEND)} + } + }, + ) + tmp_pixi_workspace.joinpath("recipe.yaml").write_text( + script_recipe("consumer", build=["tool-x", "pkg-y"]) + ) + manifest = write_manifest( + tmp_pixi_workspace / "pixi.toml", + { + "workspace": workspace_table(name="consumer", version="0.1.0"), + "dependencies": {"consumer": {"path": "."}}, + "package": { + "build": {"backend": RATTLER_BACKEND}, + "build-dependencies": { + "tool-x": {"path": "x_pkg"}, + "pkg-y": {"path": "y_pkg"}, + }, + }, + }, + ) + pixi_install(pixi, manifest) + + # --------------------------------------------------------------------------- # Build-level tests: diagnostics # --------------------------------------------------------------------------- From dde5a3963f92dab1d4ecdb1b81c5e129f4ce6301 Mon Sep 17 00:00:00 2001 From: Julian Hofer Date: Tue, 7 Jul 2026 14:56:59 +0000 Subject: [PATCH 16/17] fix: re-discover records whose inline definition arrives in a later harvest round --- .../src/install_pixi/ext.rs | 54 +++++++++++++------ 1 file changed, 39 insertions(+), 15 deletions(-) diff --git a/crates/pixi_command_dispatcher/src/install_pixi/ext.rs b/crates/pixi_command_dispatcher/src/install_pixi/ext.rs index 182b427f38..c967ad28cd 100644 --- a/crates/pixi_command_dispatcher/src/install_pixi/ext.rs +++ b/crates/pixi_command_dispatcher/src/install_pixi/ext.rs @@ -13,6 +13,7 @@ use std::{ const INSTALL_LOCK_PROGRESS_INTERVAL: Duration = Duration::from_secs(30); use pixi_compute_engine::{ComputeCtx, DataStore}; +use pixi_manifest::InlineContentHash; use pixi_record::UnresolvedPixiRecord; use rattler::install::{Installer, InstallerError, PythonInfo, Transaction}; use rattler_conda_types::{PackageName, Platform, RepoDataRecord}; @@ -184,21 +185,35 @@ async fn install_inner( // source records of this same install (its own dependencies). Collect // them by discovering each record's backend; the discovery is in-memory // cached and spawns no backend. A record that is itself inline-defined - // has no on-disk manifest, so its discovery needs its definition first — - // iterate until no new definitions turn up (a chain A→B→C resolves one - // link per round). Checkout/discovery failures are skipped here; the - // real error resurfaces with proper context in the build below. + // has no usable on-disk manifest, so its discovery needs its definition + // first — and a record whose definition only turns up in a later round + // must be discovered again: its first discovery read the on-disk + // manifest, which carries neither the right backend nor the definitions + // nested inside the inline manifest (a chain A→B→C resolves one link per + // round). Contributions are tracked per record and the pool is rebuilt + // after every discovery, so a re-discovery replaces the record's earlier + // on-disk harvest. The loop terminates because every attempt consumes a + // fresh `(record, definition)` pair and the definitions all come from + // finitely nested static manifests. Checkout/discovery failures are + // skipped here; the real error resurfaces with proper context in the + // build below. { use pixi_compute_sources::SourceCheckoutExt; - let mut discovered: std::collections::HashSet = - std::collections::HashSet::new(); + // The definition each record was last attempted with; a change + // re-triggers discovery, an unchanged one (including after a + // failure) does not. + let mut attempted: HashMap> = HashMap::new(); + let mut contributions: HashMap>> = + HashMap::new(); loop { let mut progressed = false; for record in &source_records { - if discovered.contains(record.name()) { + let inline = select_inline(&consumer_inline, &package_inline, record.name()); + let inline_hash = inline.as_ref().map(|inline| inline.content_hash); + if attempted.get(record.name()) == Some(&inline_hash) { continue; } - let inline = select_inline(&consumer_inline, &package_inline, record.name()); + attempted.insert(record.name().clone(), inline_hash); let Ok(checkout) = ctx .checkout_pinned_source(record.manifest_source.clone()) .await @@ -214,14 +229,23 @@ async fn install_inner( else { continue; }; - discovered.insert(record.name().clone()); progressed = true; - for (name, inline) in - crate::inline_package::inline_packages_from_backend(&backend).iter() - { - package_inline - .entry(name.clone()) - .or_insert_with(|| inline.clone()); + contributions.insert( + record.name().clone(), + crate::inline_package::inline_packages_from_backend(&backend), + ); + // Rebuild the pool in record order so the first declaring + // record wins, independent of discovery order. + package_inline.clear(); + for source in &source_records { + let Some(declared) = contributions.get(source.name()) else { + continue; + }; + for (name, inline) in declared.iter() { + package_inline + .entry(name.clone()) + .or_insert_with(|| inline.clone()); + } } } if !progressed { From b305473b1e7a582fed234cca881efdc2320fbc25 Mon Sep 17 00:00:00 2001 From: Julian Hofer Date: Tue, 7 Jul 2026 15:00:49 +0000 Subject: [PATCH 17/17] refactor: discover inline-definition backends concurrently and hoist function-scope imports --- .../src/install_pixi/ext.rs | 118 +++++++++++------- ...fiability@changed-platform-subdir.snap.new | 8 ++ crates/pixi_manifest/src/target.rs | 2 +- .../pixi_manifest/src/toml/package_target.rs | 4 +- 4 files changed, 86 insertions(+), 46 deletions(-) create mode 100644 crates/pixi_core/crates/pixi_core/src/lock_file/satisfiability/snapshots/pixi_core__lock_file__satisfiability__tests__failing_satisfiability@changed-platform-subdir.snap.new diff --git a/crates/pixi_command_dispatcher/src/install_pixi/ext.rs b/crates/pixi_command_dispatcher/src/install_pixi/ext.rs index c967ad28cd..7470408665 100644 --- a/crates/pixi_command_dispatcher/src/install_pixi/ext.rs +++ b/crates/pixi_command_dispatcher/src/install_pixi/ext.rs @@ -35,6 +35,7 @@ use crate::keys::{ArtifactCache, SourceBuildKey, SourceBuildSpec, WorkspaceCache use crate::reporter::PixiInstallReporter; use pixi_compute_cache_dirs::CacheDirsExt; use pixi_compute_network::HasDownloadClient; +use pixi_compute_sources::SourceCheckoutExt; /// Extension trait on [`ComputeCtx`] that installs a pixi environment /// with source-build recursion routed through [`SourceBuildKey`]. @@ -191,14 +192,13 @@ async fn install_inner( // manifest, which carries neither the right backend nor the definitions // nested inside the inline manifest (a chain A→B→C resolves one link per // round). Contributions are tracked per record and the pool is rebuilt - // after every discovery, so a re-discovery replaces the record's earlier + // after every round, so a re-discovery replaces the record's earlier // on-disk harvest. The loop terminates because every attempt consumes a // fresh `(record, definition)` pair and the definitions all come from // finitely nested static manifests. Checkout/discovery failures are - // skipped here; the real error resurfaces with proper context in the - // build below. + // skipped here (and not retried until the record's definition changes); + // the real error resurfaces with proper context in the build below. { - use pixi_compute_sources::SourceCheckoutExt; // The definition each record was last attempted with; a change // re-triggers discovery, an unchanged one (including after a // failure) does not. @@ -206,51 +206,85 @@ async fn install_inner( let mut contributions: HashMap>> = HashMap::new(); loop { - let mut progressed = false; - for record in &source_records { - let inline = select_inline(&consumer_inline, &package_inline, record.name()); - let inline_hash = inline.as_ref().map(|inline| inline.content_hash); - if attempted.get(record.name()) == Some(&inline_hash) { - continue; - } - attempted.insert(record.name().clone(), inline_hash); - let Ok(checkout) = ctx - .checkout_pinned_source(record.manifest_source.clone()) - .await - else { - continue; - }; - let Ok(backend) = crate::inline_package::discover_backend( - ctx, - checkout.path.as_std_path(), - inline.as_ref(), + // Records whose applicable definition changed since their last + // attempt (or that were never attempted). + let due: Vec<( + Arc, + Option, + )> = source_records + .iter() + .filter_map(|record| { + let inline = select_inline(&consumer_inline, &package_inline, record.name()); + let inline_hash = inline.as_ref().map(|inline| inline.content_hash); + (attempted.get(record.name()) != Some(&inline_hash)) + .then(|| (record.clone(), inline)) + }) + .collect(); + if due.is_empty() { + break; + } + for (record, inline) in &due { + attempted.insert( + record.name().clone(), + inline.as_ref().map(|inline| inline.content_hash), + ); + } + + // Check out and discover this round's records concurrently. + let discovered = ctx + .try_compute_join( + due, + async |sub_ctx: &mut ComputeCtx, + (record, inline): ( + Arc, + Option, + )| + -> Result<_, InstallPixiEnvironmentError> { + let Ok(checkout) = sub_ctx + .checkout_pinned_source(record.manifest_source.clone()) + .await + else { + return Ok(None); + }; + let Ok(backend) = crate::inline_package::discover_backend( + sub_ctx, + checkout.path.as_std_path(), + inline.as_ref(), + ) + .await + else { + return Ok(None); + }; + Ok(Some(( + record.name().clone(), + crate::inline_package::inline_packages_from_backend(&backend), + ))) + }, ) .await - else { - continue; - }; + .map_err(CommandDispatcherError::Failed)?; + + let mut progressed = false; + for (name, declared) in discovered.into_iter().flatten() { progressed = true; - contributions.insert( - record.name().clone(), - crate::inline_package::inline_packages_from_backend(&backend), - ); - // Rebuild the pool in record order so the first declaring - // record wins, independent of discovery order. - package_inline.clear(); - for source in &source_records { - let Some(declared) = contributions.get(source.name()) else { - continue; - }; - for (name, inline) in declared.iter() { - package_inline - .entry(name.clone()) - .or_insert_with(|| inline.clone()); - } - } + contributions.insert(name, declared); } if !progressed { break; } + // Rebuild the pool in record order so the first declaring record + // wins, independent of discovery order. + package_inline.clear(); + for source in &source_records { + let Some(declared) = contributions.get(source.name()) else { + continue; + }; + for (name, inline) in declared.iter() { + package_inline + .entry(name.clone()) + .or_insert_with(|| inline.clone()); + } + } } } diff --git a/crates/pixi_core/crates/pixi_core/src/lock_file/satisfiability/snapshots/pixi_core__lock_file__satisfiability__tests__failing_satisfiability@changed-platform-subdir.snap.new b/crates/pixi_core/crates/pixi_core/src/lock_file/satisfiability/snapshots/pixi_core__lock_file__satisfiability__tests__failing_satisfiability@changed-platform-subdir.snap.new new file mode 100644 index 0000000000..f8ab9ba2ae --- /dev/null +++ b/crates/pixi_core/crates/pixi_core/src/lock_file/satisfiability/snapshots/pixi_core__lock_file__satisfiability__tests__failing_satisfiability@changed-platform-subdir.snap.new @@ -0,0 +1,8 @@ +--- +source: crates/pixi_core/src/lock_file/satisfiability/tests.rs +assertion_line: 263 +expression: s +--- +environment 'default' does not satisfy the requirements of the project + Diagnostic severity: error + Caused by: the workspace platform 'extra' uses subdir 'linux-aarch64' but the lock-file was solved with subdir 'linux-64' diff --git a/crates/pixi_manifest/src/target.rs b/crates/pixi_manifest/src/target.rs index 5e550e9d15..9f2e50201f 100644 --- a/crates/pixi_manifest/src/target.rs +++ b/crates/pixi_manifest/src/target.rs @@ -12,6 +12,7 @@ use pixi_spec::PixiSpec; use pixi_spec_containers::DependencyMap; use pixi_stable_hash::StableHashBuilder; use rattler_conda_types::{PackageName, ParsePlatformError, Platform}; +use xxhash_rust::xxh3::Xxh3; use super::error::DependencyError; use crate::{ @@ -96,7 +97,6 @@ impl InlinePackageManifest { /// name is folded in so two identical inline tables declared under /// different names stay distinct. pub fn from_named_manifest(name: &PackageName, manifest: PackageManifest) -> Self { - use xxhash_rust::xxh3::Xxh3; let content_hash = { let mut hasher = Xxh3::new(); name.as_normalized().hash(&mut hasher); diff --git a/crates/pixi_manifest/src/toml/package_target.rs b/crates/pixi_manifest/src/toml/package_target.rs index 811427fbba..a738c3dea3 100644 --- a/crates/pixi_manifest/src/toml/package_target.rs +++ b/crates/pixi_manifest/src/toml/package_target.rs @@ -258,6 +258,7 @@ mod test { use super::*; use crate::toml::FromTomlStr; + use crate::toml::TomlWorkspace; fn into_package_target( target: TomlPackageTarget, @@ -276,7 +277,6 @@ mod test { /// Build workspace properties whose pool declares a single git source /// dependency `name` carrying an inline definition. fn pool_properties_with_inline(name: &str) -> WorkspacePackageProperties { - use crate::toml::TomlWorkspace; let doc = format!( r#" name = "ws" @@ -611,7 +611,6 @@ mod test { /// Build workspace properties whose pool declares a single binary /// dependency `name`. fn pool_properties_with_binary(name: &str) -> WorkspacePackageProperties { - use crate::toml::TomlWorkspace; let doc = format!( r#" name = "ws" @@ -673,7 +672,6 @@ mod test { fn test_workspace_marker_on_source_pool_entry_without_definition() { // A source pool entry without an inline definition is inherited as a // plain source dependency; discovery will use the on-disk manifest. - use crate::toml::TomlWorkspace; let doc = r#" name = "ws" channels = []