From 4e7e0ed9bc1a9908b2523b39be595a149befcc3c Mon Sep 17 00:00:00 2001 From: Ana Hobden Date: Tue, 3 Dec 2024 10:51:23 -0800 Subject: [PATCH] criticalup run without proxies --- crates/criticalup-cli/src/commands/run.rs | 60 +++++-------------- crates/criticalup-cli/src/commands/which.rs | 31 +++++++--- ...simple_run_command_manifest_not_found.snap | 3 +- 3 files changed, 40 insertions(+), 54 deletions(-) diff --git a/crates/criticalup-cli/src/commands/run.rs b/crates/criticalup-cli/src/commands/run.rs index 4aea8ea1..30773e02 100644 --- a/crates/criticalup-cli/src/commands/run.rs +++ b/crates/criticalup-cli/src/commands/run.rs @@ -1,11 +1,10 @@ // SPDX-FileCopyrightText: The Ferrocene Developers // SPDX-License-Identifier: MIT OR Apache-2.0 +use crate::commands::which::locate_binary; use crate::errors::Error; -use crate::errors::Error::BinaryNotInstalled; use crate::spawn::spawn_command; use crate::Context; -use criticalup_core::project_manifest::ProjectManifest; use std::path::PathBuf; // We *deliberately* use a sync Command here, since we are spawning a process to replace the current one. use std::process::{Command, Stdio}; @@ -15,48 +14,21 @@ pub(crate) async fn run( command: Vec, project: Option, ) -> Result<(), Error> { - // We try to fetch the manifest early on because it makes failing fast easy. Given that we need - // this variable to set the env var later for child process, it is important to try to get the - // canonical path first. - let manifest_path = ProjectManifest::discover_canonical_path(project.as_deref()).await?; - - // This dir has all the binaries that are proxied. - let proxies_dir = &ctx.config.paths.proxies_dir; - - if let Some(binary_command) = command.first() { - let mut binary_executable = PathBuf::new(); - binary_executable.set_file_name(binary_command); - // On Windows, the user can pass (for example) `cargo` or `cargo.exe` - #[cfg(windows)] - binary_executable.set_extension("exe"); - - let binary_path = proxies_dir.join(binary_executable); - - if binary_path.exists() { - let args = command.get(1..).unwrap_or(&[]); - let mut cmd = Command::new(binary_path); - cmd.args(args) - .stdout(Stdio::inherit()) - .stdout(Stdio::inherit()) - .stderr(Stdio::inherit()); - - // Set the manifest path env CRITICALUP_CURRENT_PROJ_MANIFEST_CANONICAL_PATH var which is used - // by the function `crates::criticalup-cli::binary_proxies::proxy` to find the correct project - // manifest. - // - // Important: This env var is strictly for internal use! - if manifest_path.exists() { - cmd.env( - "CRITICALUP_CURRENT_PROJ_MANIFEST_CANONICAL_PATH", - manifest_path.as_os_str(), - ); - } - - spawn_command(cmd)?; - } else { - return Err(BinaryNotInstalled(binary_command.into())); - } - } + let binary = if let Some(binary) = command.first() { + binary.clone() + } else { + return Err(Error::BinaryNotInstalled(String::new())); + }; + let found_binary = locate_binary(ctx, binary, project).await?; + + let args = command.get(1..).unwrap_or(&[]); + let mut cmd = Command::new(found_binary); + cmd.args(args) + .stdout(Stdio::inherit()) + .stdout(Stdio::inherit()) + .stderr(Stdio::inherit()); + + spawn_command(cmd)?; Ok(()) } diff --git a/crates/criticalup-cli/src/commands/which.rs b/crates/criticalup-cli/src/commands/which.rs index f39fbb43..6ca31e2e 100644 --- a/crates/criticalup-cli/src/commands/which.rs +++ b/crates/criticalup-cli/src/commands/which.rs @@ -9,25 +9,38 @@ use std::path::PathBuf; pub(crate) async fn run( ctx: &Context, - tool: String, + binary: String, project: Option, ) -> Result<(), Error> { + let found_binary = locate_binary(ctx, binary, project).await?; + println!("{}\n", found_binary.display()); + + Ok(()) +} + +#[tracing::instrument(level = "debug", skip_all, fields(binary, project))] +pub(crate) async fn locate_binary( + ctx: &Context, + binary: String, + project: Option, +) -> Result { let manifest = ProjectManifest::get(project).await?; let installation_dir = &ctx.config.paths.installation_dir; + let mut found_binary = None; for product in manifest.products() { let abs_installation_dir_path = installation_dir.join(product.installation_id()); let bin_path = PathBuf::from("bin"); let mut tool_executable = PathBuf::new(); - tool_executable.set_file_name(&tool); + tool_executable.set_file_name(&binary); let tools_bin_path = abs_installation_dir_path.join(bin_path.join(&tool_executable)); if tools_bin_path.exists() { - println!("{}\n", tools_bin_path.display()); + found_binary = Some(tools_bin_path); } else { // On Windows, the user can pass (for example) `cargo` or `cargo.exe` #[cfg(windows)] @@ -35,15 +48,15 @@ pub(crate) async fn run( let mut tools_bin_path_with_exe = tools_bin_path.clone(); tools_bin_path_with_exe.set_extension("exe"); if tools_bin_path_with_exe.exists() { - println!("{}\n", tools_bin_path_with_exe.display()); - } else { - return Err(BinaryNotInstalled(tool)); + found_binary = Some(tools_bin_path_with_exe); } } - #[cfg(not(windows))] - return Err(BinaryNotInstalled(tool)); } } - Ok(()) + if let Some(found_binary) = found_binary { + Ok(found_binary) + } else { + Err(BinaryNotInstalled(binary)) + } } diff --git a/crates/criticalup-cli/tests/snapshots/cli__run__simple_run_command_manifest_not_found.snap b/crates/criticalup-cli/tests/snapshots/cli__run__simple_run_command_manifest_not_found.snap index bf58befa..d02d6f6f 100644 --- a/crates/criticalup-cli/tests/snapshots/cli__run__simple_run_command_manifest_not_found.snap +++ b/crates/criticalup-cli/tests/snapshots/cli__run__simple_run_command_manifest_not_found.snap @@ -8,6 +8,7 @@ empty stdout stderr ------ -error: Failed to find canonical path for /path/to/criticalup.toml. +error: Failed to load the project manifest at /path/to/criticalup.toml. + caused by: Failed to read the file. caused by: No such file or directory (os error 2) ------