Skip to content

Commit c7d06a6

Browse files
committed
feat: Use gix_path::env::shell() in gix-command for sh
`gix_command::Prepare` previously used `sh` on Windows rather than first checking for a usable `sh` implementation associated with a Git for Windows installation. This changes it to use the `gix-path` facility for finding what is likely the best 'sh' implementation for POSIX shell scripts that will operate on Git repositories. This increases consistency across how different crates find 'sh', and brings the benefit of preferring the Git for Windows `sh.exe` on Windows when it can be found reliably.
1 parent 091d994 commit c7d06a6

File tree

4 files changed

+26
-18
lines changed

4 files changed

+26
-18
lines changed

Cargo.lock

Lines changed: 1 addition & 0 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

gix-command/Cargo.toml

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -24,3 +24,4 @@ shell-words = "1.0"
2424

2525
[dev-dependencies]
2626
gix-testtools = { path = "../tests/tools" }
27+
once_cell = "1.17.1"

gix-command/src/lib.rs

Lines changed: 2 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -280,10 +280,8 @@ mod prepare {
280280
cmd
281281
}
282282
None => {
283-
let mut cmd = Command::new(
284-
prep.shell_program
285-
.unwrap_or(if cfg!(windows) { "sh" } else { "/bin/sh" }.into()),
286-
);
283+
let shell = prep.shell_program.unwrap_or_else(|| gix_path::env::shell().into());
284+
let mut cmd = Command::new(shell);
287285
cmd.arg("-c");
288286
if !prep.args.is_empty() {
289287
if prep.command.to_str().map_or(true, |cmd| !cmd.contains("$@")) {

gix-command/tests/command.rs

Lines changed: 22 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -222,10 +222,13 @@ mod context {
222222
}
223223

224224
mod prepare {
225-
#[cfg(windows)]
226-
const SH: &str = "sh";
227-
#[cfg(not(windows))]
228-
const SH: &str = "/bin/sh";
225+
use once_cell::sync::Lazy;
226+
227+
static SH: Lazy<&'static str> = Lazy::new(|| {
228+
gix_path::env::shell()
229+
.to_str()
230+
.expect("`prepare` tests must be run where 'sh' path is valid Unicode")
231+
});
229232

230233
fn quoted(input: &[&str]) -> String {
231234
input.iter().map(|s| format!("\"{s}\"")).collect::<Vec<_>>().join(" ")
@@ -275,7 +278,7 @@ mod prepare {
275278
if cfg!(windows) {
276279
quoted(&["ls", "first", "second", "third"])
277280
} else {
278-
quoted(&[SH, "-c", "ls first second third", "--"])
281+
quoted(&[*SH, "-c", "ls first second third", "--"])
279282
},
280283
"with shell, this works as it performs word splitting"
281284
);
@@ -311,7 +314,8 @@ mod prepare {
311314
if cfg!(windows) {
312315
quoted(&["ls", "--foo", "a b", "additional"])
313316
} else {
314-
format!(r#""{SH}" "-c" "ls --foo \"a b\" \"$@\"" "--" "additional""#)
317+
let sh = *SH;
318+
format!(r#""{sh}" "-c" "ls --foo \"a b\" \"$@\"" "--" "additional""#)
315319
},
316320
"with shell, this works as it performs word splitting"
317321
);
@@ -334,7 +338,7 @@ mod prepare {
334338
let cmd = std::process::Command::from(
335339
gix_command::prepare("ls --foo=\"a b\"").command_may_be_shell_script_disallow_manual_argument_splitting(),
336340
);
337-
assert_eq!(format!("{cmd:?}"), quoted(&[SH, "-c", r#"ls --foo=\"a b\""#, "--"]));
341+
assert_eq!(format!("{cmd:?}"), quoted(&[*SH, "-c", r#"ls --foo=\"a b\""#, "--"]));
338342
}
339343

340344
#[test]
@@ -347,7 +351,7 @@ mod prepare {
347351
);
348352
assert_eq!(
349353
format!("{cmd:?}"),
350-
quoted(&[SH, "-c", "ls \\\"$@\\\"", "--", "--foo=a b"])
354+
quoted(&[*SH, "-c", "ls \\\"$@\\\"", "--", "--foo=a b"])
351355
);
352356
}
353357

@@ -362,7 +366,7 @@ mod prepare {
362366
);
363367
assert_eq!(
364368
format!("{cmd:?}"),
365-
quoted(&[SH, "-c", "\\'ls\\' \\\"$@\\\"", "--", "--foo=a b"]),
369+
quoted(&[*SH, "-c", "\\'ls\\' \\\"$@\\\"", "--", "--foo=a b"]),
366370
"looks strange thanks to debug printing, but is the right amount of quotes actually"
367371
);
368372
}
@@ -379,7 +383,7 @@ mod prepare {
379383
assert_eq!(
380384
format!("{cmd:?}"),
381385
quoted(&[
382-
SH,
386+
*SH,
383387
"-c",
384388
"\\'C:\\\\Users\\\\O\\'\\\\\\'\\'Shaughnessy\\\\with space.exe\\' \\\"$@\\\"",
385389
"--",
@@ -394,9 +398,10 @@ mod prepare {
394398
let cmd = std::process::Command::from(
395399
gix_command::prepare("ls --foo=~/path").command_may_be_shell_script_allow_manual_argument_splitting(),
396400
);
401+
let sh = *SH;
397402
assert_eq!(
398403
format!("{cmd:?}"),
399-
format!(r#""{SH}" "-c" "ls --foo=~/path" "--""#),
404+
format!(r#""{sh}" "-c" "ls --foo=~/path" "--""#),
400405
"splitting can also handle quotes"
401406
);
402407
}
@@ -405,9 +410,10 @@ mod prepare {
405410
fn tilde_path_and_multiple_arguments_as_part_of_command_with_shell() {
406411
let cmd =
407412
std::process::Command::from(gix_command::prepare("~/bin/exe --foo \"a b\"").command_may_be_shell_script());
413+
let sh = *SH;
408414
assert_eq!(
409415
format!("{cmd:?}"),
410-
format!(r#""{SH}" "-c" "~/bin/exe --foo \"a b\"" "--""#),
416+
format!(r#""{sh}" "-c" "~/bin/exe --foo \"a b\"" "--""#),
411417
"this always needs a shell as we need tilde expansion"
412418
);
413419
}
@@ -419,9 +425,10 @@ mod prepare {
419425
.command_may_be_shell_script()
420426
.arg("store"),
421427
);
428+
let sh = *SH;
422429
assert_eq!(
423430
format!("{cmd:?}"),
424-
format!(r#""{SH}" "-c" "echo \"$@\" >&2" "--" "store""#),
431+
format!(r#""{sh}" "-c" "echo \"$@\" >&2" "--" "store""#),
425432
"this is how credential helpers have to work as for some reason they don't get '$@' added in Git.\
426433
We deal with it by not doubling the '$@' argument, which seems more flexible."
427434
);
@@ -435,9 +442,10 @@ mod prepare {
435442
.with_quoted_command()
436443
.arg("store"),
437444
);
445+
let sh = *SH;
438446
assert_eq!(
439447
format!("{cmd:?}"),
440-
format!(r#""{SH}" "-c" "echo \"$@\" >&2" "--" "store""#)
448+
format!(r#""{sh}" "-c" "echo \"$@\" >&2" "--" "store""#)
441449
);
442450
}
443451
}

0 commit comments

Comments
 (0)