Skip to content

Commit

Permalink
fix: Use proto run when available for parent execution. (#313)
Browse files Browse the repository at this point in the history
  • Loading branch information
milesj authored Dec 1, 2023
1 parent 9f07d38 commit f4c083f
Showing 1 changed file with 34 additions and 13 deletions.
47 changes: 34 additions & 13 deletions crates/cli/src/commands/run.rs
Original file line number Diff line number Diff line change
Expand Up @@ -122,26 +122,47 @@ fn create_command<I: IntoIterator<Item = A>, A: AsRef<OsStr>>(
tool: &Tool,
exe_config: &ExecutableConfig,
args: I,
) -> Command {
) -> miette::Result<Command> {
let exe_path = exe_config.exe_path.as_ref().unwrap();
let args = args
.into_iter()
.map(|arg| arg.as_ref().to_os_string())
.collect::<Vec<_>>();

let command = if let Some(parent_exe) = &exe_config.parent_exe_name {
// Force an .exe extension on Windows because Windows shims are very brittle.
#[allow(unused_mut)]
let mut parent_exe_path = parent_exe.to_owned();
let mut exe_args = vec![];

// Avoid using parent shims on Windows because Windows shims are very brittle.
// For example, if we execute "npm serve", this becomes "node ~/npm.js serve",
// which results in "node" becoming "node.cmd" because of PATH resolution,
// and .cmd files do not handle signals/pipes correctly, especially when a child
// process. So forcing the parent to always use .exe seems like a good solution.
let parent_exe_path = if cfg!(windows) && !parent_exe.ends_with(".exe") {
format!("{parent_exe}.exe")
} else {
parent_exe.to_owned()
};
// which results in "node" becoming "node.cmd" because of `PATH` resolution,
// and .cmd files do not handle signals/pipes correctly.
#[cfg(windows)]
if !parent_exe.ends_with(".exe") {
use std::ffi::OsString;

let mut config = proto_core::ToolsConfig::load_upwards()?;
config.inherit_builtin_plugins();

// Attempt to use `proto run <tool>` first instead of a hard-coded .exe.
// This way we rely on proto's executable discovery functionality.
if config.plugins.contains_key(parent_exe)
&& tool.proto.tools_dir.join(parent_exe).exists()
{
parent_exe_path = "proto.exe".to_owned();

exe_args.push(OsString::from("run"));
exe_args.push(OsString::from(parent_exe));
exe_args.push(OsString::from("--"));
}
// Otherwise use a hard-coded .exe and rely on OS path resolution.
else {
parent_exe_path = format!("{parent_exe}.exe");
}
}

let mut exe_args = vec![exe_path.as_os_str().to_os_string()];
exe_args.push(exe_path.as_os_str().to_os_string());
exe_args.extend(args);

debug!(bin = ?parent_exe_path, args = ?exe_args, "Running {}", tool.get_name());
Expand All @@ -154,7 +175,7 @@ fn create_command<I: IntoIterator<Item = A>, A: AsRef<OsStr>>(
};

// Convert std to tokio
Command::from(command)
Ok(Command::from(command))
}

#[system]
Expand Down Expand Up @@ -211,7 +232,7 @@ pub async fn run(args: ArgsRef<RunArgs>) -> SystemResult {
})?;

// Run the command
let status = create_command(&tool, &exe_config, &args.passthrough)
let status = create_command(&tool, &exe_config, &args.passthrough)?
.env(
format!("{}_VERSION", tool.get_env_var_prefix()),
tool.get_resolved_version().to_string(),
Expand Down

0 comments on commit f4c083f

Please sign in to comment.