Skip to content

Commit

Permalink
feat: move package fields under [package]. (#2731)
Browse files Browse the repository at this point in the history
This PR refactors the manifest to move all keys related to a package under the package key itself. It also removes the build-backend table in favor of making backends read the tool section instead. The build backends have been updated.
  • Loading branch information
baszalmstra authored Jan 7, 2025
1 parent c464ed7 commit bb6cd82
Show file tree
Hide file tree
Showing 57 changed files with 985 additions and 1,080 deletions.
1 change: 0 additions & 1 deletion Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

62 changes: 16 additions & 46 deletions crates/pixi_build_frontend/src/protocols/builders/pixi.rs
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,6 @@ use miette::Diagnostic;
use pixi_consts::consts;
use pixi_manifest::{Manifest, PackageManifest, PrioritizedChannel, WorkspaceManifest};
use rattler_conda_types::{ChannelConfig, MatchSpec};
use serde::{de::IntoDeserializer, Deserialize};
use thiserror::Error;
use which::Error;

Expand Down Expand Up @@ -143,35 +142,28 @@ impl ProtocolBuilder {
}

fn get_tool_spec(&self, channel_config: &ChannelConfig) -> Result<ToolSpec, FinishError> {
// The tool is either overridden or its not, with pixi the backend is specified in the toml
// so it's unclear if we need to override the tool until this point, lets check that now
// The tool is either overridden or its not, with pixi the backend is specified
// in the toml so it's unclear if we need to override the tool until
// this point, lets check that now
if let Some(backend_override) = self.backend_override.as_ref() {
let tool_name = self
.package_manifest
.build_system
.build_backend
.name
.clone();
let tool_name = self.package_manifest.build.backend.name.clone();
if let Some(tool) = backend_override.overridden_tool(tool_name.as_normalized()) {
return Ok(tool.as_spec());
}
}

// If we get here the tool is not overridden, so we use the isolated variant
let build_system = &self.package_manifest.build_system;
let specs = [(
&build_system.build_backend.name,
&build_system.build_backend.spec,
)]
.into_iter()
.chain(build_system.additional_dependencies.iter())
.map(|(name, spec)| {
spec.clone()
.try_into_nameless_match_spec(channel_config)
.map(|spec| MatchSpec::from_nameless(spec, Some(name.clone())))
})
.collect::<Result<_, _>>()
.map_err(FinishError::SpecConversionError)?;
let build_system = &self.package_manifest.build;
let specs = [(&build_system.backend.name, &build_system.backend.spec)]
.into_iter()
.chain(build_system.additional_dependencies.iter())
.map(|(name, spec)| {
spec.clone()
.try_into_nameless_match_spec(channel_config)
.map(|spec| MatchSpec::from_nameless(spec, Some(name.clone())))
})
.collect::<Result<_, _>>()
.map_err(FinishError::SpecConversionError)?;

// Figure out the channels to use
let channels = build_system.channels.clone().unwrap_or_else(|| {
Expand All @@ -184,7 +176,7 @@ impl ProtocolBuilder {

Ok(ToolSpec::Isolated(IsolatedToolSpec {
specs,
command: build_system.build_backend.name.as_source().to_string(),
command: build_system.backend.name.as_source().to_string(),
channels,
}))
}
Expand All @@ -210,20 +202,9 @@ impl ProtocolBuilder {
.await
.map_err(FinishError::Tool)?;

let configuration = self
.package_manifest
.build_system
.build_backend
.additional_args
.map_or(serde_json::Value::Null, |value| {
let deserializer = value.into_deserializer();
serde_json::Value::deserialize(deserializer).unwrap_or(serde_json::Value::Null)
});

Ok(JsonRPCBuildProtocol::setup(
self.source_dir,
self.manifest_path,
configuration,
build_id,
self.cache_dir,
tool,
Expand All @@ -238,21 +219,10 @@ impl ProtocolBuilder {
ipc: InProcessBackend,
build_id: usize,
) -> Result<JsonRPCBuildProtocol, FinishError> {
let configuration = self
.package_manifest
.build_system
.build_backend
.additional_args
.map_or(serde_json::Value::Null, |value| {
let deserializer = value.into_deserializer();
serde_json::Value::deserialize(deserializer).unwrap_or(serde_json::Value::Null)
});

Ok(JsonRPCBuildProtocol::setup_with_transport(
"<IPC>".to_string(),
self.source_dir,
self.manifest_path,
configuration,
build_id,
self.cache_dir,
Sender::from(ipc.rpc_out),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -145,7 +145,6 @@ impl ProtocolBuilder {
Ok(JsonRPCBuildProtocol::setup(
self.source_dir,
self.recipe_dir.join("recipe.yaml"),
serde_json::Value::Null,
build_id,
self.cache_dir,
tool,
Expand Down
4 changes: 0 additions & 4 deletions crates/pixi_build_frontend/src/protocols/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -147,7 +147,6 @@ impl JsonRPCBuildProtocol {
async fn setup(
source_dir: PathBuf,
manifest_path: PathBuf,
configuration: serde_json::Value,
build_id: usize,
cache_dir: Option<PathBuf>,
tool: Tool,
Expand Down Expand Up @@ -180,7 +179,6 @@ impl JsonRPCBuildProtocol {
backend_identifier,
source_dir,
manifest_path,
configuration,
build_id,
cache_dir,
tx,
Expand All @@ -197,7 +195,6 @@ impl JsonRPCBuildProtocol {
source_dir: PathBuf,
// In case of rattler-build it's recipe.yaml
manifest_path: PathBuf,
configuration: serde_json::Value,
build_id: usize,
cache_dir: Option<PathBuf>,
sender: impl TransportSenderT + Send,
Expand Down Expand Up @@ -234,7 +231,6 @@ impl JsonRPCBuildProtocol {
RpcParams::from(InitializeParams {
manifest_path: manifest_path.clone(),
cache_directory: cache_dir,
configuration,
}),
)
.await
Expand Down
74 changes: 3 additions & 71 deletions crates/pixi_build_frontend/tests/diagnostics.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,12 +2,10 @@ use std::path::Path;

use bytes::Bytes;
use futures::{SinkExt, StreamExt};
use jsonrpsee::types::Request;
use miette::{Diagnostic, GraphicalReportHandler, GraphicalTheme};
use pixi_build_frontend::{BuildFrontend, InProcessBackend, SetupRequest};
use pixi_build_types::procedures::initialize::InitializeParams;
use pixi_manifest::toml::{ExternalWorkspaceProperties, FromTomlStr, TomlManifest};
use tokio::io::{AsyncBufReadExt, AsyncRead, AsyncWrite, BufReader};
use tokio::io::{AsyncRead, AsyncWrite};
use tokio_stream::wrappers::ReceiverStream;
use tokio_util::{
io::{CopyToBytes, SinkWriter, StreamReader},
Expand Down Expand Up @@ -136,8 +134,8 @@ async fn test_invalid_backend() {
version = "0.1.0"
name = "project"
[build-system]
build-backend = { name = "ipc", version = "*" }
[package.build]
backend = { name = "ipc", version = "*" }
"#;

let (frontend_tx, backend_rx) = pipe();
Expand Down Expand Up @@ -171,72 +169,6 @@ async fn test_invalid_backend() {
insta::assert_snapshot!(snapshot);
}

#[tokio::test]
#[ignore]
async fn test_backend_configuration() {
let toml = r#"
[workspace]
platforms = []
channels = []
preview = ['pixi-build']
[package]
version = "0.1.0"
name = "project"
[build-system]
build-backend = { name = "ipc", version = "*" }
[build-backend.ipc]
hello = "world"
"#;

let source_dir = tempfile::TempDir::new().unwrap();
let manifest = source_dir
.path()
.join(pixi_consts::consts::PROJECT_MANIFEST);

let (frontend_tx, backend_rx) = pipe();
let (backend_tx, frontend_rx) = pipe();
let ipc = InProcessBackend {
rpc_in: Box::new(frontend_rx),
rpc_out: Box::new(frontend_tx),
};

let protocol_setup = tokio::spawn(async move {
let (workspace, package) = TomlManifest::from_toml_str(toml)
.unwrap()
.into_manifests(ExternalWorkspaceProperties::default())
.unwrap();
pixi_build_frontend::pixi_protocol::ProtocolBuilder::new(
source_dir.path().to_path_buf(),
manifest.to_path_buf(),
workspace,
package.unwrap(),
)
.finish_with_ipc(ipc, 0)
.await
.expect_err("the test never sends a response to the initialize request");
});

let read_initialize_message = async move {
let initialize_line = BufReader::new(backend_rx)
.lines()
.next_line()
.await
.unwrap()
.unwrap();
let request: Request = serde_json::from_str(&initialize_line).unwrap();
let init_params: InitializeParams = request.params().parse().unwrap();
drop(backend_tx); // Simulates the backend closing the connection.
init_params
};

let (_, init_params) = tokio::join!(protocol_setup, read_initialize_message);

insta::assert_snapshot!(serde_json::to_string_pretty(&init_params.configuration).unwrap());
}

/// Creates a pipe that connects an async write instance to an async read
/// instance.
pub fn pipe() -> (
Expand Down
1 change: 0 additions & 1 deletion crates/pixi_build_types/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,5 @@ version = "0.1.0"
[dependencies]
rattler_conda_types = { workspace = true }
serde = { workspace = true, features = ["derive"] }
serde_json = { workspace = true }
serde_with = { workspace = true }
url = { workspace = true }
4 changes: 0 additions & 4 deletions crates/pixi_build_types/src/procedures/initialize.rs
Original file line number Diff line number Diff line change
Expand Up @@ -23,10 +23,6 @@ pub struct InitializeParams {
/// The manifest that the build backend should use.
pub manifest_path: PathBuf,

/// Additional configuration to configure the backend. This configuration is
/// specific to the backend.
pub configuration: serde_json::Value,

/// Optionally the cache directory to use for any caching activity.
pub cache_directory: Option<PathBuf>,
}
Expand Down
20 changes: 8 additions & 12 deletions crates/pixi_manifest/src/build_system.rs
Original file line number Diff line number Diff line change
Expand Up @@ -5,14 +5,14 @@ use pixi_spec::BinarySpec;
use rattler_conda_types::NamedChannelOrUrl;

use crate::toml::FromTomlStr;
use crate::{toml::TomlBuildSystem, TomlError};
use crate::{toml::TomlPackageBuild, TomlError};

/// A build section in the pixi manifest.
/// that defines what backend is used to build the project.
#[derive(Debug, Clone)]
pub struct BuildSystem {
pub struct PackageBuild {
/// Information about the build backend
pub build_backend: BuildBackend,
pub backend: BuildBackend,

/// Additional dependencies that should be installed alongside the backend.
pub additional_dependencies: IndexMap<rattler_conda_types::PackageName, BinarySpec>,
Expand All @@ -29,16 +29,12 @@ pub struct BuildBackend {

/// The spec for the backend
pub spec: BinarySpec,

/// Additional arguments to pass to the build backend. In the manifest these are read from the
/// `[build-backend]` section.
pub additional_args: Option<serde_value::Value>,
}

impl BuildSystem {
impl PackageBuild {
/// Parses the specified string as a toml representation of a build system.
pub fn from_toml_str(source: &str) -> Result<Self, TomlError> {
TomlBuildSystem::from_toml_str(source).and_then(TomlBuildSystem::into_build_system)
TomlPackageBuild::from_toml_str(source).and_then(TomlPackageBuild::into_build_system)
}
}

Expand All @@ -49,10 +45,10 @@ mod tests {
#[test]
fn deserialize_build() {
let toml = r#"
build-backend = { name = "pixi-build-python", version = "12.*" }
backend = { name = "pixi-build-python", version = "12.*" }
"#;

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

0 comments on commit bb6cd82

Please sign in to comment.