From db39c1371f2d40ade814b06cb3f693afc40ef262 Mon Sep 17 00:00:00 2001 From: Claude Date: Tue, 17 Mar 2026 18:10:02 +0000 Subject: [PATCH 1/5] feat: support flexible selector expressions in [package.target] Allow arbitrary selector expressions (e.g. "host_platform == build_platform") as target keys in the [package] section. These are passed through directly to rattler-build, enabling full control over conditional dependencies. Expression selectors are only valid in [package.target] and [package.build.target] sections. In workspace and feature sections, unknown platform strings still produce proper error messages. Closes #4625 https://claude.ai/code/session_013pXmuPTy8PtK6p6jrRKWJc --- .../src/specs_conversion.rs | 37 +++++++ .../pixi_build_backend/src/traits/targets.rs | 1 + .../src/project_model.rs | 1 + crates/pixi_build_types/src/project_model.rs | 21 +++- crates/pixi_manifest/src/target.rs | 96 ++++++++++++++++++- .../pixi_manifest/src/toml/build_backend.rs | 8 +- crates/pixi_manifest/src/toml/manifest.rs | 18 ++++ crates/pixi_manifest/src/toml/package.rs | 68 ++++++++++++- ...selector_rejected_in_workspace_target.snap | 13 +++ 9 files changed, 251 insertions(+), 12 deletions(-) create mode 100644 crates/pixi_manifest/src/toml/snapshots/pixi_manifest__toml__manifest__test__expression_selector_rejected_in_workspace_target.snap diff --git a/crates/pixi_build_backend/src/specs_conversion.rs b/crates/pixi_build_backend/src/specs_conversion.rs index 9560b9a76c..223f3ad857 100644 --- a/crates/pixi_build_backend/src/specs_conversion.rs +++ b/crates/pixi_build_backend/src/specs_conversion.rs @@ -76,6 +76,8 @@ pub fn convert_variant_to_pixi_build_types( pub fn to_rattler_build_selector(selector: &TargetSelector, platform_kind: PlatformKind) -> String { match selector { TargetSelector::Platform(p) => format!("{platform_kind}_platform == '{p}'"), + // Expression selectors are passed through directly to rattler-build. + TargetSelector::Expression(expr) => expr.clone(), _ => selector.to_string(), } } @@ -419,4 +421,39 @@ mod test { let match_spec = binary_package_spec_to_package_dependency(name, spec); assert_eq!(match_spec.to_string(), "python"); } + + #[test] + fn test_to_rattler_build_selector_platform() { + let selector = TargetSelector::Platform("linux-64".to_string()); + assert_eq!( + to_rattler_build_selector(&selector, PlatformKind::Host), + "host_platform == 'linux-64'" + ); + } + + #[test] + fn test_to_rattler_build_selector_expression_passthrough() { + let selector = + TargetSelector::Expression("host_platform == build_platform".to_string()); + assert_eq!( + to_rattler_build_selector(&selector, PlatformKind::Host), + "host_platform == build_platform" + ); + + let selector2 = + TargetSelector::Expression("target_platform != 'linux-64'".to_string()); + assert_eq!( + to_rattler_build_selector(&selector2, PlatformKind::Build), + "target_platform != 'linux-64'" + ); + } + + #[test] + fn test_to_rattler_build_selector_family() { + let selector = TargetSelector::Unix; + assert_eq!( + to_rattler_build_selector(&selector, PlatformKind::Host), + "unix" + ); + } } diff --git a/crates/pixi_build_backend/src/traits/targets.rs b/crates/pixi_build_backend/src/traits/targets.rs index f2d7c3fe37..ad8841ad9f 100644 --- a/crates/pixi_build_backend/src/traits/targets.rs +++ b/crates/pixi_build_backend/src/traits/targets.rs @@ -140,6 +140,7 @@ impl TargetSelector for pbt::TargetSelector { pbt::TargetSelector::Unix => platform.is_unix(), pbt::TargetSelector::Win => platform.is_windows(), pbt::TargetSelector::MacOs => platform.is_osx(), + pbt::TargetSelector::Expression(_) => false, } } } diff --git a/crates/pixi_build_type_conversions/src/project_model.rs b/crates/pixi_build_type_conversions/src/project_model.rs index 3282d7df82..1c08dd1618 100644 --- a/crates/pixi_build_type_conversions/src/project_model.rs +++ b/crates/pixi_build_type_conversions/src/project_model.rs @@ -175,6 +175,7 @@ pub fn to_target_selector_v1(selector: &TargetSelector) -> pbt::TargetSelector { TargetSelector::Linux => pbt::TargetSelector::Linux, TargetSelector::Win => pbt::TargetSelector::Win, TargetSelector::MacOs => pbt::TargetSelector::MacOs, + TargetSelector::Expression(expr) => pbt::TargetSelector::Expression(expr.clone()), } } diff --git a/crates/pixi_build_types/src/project_model.rs b/crates/pixi_build_types/src/project_model.rs index 246c097430..2c271a9a7e 100644 --- a/crates/pixi_build_types/src/project_model.rs +++ b/crates/pixi_build_types/src/project_model.rs @@ -76,8 +76,11 @@ impl IsDefault for ProjectModel { } } -/// Represents a target selector. Currently, we only support explicit platform -/// selection. +/// Represents a target selector. +/// +/// In addition to explicit platform selection, the `Expression` variant allows +/// arbitrary selector expressions (e.g. `"host_platform == build_platform"`) +/// that are passed through directly to rattler-build. #[derive(Debug, Clone, DeserializeFromStr, SerializeDisplay, Eq, PartialEq)] #[cfg_attr(feature = "schemars", derive(schemars::JsonSchema))] pub enum TargetSelector { @@ -87,7 +90,8 @@ pub enum TargetSelector { Win, MacOs, Platform(String), - // TODO: Add minijinja coolness here. + /// A free-form selector expression passed through to rattler-build. + Expression(String), } impl Display for TargetSelector { @@ -98,6 +102,7 @@ impl Display for TargetSelector { TargetSelector::Win => write!(f, "win"), TargetSelector::MacOs => write!(f, "macos"), TargetSelector::Platform(p) => write!(f, "{p}"), + TargetSelector::Expression(expr) => write!(f, "{expr}"), } } } @@ -110,6 +115,12 @@ impl FromStr for TargetSelector { "linux" => Ok(TargetSelector::Linux), "win" => Ok(TargetSelector::Win), "macos" => Ok(TargetSelector::MacOs), + // Expression selectors contain operators or spaces (e.g. + // "host_platform == build_platform"), whereas platform names are + // simple identifiers like "linux-64" or "osx-arm64". + _ if s.contains(' ') || s.contains("==") || s.contains("!=") => { + Ok(TargetSelector::Expression(s.to_string())) + } _ => Ok(TargetSelector::Platform(s.to_string())), } } @@ -575,6 +586,10 @@ impl Hash for TargetSelector { 4u8.hash(state); p.hash(state); } + TargetSelector::Expression(expr) => { + 5u8.hash(state); + expr.hash(state); + } } } } diff --git a/crates/pixi_manifest/src/target.rs b/crates/pixi_manifest/src/target.rs index d34221c037..5f9bcd7460 100644 --- a/crates/pixi_manifest/src/target.rs +++ b/crates/pixi_manifest/src/target.rs @@ -437,8 +437,12 @@ impl PackageTarget { } } -/// Represents a target selector. Currently we only support explicit platform -/// selection. +/// Represents a target selector. +/// +/// In addition to explicit platform selection, the `Expression` variant allows +/// arbitrary selector expressions (e.g. `"host_platform == build_platform"`) +/// that are passed through directly to rattler-build. Expression selectors are +/// only valid in the `[package]` section of the manifest. #[derive(Debug, Clone, Eq, PartialEq, Hash)] pub enum TargetSelector { // Platform specific configuration @@ -447,11 +451,16 @@ pub enum TargetSelector { Linux, Win, MacOs, - // TODO: Add minijinja coolness here. + /// A free-form selector expression that is passed through to rattler-build. + /// Only valid in the `[package]` section. + Expression(String), } impl TargetSelector { /// Returns true if this selector matches the given platform. + /// + /// Expression selectors never match a specific platform since they are + /// evaluated by rattler-build, not by pixi. pub fn matches(&self, platform: Platform) -> bool { match self { TargetSelector::Platform(p) => p == &platform, @@ -459,8 +468,14 @@ impl TargetSelector { TargetSelector::Unix => platform.is_unix(), TargetSelector::Win => platform.is_windows(), TargetSelector::MacOs => platform.is_osx(), + TargetSelector::Expression(_) => false, } } + + /// Returns `true` if this is a free-form expression selector. + pub fn is_expression(&self) -> bool { + matches!(self, TargetSelector::Expression(_)) + } } impl std::fmt::Display for TargetSelector { @@ -471,6 +486,7 @@ impl std::fmt::Display for TargetSelector { TargetSelector::Unix => write!(f, "unix"), TargetSelector::Win => write!(f, "win"), TargetSelector::MacOs => write!(f, "osx"), + TargetSelector::Expression(expr) => write!(f, "{expr}"), } } } @@ -495,6 +511,16 @@ impl FromStr for TargetSelector { } } +impl TargetSelector { + /// Parse a target selector, falling back to an `Expression` variant for + /// strings that are not known platform names. This is used in the + /// `[package]` section where arbitrary rattler-build selector expressions + /// are allowed. + pub fn from_str_or_expression(s: &str) -> Self { + s.parse().unwrap_or_else(|_| TargetSelector::Expression(s.to_string())) + } +} + /// A collect of targets including a default target. #[derive(Debug, Clone, Default)] pub struct Targets { @@ -975,4 +1001,68 @@ mod tests { "Expected foo=1.0 on osx-arm64" ); } + + #[test] + fn test_target_selector_parsing() { + use crate::TargetSelector; + + // Platform family selectors + assert_eq!( + TargetSelector::from_str("linux").unwrap(), + TargetSelector::Linux + ); + assert_eq!( + TargetSelector::from_str("unix").unwrap(), + TargetSelector::Unix + ); + assert_eq!( + TargetSelector::from_str("win").unwrap(), + TargetSelector::Win + ); + assert_eq!( + TargetSelector::from_str("osx").unwrap(), + TargetSelector::MacOs + ); + + // Platform-specific selectors + assert!(matches!( + TargetSelector::from_str("linux-64").unwrap(), + TargetSelector::Platform(_) + )); + + // Unknown strings should fail with from_str + assert!(TargetSelector::from_str("host_platform == build_platform").is_err()); + assert!(TargetSelector::from_str("foobar").is_err()); + } + + #[test] + fn test_target_selector_from_str_or_expression() { + use crate::TargetSelector; + + // Known platforms still parse as platform selectors + assert_eq!( + TargetSelector::from_str_or_expression("linux"), + TargetSelector::Linux + ); + assert!(matches!( + TargetSelector::from_str_or_expression("linux-64"), + TargetSelector::Platform(_) + )); + + // Unknown strings become expression selectors + let expr = TargetSelector::from_str_or_expression("host_platform == build_platform"); + assert!( + matches!(expr, TargetSelector::Expression(ref s) if s == "host_platform == build_platform") + ); + assert!(expr.is_expression()); + assert_eq!(expr.to_string(), "host_platform == build_platform"); + + // Expression selectors don't match any platform + assert!(!expr.matches(rattler_conda_types::Platform::Linux64)); + assert!(!expr.matches(rattler_conda_types::Platform::OsxArm64)); + + // Another expression + let expr2 = TargetSelector::from_str_or_expression("host_platform != 'linux-64'"); + assert!(expr2.is_expression()); + } } diff --git a/crates/pixi_manifest/src/toml/build_backend.rs b/crates/pixi_manifest/src/toml/build_backend.rs index 457f547e17..a670415665 100644 --- a/crates/pixi_manifest/src/toml/build_backend.rs +++ b/crates/pixi_manifest/src/toml/build_backend.rs @@ -8,10 +8,10 @@ use std::borrow::Cow; use toml_span::{DeserError, Error, Spanned, Value, de_helpers::TableHelper, value::ValueInner}; use crate::{ - PackageBuild, TargetSelector, TomlError, WithWarnings, + PackageBuild, TomlError, WithWarnings, build_system::BuildBackend, error::GenericError, - toml::build_target::TomlPackageBuildTarget, + toml::{build_target::TomlPackageBuildTarget, package::PackageTargetKey}, utils::{PixiSpanned, package_map::UniquePackageMap}, warning::Deprecation, }; @@ -23,7 +23,7 @@ pub struct TomlPackageBuild { pub additional_dependencies: UniquePackageMap, pub source: Option, pub configuration: Option, - pub target: IndexMap, TomlPackageBuildTarget>, + pub target: IndexMap, TomlPackageBuildTarget>, pub warnings: Vec, } @@ -82,7 +82,7 @@ impl TomlPackageBuild { .target .into_iter() .flat_map(|(selector, target)| { - target.config.map(|config| (selector.into_inner(), config)) + target.config.map(|config| (selector.into_inner().0, config)) }) .collect::>(); diff --git a/crates/pixi_manifest/src/toml/manifest.rs b/crates/pixi_manifest/src/toml/manifest.rs index 2b4751d1cc..9206c9827c 100644 --- a/crates/pixi_manifest/src/toml/manifest.rs +++ b/crates/pixi_manifest/src/toml/manifest.rs @@ -928,6 +928,24 @@ mod test { )); } + #[test] + fn test_expression_selector_rejected_in_workspace_target() { + // Expression selectors should be rejected in the workspace target section + // because TargetSelector::from_str doesn't accept arbitrary expressions. + // Only the [package.target] section accepts expression selectors. + assert_snapshot!(expect_parse_failure( + r#" + [workspace] + name = "test" + channels = [] + platforms = ['linux-64'] + + [target."host_platform == build_platform".dependencies] + foo = "*" + "#, + )); + } + #[test] fn test_unknown_feature() { assert_snapshot!(expect_parse_failure( diff --git a/crates/pixi_manifest/src/toml/package.rs b/crates/pixi_manifest/src/toml/package.rs index 7bf5dcc185..ce8d76600b 100644 --- a/crates/pixi_manifest/src/toml/package.rs +++ b/crates/pixi_manifest/src/toml/package.rs @@ -2,7 +2,7 @@ use std::path::{Path, PathBuf}; use indexmap::IndexMap; pub use pixi_toml::TomlFromStr; -use pixi_toml::{DeserializeAs, Same, TomlIndexMap, TomlWith}; +use pixi_toml::{DeserializeAs, FromKey, Same, TomlIndexMap, TomlWith}; use rattler_conda_types::Version; use thiserror::Error; use toml_span::{DeserError, Span, Spanned, Value, de_helpers::TableHelper}; @@ -18,6 +18,21 @@ use crate::{ utils::{PixiSpanned, package_map::UniquePackageMap}, }; +/// A wrapper around [`TargetSelector`] for use as a TOML key in the +/// `[package.target]` section. Unlike `TargetSelector::from_str`, this +/// accepts arbitrary strings as expression selectors that are passed through +/// to rattler-build. +#[derive(Debug, Clone, Eq, PartialEq, Hash)] +pub(crate) struct PackageTargetKey(pub(crate) TargetSelector); + +impl<'de> FromKey<'de> for PackageTargetKey { + type Err = std::convert::Infallible; + + fn from_key(key: toml_span::value::Key<'de>) -> Result { + Ok(PackageTargetKey(TargetSelector::from_str_or_expression(&key.name))) + } +} + /// Represents a field that can either have a direct value or inherit from /// workspace #[derive(Debug, Clone)] @@ -135,7 +150,7 @@ pub struct TomlPackage { pub host_dependencies: Option>, pub build_dependencies: Option>, pub run_dependencies: Option>, - pub target: IndexMap, TomlPackageTarget>, + pub target: IndexMap, TomlPackageTarget>, pub span: Span, } @@ -345,6 +360,10 @@ impl TomlPackage { .into_iter() .map(|(selector, target)| { let target = target.into_package_target(preview)?; + let selector = PixiSpanned { + value: selector.value.0, + span: selector.span, + }; Ok::<_, TomlError>((selector, target)) }) .collect::>()?; @@ -1114,4 +1133,49 @@ mod test { let manifest = result.unwrap().value; assert!(manifest.package.readme.is_some()); } + + #[test] + fn test_expression_selector_in_package_target() { + let input = r#" + name = "package-name" + version = "1.0.0" + + [build] + backend = { name = "bla", version = "1.0" } + + [target."host_platform == build_platform".build-dependencies] + cmake = "*" + "#; + let package = TomlPackage::from_toml_str(input).unwrap(); + let workspace = WorkspacePackageProperties::default(); + + let parsed = package + .into_manifest( + workspace, + PackageDefaults::default(), + &Preview::default(), + None, + ) + .unwrap(); + + // Verify the expression selector target was parsed correctly + let targets = &parsed.value.targets; + let expr_target = targets + .iter() + .find_map(|(target, selector)| { + selector.and_then(|s| { + if s.is_expression() { + Some((s, target)) + } else { + None + } + }) + }) + .expect("expected an expression selector target"); + assert_eq!( + expr_target.0.to_string(), + "host_platform == build_platform" + ); + assert!(expr_target.1.build_dependencies().is_some()); + } } diff --git a/crates/pixi_manifest/src/toml/snapshots/pixi_manifest__toml__manifest__test__expression_selector_rejected_in_workspace_target.snap b/crates/pixi_manifest/src/toml/snapshots/pixi_manifest__toml__manifest__test__expression_selector_rejected_in_workspace_target.snap new file mode 100644 index 0000000000..600dcc4f1c --- /dev/null +++ b/crates/pixi_manifest/src/toml/snapshots/pixi_manifest__toml__manifest__test__expression_selector_rejected_in_workspace_target.snap @@ -0,0 +1,13 @@ +--- +source: crates/pixi_manifest/src/toml/manifest.rs +expression: "expect_parse_failure(r#\"\n [workspace]\n name = \"test\"\n channels = []\n platforms = ['linux-64']\n\n [target.\"host_platform == build_platform\".dependencies]\n foo = \"*\"\n \"#,)" +--- + × 'host_platform == build_platform' is not a known platform. Valid platforms are 'noarch', 'unknown', 'linux-32', 'linux-64', 'linux-aarch64', 'linux-armv6l', 'linux-armv7l', 'linux-loongarch64', + │ 'linux-ppc64le', 'linux-ppc64', 'linux-ppc', 'linux-s390x', 'linux-riscv32', 'linux-riscv64', 'freebsd-64', 'osx-64', 'osx-arm64', 'win-32', 'win-64', 'win-arm64', 'emscripten-wasm32', 'wasi- + │ wasm32', 'zos-z' + ╭─[pixi.toml:7:18] + 6 │ + 7 │ [target."host_platform == build_platform".dependencies] + · ─────────────────────────────── + 8 │ foo = "*" + ╰──── From 3f878f4e267fb5ec8c523516b4c123d3ccb53481 Mon Sep 17 00:00:00 2001 From: Wolf Vollprecht Date: Wed, 20 May 2026 14:31:02 +0200 Subject: [PATCH 2/5] test: update expression-selector snapshot for new platform list Co-Authored-By: Claude Opus 4.7 (1M context) --- ...st__expression_selector_rejected_in_workspace_target.snap | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/crates/pixi_manifest/src/toml/snapshots/pixi_manifest__toml__manifest__test__expression_selector_rejected_in_workspace_target.snap b/crates/pixi_manifest/src/toml/snapshots/pixi_manifest__toml__manifest__test__expression_selector_rejected_in_workspace_target.snap index 600dcc4f1c..e2c21c155c 100644 --- a/crates/pixi_manifest/src/toml/snapshots/pixi_manifest__toml__manifest__test__expression_selector_rejected_in_workspace_target.snap +++ b/crates/pixi_manifest/src/toml/snapshots/pixi_manifest__toml__manifest__test__expression_selector_rejected_in_workspace_target.snap @@ -1,10 +1,11 @@ --- source: crates/pixi_manifest/src/toml/manifest.rs +assertion_line: 980 expression: "expect_parse_failure(r#\"\n [workspace]\n name = \"test\"\n channels = []\n platforms = ['linux-64']\n\n [target.\"host_platform == build_platform\".dependencies]\n foo = \"*\"\n \"#,)" --- × 'host_platform == build_platform' is not a known platform. Valid platforms are 'noarch', 'unknown', 'linux-32', 'linux-64', 'linux-aarch64', 'linux-armv6l', 'linux-armv7l', 'linux-loongarch64', - │ 'linux-ppc64le', 'linux-ppc64', 'linux-ppc', 'linux-s390x', 'linux-riscv32', 'linux-riscv64', 'freebsd-64', 'osx-64', 'osx-arm64', 'win-32', 'win-64', 'win-arm64', 'emscripten-wasm32', 'wasi- - │ wasm32', 'zos-z' + │ 'linux-ppc64le', 'linux-ppc64', 'linux-ppc', 'linux-s390x', 'linux-riscv32', 'linux-riscv64', 'freebsd-32', 'freebsd-64', 'freebsd-arm64', 'osx-64', 'osx-arm64', 'win-32', 'win-64', 'win-arm64', + │ 'emscripten-wasm32', 'wasi-wasm32', 'zos-z' ╭─[pixi.toml:7:18] 6 │ 7 │ [target."host_platform == build_platform".dependencies] From a03ca20e0f1b958b82c31cb4d5429b470fe7afe8 Mon Sep 17 00:00:00 2001 From: Wolf Vollprecht Date: Wed, 20 May 2026 14:42:11 +0200 Subject: [PATCH 3/5] feat: require if(...) wrapper for expression target selectors Expression selectors in [package.target] must now be wrapped in `if()`, e.g. `[target."if(host_platform == 'linux-64')".build-dependencies]`, replacing the previous heuristic that auto-detected expressions by scanning for spaces or comparison operators. The stored Expression value is the bare inner expression; Display re-wraps with `if(...)` for round-tripping. Unknown keys that are neither a known platform/family nor wrapped in `if(...)` now produce a parse error instead of silently becoming an expression. Co-Authored-By: Claude Opus 4.7 (1M context) --- crates/pixi_build_types/src/project_model.rs | 39 ++++++++++------- crates/pixi_manifest/src/target.rs | 46 ++++++++++++++------ crates/pixi_manifest/src/toml/package.rs | 34 ++++++++++++--- 3 files changed, 84 insertions(+), 35 deletions(-) diff --git a/crates/pixi_build_types/src/project_model.rs b/crates/pixi_build_types/src/project_model.rs index d88efda922..28553a770a 100644 --- a/crates/pixi_build_types/src/project_model.rs +++ b/crates/pixi_build_types/src/project_model.rs @@ -86,8 +86,10 @@ impl IsDefault for ProjectModel { /// Represents a target selector. /// /// In addition to explicit platform selection, the `Expression` variant allows -/// arbitrary selector expressions (e.g. `"host_platform == build_platform"`) -/// that are passed through directly to rattler-build. +/// arbitrary selector expressions wrapped in `if(...)` (e.g. +/// `"if(host_platform == build_platform)"`) that are passed through directly to +/// rattler-build. The stored string is the bare inner expression without the +/// `if(...)` wrapper. #[derive(Debug, Clone, DeserializeFromStr, SerializeDisplay, Eq, PartialEq)] #[cfg_attr(feature = "schemars", derive(schemars::JsonSchema))] pub enum TargetSelector { @@ -98,6 +100,8 @@ pub enum TargetSelector { MacOs, Platform(String), /// A free-form selector expression passed through to rattler-build. + /// Written by users as `if()`; the stored value is the bare + /// inner expression. Expression(String), } @@ -109,7 +113,7 @@ impl Display for TargetSelector { TargetSelector::Win => write!(f, "win"), TargetSelector::MacOs => write!(f, "macos"), TargetSelector::Platform(p) => write!(f, "{p}"), - TargetSelector::Expression(expr) => write!(f, "{expr}"), + TargetSelector::Expression(expr) => write!(f, "if({expr})"), } } } @@ -117,22 +121,27 @@ impl FromStr for TargetSelector { type Err = Infallible; fn from_str(s: &str) -> Result { - match s { - "unix" => Ok(TargetSelector::Unix), - "linux" => Ok(TargetSelector::Linux), - "win" => Ok(TargetSelector::Win), - "macos" => Ok(TargetSelector::MacOs), - // Expression selectors contain operators or spaces (e.g. - // "host_platform == build_platform"), whereas platform names are - // simple identifiers like "linux-64" or "osx-arm64". - _ if s.contains(' ') || s.contains("==") || s.contains("!=") => { - Ok(TargetSelector::Expression(s.to_string())) - } - _ => Ok(TargetSelector::Platform(s.to_string())), + // Expression selectors are wrapped in `if(...)`, e.g. + // `if(host_platform == build_platform)`. + if let Some(inner) = strip_if_wrapper(s) { + return Ok(TargetSelector::Expression(inner.to_string())); } + Ok(match s { + "unix" => TargetSelector::Unix, + "linux" => TargetSelector::Linux, + "win" => TargetSelector::Win, + "macos" => TargetSelector::MacOs, + _ => TargetSelector::Platform(s.to_string()), + }) } } +/// If `s` is wrapped as `if()`, return the trimmed inner expression. +fn strip_if_wrapper(s: &str) -> Option<&str> { + let inner = s.strip_prefix("if(")?.strip_suffix(')')?; + Some(inner.trim()) +} + /// A collect of targets including a default target. #[derive(Debug, Clone, Default, Serialize, Deserialize, PartialEq, Eq)] #[cfg_attr(feature = "schemars", derive(schemars::JsonSchema))] diff --git a/crates/pixi_manifest/src/target.rs b/crates/pixi_manifest/src/target.rs index 24b6e5c104..899caec980 100644 --- a/crates/pixi_manifest/src/target.rs +++ b/crates/pixi_manifest/src/target.rs @@ -491,7 +491,7 @@ impl std::fmt::Display for TargetSelector { TargetSelector::Unix => write!(f, "unix"), TargetSelector::Win => write!(f, "win"), TargetSelector::MacOs => write!(f, "osx"), - TargetSelector::Expression(expr) => write!(f, "{expr}"), + TargetSelector::Expression(expr) => write!(f, "if({expr})"), } } } @@ -517,12 +517,18 @@ impl FromStr for TargetSelector { } impl TargetSelector { - /// Parse a target selector, falling back to an `Expression` variant for - /// strings that are not known platform names. This is used in the - /// `[package]` section where arbitrary rattler-build selector expressions - /// are allowed. - pub fn from_str_or_expression(s: &str) -> Self { - s.parse().unwrap_or_else(|_| TargetSelector::Expression(s.to_string())) + /// Parse a target selector that accepts either a platform name (or + /// `unix`/`linux`/`win`/`osx` family) or an `if()` wrapper for + /// arbitrary rattler-build selector expressions. This is used in the + /// `[package]` section where expression selectors are allowed. + /// + /// The stored expression is the bare inner string without the `if(...)` + /// wrapper. + pub fn from_str_or_expression(s: &str) -> Result { + if let Some(inner) = s.strip_prefix("if(").and_then(|r| r.strip_suffix(')')) { + return Ok(TargetSelector::Expression(inner.trim().to_string())); + } + s.parse() } } @@ -1054,28 +1060,40 @@ mod tests { // Known platforms still parse as platform selectors assert_eq!( - TargetSelector::from_str_or_expression("linux"), + TargetSelector::from_str_or_expression("linux").unwrap(), TargetSelector::Linux ); assert!(matches!( - TargetSelector::from_str_or_expression("linux-64"), + TargetSelector::from_str_or_expression("linux-64").unwrap(), TargetSelector::Platform(_) )); - // Unknown strings become expression selectors - let expr = TargetSelector::from_str_or_expression("host_platform == build_platform"); + // `if(...)` strings become expression selectors, storing the bare + // inner expression. + let expr = + TargetSelector::from_str_or_expression("if(host_platform == build_platform)").unwrap(); assert!( matches!(expr, TargetSelector::Expression(ref s) if s == "host_platform == build_platform") ); assert!(expr.is_expression()); - assert_eq!(expr.to_string(), "host_platform == build_platform"); + // Display re-wraps with `if(...)` for round-tripping. + assert_eq!(expr.to_string(), "if(host_platform == build_platform)"); // Expression selectors don't match any platform assert!(!expr.matches(rattler_conda_types::Platform::Linux64)); assert!(!expr.matches(rattler_conda_types::Platform::OsxArm64)); - // Another expression - let expr2 = TargetSelector::from_str_or_expression("host_platform != 'linux-64'"); + // Another expression — inner whitespace is trimmed. + let expr2 = + TargetSelector::from_str_or_expression("if( host_platform != 'linux-64' )").unwrap(); assert!(expr2.is_expression()); + if let TargetSelector::Expression(inner) = &expr2 { + assert_eq!(inner, "host_platform != 'linux-64'"); + } + + // Bare expression strings without `if(...)` are rejected. + assert!( + TargetSelector::from_str_or_expression("host_platform == build_platform").is_err() + ); } } diff --git a/crates/pixi_manifest/src/toml/package.rs b/crates/pixi_manifest/src/toml/package.rs index 8a52bbd82e..b834d2d760 100644 --- a/crates/pixi_manifest/src/toml/package.rs +++ b/crates/pixi_manifest/src/toml/package.rs @@ -20,16 +20,16 @@ use crate::{ /// A wrapper around [`TargetSelector`] for use as a TOML key in the /// `[package.target]` section. Unlike `TargetSelector::from_str`, this -/// accepts arbitrary strings as expression selectors that are passed through -/// to rattler-build. +/// additionally accepts `if()` wrappers for selector expressions +/// passed through to rattler-build. #[derive(Debug, Clone, Eq, PartialEq, Hash)] pub struct PackageTargetKey(pub TargetSelector); impl<'de> FromKey<'de> for PackageTargetKey { - type Err = std::convert::Infallible; + type Err = rattler_conda_types::ParsePlatformError; fn from_key(key: toml_span::value::Key<'de>) -> Result { - Ok(PackageTargetKey(TargetSelector::from_str_or_expression(&key.name))) + TargetSelector::from_str_or_expression(&key.name).map(PackageTargetKey) } } @@ -1335,7 +1335,7 @@ mod test { [build] backend = { name = "bla", version = "1.0" } - [target."host_platform == build_platform".build-dependencies] + [target."if(host_platform == build_platform)".build-dependencies] cmake = "*" "#; let package = TomlPackage::from_toml_str(input).unwrap(); @@ -1364,10 +1364,32 @@ mod test { }) }) .expect("expected an expression selector target"); + // Display re-wraps the expression in `if(...)` for round-tripping. assert_eq!( expr_target.0.to_string(), - "host_platform == build_platform" + "if(host_platform == build_platform)" ); assert!(expr_target.1.build_dependencies().is_some()); } + + #[test] + fn test_bare_expression_in_package_target_is_rejected() { + // Without the `if(...)` wrapper, an expression-like key must fail to + // parse as a platform. + let input = r#" + name = "package-name" + version = "1.0.0" + + [build] + backend = { name = "bla", version = "1.0" } + + [target."host_platform == build_platform".build-dependencies] + cmake = "*" + "#; + let result = TomlPackage::from_toml_str(input); + assert!( + result.is_err(), + "bare expression keys without if(...) should be rejected, got: {result:?}" + ); + } } From 79e5f5e5a06640212ebb7e75dd9dc972b303116c Mon Sep 17 00:00:00 2001 From: Wolf Vollprecht Date: Wed, 20 May 2026 17:30:03 +0200 Subject: [PATCH 4/5] style: apply cargo fmt Co-Authored-By: Claude Opus 4.7 (1M context) --- crates/pixi_build_backend/src/specs_conversion.rs | 6 ++---- crates/pixi_manifest/src/target.rs | 4 +--- crates/pixi_manifest/src/toml/build_backend.rs | 4 +++- 3 files changed, 6 insertions(+), 8 deletions(-) diff --git a/crates/pixi_build_backend/src/specs_conversion.rs b/crates/pixi_build_backend/src/specs_conversion.rs index 4ae31c3f7c..c9da006d68 100644 --- a/crates/pixi_build_backend/src/specs_conversion.rs +++ b/crates/pixi_build_backend/src/specs_conversion.rs @@ -505,15 +505,13 @@ mod test { #[test] fn test_to_rattler_build_selector_expression_passthrough() { - let selector = - TargetSelector::Expression("host_platform == build_platform".to_string()); + let selector = TargetSelector::Expression("host_platform == build_platform".to_string()); assert_eq!( to_rattler_build_selector(&selector, PlatformKind::Host), "host_platform == build_platform" ); - let selector2 = - TargetSelector::Expression("target_platform != 'linux-64'".to_string()); + let selector2 = TargetSelector::Expression("target_platform != 'linux-64'".to_string()); assert_eq!( to_rattler_build_selector(&selector2, PlatformKind::Build), "target_platform != 'linux-64'" diff --git a/crates/pixi_manifest/src/target.rs b/crates/pixi_manifest/src/target.rs index 899caec980..4a4475688e 100644 --- a/crates/pixi_manifest/src/target.rs +++ b/crates/pixi_manifest/src/target.rs @@ -1092,8 +1092,6 @@ mod tests { } // Bare expression strings without `if(...)` are rejected. - assert!( - TargetSelector::from_str_or_expression("host_platform == build_platform").is_err() - ); + assert!(TargetSelector::from_str_or_expression("host_platform == build_platform").is_err()); } } diff --git a/crates/pixi_manifest/src/toml/build_backend.rs b/crates/pixi_manifest/src/toml/build_backend.rs index 6844881e3a..4e563f0fcf 100644 --- a/crates/pixi_manifest/src/toml/build_backend.rs +++ b/crates/pixi_manifest/src/toml/build_backend.rs @@ -89,7 +89,9 @@ impl TomlPackageBuild { .target .into_iter() .flat_map(|(selector, target)| { - target.config.map(|config| (selector.into_inner().0, config)) + target + .config + .map(|config| (selector.into_inner().0, config)) }) .collect::>(); From e52713acfb346fcae5d5e8f82e764e66c0057385 Mon Sep 17 00:00:00 2001 From: Wolf Vollprecht Date: Fri, 22 May 2026 15:41:13 +0200 Subject: [PATCH 5/5] feat: hint at if(...) wrapper when a target key looks like an expression Add `ParseTargetSelectorError` that wraps `ParsePlatformError` and, when the offending string contains characters outside `[a-z0-9-]`, emits a miette `help:` section pointing at the required `if(...)` wrapper. Convert the relevant tests to inline snapshots so the new error rendering is locked in next to the code. Co-Authored-By: Claude Opus 4.7 (1M context) --- crates/pixi_manifest/src/lib.rs | 4 +- crates/pixi_manifest/src/target.rs | 114 +++++++++++++++++- crates/pixi_manifest/src/toml/manifest.rs | 16 ++- crates/pixi_manifest/src/toml/package.rs | 27 +++-- ...selector_rejected_in_workspace_target.snap | 14 --- 5 files changed, 146 insertions(+), 29 deletions(-) delete mode 100644 crates/pixi_manifest/src/toml/snapshots/pixi_manifest__toml__manifest__test__expression_selector_rejected_in_workspace_target.snap diff --git a/crates/pixi_manifest/src/lib.rs b/crates/pixi_manifest/src/lib.rs index c124ecbe67..414691cb90 100644 --- a/crates/pixi_manifest/src/lib.rs +++ b/crates/pixi_manifest/src/lib.rs @@ -54,7 +54,9 @@ pub use spec_type::SpecType; pub use system_requirements::{ GLIBC_FAMILY, LibCFamilyAndVersion, LibCSystemRequirement, MUSL_FAMILY, SystemRequirements, }; -pub use target::{PackageTarget, TargetSelector, Targets, WorkspaceTarget}; +pub use target::{ + PackageTarget, ParseTargetSelectorError, TargetSelector, Targets, WorkspaceTarget, +}; pub use task::{Task, TaskName}; use thiserror::Error; pub use warning::{Warning, WarningWithSource, WithWarnings}; diff --git a/crates/pixi_manifest/src/target.rs b/crates/pixi_manifest/src/target.rs index 4a4475688e..1139911e92 100644 --- a/crates/pixi_manifest/src/target.rs +++ b/crates/pixi_manifest/src/target.rs @@ -502,8 +502,53 @@ impl From for TargetSelector { } } +/// Error returned when a target selector key cannot be parsed. +/// +/// Wraps [`ParsePlatformError`] and adds a hint when the input looks like a +/// rattler-build selector expression (contains characters outside +/// `[a-z0-9-]`), nudging users toward the required `if(...)` wrapper. +#[derive(Debug, thiserror::Error)] +pub enum ParseTargetSelectorError { + LooksLikeExpression { + source: ParsePlatformError, + input: String, + }, + + #[error(transparent)] + Platform(#[from] ParsePlatformError), +} + +impl std::fmt::Display for ParseTargetSelectorError { + fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { + match self { + Self::Platform(source) => write!(f, "{source}"), + Self::LooksLikeExpression { source, input } => { + // Encode the help text so `TomlDiagnostic` can render it as a + // separate `help:` section in miette output. + let help = format!( + "'{input}' looks like a selector expression. Wrap it in `if(...)` to pass it through to rattler-build, e.g. `if({input})`." + ); + write!( + f, + "{}", + pixi_toml::custom_error_message_with_help(&source.to_string(), &help) + ) + } + } + } +} + +/// Returns true if `s` cannot be a platform or family name because it contains +/// characters outside `[a-z0-9-]`. Such strings most likely intend to be +/// rattler-build selector expressions and should be wrapped with `if(...)`. +fn looks_like_expression(s: &str) -> bool { + !s.is_empty() + && s.chars() + .any(|c| !(c.is_ascii_lowercase() || c.is_ascii_digit() || c == '-')) +} + impl FromStr for TargetSelector { - type Err = ParsePlatformError; + type Err = ParseTargetSelectorError; fn from_str(s: &str) -> Result { match s { @@ -511,7 +556,18 @@ impl FromStr for TargetSelector { "unix" => Ok(TargetSelector::Unix), "win" => Ok(TargetSelector::Win), "osx" => Ok(TargetSelector::MacOs), - _ => Platform::from_str(s).map(TargetSelector::Platform), + _ => Platform::from_str(s) + .map(TargetSelector::Platform) + .map_err(|e| { + if looks_like_expression(s) { + ParseTargetSelectorError::LooksLikeExpression { + source: e, + input: s.to_string(), + } + } else { + ParseTargetSelectorError::Platform(e) + } + }), } } } @@ -524,7 +580,7 @@ impl TargetSelector { /// /// The stored expression is the bare inner string without the `if(...)` /// wrapper. - pub fn from_str_or_expression(s: &str) -> Result { + pub fn from_str_or_expression(s: &str) -> Result { if let Some(inner) = s.strip_prefix("if(").and_then(|r| r.strip_suffix(')')) { return Ok(TargetSelector::Expression(inner.trim().to_string())); } @@ -1054,6 +1110,58 @@ mod tests { assert!(TargetSelector::from_str("foobar").is_err()); } + #[test] + fn test_target_selector_expression_hint_on_unknown_platform() { + use crate::{TargetSelector, target::ParseTargetSelectorError}; + + // Strings containing characters outside `[a-z0-9-]` look like + // selector expressions and produce a hint pointing to `if(...)`. + let err = TargetSelector::from_str("host_platform == build_platform").unwrap_err(); + assert!( + matches!(err, ParseTargetSelectorError::LooksLikeExpression { .. }), + "expected LooksLikeExpression hint, got: {err:?}" + ); + let message = err.to_string(); + assert!( + message.contains("looks like a selector expression"), + "hint missing from error message: {message}" + ); + assert!( + message.contains("if(host_platform == build_platform)"), + "if(...) suggestion missing from error message: {message}" + ); + + // Plain unknown strings (matching `[a-z0-9-]`) just report the + // platform parse error without the expression hint. + let err = TargetSelector::from_str("foobar").unwrap_err(); + assert!( + matches!(err, ParseTargetSelectorError::Platform(_)), + "expected plain platform error, got: {err:?}" + ); + } + + #[test] + fn test_target_selector_expression_hint_triggers() { + use crate::TargetSelector; + // Anything outside `[a-z0-9-]` should trigger the expression hint. + let cases = [ + "host_platform == build_platform", + "host_platform != 'linux-64'", + "matches(python, '>=3.10')", + "linux 64", // contains a space + ]; + for case in cases { + let err = TargetSelector::from_str(case).unwrap_err(); + assert!( + matches!( + err, + super::ParseTargetSelectorError::LooksLikeExpression { .. } + ), + "expected expression hint for '{case}', got: {err:?}" + ); + } + } + #[test] fn test_target_selector_from_str_or_expression() { use crate::TargetSelector; diff --git a/crates/pixi_manifest/src/toml/manifest.rs b/crates/pixi_manifest/src/toml/manifest.rs index e4b7cfacca..d6bf5fccc5 100644 --- a/crates/pixi_manifest/src/toml/manifest.rs +++ b/crates/pixi_manifest/src/toml/manifest.rs @@ -1166,7 +1166,8 @@ mod test { fn test_expression_selector_rejected_in_workspace_target() { // Expression selectors should be rejected in the workspace target section // because TargetSelector::from_str doesn't accept arbitrary expressions. - // Only the [package.target] section accepts expression selectors. + // Only the [package.target] section accepts expression selectors. The + // error must include a hint pointing users at the `if(...)` wrapper. assert_snapshot!(expect_parse_failure( r#" [workspace] @@ -1177,7 +1178,18 @@ mod test { [target."host_platform == build_platform".dependencies] foo = "*" "#, - )); + ), @r###" + × 'host_platform == build_platform' is not a known platform. Valid platforms are 'noarch', 'unknown', 'linux-32', 'linux-64', 'linux-aarch64', 'linux-armv6l', 'linux-armv7l', 'linux-loongarch64', + │ 'linux-ppc64le', 'linux-ppc64', 'linux-ppc', 'linux-s390x', 'linux-riscv32', 'linux-riscv64', 'freebsd-32', 'freebsd-64', 'freebsd-arm64', 'osx-64', 'osx-arm64', 'win-32', 'win-64', 'win-arm64', + │ 'emscripten-wasm32', 'wasi-wasm32', 'zos-z' + ╭─[pixi.toml:7:18] + 6 │ + 7 │ [target."host_platform == build_platform".dependencies] + · ─────────────────────────────── + 8 │ foo = "*" + ╰──── + help: 'host_platform == build_platform' looks like a selector expression. Wrap it in `if(...)` to pass it through to rattler-build, e.g. `if(host_platform == build_platform)`. + "###); } #[test] diff --git a/crates/pixi_manifest/src/toml/package.rs b/crates/pixi_manifest/src/toml/package.rs index 20d1b152e9..be710104b7 100644 --- a/crates/pixi_manifest/src/toml/package.rs +++ b/crates/pixi_manifest/src/toml/package.rs @@ -27,7 +27,7 @@ use crate::{ pub struct PackageTargetKey(pub TargetSelector); impl<'de> FromKey<'de> for PackageTargetKey { - type Err = rattler_conda_types::ParsePlatformError; + type Err = crate::target::ParseTargetSelectorError; fn from_key(key: toml_span::value::Key<'de>) -> Result { TargetSelector::from_str_or_expression(&key.name).map(PackageTargetKey) @@ -1419,8 +1419,10 @@ mod test { #[test] fn test_bare_expression_in_package_target_is_rejected() { // Without the `if(...)` wrapper, an expression-like key must fail to - // parse as a platform. - let input = r#" + // parse as a platform. The error must include a hint pointing users + // at the `if(...)` wrapper. + assert_snapshot!(expect_parse_failure( + r#" name = "package-name" version = "1.0.0" @@ -1429,12 +1431,19 @@ mod test { [target."host_platform == build_platform".build-dependencies] cmake = "*" - "#; - let result = TomlPackage::from_toml_str(input); - assert!( - result.is_err(), - "bare expression keys without if(...) should be rejected, got: {result:?}" - ); + "#, + ), @r###" + × 'host_platform == build_platform' is not a known platform. Valid platforms are 'noarch', 'unknown', 'linux-32', 'linux-64', 'linux-aarch64', 'linux-armv6l', 'linux-armv7l', 'linux-loongarch64', + │ 'linux-ppc64le', 'linux-ppc64', 'linux-ppc', 'linux-s390x', 'linux-riscv32', 'linux-riscv64', 'freebsd-32', 'freebsd-64', 'freebsd-arm64', 'osx-64', 'osx-arm64', 'win-32', 'win-64', 'win-arm64', + │ 'emscripten-wasm32', 'wasi-wasm32', 'zos-z' + ╭─[pixi.toml:8:18] + 7 │ + 8 │ [target."host_platform == build_platform".build-dependencies] + · ─────────────────────────────── + 9 │ cmake = "*" + ╰──── + help: 'host_platform == build_platform' looks like a selector expression. Wrap it in `if(...)` to pass it through to rattler-build, e.g. `if(host_platform == build_platform)`. + "###); } #[test] diff --git a/crates/pixi_manifest/src/toml/snapshots/pixi_manifest__toml__manifest__test__expression_selector_rejected_in_workspace_target.snap b/crates/pixi_manifest/src/toml/snapshots/pixi_manifest__toml__manifest__test__expression_selector_rejected_in_workspace_target.snap deleted file mode 100644 index e2c21c155c..0000000000 --- a/crates/pixi_manifest/src/toml/snapshots/pixi_manifest__toml__manifest__test__expression_selector_rejected_in_workspace_target.snap +++ /dev/null @@ -1,14 +0,0 @@ ---- -source: crates/pixi_manifest/src/toml/manifest.rs -assertion_line: 980 -expression: "expect_parse_failure(r#\"\n [workspace]\n name = \"test\"\n channels = []\n platforms = ['linux-64']\n\n [target.\"host_platform == build_platform\".dependencies]\n foo = \"*\"\n \"#,)" ---- - × 'host_platform == build_platform' is not a known platform. Valid platforms are 'noarch', 'unknown', 'linux-32', 'linux-64', 'linux-aarch64', 'linux-armv6l', 'linux-armv7l', 'linux-loongarch64', - │ 'linux-ppc64le', 'linux-ppc64', 'linux-ppc', 'linux-s390x', 'linux-riscv32', 'linux-riscv64', 'freebsd-32', 'freebsd-64', 'freebsd-arm64', 'osx-64', 'osx-arm64', 'win-32', 'win-64', 'win-arm64', - │ 'emscripten-wasm32', 'wasi-wasm32', 'zos-z' - ╭─[pixi.toml:7:18] - 6 │ - 7 │ [target."host_platform == build_platform".dependencies] - · ─────────────────────────────── - 8 │ foo = "*" - ╰────