refactor: remove ipc override from options and give it manually to te…

…st (#2629)

Co-authored-by: Bas Zalmstra <[email protected]>

crates/pixi_build_frontend/src/lib.rs

@@ -5,11 +5,11 @@ mod protocols;
 
 use std::fmt::{Debug, Formatter};
 
-pub(crate) use protocols::builders::{conda_protocol, pixi_protocol, rattler_build_protocol};
+pub use protocols::builders::{conda_protocol, pixi_protocol, rattler_build_protocol};
 
 mod protocol_builder;
 mod reporters;
-mod tool;
+pub mod tool;
 
 use std::path::PathBuf;
 
@@ -32,9 +32,6 @@ pub enum BackendOverride {
 
     /// Overwrite the backend with a executable path.
     System(String),
-
-    /// Override with a specific IPC channel.
-    Io(InProcessBackend),
 }
 
 impl BackendOverride {
@@ -49,18 +46,6 @@ impl BackendOverride {
     }
 }
 
-impl From<InProcessBackend> for BackendOverride {
-    fn from(value: InProcessBackend) -> Self {
-        Self::Io(value)
-    }
-}
-
-impl From<InProcessBackend> for Option<BackendOverride> {
-    fn from(value: InProcessBackend) -> Self {
-        Some(value.into())
-    }
-}
-
 /// A backend communication protocol that can run in the same process.
 pub struct InProcessBackend {
     pub rpc_in: Box<dyn AsyncRead + Send + Sync + Unpin>,

crates/pixi_build_frontend/src/protocols/builders/conda_build/protocol.rs

@@ -44,10 +44,6 @@ impl Protocol {
         &self,
         request: &CondaMetadataParams,
     ) -> miette::Result<CondaMetadataResult> {
-        let Some(tool) = self.tool.as_executable() else {
-            miette::bail!("Cannot use a non-executable tool to render conda metadata");
-        };
-
         // Construct a new tool that can be used to invoke conda-render instead of the
         // original tool.
         let conda_render_executable = String::from("conda-render");
@@ -57,7 +53,7 @@ impl Protocol {
             conda_render_executable
         };
 
-        let conda_render_tool = tool.with_executable(conda_render_executable);
+        let conda_render_tool = self.tool.with_executable(conda_render_executable);
 
         // TODO: Properly pass channels
         // TODO: Setup --exclusive-config-files

crates/pixi_build_frontend/src/protocols/builders/pixi.rs

@@ -7,17 +7,16 @@ use std::{
 use miette::Diagnostic;
 use pixi_consts::consts;
 use pixi_manifest::{Manifest, PackageManifest, PrioritizedChannel, WorkspaceManifest};
-// pub use protocol::Protocol;
 use rattler_conda_types::{ChannelConfig, MatchSpec};
 use thiserror::Error;
 use which::Error;
 
 use crate::{
+    jsonrpc::{Receiver, Sender},
     protocols::{InitializeError, JsonRPCBuildProtocol},
     tool::{IsolatedToolSpec, ToolCacheError, ToolSpec},
-    BackendOverride, ToolContext,
+    BackendOverride, InProcessBackend, ToolContext,
 };
-
 // use super::{InitializeError, JsonRPCBuildProtocol};
 
 /// A protocol that uses a pixi manifest to invoke a build backend .
@@ -74,7 +73,7 @@ impl Display for FinishError {
 
 impl ProtocolBuilder {
     /// Constructs a new instance from a manifest.
-    pub(crate) fn new(
+    pub fn new(
         source_dir: PathBuf,
         manifest_path: PathBuf,
         workspace_manifest: WorkspaceManifest,
@@ -205,6 +204,26 @@ impl ProtocolBuilder {
         )
         .await?)
     }
+
+    /// Finish the construction of the protocol with the given
+    /// [`InProcessBackend`] representing the build backend.
+    pub async fn finish_with_ipc(
+        self,
+        ipc: InProcessBackend,
+        build_id: usize,
+    ) -> Result<JsonRPCBuildProtocol, FinishError> {
+        Ok(JsonRPCBuildProtocol::setup_with_transport(
+            "<IPC>".to_string(),
+            self.source_dir,
+            self.manifest_path,
+            build_id,
+            self.cache_dir,
+            Sender::from(ipc.rpc_out),
+            Receiver::from(ipc.rpc_in),
+            None,
+        )
+        .await?)
+    }
 }
 
 /// Try to find a pixi manifest in the given source directory.

crates/pixi_build_frontend/src/protocols/builders/rattler_build.rs

@@ -39,7 +39,7 @@ pub enum ProtocolBuildError {
     FailedToBuildPixi(#[from] PixiProtocolBuildError),
 }
 
-/// A builder for constructing a [`protocol::Protocol`] instance.
+/// A builder for constructing a [`crate::protocol::Protocol`] instance.
 #[derive(Debug)]
 pub struct ProtocolBuilder {
     /// The directory that contains the source files.

crates/pixi_build_frontend/src/protocols/mod.rs

@@ -32,7 +32,7 @@ use tokio::{
 };
 
 use crate::{
-    jsonrpc::{stdio_transport, Receiver, RpcParams, Sender},
+    jsonrpc::{stdio_transport, RpcParams},
     tool::Tool,
     CondaBuildReporter, CondaMetadataReporter,
 };
@@ -149,63 +149,46 @@ impl JsonRPCBuildProtocol {
         cache_dir: Option<PathBuf>,
         tool: Tool,
     ) -> Result<Self, InitializeError> {
-        match tool.try_into_executable() {
-            Ok(tool) => {
-                // Spawn the tool and capture stdin/stdout.
-                let mut process = tokio::process::Command::from(tool.command())
-                    .stdout(std::process::Stdio::piped())
-                    .stdin(std::process::Stdio::piped())
-                    .stderr(std::process::Stdio::piped())
-                    .spawn()?;
-
-                let backend_identifier = tool.executable().clone();
-
-                // Acquire the stdin/stdout handles.
-                let stdin = process
-                    .stdin
-                    .take()
-                    .expect("since we piped stdin we expect a valid value here");
-                let stdout = process
-                    .stdout
-                    .expect("since we piped stdout we expect a valid value here");
-                let stderr = process
-                    .stderr
-                    .map(|stderr| BufReader::new(stderr).lines())
-                    .expect("since we piped stderr we expect a valid value here");
-
-                // Construct a JSON-RPC client to communicate with the backend process.
-                let (tx, rx) = stdio_transport(stdin, stdout);
-                Self::setup_with_transport(
-                    backend_identifier,
-                    source_dir,
-                    manifest_path,
-                    build_id,
-                    cache_dir,
-                    tx,
-                    rx,
-                    Some(stderr),
-                )
-                .await
-            }
-            Err(ipc) => {
-                Self::setup_with_transport(
-                    "<IPC>".to_string(),
-                    source_dir,
-                    manifest_path,
-                    build_id,
-                    cache_dir,
-                    Sender::from(ipc.rpc_out),
-                    Receiver::from(ipc.rpc_in),
-                    None,
-                )
-                .await
-            }
-        }
+        // Spawn the tool and capture stdin/stdout.
+        let mut process = tokio::process::Command::from(tool.command())
+            .stdout(std::process::Stdio::piped())
+            .stdin(std::process::Stdio::piped())
+            .stderr(std::process::Stdio::piped())
+            .spawn()?;
+
+        let backend_identifier = tool.executable().clone();
+
+        // Acquire the stdin/stdout handles.
+        let stdin = process
+            .stdin
+            .take()
+            .expect("since we piped stdin we expect a valid value here");
+        let stdout = process
+            .stdout
+            .expect("since we piped stdout we expect a valid value here");
+        let stderr = process
+            .stderr
+            .map(|stderr| BufReader::new(stderr).lines())
+            .expect("since we piped stderr we expect a valid value here");
+
+        // Construct a JSON-RPC client to communicate with the backend process.
+        let (tx, rx) = stdio_transport(stdin, stdout);
+        Self::setup_with_transport(
+            backend_identifier,
+            source_dir,
+            manifest_path,
+            build_id,
+            cache_dir,
+            tx,
+            rx,
+            Some(stderr),
+        )
+        .await
     }
 
     /// Setup a new protocol instance with a given transport.
     #[allow(clippy::too_many_arguments)]
-    async fn setup_with_transport(
+    pub(crate) async fn setup_with_transport(
         backend_identifier: String,
         source_dir: PathBuf,
         // In case of rattler-build it's recipe.yaml

crates/pixi_build_frontend/src/tool/cache.rs

@@ -208,9 +208,7 @@ mod tests {
             .await
             .unwrap();
 
-        let exec = tool.as_executable().unwrap();
-
-        exec.command().arg("--version").spawn().unwrap();
+        tool.command().arg("--version").spawn().unwrap();
     }
 
     #[tokio::test(flavor = "multi_thread", worker_threads = 2)]

crates/pixi_build_frontend/src/tool/installer.rs

@@ -157,7 +157,6 @@ impl ToolContext {
         channel_config: &ChannelConfig,
     ) -> Result<Tool, ToolCacheError> {
         let spec = match spec {
-            ToolSpec::Io(ipc) => return Ok(Tool::Io(ipc)),
             ToolSpec::Isolated(isolated) => {
                 if isolated.specs.is_empty() {
                     return Err(ToolCacheError::Install(miette!(

crates/pixi_build_frontend/src/tool/mod.rs

@@ -7,32 +7,13 @@ use std::{collections::HashMap, path::PathBuf};
 pub use cache::ToolCacheError;
 pub use spec::{IsolatedToolSpec, SystemToolSpec, ToolSpec};
 
-use crate::InProcessBackend;
-
 pub use installer::ToolContext;
 
 /// A tool that can be invoked.
 #[derive(Debug)]
 pub enum Tool {
     Isolated(IsolatedTool),
     System(SystemTool),
-    Io(InProcessBackend),
-}
-
-impl Tool {
-    pub fn as_isolated(&self) -> Option<&IsolatedTool> {
-        match self {
-            Tool::Isolated(tool) => Some(tool),
-            Tool::System(_) => None,
-            Tool::Io(_) => None,
-        }
-    }
-}
-
-#[derive(Debug)]
-pub enum ExecutableTool {
-    Isolated(IsolatedTool),
-    System(SystemTool),
 }
 
 /// A tool that is pre-installed on the system.
@@ -89,54 +70,43 @@ impl IsolatedTool {
 }
 
 impl Tool {
-    pub fn as_executable(&self) -> Option<ExecutableTool> {
-        match self {
-            Tool::Isolated(tool) => Some(ExecutableTool::Isolated(tool.clone())),
-            Tool::System(tool) => Some(ExecutableTool::System(tool.clone())),
-            Tool::Io(_) => None,
-        }
-    }
-
-    pub fn try_into_executable(self) -> Result<ExecutableTool, InProcessBackend> {
+    pub fn as_isolated(&self) -> Option<&IsolatedTool> {
         match self {
-            Tool::Isolated(tool) => Ok(ExecutableTool::Isolated(tool)),
-            Tool::System(tool) => Ok(ExecutableTool::System(tool)),
-            Tool::Io(ipc) => Err(ipc),
+            Tool::Isolated(tool) => Some(tool),
+            Tool::System(_) => None,
         }
     }
-}
 
-impl ExecutableTool {
     /// Returns the full path to the executable to invoke.
     pub fn executable(&self) -> &String {
         match self {
-            ExecutableTool::Isolated(tool) => &tool.command,
-            ExecutableTool::System(tool) => &tool.command,
+            Tool::Isolated(tool) => &tool.command,
+            Tool::System(tool) => &tool.command,
         }
     }
 
     /// Construct a new tool that calls another executable.
     pub fn with_executable(&self, executable: impl Into<String>) -> Self {
         match self {
-            ExecutableTool::Isolated(tool) => ExecutableTool::Isolated(IsolatedTool::new(
+            Tool::Isolated(tool) => Tool::Isolated(IsolatedTool::new(
                 executable,
                 tool.prefix.clone(),
                 tool.activation_scripts.clone(),
             )),
-            ExecutableTool::System(_) => ExecutableTool::System(SystemTool::new(executable)),
+            Tool::System(_) => Tool::System(SystemTool::new(executable)),
         }
     }
 
     /// Construct a new command that enables invocation of the tool.
     pub fn command(&self) -> std::process::Command {
         match self {
-            ExecutableTool::Isolated(tool) => {
+            Tool::Isolated(tool) => {
                 let mut cmd = std::process::Command::new(&tool.command);
                 cmd.envs(tool.activation_scripts.clone());
 
                 cmd
             }
-            ExecutableTool::System(tool) => std::process::Command::new(&tool.command),
+            Tool::System(tool) => std::process::Command::new(&tool.command),
         }
     }
 }

crates/pixi_build_frontend/src/tool/spec.rs

@@ -1,14 +1,13 @@
 use rattler_conda_types::{MatchSpec, NamedChannelOrUrl};
 
-use crate::{BackendOverride, InProcessBackend};
+use crate::BackendOverride;
 
 /// Describes the specification of the tool. This can be used to cache tool
 /// information.
 #[derive(Debug)]
 pub enum ToolSpec {
     Isolated(IsolatedToolSpec),
     System(SystemToolSpec),
-    Io(InProcessBackend),
 }
 
 /// A build tool that can be installed through a conda package.
@@ -72,7 +71,6 @@ impl BackendOverride {
                 IsolatedToolSpec::from_specs(vec![spec], channels.into_iter().flatten()),
             ),
             BackendOverride::System(command) => ToolSpec::System(SystemToolSpec { command }),
-            BackendOverride::Io(process) => ToolSpec::Io(process),
         }
     }
 }