Skip to content

Commit 572fb96

Browse files
committed
fix: Prefer / in bash.exe path (for fixtures)
Starting in #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, when running `bash` on Windows, the path will either not be directly relevant (because it will turn into something like `/usr/bin/bash` when accessed through the shell itself such as in `$0`), or it will be used in such a way that it may either need to be quoted or appear ambiguous when examined in logs. Furthermore, the path previously mixed `/` and `\` directory separators except in the unusual case that the `GIT_EXEC_PATH` environment variable was set explicitly and its value used `\` separators, since otherwise `git --exec-path` outputs a path with `/` separators. A path with all `/` separators, provided it is a correct path, should be at least as usable as one that mixes `/` and `\`, and may make any error messages where it appears (such as in test failures) more readable. This also refactors for clarity, and adds new tests related to the change.
1 parent d0ef276 commit 572fb96

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)