Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
113 changes: 110 additions & 3 deletions cli/src/cli/shell.rs
Original file line number Diff line number Diff line change
Expand Up @@ -49,7 +49,11 @@ impl Shell {
let parsed = usage::parse::parse(&spec, &args)?;
debug!("{parsed:?}");

let mut cmd = std::process::Command::new(shell);
let overridden = env::shell_program_override(shell, |key| env::var(key).ok());
let program = overridden.clone().unwrap_or_else(|| shell.to_string());
debug!("running {program}");

let mut cmd = std::process::Command::new(&program);
cmd.stdin(Stdio::inherit());
cmd.stdout(Stdio::inherit());
cmd.stderr(Stdio::inherit());
Expand All @@ -64,10 +68,25 @@ impl Shell {

env::apply_parsed_env(&mut cmd, &parsed.as_env());

let result = cmd.spawn().into_diagnostic()?.wait().into_diagnostic()?;
// Name the program: the bare io error says only "No such file or directory", which for
// an overridden shell gives no clue that the variable is what pointed here.
let mut child = cmd.spawn().map_err(|err| match &overridden {
Some(_) => miette::miette!(
"failed to run `{program}` (from ${}): {err}",
env::shell_var_name(shell)
),
None => miette::miette!("failed to run `{program}`: {err}"),
})?;
let result = child.wait().into_diagnostic()?;

if !result.success() {
std::process::exit(result.code().unwrap_or(1));
let code = result.code().unwrap_or(1);
if cfg!(windows) && overridden.is_none() {
if let Some(hint) = wsl_path_hint(shell, code, script_path) {
eprintln!("{hint}");
}
}
std::process::exit(code);
}

Ok(())
Expand All @@ -79,3 +98,91 @@ impl Shell {
Ok(())
}
}

/// Whether `path` is a path only Windows understands — a drive letter or a UNC share.
///
/// Hand-written rather than `Path::is_absolute`, whose answer depends on the platform it was
/// compiled for and so cannot be tested from the Linux CI. `/c/foo` (msys) and plain relative
/// paths are not included: both work through the WSL launcher, which translates the working
/// directory for them.
///
/// A drive letter counts even without a following separator. `C:x.sh` is drive-*relative*
/// rather than absolute, but it is still a spelling only Windows resolves — WSL looks for a
/// file named `C:x.sh` and reports 127 exactly as it does for `C:\x.sh`. Requiring the
/// separator would drop the hint for a case that needs it.
fn looks_like_windows_path(path: &str) -> bool {
let bytes = path.as_bytes();
let drive_letter = bytes.len() >= 2 && bytes[0].is_ascii_alphabetic() && bytes[1] == b':';
drive_letter || path.starts_with(r"\\")
Comment thread
greptile-apps[bot] marked this conversation as resolved.
}

/// The guess to print when `bash` on Windows could not find a script it was handed.
///
/// Narrow on purpose. `bash` is the only one of the four shells that ships in the system
/// directory, which Windows searches ahead of `PATH`, so on a machine with WSL installed it is
/// the only one that silently resolves to something that cannot read a `C:\…` path. 127 is the
/// shell's own "command not found", which is what the WSL launcher passes back up.
fn wsl_path_hint(shell: &str, code: i32, script: &str) -> Option<String> {
if shell != "bash" || code != 127 || !looks_like_windows_path(script) {
return None;
}
Some(format!(
"usage: `bash` exited 127 (command not found) and the script was given as a Windows path.\n\
usage: On Windows the system directory is searched before $PATH, so `bash` resolves to\n\
usage: C:\\Windows\\System32\\bash.exe — the WSL launcher — which cannot open `{script}`.\n\
usage: If that is what happened, pass the script by relative path, or point usage at the\n\
usage: bash you meant:\n\
usage: set {}=C:\\Program Files\\Git\\bin\\bash.exe\n\
usage: If the script really did exit 127 on its own, ignore this.",
env::shell_var_name(shell)
))
}

#[cfg(test)]
mod tests {
use super::*;

#[test]
fn windows_paths_are_recognized() {
for path in [r"C:\x", "C:/x", "c:/x", r"\\srv\share\x"] {
assert!(looks_like_windows_path(path), "{path}");
}
}

#[test]
fn a_drive_relative_path_counts_too() {
// Not absolute — `C:x.sh` means "x.sh on drive C's current directory" — but still a
// spelling only Windows resolves. Measured: `usage bash C:x.sh` reaches WSL, which
// looks for a file called `C:x.sh` and exits 127, so the hint belongs there.
for path in ["C:x.sh", "c:x.sh"] {
assert!(looks_like_windows_path(path), "{path}");
}
}

#[test]
fn paths_a_posix_shell_can_open_are_not_windows_paths() {
for path in ["/c/x", "./x.sh", "x.sh", "/usr/local/bin/x", "", "C"] {
assert!(!looks_like_windows_path(path), "{path}");
}
}

#[test]
fn the_hint_names_the_script_and_the_override() {
let hint = wsl_path_hint("bash", 127, r"C:\Users\me\script.sh").unwrap();
assert!(hint.contains(r"C:\Users\me\script.sh"), "{hint}");
assert!(hint.contains("USAGE_SHELL_BASH"), "{hint}");
// Hedged, because a script really can exit 127 on its own.
assert!(hint.contains("ignore this"), "{hint}");
}

#[test]
fn the_hint_stays_quiet_when_it_would_be_guessing() {
// A script that failed for its own reasons.
assert!(wsl_path_hint("bash", 1, r"C:\x").is_none());
// Shells that do not exist in the system directory.
assert!(wsl_path_hint("zsh", 127, r"C:\x").is_none());
assert!(wsl_path_hint("pwsh", 127, r"C:\x").is_none());
// A path the WSL launcher can resolve anyway.
assert!(wsl_path_hint("bash", 127, "./x.sh").is_none());
}
}
93 changes: 93 additions & 0 deletions cli/src/env.rs
Original file line number Diff line number Diff line change
Expand Up @@ -106,6 +106,36 @@ pub fn append_to_wslenv<'a>(
entries.join(":")
}

/// Keyed by the *program* rather than the subcommand, because that is what the value names.
/// `usage powershell` runs `pwsh`, so its variable is `USAGE_SHELL_PWSH`.
pub fn shell_var_name(shell: &str) -> String {
format!("USAGE_SHELL_{}", shell.to_ascii_uppercase())
}

/// The shell program to run in place of `shell`, if one was configured.
///
/// `None` means run `shell` as before. The value is a program path or a name to look up on
/// `PATH` — not a command line: shells on Windows live at paths like
/// `C:\Program Files\Git\bin\bash.exe`, and treating the value as a command line would make
/// usage responsible for quoting rules it has no reason to own. `Command` passes the program
/// and each argument separately, so a path with spaces needs no quoting.
///
/// An empty or blank value reads as unset, matching the `FOO= cmd` convention for switching
/// something off. Nothing checks that the program exists: the value need not be an absolute
/// path, so deciding would mean reimplementing `PATH`, `PATHEXT` and permission lookup, and
/// racing the spawn that follows. A bad value surfaces as a spawn error naming it.
///
/// `lookup` is injected rather than read from the environment so this stays testable without
/// mutating process-wide state — the same shape as `parse_partial_with_env` in usage-lib.
pub fn shell_program_override(
shell: &str,
lookup: impl Fn(&str) -> Option<String>,
) -> Option<String> {
let value = lookup(&shell_var_name(shell))?;
let value = value.trim();
(!value.is_empty()).then(|| value.to_string())
}

#[cfg(test)]
mod tests {
use super::*;
Expand Down Expand Up @@ -228,4 +258,67 @@ mod tests {
);
}
}

fn from(pairs: &[(&str, &str)]) -> impl Fn(&str) -> Option<String> + use<> {
let pairs: Vec<(String, String)> = pairs
.iter()
.map(|(k, v)| ((*k).to_string(), (*v).to_string()))
.collect();
move |key| {
pairs
.iter()
.find(|(k, _)| k == key)
.map(|(_, v)| v.to_string())
}
}

#[test]
fn shell_var_names_cover_every_shell_subcommand() {
// These are the four programs `Cli::run` dispatches to; `powershell` runs `pwsh`.
assert_eq!(shell_var_name("bash"), "USAGE_SHELL_BASH");
assert_eq!(shell_var_name("zsh"), "USAGE_SHELL_ZSH");
assert_eq!(shell_var_name("fish"), "USAGE_SHELL_FISH");
assert_eq!(shell_var_name("pwsh"), "USAGE_SHELL_PWSH");
}

#[test]
fn unset_means_no_override() {
assert_eq!(shell_program_override("bash", from(&[])), None);
}

#[test]
fn a_blank_value_means_no_override() {
for blank in ["", " ", "\t"] {
assert_eq!(
shell_program_override("bash", from(&[("USAGE_SHELL_BASH", blank)])),
None,
"blank value {blank:?} should read as unset"
);
}
}

#[test]
fn surrounding_whitespace_is_trimmed() {
assert_eq!(
shell_program_override("bash", from(&[("USAGE_SHELL_BASH", " /usr/bin/bash ")])),
Some("/usr/bin/bash".to_string())
);
}

#[test]
fn a_path_with_spaces_survives_intact() {
let path = r"C:\Program Files\Git\bin\bash.exe";
assert_eq!(
shell_program_override("bash", from(&[("USAGE_SHELL_BASH", path)])),
Some(path.to_string())
);
}

#[test]
fn another_shells_variable_is_not_picked_up() {
assert_eq!(
shell_program_override("bash", from(&[("USAGE_SHELL_ZSH", "/bin/zsh")])),
None
);
}
}
81 changes: 81 additions & 0 deletions cli/tests/shell_override.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,81 @@
//! `USAGE_SHELL_<SHELL>` replaces the program `usage <shell>` runs.
//!
//! Unix-only because they pin the substitute to a real path (`/bin/echo`, `/bin/sh`). The
//! variable itself is not Unix-specific — it exists for Windows, where `bash` resolves to the
//! WSL launcher ahead of `PATH` — but the CI that runs these is Linux.
#![cfg(unix)]

use assert_cmd::cargo;
use assert_cmd::prelude::*;
use predicates::prelude::*;
use predicates::str::contains;
use std::process::Command;

const SCRIPT: &str = "../examples/test-new-usage-syntax.sh";

/// `usage bash <script> --foo`, with every `USAGE_SHELL_*` this test cares about cleared so a
/// developer's own environment cannot change the result.
fn usage_bash() -> Command {
let mut cmd = Command::new(cargo::cargo_bin!("usage"));
cmd.env_remove("USAGE_SHELL_BASH")
.env_remove("USAGE_SHELL_ZSH")
.args(["bash", SCRIPT, "--foo"]);
cmd
}

#[test]
fn the_override_replaces_the_program() {
// `/bin/echo` prints the argv it was handed, which shows both that a different program ran
// and that the script path and arguments reached it untouched.
usage_bash()
.env("USAGE_SHELL_BASH", "/bin/echo")
.assert()
.success()
.stdout(contains(SCRIPT))
.stdout(contains("foo: true").not());
}

#[test]
fn a_blank_override_runs_the_default_shell() {
usage_bash()
.env("USAGE_SHELL_BASH", "")
.assert()
.success()
.stdout(contains("foo: true"));
}

#[test]
fn the_override_may_name_a_program_on_path() {
// The value does not have to be an absolute path — a name resolved through PATH works
// too, which is what the docs promise. `bash` names the same shell that would have run
// anyway, so the script's output must be unchanged.
//
// Not a different shell: every fixture here uses `set -o pipefail`, so a stand-in has to
// be one that understands it.
let expected = usage_bash().output().unwrap().stdout;
usage_bash()
.env("USAGE_SHELL_BASH", "bash")
.assert()
.success()
.stdout(String::from_utf8(expected).unwrap());
}

#[test]
fn a_program_that_cannot_be_started_names_itself_and_the_variable() {
usage_bash()
.env("USAGE_SHELL_BASH", "/nonexistent/bash-xyz")
.assert()
.failure()
.stderr(contains("/nonexistent/bash-xyz"))
.stderr(contains("USAGE_SHELL_BASH"));
}

#[test]
fn another_shells_variable_is_ignored() {
usage_bash()
.env("USAGE_SHELL_ZSH", "/bin/echo")
.assert()
.success()
.stdout(contains("foo: true"));
}
50 changes: 50 additions & 0 deletions docs/cli/scripts.md
Original file line number Diff line number Diff line change
Expand Up @@ -105,3 +105,53 @@ by spaces. If an arg itself has a space, then it will have quotes around it. Thi
by [`shell_words::join()`](https://docs.rs/shell-words/latest/shell_words/fn.join.html). For now,
this is not customizable behavior. It would be possible to
support [alternatives](https://github.com/jdx/usage/issues/189) though.

## Windows

`usage bash ./mycli` runs whatever `bash` Windows resolves to, and the executable search order
there puts the system directory ahead of `PATH`. Installing WSL puts `bash.exe` in that
directory, so on such a machine `bash` is the WSL launcher no matter what else is installed —
and WSL cannot open a Windows path:

```console
$ usage bash C:/work/mycli
/bin/bash: C:/work/mycli: No such file or directory
```

Two ways out. Passing the script by a **relative path** works, because the launcher translates
the working directory. Or name the shell you actually meant:

```batch
:: Command Prompt
set USAGE_SHELL_BASH=C:\Program Files\Git\bin\bash.exe
```

```powershell
# PowerShell
$env:USAGE_SHELL_BASH = 'C:\Program Files\Git\bin\bash.exe'
```

Each shell subcommand reads the variable for the program it runs:

| Command | Variable |
| ------------------ | ------------------ |
| `usage bash` | `USAGE_SHELL_BASH` |
| `usage zsh` | `USAGE_SHELL_ZSH` |
| `usage fish` | `USAGE_SHELL_FISH` |
| `usage powershell` | `USAGE_SHELL_PWSH` |

`usage powershell` runs `pwsh`, so its variable is named for that — which also lets you point it
at `powershell.exe` on a machine that only has Windows PowerShell.

The value is a program: an absolute path, or a name to look up on `PATH`. It is not a command
line, so it takes no arguments and needs no quoting even where the path contains spaces. An
empty or whitespace-only value reads the same as an unset one. The variable is inherited by the
script, so a script
that invokes `usage` again gets the same shell.
Comment thread
coderabbitai[bot] marked this conversation as resolved.

`usage exec` needs none of this — it already names the interpreter, so a shebang can point
straight at one:

```bash
#!/usr/bin/env -S usage exec "C:/msys64/usr/bin/bash.exe"
```