fix: Use / in gix_path::env::shell() and check existence

This makes the path returned by `gix_path::env::shell()` on Windows
more usable by:

1. Adding components with `/` separators. While in principle a `\`
   should work, the path of the shell itself is used in shell
   scripts (script files and `sh -c` operands) that may not account
   for the presence of backslashes, and it is also harder to read
   paths with `\` in contexts where it appears escaped, which may
   include various messages from Rust code and shell scripts.

   The path before what we add will already use `/` and never `\`,
   unless `GIT_EXEC_PATH` has been set to a strange value, because
   it is based on `git --exec-path`, which by default gives a path
   with `/` separators. Thus, ensuring that the part we add uses `/`
   should be sufficient to get a path without `\` in all cases when
   it is clearly reasonable to do so. This therefore also usually
   increases stylistic consistency of the path, which is another
   factor that makes it more user-friendly in messages.

   This is needed to get tests to pass since changing `gix-command`
   to use `gix_path::env::shell()` on Windows, where a path is
   formatted in away that sometimes quotes `\` characters. Their
   expectations could be adjusted, but it seems likely that various
   other software, much of which may otherwise be working, has
   similar expectations. Using `/` instead of `\` works whether `\`
   is expected to be displayed quoted or not.

2. Check that the path to the shell plausibly has a shell there,
   only using it if it a file or a non-broken file symlink. When
   this is not the case, the fallback short name is used instead.

3. The fallback short name is changed from `sh` to `sh.exe`, since
   the `.exe` suffix is appended in other short names on Windows,
   such as `git.exe`, as well as being part of the filename
   component of the path we build for the shell when using the
   implementation provided as part of Git for Windows.

Those changes only affect Windows.

This also adds tests for (1) and (2) above, as well as for the
expectation that we get an absolute path, to make sure we don't
build a path that would be absolute on a Unix-like system but is
relative on Windows (a path that starts with just one `/` or `\`).

These tests are not Windows-specific, since all these expectations
should already hold on Unix-like systems, where currently we are
using the hard-coded path `/bin/sh`, which is an absolute path on
those systems. (Some Unix-like systems may technically not have
`/bin/sh` or it may not be the best path to use for a shell that
should be POSIX-compatible, but we are already relying on this,
and handling that better is outside the scope of the changes here.)

Author: EliahKagan

gix-path/src/env/mod.rs

@@ -30,18 +30,35 @@ pub fn installation_config_prefix() -> Option<&'static Path> {
 
 /// Return the shell that Git would use, the shell to execute commands from.
 ///
-/// On Windows, this is the full path to `sh.exe` bundled with Git, and on
-/// Unix it's `/bin/sh` as posix compatible shell.
-/// If the bundled shell on Windows cannot be found, `sh` is returned as the name of a shell
-/// as it could possibly be found in `PATH`.
-/// Note that the returned path might not be a path on disk.
+/// On Windows, this is the full path to `sh.exe` bundled with Git for Windows if we can find it.
+/// If the bundled shell on Windows cannot be found, `sh.exe` is returned as the name of a shell,
+/// as it could possibly be found in `PATH`. On Unix it's `/bin/sh` as the POSIX-compatible shell.
+///
+/// Note that the returned path might not be a path on disk, if it is a fallback path or if the
+/// file was moved or deleted since the first time this function is called.
 pub fn shell() -> &'static OsStr {
     static PATH: Lazy<OsString> = Lazy::new(|| {
         if cfg!(windows) {
             core_dir()
-                .and_then(|p| p.ancestors().nth(3)) // Skip something like mingw64/libexec/git-core.
-                .map(|p| p.join("usr").join("bin").join("sh.exe"))
-                .map_or_else(|| OsString::from("sh"), Into::into)
+                .and_then(|git_core| {
+                    // Go up above something that is expected to be like mingw64/libexec/git-core.
+                    git_core.ancestors().nth(3)
+                })
+                .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
+                    // 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()
+                })
+                .unwrap_or_else(|| "sh.exe".into())
         } else {
             "/bin/sh".into()
         }

gix-path/tests/path/env.rs

@@ -1,3 +1,5 @@
+use std::path::Path;
+
 #[test]
 fn exe_invocation() {
     let actual = gix_path::env::exe_invocation();
@@ -10,8 +12,27 @@ fn exe_invocation() {
 #[test]
 fn shell() {
     assert!(
-        std::path::Path::new(gix_path::env::shell()).exists(),
-        "On CI and on Unix we'd expect a full path to the shell that exists on disk"
+        Path::new(gix_path::env::shell()).exists(),
+        "On CI and on Unix we expect a usable path to the shell that exists on disk"
+    );
+}
+
+#[test]
+fn shell_absolute() {
+    assert!(
+        Path::new(gix_path::env::shell()).is_absolute(),
+        "On CI and on Unix we currently expect the path to the shell always to be absolute"
+    );
+}
+
+#[test]
+fn shell_unix_path() {
+    let shell = gix_path::env::shell()
+        .to_str()
+        .expect("This test depends on the shell path being valid Unicode");
+    assert!(
+        !shell.contains('\\'),
+        "The path to the shell should have no backslashes, barring strange `GIT_EXEC_PATH` values"
     );
 }