Skip to content

Commit a75b4e9

Browse files
committed
fix: Prefer / in bash.exe path (for fixtures)
Starting in GitoxideLabs#1712, `gix-testtools` looks for `bash.exe` on Windows in one of its common locations as provided by Git for Windows, by taking the path given by `git --exec-path`, going up by three components, and going down to `bin/bash.exe` under that. But the `bin` and `bash.exe` components were appended in a way that used `\` directory separators. Ordinarily that would be ideal, since `\` is the primary directory separator on Windows. However: - Because `bash` is a Bourne-style shell, the path used to invoke it appears as `$0` in some circumstances, such as when it is used to run a command with `-c` with no arguments after its operand. Ideally it will never be assumed to use `/` separators, and ideally it will never be used in a way that treats backslashes appearing in it as escape characters. But in practice this is sometimes hard to avoid. - The prefix, which is part of the output of `git --exec-path`, already uses `/` separatros, except in the unusual case that it overridden to a value containing `\` by setting the `GIT_EXEC_PATH` environment. So we are usually mixing `/` and `\` separators if we append with `\`, and usually avoiding mixing them if we append with `/`. Mixing directory separators is permitted, but the resulting paths are often more confusing when they appear in messages, and more broadly combine the disadvantages of each form. This also refactors for clarity, and adds new tests related to the change.
1 parent 2efce72 commit a75b4e9

File tree

1 file changed

+47
-12
lines changed

1 file changed

+47
-12
lines changed

tests/tools/src/lib.rs

+47-12
Original file line numberDiff line numberDiff line change
@@ -653,20 +653,27 @@ fn configure_command<'a, I: IntoIterator<Item = S>, S: AsRef<OsStr>>(
653653
}
654654

655655
fn bash_program() -> &'static Path {
656-
if cfg!(windows) {
657-
// TODO(deps): Once `gix_path::env::shell()` is available, maybe do `shell().parent()?.join("bash.exe")`
658-
static GIT_BASH: Lazy<Option<PathBuf>> = Lazy::new(|| {
656+
// TODO(deps): *Maybe* add something to `gix-path` like `env::shell()` that can be used to
657+
// find bash and, once the version `gix-testtools` depends on has it, use it.
658+
static GIT_BASH: Lazy<PathBuf> = Lazy::new(|| {
659+
if cfg!(windows) {
659660
GIT_CORE_DIR
660-
.parent()?
661-
.parent()?
662-
.parent()
663-
.map(|installation_dir| installation_dir.join("bin").join("bash.exe"))
661+
.ancestors()
662+
.nth(3)
663+
.map(OsString::from)
664+
.map(|mut raw_path| {
665+
// Go down to where `bash.exe` usually is. Keep using `/` separators (not `\`).
666+
raw_path.push("/bin/bash.exe");
667+
raw_path
668+
})
669+
.map(PathBuf::from)
664670
.filter(|bash| bash.is_file())
665-
});
666-
GIT_BASH.as_deref().unwrap_or(Path::new("bash.exe"))
667-
} else {
668-
Path::new("bash")
669-
}
671+
.unwrap_or_else(|| "bash.exe".into())
672+
} else {
673+
"bash".into()
674+
}
675+
});
676+
GIT_BASH.as_ref()
670677
}
671678

672679
fn write_failure_marker(failure_marker: &Path) {
@@ -1059,4 +1066,32 @@ mod tests {
10591066
fn bash_program_ok_for_platform() {
10601067
assert_eq!(bash_program(), Path::new("bash"));
10611068
}
1069+
1070+
#[test]
1071+
fn bash_program_unix_path() {
1072+
let path = bash_program()
1073+
.to_str()
1074+
.expect("This test depends on the bash path being valid Unicode");
1075+
assert!(
1076+
!path.contains('\\'),
1077+
"The path to bash should have no backslashes, barring very unusual environments"
1078+
);
1079+
}
1080+
1081+
fn is_rooted_relative(path: impl AsRef<Path>) -> bool {
1082+
let p = path.as_ref();
1083+
p.is_relative() && p.has_root()
1084+
}
1085+
1086+
#[test]
1087+
#[cfg(windows)]
1088+
fn unix_style_absolute_is_rooted_relative() {
1089+
assert!(is_rooted_relative("/bin/bash"), "can detect paths like /bin/bash");
1090+
}
1091+
1092+
#[test]
1093+
fn bash_program_absolute_or_unrooted() {
1094+
let bash = bash_program();
1095+
assert!(!is_rooted_relative(bash), "{bash:?}");
1096+
}
10621097
}

0 commit comments

Comments
 (0)