Skip to content
Merged
Show file tree
Hide file tree
Changes from 3 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions crates/pixi_build_discovery/src/discovery.rs
Original file line number Diff line number Diff line change
Expand Up @@ -254,12 +254,12 @@ impl DiscoveredBackend {
.expect("points to a file")
.to_path_buf(),
project_model: Some(project_model),
configuration: build_system.configuration.map(|config| {
configuration: build_system.config.map(|config| {
config
.deserialize_into()
.expect("Configuration dictionary needs to be serializable to JSON")
}),
target_configuration: build_system.target_configuration.map(|c| {
target_configuration: build_system.target_config.map(|c| {
c.into_iter()
.map(|(selector, config)| {
(
Expand Down
71 changes: 43 additions & 28 deletions crates/pixi_manifest/src/build_system.rs
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ use rattler_conda_types::NamedChannelOrUrl;

use crate::TargetSelector;
use crate::{
TomlError,
TomlError, WithWarnings,
toml::{FromTomlStr, TomlPackageBuild},
};

Expand All @@ -28,10 +28,10 @@ pub struct PackageBuild {
pub source: Option<SourceLocationSpec>,

/// Additional configuration for the build backend.
pub configuration: Option<serde_value::Value>,
pub config: Option<serde_value::Value>,

/// Target-specific configuration for different platforms
pub target_configuration: Option<IndexMap<TargetSelector, serde_value::Value>>,
pub target_config: Option<IndexMap<TargetSelector, serde_value::Value>>,
}

impl PackageBuild {
Expand All @@ -42,8 +42,8 @@ impl PackageBuild {
channels: Some(channels),
additional_dependencies: IndexMap::default(),
source: None,
configuration: None,
target_configuration: None,
config: None,
target_config: None,
}
}
}
Expand All @@ -59,7 +59,7 @@ pub struct BuildBackend {

impl PackageBuild {
/// Parses the specified string as a toml representation of a build system.
pub fn from_toml_str(source: &str) -> Result<Self, TomlError> {
pub fn from_toml_str(source: &str) -> Result<WithWarnings<Self>, TomlError> {
TomlPackageBuild::from_toml_str(source).and_then(TomlPackageBuild::into_build_system)
}
}
Expand All @@ -75,7 +75,7 @@ mod tests {
"#;

let build = PackageBuild::from_toml_str(toml).unwrap();
assert_eq!(build.backend.name.as_source(), "pixi-build-python");
assert_eq!(build.value.backend.name.as_source(), "pixi-build-python");
}

#[test]
Expand All @@ -90,9 +90,12 @@ mod tests {
"#;

let build = PackageBuild::from_toml_str(toml).unwrap();
assert_eq!(build.backend.name.as_source(), "pixi-build-rattler-build");
assert!(build.source.is_some());
assert!(!build.source.unwrap().is_git());
assert_eq!(
build.value.backend.name.as_source(),
"pixi-build-rattler-build"
);
assert!(build.value.source.is_some());
assert!(!build.value.source.unwrap().is_git());
}

#[test]
Expand All @@ -107,9 +110,12 @@ mod tests {
"#;

let build = PackageBuild::from_toml_str(toml).unwrap();
assert_eq!(build.backend.name.as_source(), "pixi-build-rattler-build");
assert!(build.source.is_some());
assert!(build.source.unwrap().is_git());
assert_eq!(
build.value.backend.name.as_source(),
"pixi-build-rattler-build"
);
assert!(build.value.source.is_some());
assert!(build.value.source.unwrap().is_git());
}

#[test]
Expand All @@ -124,9 +130,12 @@ mod tests {
"#;

let build = PackageBuild::from_toml_str(toml).unwrap();
assert_eq!(build.backend.name.as_source(), "pixi-build-rattler-build");
assert!(build.source.is_some());
assert!(!build.source.as_ref().unwrap().is_git());
assert_eq!(
build.value.backend.name.as_source(),
"pixi-build-rattler-build"
);
assert!(build.value.source.is_some());
assert!(!build.value.source.as_ref().unwrap().is_git());
}

#[test]
Expand All @@ -141,11 +150,14 @@ mod tests {
"#;

let build = PackageBuild::from_toml_str(toml).unwrap();
assert_eq!(build.backend.name.as_source(), "pixi-build-rattler-build");
assert!(build.source.is_some());
assert_eq!(
build.value.backend.name.as_source(),
"pixi-build-rattler-build"
);
assert!(build.value.source.is_some());

// Verify it's a path source and contains the relative path
if let Some(source) = &build.source {
if let Some(source) = &build.value.source {
match &source {
pixi_spec::SourceLocationSpec::Path(path_spec) => {
assert_eq!(path_spec.path.as_str(), "../other-source");
Expand All @@ -167,11 +179,14 @@ mod tests {
"#;

let build = PackageBuild::from_toml_str(toml).unwrap();
assert_eq!(build.backend.name.as_source(), "pixi-build-rattler-build");
assert!(build.source.is_some());
assert_eq!(
build.value.backend.name.as_source(),
"pixi-build-rattler-build"
);
assert!(build.value.source.is_some());

// Verify it's a path source and contains the home directory path
if let Some(source) = &build.source {
if let Some(source) = &build.value.source {
match &source {
pixi_spec::SourceLocationSpec::Path(path_spec) => {
assert_eq!(path_spec.path.as_str(), "~/my-source");
Expand All @@ -195,9 +210,9 @@ mod tests {
"#;

let build = PackageBuild::from_toml_str(toml).unwrap();
assert!(build.source.is_some());
assert!(build.value.source.is_some());

if let Some(source) = &build.source {
if let Some(source) = &build.value.source {
match &source {
pixi_spec::SourceLocationSpec::Path(path_spec) => {
// Test that the path spec can resolve relative paths correctly
Expand All @@ -224,9 +239,9 @@ mod tests {
"#;

let build = PackageBuild::from_toml_str(toml).unwrap();
assert!(build.source.is_some());
assert!(build.value.source.is_some());

if let Some(source) = &build.source {
if let Some(source) = &build.value.source {
match &source {
pixi_spec::SourceLocationSpec::Path(path_spec) => {
// Test that absolute paths are returned as-is during resolution
Expand Down Expand Up @@ -261,9 +276,9 @@ mod tests {
);

let build = PackageBuild::from_toml_str(&toml).unwrap();
assert!(build.source.is_some(), "Failed for path: {}", path);
assert!(build.value.source.is_some(), "Failed for path: {}", path);

if let Some(source) = &build.source {
if let Some(source) = &build.value.source {
match &source {
pixi_spec::SourceLocationSpec::Path(path_spec) => {
// Verify the path is preserved correctly
Expand Down
84 changes: 64 additions & 20 deletions crates/pixi_manifest/src/toml/build_backend.rs
Original file line number Diff line number Diff line change
Expand Up @@ -8,11 +8,12 @@ use std::borrow::Cow;
use toml_span::{DeserError, Error, Spanned, Value, de_helpers::TableHelper, value::ValueInner};

use crate::{
PackageBuild, TargetSelector, TomlError,
PackageBuild, TargetSelector, TomlError, WithWarnings,
build_system::BuildBackend,
error::GenericError,
toml::build_target::TomlPackageBuildTarget,
utils::{PixiSpanned, package_map::UniquePackageMap},
warning::Deprecation,
};

#[derive(Debug)]
Expand All @@ -23,6 +24,7 @@ pub struct TomlPackageBuild {
pub source: Option<SourceLocationSpec>,
pub configuration: Option<serde_value::Value>,
pub target: IndexMap<PixiSpanned<TargetSelector>, TomlPackageBuildTarget>,
pub warnings: Vec<crate::Warning>,
}

#[derive(Debug)]
Expand All @@ -32,7 +34,7 @@ pub struct TomlBuildBackend {
}

impl TomlPackageBuild {
pub fn into_build_system(self) -> Result<PackageBuild, TomlError> {
pub fn into_build_system(self) -> Result<WithWarnings<PackageBuild>, TomlError> {
// Parse the build backend and ensure it is a binary spec.
let build_backend_spec = self.backend.value.spec.into_spec().map_err(|e| {
TomlError::Generic(
Expand All @@ -54,7 +56,7 @@ impl TomlPackageBuild {
}

// Convert target-specific build configurations
let target_configuration = self
let target_config = self
.target
.into_iter()
.flat_map(|(selector, target)| {
Expand All @@ -64,20 +66,23 @@ impl TomlPackageBuild {
})
.collect::<IndexMap<_, _>>();

Ok(PackageBuild {
backend: BuildBackend {
name: self.backend.value.name.value,
spec: build_backend_spec,
},
additional_dependencies,
channels: self.channels.map(|channels| channels.value),
source: self.source,
configuration: self.configuration,
target_configuration: if target_configuration.is_empty() {
None
} else {
Some(target_configuration)
Ok(WithWarnings {
value: PackageBuild {
backend: BuildBackend {
name: self.backend.value.name.value,
spec: build_backend_spec,
},
additional_dependencies,
channels: self.channels.map(|channels| channels.value),
source: self.source,
config: self.configuration,
target_config: if target_config.is_empty() {
None
} else {
Some(target_config)
},
},
warnings: self.warnings,
})
}
}
Expand Down Expand Up @@ -144,6 +149,8 @@ fn spec_from_spanned_toml_location(
impl<'de> toml_span::Deserialize<'de> for TomlPackageBuild {
fn deserialize(value: &mut Value<'de>) -> Result<Self, DeserError> {
let mut th = TableHelper::new(value)?;
let mut warnings = Vec::new();

let build_backend = th.required_s("backend")?.into();
let channels = th
.optional_s::<TomlWith<_, Vec<TomlFromStr<_>>>>("channels")
Expand All @@ -158,10 +165,15 @@ impl<'de> toml_span::Deserialize<'de> for TomlPackageBuild {
.map(spec_from_spanned_toml_location)
.transpose()?;

let configuration = th
.take("configuration")
.map(|(_, mut value)| convert_toml_to_serde(&mut value))
.transpose()?;
// Try the new "config" key first, then fall back to deprecated "configuration"
let configuration = if let Some((_, mut value)) = th.take("config") {
Some(convert_toml_to_serde(&mut value)?)
} else if let Some((key, mut value)) = th.table.remove_entry("configuration") {
warnings.push(Deprecation::renamed_field("configuration", "config", key.span).into());
Some(convert_toml_to_serde(&mut value)?)
} else {
None
};

let target = th
.optional::<TomlWith<_, TomlIndexMap<_, Same>>>("target")
Expand All @@ -176,6 +188,7 @@ impl<'de> toml_span::Deserialize<'de> for TomlPackageBuild {
source,
configuration,
target,
warnings,
})
}
}
Expand Down Expand Up @@ -207,6 +220,37 @@ mod test {
insta::assert_debug_snapshot!(parsed);
}

#[test]
fn test_config_parsing() {
let toml = r#"
backend = { name = "foobar", version = "*" }
config = { key = "value", other = ["foo", "bar"], integer = 1234, nested = { abc = "def" } }
"#;
let parsed = <TomlPackageBuild as crate::toml::FromTomlStr>::from_toml_str(toml)
.and_then(TomlPackageBuild::into_build_system)
.expect("parsing should succeed");

assert!(parsed.warnings.is_empty());
insta::assert_debug_snapshot!(parsed.value);
}

#[test]
fn test_configuration_deprecation_warning() {
let toml = r#"
backend = { name = "foobar", version = "*" }
configuration = { key = "value" }
"#;
let parsed = <TomlPackageBuild as crate::toml::FromTomlStr>::from_toml_str(toml)
.and_then(TomlPackageBuild::into_build_system)
.expect("parsing should succeed");

assert_eq!(parsed.warnings.len(), 1);
insta::assert_snapshot!(format_parse_error(
toml,
parsed.warnings.into_iter().next().unwrap()
));
}

#[test]
fn test_missing_version_specifier() {
assert_snapshot!(expect_parse_failure(
Expand Down
7 changes: 5 additions & 2 deletions crates/pixi_manifest/src/toml/package.rs
Original file line number Diff line number Diff line change
Expand Up @@ -351,7 +351,10 @@ impl TomlPackage {
preview: &Preview,
root_directory: Option<&Path>,
) -> Result<WithWarnings<PackageManifest>, TomlError> {
let warnings = Vec::new();
let mut warnings = Vec::new();

let build_result = self.build.into_build_system()?;
warnings.extend(build_result.warnings);

// Resolve fields with 3-tier hierarchy: direct → workspace → package defaults →
// error
Expand Down Expand Up @@ -496,7 +499,7 @@ impl TomlPackage {
"documentation",
)?,
},
build: self.build.into_build_system()?,
build: build_result.value,
targets: Targets::from_default_and_user_defined(default_package_target, targets),
})
.with_warnings(warnings))
Expand Down
Original file line number Diff line number Diff line change
@@ -1,8 +1,9 @@
---
source: crates/pixi_manifest/src/toml/build_backend.rs
expression: "expect_parse_failure(r#\"\n backend = { name = \"foobar\", version = \"*\" }\n additional = \"key\"\n \"#)"
snapshot_kind: text
---
× Unexpected keys, expected only 'backend', 'channels', 'additional-dependencies', 'source', 'configuration', 'target'
× Unexpected keys, expected only 'backend', 'channels', 'additional-dependencies', 'source', 'config', 'target'
╭─[pixi.toml:3:13]
2 │ backend = { name = "foobar", version = "*" }
3 │ additional = "key"
Expand Down
Loading
Loading