fix: Check prefix and prefer shim in gix_path::env::shell()

This makes a few changes to make `shell()` more robust:

1. Check the last two components of the path `git --exec-path`
   gave, to make sure they are `libexec/git-core`.

   (The check is done in such a way that the separator may be `/`
   or `\`, though a `\` separator here would be unexpected. We
   permit it because it may plausibly be present due to an
   overriden `GIT_EXEC_PATH` that breaks with Git's own behavior of
   using `/` but that is otherwise fully usable.)

   If the directory is not named `git-core`, or it is a top-level
   directory (no parent),  or its parent is not named `libexec`,
   then it is not reasonable to guess that this is in a directory
   where it would be safe to use `sh.exe` in the expected relative
   location. (Even if safe, such a layout does not suggest that a
   `sh.exe` found in it would be better choice than the fallback of
   just doing a `PATH` search.)

2. Check the grandparent component (that `../..` would go to) of
   the path `git --exec-path` gave, to make sure it is recognized
   name of a platform-specific `usr`-like directory that has been
   used in MSYS2.

   This is to avoid traversing up out of less common directory
   trees that have some different and shallower structure than
   found in a typical Git for Windows or MSYS2 installation.

3. Instead of using only the `(git root)/usr/bin/sh.exe` non-shim,
   prefer the `(git root)/bin/sh.exe` shim. If that is not found,
   fall back to the `(git root)/usr/bin/sh.exe` non-shim, mainly to
   support the Git for Windows SDK, which doesn't have the shim.

   The reason to prefer the shim is that it sets environment
   variables, including prepending `bin` directories that provide
   tools one would expect to have when using it. Without this,
   common POSIX commands may be unavailable, or different and
   incompatible implementations of them may be found.

   In particular, if they are found in a different MSYS2
   installation whose `msys-2.0.dll` is of a different version or
   otherwise a different build, then calling them directly may
   produce strange behavior. See:

   - https://cygwin.com/faq.html#faq.using.multiple-copies
   - GitoxideLabs#1862 (comment)

   This makes things more robust overall than either preferring the
   non-shim or just doing a path search for `sh` as was done before
   that. But it exacerbates GitoxideLabs#1868 (as described there), so if the
   Git for Windows `sh.exe` shim continues to work as it currently
   does, then further improvements may be called for here.

Author: EliahKagan

Diff for: gix-path/src/env/mod.rs

@@ -39,25 +39,42 @@ pub fn installation_config_prefix() -> Option<&'static Path> {
 pub fn shell() -> &'static OsStr {
     static PATH: Lazy<OsString> = Lazy::new(|| {
         if cfg!(windows) {
+            const MSYS_PREFIX_NAMES: &[&str] = &[
+                "mingw64",
+                "mingw32",
+                "clangarm64",
+                "clang64",
+                "clang32",
+                "ucrt64",
+                "usr",
+            ];
+            const RAW_SUFFIXES: &[&str] = &[
+                "/bin/sh.exe", // Usually a shim, which currently we prefer, if available.
+                "/usr/bin/sh.exe",
+            ];
+            fn raw_join(path: &Path, raw_suffix: &str) -> OsString {
+                let mut raw_path = OsString::from(path);
+                raw_path.push(raw_suffix);
+                raw_path
+            }
             core_dir()
-                .and_then(|git_core| {
-                    // Go up above something that is expected to be like mingw64/libexec/git-core.
-                    git_core.ancestors().nth(3)
+                .filter(|core| core.is_absolute() && core.ends_with("libexec/git-core"))
+                .and_then(|core| core.ancestors().nth(2))
+                .filter(|prefix| {
+                    // Only use `libexec/git-core` from inside something `usr`-like, such as `mingw64`.
+                    MSYS_PREFIX_NAMES.iter().any(|name| prefix.ends_with(name))
                 })
-                .map(OsString::from)
-                .map(|mut raw_path| {
-                    // Go down to where `sh.exe` usually is. To avoid breaking shell scripts that
-                    // wrongly assume the shell's own path contains no `\`, as well as to produce
+                .and_then(|prefix| prefix.parent())
+                .into_iter()
+                .flat_map(|git_root| {
+                    // Enumerate the locations where `sh.exe` usually is. To avoid breaking shell
+                    // scripts that assume the shell's own path contains no `\`, and to produce
                     // more readable messages, append literally with `/` separators. The path from
                     // `git --exec-path` will already have all `/` separators (and no trailing `/`)
                     // unless it was explicitly overridden to an unusual value via `GIT_EXEC_PATH`.
-                    raw_path.push("/usr/bin/sh.exe");
-                    raw_path
-                })
-                .filter(|raw_path| {
-                    // Check if there is something that could be a usable shell there.
-                    Path::new(raw_path).is_file()
+                    RAW_SUFFIXES.iter().map(|raw_suffix| raw_join(git_root, raw_suffix))
                 })
+                .find(|raw_path| Path::new(raw_path).is_file())
                 .unwrap_or_else(|| "sh.exe".into())
         } else {
             "/bin/sh".into()