Skip to content

Commit

Permalink
refactor(cli): removed shell dependency
Browse files Browse the repository at this point in the history
This should make the CLI significantly more robust to exotic
configurations. It may cause issues on Windows? We'll see.
  • Loading branch information
arctic-hen7 committed Dec 17, 2023
1 parent 276a0c1 commit 0b98318
Show file tree
Hide file tree
Showing 5 changed files with 21 additions and 56 deletions.
1 change: 1 addition & 0 deletions packages/perseus-cli/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@ path = "tests/lib.rs"
# See more keys and their definitions at https://doc.rust-lang.org/cargo/reference/manifest.html

[dependencies]
shell-words = "1"
include_dir = "0.7"
thiserror = "1"
fmterr = "0.1"
Expand Down
19 changes: 7 additions & 12 deletions packages/perseus-cli/src/cmd.rs
Original file line number Diff line number Diff line change
Expand Up @@ -23,20 +23,15 @@ pub fn run_cmd(
pre_dump: impl Fn(),
full_logging: bool,
) -> Result<(String, String, i32), ExecutionError> {
// We run the command in a shell so that NPM/Yarn binaries can be recognized
// (see #5)
#[cfg(unix)]
let shell_exec = "sh";
#[cfg(windows)]
let shell_exec = "powershell";
#[cfg(unix)]
let shell_param = "-c";
#[cfg(windows)]
let shell_param = "-command";
let cmd_parts = shell_words::split(&cmd).map_err(|err| ExecutionError::CmdParseFailed {
cmd: cmd.clone(),
source: err,
})?;
let cmd_exec = &cmd_parts[0];

// This will NOT pipe output/errors to the console
let output = Command::new(shell_exec)
.args([shell_param, &cmd])
let output = Command::new(cmd_exec)
.args(&cmd_parts[1..])
.envs(envs)
.current_dir(dir)
// A pipe is set for stdio
Expand Down
7 changes: 7 additions & 0 deletions packages/perseus-cli/src/errors.rs
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,13 @@ pub enum Error {
/// `build`/`serve` (export errors are separate).
#[derive(Error, Debug)]
pub enum ExecutionError {
#[error("failed to parse command for execution")]
CmdParseFailed {
cmd: String,
// Right now, this can only be an unmatched quote error
#[source]
source: shell_words::ParseError,
},
#[error("couldn't execute command '{cmd}' (this doesn't mean it threw an error, it means it couldn't be run at all)")]
CmdExecFailed {
cmd: String,
Expand Down
15 changes: 2 additions & 13 deletions packages/perseus-cli/src/install.rs
Original file line number Diff line number Diff line change
Expand Up @@ -530,19 +530,8 @@ impl Tool {
/// error, and hence it should only be called after all other options
/// have been exhausted.
fn get_path_to_preinstalled(&self) -> Result<String, InstallError> {
#[cfg(unix)]
let shell_exec = "sh";
#[cfg(windows)]
let shell_exec = "powershell";
#[cfg(unix)]
let shell_param = "-c";
#[cfg(windows)]
let shell_param = "-command";

let check_cmd = format!("{} --version", self.name); // Not exactly bulletproof, but it works!
let res = Command::new(shell_exec)
.args([shell_param, &check_cmd])
.output();
// Not exactly bulletproof, but it works!
let res = Command::new(&self.name).arg("--version").output();
if let Err(err) = res {
// Unlike `wasm-pack`, we don't try to install with `cargo install`, because
// that's a little pointless to me (the user will still have to get
Expand Down
35 changes: 4 additions & 31 deletions packages/perseus-cli/src/prepare.rs
Original file line number Diff line number Diff line change
Expand Up @@ -13,35 +13,9 @@ use std::process::Command;
/// Checks if the user has `cargo` installed, and tries to install the
/// `wasm32-unknown-unknown` target with `rustup` if it's available.
pub fn check_env(global_opts: &Opts) -> Result<(), Error> {
#[cfg(unix)]
let shell_exec = "sh";
#[cfg(windows)]
let shell_exec = "powershell";
#[cfg(unix)]
let shell_param = "-c";
#[cfg(windows)]
let shell_param = "-command";

// Check for the shell before anything else (#314)
let shell_res = Command::new(shell_exec)
.arg("--version")
.output()
.map_err(|err| Error::ShellNotPresent { source: err })?;
let exit_code = match shell_res.status.code() {
Some(exit_code) => exit_code,
None if shell_res.status.success() => 0,
None => 1,
};
if exit_code != 0 {
return Err(Error::ShellNotPresent {
source: std::io::Error::new(std::io::ErrorKind::NotFound, "non-zero exit code"),
});
}

// Check for `cargo`
let cargo_cmd = global_opts.cargo_engine_path.to_string() + " --version";
let cargo_res = Command::new(shell_exec)
.args([shell_param, &cargo_cmd])
let cargo_res = Command::new(global_opts.cargo_engine_path.to_string())
.arg("--version")
.output()
.map_err(|err| Error::CargoNotPresent { source: err })?;
let exit_code = match cargo_res.status.code() {
Expand All @@ -56,9 +30,8 @@ pub fn check_env(global_opts: &Opts) -> Result<(), Error> {
}
// If the user has `rustup`, make sure they have `wasm32-unknown-unknown`
// installed If they don'aren't using `rustup`, we won't worry about this
let rustup_cmd = global_opts.rustup_path.to_string() + " target list";
let rustup_res = Command::new(shell_exec)
.args([shell_param, &rustup_cmd])
let rustup_res = Command::new(global_opts.rustup_path.to_string())
.args(["target", "list"])
.output();
if let Ok(rustup_res) = rustup_res {
let exit_code = match rustup_res.status.code() {
Expand Down

0 comments on commit 0b98318

Please sign in to comment.