Skip to content

Commit

Permalink
fix: Run test fixture scripts on Windows with Git Bash
Browse files Browse the repository at this point in the history
Previously they had run on Windows with whatever `bash` was found
in a `PATH` search, which had two problems:

- On most, but not all, Windows systems, except when running in a
  specialized environment such as a Git Bash shell where `PATH` is
  changed, the `System32` directory contains a `bash.exe` program
  associated with WSL (that attempts to use WSL to run `bash` in an
  installed distrobution) and appears in the value of `PATH` before
  any other directory that contains a `bash.exe` program. This
  makes it so that, on most Windows systems, it has been necessary
  to run the test suite from Git Bash or another MSYS2 environment,
  or otherwise to adjust the `PATH`.

- That adjusting the path, even by using a Git Bash environment is
  *sufficient* to work around this on most Windows systems relies
  on non-guaranteed behavior of `std::process::Command` executable
  search, which may change without warning in the future.

  On Windows, the usual `CreateProcessW` path search behavior
  actually searches various locations before examining `PATH`
  directories, and the `System32` directory is one of these.
  To avoid including the current directory in the search,
  `std::process::Command` does its own search, which is otherwise
  mostly similar to how `CreateProcessW` searches. But one other
  way it differs, which is subject to change, is that whenever any
  environment variable is set, it first searches the directories in
  (what will be) the child environment.

  If in the future this is not done, or narrowed to be done only
  when `PATH` is one of the environment variables customized for
  the child process, then putting the directory with the desired
  `bash.exe` earlier than the `System32` directory in `PATH` will
  no longer prevent `std::proces::Command` from finding the
  `bash.exe` in `System32` as `CreateProcessW` would and using it.
  Then it would be nontrivial to run the test suite on Windows.
  • Loading branch information
EliahKagan committed Nov 30, 2024
1 parent 479c06b commit 67fb22b
Showing 1 changed file with 79 additions and 22 deletions.
101 changes: 79 additions & 22 deletions tests/tools/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -93,6 +93,23 @@ const GIT_PROGRAM: &str = "git.exe";
#[cfg(not(windows))]
const GIT_PROGRAM: &str = "git";

static GIT_CORE_DIR: Lazy<PathBuf> = Lazy::new(|| {
let output = std::process::Command::new(GIT_PROGRAM)
.arg("--exec-path")
.output()
.expect("can execute `git --exec-path`");

assert!(output.status.success(), "`git --exec-path` failed");

output
.stdout
.strip_suffix(b"\n")
.expect("malformed output from `git --exec-path`")
.to_os_str()
.expect("no invalid UTF-8 in `--exec-path` except as OS allows")
.into()
});

/// The major, minor and patch level of the git version on the system.
pub static GIT_VERSION: Lazy<(u8, u8, u8)> = Lazy::new(|| parse_git_version().expect("git version to be parsable"));

Expand Down Expand Up @@ -186,22 +203,6 @@ pub fn run_git(working_dir: &Path, args: &[&str]) -> std::io::Result<std::proces

/// Spawn a git daemon process to host all repository at or below `working_dir`.
pub fn spawn_git_daemon(working_dir: impl AsRef<Path>) -> std::io::Result<GitDaemon> {
static EXEC_PATH: Lazy<PathBuf> = Lazy::new(|| {
let output = std::process::Command::new(GIT_PROGRAM)
.arg("--exec-path")
.output()
.expect("can execute `git --exec-path`");

assert!(output.status.success(), "`git --exec-path` failed");

output
.stdout
.strip_suffix(b"\n")
.expect("malformed output from `git --exec-path`")
.to_os_str()
.expect("no invalid UTF-8 in `--exec-path` except as OS allows")
.into()
});
let mut ports: Vec<_> = (9419u16..9419 + 100).collect();
fastrand::shuffle(&mut ports);
let addr_at = |port| std::net::SocketAddr::from(([127, 0, 0, 1], port));
Expand All @@ -210,11 +211,12 @@ pub fn spawn_git_daemon(working_dir: impl AsRef<Path>) -> std::io::Result<GitDae
listener.local_addr().expect("listener address is available").port()
};

let child = std::process::Command::new(EXEC_PATH.join(if cfg!(windows) { "git-daemon.exe" } else { "git-daemon" }))
.current_dir(working_dir)
.args(["--verbose", "--base-path=.", "--export-all", "--user-path"])
.arg(format!("--port={free_port}"))
.spawn()?;
let child =
std::process::Command::new(GIT_CORE_DIR.join(if cfg!(windows) { "git-daemon.exe" } else { "git-daemon" }))
.current_dir(working_dir)
.args(["--verbose", "--base-path=.", "--export-all", "--user-path"])
.arg(format!("--port={free_port}"))
.spawn()?;

let server_addr = addr_at(free_port);
for time in gix_lock::backoff::Exponential::default_with_random() {
Expand Down Expand Up @@ -566,7 +568,7 @@ fn scripted_fixture_read_only_with_args_inner(
Err(err)
if err.kind() == std::io::ErrorKind::PermissionDenied || err.raw_os_error() == Some(193) /* windows */ =>
{
cmd = std::process::Command::new("bash");
cmd = std::process::Command::new(bash_program());
configure_command(cmd.arg(script_absolute_path), &args, &script_result_directory).output()?
}
Err(err) => return Err(err.into()),
Expand Down Expand Up @@ -642,6 +644,22 @@ fn configure_command<'a, I: IntoIterator<Item = S>, S: AsRef<OsStr>>(
.env("GIT_CONFIG_VALUE_3", "always")
}

fn bash_program() -> &'static Path {
if cfg!(windows) {
static GIT_BASH: Lazy<Option<PathBuf>> = Lazy::new(|| {
GIT_CORE_DIR
.parent()?
.parent()?
.parent()
.map(|installation_dir| installation_dir.join("bin").join("bash.exe"))
.filter(|bash| bash.is_file())
});
GIT_BASH.as_deref().unwrap_or(Path::new("bash.exe"))
} else {
Path::new("bash")
}
}

fn write_failure_marker(failure_marker: &Path) {
std::fs::write(failure_marker, []).ok();
}
Expand Down Expand Up @@ -981,4 +999,43 @@ mod tests {
assert_eq!(lines, Vec::<&str>::new(), "should be no config variables from files");
assert_eq!(status, 0, "reading the config should succeed");
}

#[test]
#[cfg(windows)]
fn bash_program_ok_for_platform() {
let path = bash_program();
assert!(path.is_absolute());

let for_version = std::process::Command::new(path)
.arg("--version")
.output()
.expect("can pass it `--version`");
assert!(for_version.status.success(), "passing `--version` succeeds");
let version_line = for_version
.stdout
.lines()
.nth(0)
.expect("`--version` output has first line");
assert!(
version_line.ends_with(b"-pc-msys)"), // On Windows, "-pc-linux-gnu)" would be WSL.
"it is an MSYS bash (such as Git Bash)"
);

let for_uname_os = std::process::Command::new(path)
.args(["-c", "uname -o"])
.output()
.expect("can tell it to run `uname -o`");
assert!(for_uname_os.status.success(), "telling it to run `uname -o` succeeds");
assert_eq!(
for_uname_os.stdout.trim_end(),
b"Msys",
"it runs commands in an MSYS environment"
);
}

#[test]
#[cfg(not(windows))]
fn bash_program_ok_for_platform() {
assert_eq!(bash_program(), Path::new("bash"));
}
}

0 comments on commit 67fb22b

Please sign in to comment.