diff --git a/cli/src/cli/complete_word.rs b/cli/src/cli/complete_word.rs index f1c1cbf9..b7065405 100644 --- a/cli/src/cli/complete_word.rs +++ b/cli/src/cli/complete_word.rs @@ -2,17 +2,16 @@ use std::collections::BTreeMap; use std::env; use std::fmt::Debug; use std::path::{Path, PathBuf}; -use std::process::Command; use std::sync::Arc; use clap::Args; use itertools::Itertools; use miette::IntoDiagnostic; use std::sync::LazyLock; -use xx::process::check_status; -use xx::{regex, XXError, XXResult}; +use xx::regex; use usage::parse::{ParseOutput, ParseValue}; +use usage::sh::sh; use usage::{Spec, SpecArg, SpecCommand, SpecComplete, SpecFlag}; use crate::cli::generate; @@ -410,19 +409,3 @@ fn zsh_shell_quote(s: &str) -> String { let escaped = s.replace('\'', "'\\''"); format!("'{escaped}'") } - -fn sh(script: &str) -> XXResult { - let output = Command::new("sh") - .arg("-c") - .arg(script) - .stdin(std::process::Stdio::null()) - .stderr(std::process::Stdio::inherit()) - .env("__USAGE", env!("CARGO_PKG_VERSION")) - .output() - .map_err(|err| XXError::ProcessError(err, format!("sh -c {script}")))?; - - check_status(output.status) - .map_err(|err| XXError::ProcessError(err, format!("sh -c {script}")))?; - let stdout = String::from_utf8(output.stdout).expect("stdout is not utf-8"); - Ok(stdout) -} diff --git a/cli/tests/complete_word.rs b/cli/tests/complete_word.rs index 172cdb70..7efd5fd0 100644 --- a/cli/tests/complete_word.rs +++ b/cli/tests/complete_word.rs @@ -7,6 +7,34 @@ use assert_cmd::prelude::*; use predicates::prelude::*; use predicates::str::contains; +/// Returns `true` if the test should be skipped because there is no usable POSIX shell. +/// Panics under `CI` (any non-empty value) so a missing shell there is a configuration bug +/// rather than a silent pass. +/// +/// The mount fixtures are `#!/usr/bin/env -S usage bash` scripts, so `mount run=` can only +/// work where `sh` can start them. Without this guard, Windows machines with no POSIX shell +/// fall through to the `cmd /c` path, which hands a `.sh` to whatever program is registered +/// for the extension — an editor window per test rather than a failure. +/// +/// The probe runs a script and checks that it exited cleanly, rather than only that something +/// spawned. On Windows `sh` can resolve to a program that starts and then fails, which would +/// otherwise read as a working shell and put the tests back in the confusing state above. +fn skip_if_posix_shell_missing() -> bool { + let usable = Command::new("sh") + .arg("-c") + .arg("exit 0") + .output() + .is_ok_and(|out| out.status.success()); + if usable { + return false; + } + if env::var("CI").is_ok_and(|v| !v.is_empty()) { + panic!("no usable POSIX shell (`sh`) but CI is set — refusing to skip"); + } + eprintln!("Skipping test - no usable POSIX shell (`sh`)"); + true +} + #[test] fn complete_word_completer() { assert_cmd("basic.usage.kdl", &["plugins", "install", "pl"]) @@ -132,6 +160,9 @@ fn complete_word_arg_completer() { #[test] fn complete_word_mounted() { + if skip_if_posix_shell_missing() { + return; + } let mut path = env::split_paths(&env::var("PATH").unwrap()).collect::>(); path.insert( 0, @@ -150,6 +181,9 @@ fn complete_word_mounted() { #[test] fn complete_word_mounted_with_global_flags() { + if skip_if_posix_shell_missing() { + return; + } let mut path = env::split_paths(&env::var("PATH").unwrap()).collect::>(); path.insert( 0, @@ -191,6 +225,9 @@ fn complete_word_mounted_global_flag_choices() { // Regression for the parser-side root cause referenced by jdx/mise#10069: // a value-taking global flag placed before a mounted subcommand must not leak its // consumed tokens into the mounted task's `choices` positional arg. + if skip_if_posix_shell_missing() { + return; + } let mut path = env::split_paths(&env::var("PATH").unwrap()).collect::>(); path.insert( 0, @@ -242,6 +279,9 @@ fn complete_word_mounted_orphan_short_flag_choices() { // Completing a mounted task with the short in front must return the task's choices rather // than bailing with "unexpected word" / "Invalid choice" (which mise worked around by // promoting the short back to global). + if skip_if_posix_shell_missing() { + return; + } let mut path = env::split_paths(&env::var("PATH").unwrap()).collect::>(); path.insert( 0, @@ -287,6 +327,9 @@ fn complete_word_mounted_orphan_short_flag_choices() { fn complete_word_mounted_does_not_offer_mounting_cli_flags() { // Regression for jdx/mise#11282: the mounting CLI's global flags must not be offered // inside a mounted command, and must not shadow the mounted command's own flags. + if skip_if_posix_shell_missing() { + return; + } let mut path = env::split_paths(&env::var("PATH").unwrap()).collect::>(); path.insert( 0, @@ -376,6 +419,9 @@ fn complete_word_mounted_does_not_offer_mounting_cli_flags() { #[test] fn complete_word_boolean_flags_dont_consume_subcommands() { + if skip_if_posix_shell_missing() { + return; + } let mut path = env::split_paths(&env::var("PATH").unwrap()).collect::>(); path.insert( 0, @@ -410,6 +456,9 @@ fn complete_word_boolean_flags_dont_consume_subcommands() { #[test] fn complete_word_non_global_flags_do_not_stop_search() { + if skip_if_posix_shell_missing() { + return; + } let mut path = env::split_paths(&env::var("PATH").unwrap()).collect::>(); path.insert( 0, @@ -439,6 +488,9 @@ fn complete_word_non_global_flags_do_not_stop_search() { #[test] fn complete_word_mixed_global_flags() { + if skip_if_posix_shell_missing() { + return; + } let mut path = env::split_paths(&env::var("PATH").unwrap()).collect::>(); path.insert( 0, diff --git a/docs/spec/reference/cmd.md b/docs/spec/reference/cmd.md index f113bd5b..12c1dfc1 100644 --- a/docs/spec/reference/cmd.md +++ b/docs/spec/reference/cmd.md @@ -78,6 +78,11 @@ Now when using completion with usage, if the user types `mycli run `, call `mycli mount-usage-tasks` and merge the emitted usage into the `run` command and display the task commands as if they were statically defined in the usage spec. +`mount run` is executed the same way as [`complete`'s `run`](./complete.md#which-shell-runs-run): +`sh -c`, falling back to `cmd /c` on Windows when there is no `sh` on `PATH`. A mount pointing at +a shebang script therefore needs a POSIX shell to be available; one that invokes a program +directly, like `mycli mount-usage-tasks` above, works either way. + ### Global flags and mounted commands A mounted command describes a different program, so the flags of the commands it is mounted under diff --git a/docs/spec/reference/complete.md b/docs/spec/reference/complete.md index 9efc0a6a..6794b262 100644 --- a/docs/spec/reference/complete.md +++ b/docs/spec/reference/complete.md @@ -51,3 +51,16 @@ complete "four" run="echo {{ words | slice(start=-4) | join(sep='\"\n\"') }}" ``` Here we just use simple commands like `ls` and `echo` but these words could be passed to any command. + +## Which shell runs `run` + +`run` is executed with `sh -c`, so it is a POSIX shell command line: pipelines, `;` sequences +and shell builtins all work. + +On Windows a POSIX shell is not guaranteed. usage still runs `sh -c` when `sh` is on `PATH` +(Git for Windows provides it), and falls back to `cmd /c` when it is not. `cmd` cannot run any +of the above — it only handles a plain command invocation — so a spec that targets Windows +should either keep `run` to a single command or state that it needs a POSIX shell. + +The script is run with stdin closed and stderr inherited, and with `__USAGE` set to the usage +version so a script can tell it was invoked by usage. diff --git a/lib/src/lib.rs b/lib/src/lib.rs index f8bbf275..95818b4d 100644 --- a/lib/src/lib.rs +++ b/lib/src/lib.rs @@ -26,7 +26,7 @@ pub use error::Result; pub mod docs; pub mod parse; pub mod sdk; -pub(crate) mod sh; +pub mod sh; pub(crate) mod string; #[cfg(test)] mod test; diff --git a/lib/src/sh.rs b/lib/src/sh.rs index 3640ab04..38bb4cfe 100644 --- a/lib/src/sh.rs +++ b/lib/src/sh.rs @@ -1,25 +1,214 @@ +//! Running the shell scripts a spec embeds in `run=`. + +use std::io; use std::process::Command; +use std::string::FromUtf8Error; use xx::process::check_status; -use xx::{XXError, XXResult}; +use xx::XXError; -pub(crate) fn sh(script: &str) -> XXResult { - #[cfg(unix)] - let (shell, flag) = ("sh", "-c"); +use crate::error::Result; + +/// The interpreter a `run=` script is handed to. +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +enum ShellKind { + /// `sh -c`, what `run=` is written for everywhere in the spec format. + Posix, + /// `cmd /c`. Windows only, and only when there is no POSIX shell to be had: + /// it cannot run a shebang script, a pipeline, or `a; b`. + Cmd, +} + +fn shell_argv(kind: ShellKind) -> (&'static str, &'static str) { + match kind { + ShellKind::Posix => ("sh", "-c"), + ShellKind::Cmd => ("cmd", "/c"), + } +} + +/// Which interpreter to try next after `kind` failed to start, if any. +/// +/// Only a missing executable falls back. Anything else — a `sh` that exists but cannot be +/// executed, say — is reported as-is: quietly demoting a broken shell to a different one is +/// the same class of silent wrong behavior this fallback exists to get rid of. +fn fallback_for(kind: ShellKind, err: io::ErrorKind) -> Option { + match (kind, err) { + (ShellKind::Posix, io::ErrorKind::NotFound) if cfg!(windows) => Some(ShellKind::Cmd), + _ => None, + } +} + +/// The first line of a script, for putting in an error message. +/// +/// A `run=` can be a whole multi-line `case … esac`, and these messages surface in a shell +/// completion, where a wall of text buries the prompt. +fn script_excerpt(script: &str) -> String { + let first_line = script.lines().next().unwrap_or_default(); + match script.lines().nth(1) { + Some(_) => format!("{first_line} …"), + None => first_line.to_string(), + } +} + +fn no_shell_message(script: &str) -> String { + format!( + "failed to run `run=` script: neither `sh` nor `cmd` could be started\n \ + script: {}\n \ + `run=` is executed with `sh -c`, falling back to `cmd /c` on Windows. \ + Install a POSIX shell (Git for Windows ships sh.exe) and make sure it is on PATH.", + script_excerpt(script) + ) +} + +fn non_utf8_message(shell: &str, flag: &str, script: &str, err: &FromUtf8Error) -> String { + format!( + "`run=` script produced output that is not valid UTF-8: {err}\n \ + script: {}\n \ + shell: {shell} {flag}", + script_excerpt(script) + ) +} - #[cfg(windows)] - let (shell, flag) = ("cmd", "/c"); +/// Run a `run=` script and return its stdout. +/// +/// Executed with `sh -c`, which is the language the spec format's `run=` is written in — the +/// reference examples use pipelines, `;` sequences and shebang scripts. On Windows, where a +/// POSIX shell is not guaranteed, a missing `sh` falls back to `cmd /c`; that runs a plain +/// command invocation but none of the above, so a spec meant to work there should keep `run=` +/// to a single command. +/// +/// stdin is closed and stderr is inherited, so a script cannot stall a completion waiting for +/// input but can still say why it failed. `__USAGE` is set to the usage version, letting a +/// script tell that it was invoked by usage. +/// +/// Output that is not valid UTF-8 is an error, not a panic and not a lossy conversion. The +/// `cmd /c` fallback in particular emits the console code page, which is not UTF-8 outside +/// English locales, and a mount's output is parsed as a spec — replacement characters there +/// would resurface as a baffling KDL syntax error instead of an encoding one. +pub fn sh(script: &str) -> Result { + let mut kind = ShellKind::Posix; + let output = loop { + let (shell, flag) = shell_argv(kind); + let err = match run(shell, flag, script) { + Ok(output) => break output, + Err(err) => err, + }; + match fallback_for(kind, err.kind()) { + Some(next) => kind = next, + None if err.kind() == io::ErrorKind::NotFound && cfg!(windows) => { + return Err(XXError::Error(no_shell_message(script)).into()); + } + None => { + return Err(XXError::ProcessError(err, format!("{shell} {flag} {script}")).into()); + } + } + }; + + let (shell, flag) = shell_argv(kind); + check_status(output.status) + .map_err(|err| XXError::ProcessError(err, format!("{shell} {flag} {script}")))?; + String::from_utf8(output.stdout) + .map_err(|err| XXError::Error(non_utf8_message(shell, flag, script, &err)).into()) +} - let output = Command::new(shell) +fn run(shell: &str, flag: &str, script: &str) -> io::Result { + Command::new(shell) .arg(flag) .arg(script) .stdin(std::process::Stdio::null()) .stderr(std::process::Stdio::inherit()) .env("__USAGE", env!("CARGO_PKG_VERSION")) .output() - .map_err(|err| XXError::ProcessError(err, format!("{shell} {flag} {script}")))?; +} - check_status(output.status) - .map_err(|err| XXError::ProcessError(err, format!("{shell} {flag} {script}")))?; - let stdout = String::from_utf8(output.stdout).expect("stdout is not utf-8"); - Ok(stdout) +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn shell_argv_maps_each_kind() { + assert_eq!(shell_argv(ShellKind::Posix), ("sh", "-c")); + assert_eq!(shell_argv(ShellKind::Cmd), ("cmd", "/c")); + } + + #[test] + fn a_missing_posix_shell_falls_back_only_on_windows() { + let fallback = fallback_for(ShellKind::Posix, io::ErrorKind::NotFound); + if cfg!(windows) { + assert_eq!(fallback, Some(ShellKind::Cmd)); + } else { + assert_eq!(fallback, None); + } + } + + #[test] + fn a_shell_that_exists_but_fails_is_not_demoted() { + // Falling back here would hide a broken `sh` behind a shell that silently + // mis-executes the script, which is the failure mode this is meant to remove. + assert_eq!( + fallback_for(ShellKind::Posix, io::ErrorKind::PermissionDenied), + None + ); + } + + #[test] + fn cmd_is_the_last_resort() { + assert_eq!(fallback_for(ShellKind::Cmd, io::ErrorKind::NotFound), None); + } + + #[test] + fn no_shell_message_names_both_shells_and_the_script() { + let msg = no_shell_message("echo hello"); + assert!(msg.contains("`sh`"), "{msg}"); + assert!(msg.contains("`cmd`"), "{msg}"); + assert!(msg.contains("echo hello"), "{msg}"); + } + + #[test] + fn no_shell_message_truncates_a_multi_line_script() { + let msg = no_shell_message("case $cur in\n a) echo a ;;\nesac"); + assert!(msg.contains("case $cur in …"), "{msg}"); + assert!(!msg.contains("esac"), "{msg}"); + } + + #[test] + fn non_utf8_message_names_the_script_and_the_shell() { + let err = String::from_utf8(vec![0xff]).unwrap_err(); + let msg = non_utf8_message("cmd", "/c", "chcp 932 && dir", &err); + assert!(msg.contains("chcp 932 && dir"), "{msg}"); + assert!(msg.contains("cmd /c"), "{msg}"); + assert!(msg.contains("not valid UTF-8"), "{msg}"); + } + + #[cfg(unix)] + #[test] + fn sh_reports_non_utf8_output_instead_of_panicking() { + // A `run=` that emits raw bytes used to take the whole process down. `cmd /c` on a + // non-English Windows reaches this through its console code page. + let err = sh(r"printf '\377'").unwrap_err(); + assert!( + err.to_string().contains("not valid UTF-8"), + "{}", + err.to_string() + ); + } + + #[test] + fn sh_returns_stdout() { + // `echo` behaves the same under `sh -c` and `cmd /c`, so this runs anywhere. + assert!(sh("echo hello").unwrap().contains("hello")); + } + + #[test] + fn sh_fails_on_a_nonzero_exit() { + assert!(sh("exit 1").is_err()); + } + + #[cfg(unix)] + #[test] + fn sh_exposes_the_usage_version() { + assert_eq!( + sh("echo $__USAGE").unwrap().trim(), + env!("CARGO_PKG_VERSION") + ); + } }