Skip to content

Commit

Permalink
fix: Prefer / in bash.exe path (for fixtures)
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
EliahKagan committed Feb 23, 2025
1 parent 6acca19 commit bf2f8df
Showing 1 changed file with 47 additions and 12 deletions.
59 changes: 47 additions & 12 deletions tests/tools/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -653,20 +653,27 @@ fn configure_command<'a, I: IntoIterator<Item = S>, S: AsRef<OsStr>>(
}

fn bash_program() -> &'static Path {
if cfg!(windows) {
// TODO(deps): Once `gix_path::env::shell()` is available, maybe do `shell().parent()?.join("bash.exe")`
static GIT_BASH: Lazy<Option<PathBuf>> = Lazy::new(|| {
// TODO(deps): *Maybe* add something to `gix-path` like `env::shell()` that can be used to
// find bash and, once the version `gix-testtools` depends on has it, use it.
static GIT_BASH: Lazy<PathBuf> = Lazy::new(|| {
if cfg!(windows) {
GIT_CORE_DIR
.parent()?
.parent()?
.parent()
.map(|installation_dir| installation_dir.join("bin").join("bash.exe"))
.ancestors()
.nth(3)
.map(OsString::from)
.map(|mut raw_path| {
// Go down to where `bash.exe` usually is. Keep using `/` separators (not `\`).
raw_path.push("/bin/bash.exe");
raw_path
})
.map(PathBuf::from)
.filter(|bash| bash.is_file())
});
GIT_BASH.as_deref().unwrap_or(Path::new("bash.exe"))
} else {
Path::new("bash")
}
.unwrap_or_else(|| "bash.exe".into())
} else {
"bash".into()
}
});
GIT_BASH.as_ref()
}

fn write_failure_marker(failure_marker: &Path) {
Expand Down Expand Up @@ -1059,4 +1066,32 @@ mod tests {
fn bash_program_ok_for_platform() {
assert_eq!(bash_program(), Path::new("bash"));
}

#[test]
fn bash_program_unix_path() {
let path = bash_program()
.to_str()
.expect("This test depends on the bash path being valid Unicode");
assert!(
!path.contains('\\'),
"The path to bash should have no backslashes, barring very unusual environments"
);
}

fn is_rooted_relative(path: impl AsRef<Path>) -> bool {
let p = path.as_ref();
p.is_relative() && p.has_root()
}

#[test]
#[cfg(windows)]
fn unix_style_absolute_is_rooted_relative() {
assert!(is_rooted_relative("/bin/bash"), "can detect paths like /bin/bash");
}

#[test]
fn bash_program_absolute_or_unrooted() {
let bash = bash_program();
assert!(!is_rooted_relative(bash), "{bash:?}");
}
}

0 comments on commit bf2f8df

Please sign in to comment.