diff --git a/Cargo.lock b/Cargo.lock index 90992bd2178..ed0f4f844dc 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -1582,6 +1582,7 @@ dependencies = [ "gix-quote 0.4.15", "gix-testtools", "gix-trace 0.1.12", + "once_cell", "shell-words", ] diff --git a/gix-command/Cargo.toml b/gix-command/Cargo.toml index e81cfedb4be..a2f9d763ddf 100644 --- a/gix-command/Cargo.toml +++ b/gix-command/Cargo.toml @@ -24,3 +24,4 @@ shell-words = "1.0" [dev-dependencies] gix-testtools = { path = "../tests/tools" } +once_cell = "1.17.1" diff --git a/gix-command/src/lib.rs b/gix-command/src/lib.rs index ca64d5cc3e7..c8d47d93e32 100644 --- a/gix-command/src/lib.rs +++ b/gix-command/src/lib.rs @@ -280,10 +280,8 @@ mod prepare { cmd } None => { - let mut cmd = Command::new( - prep.shell_program - .unwrap_or(if cfg!(windows) { "sh" } else { "/bin/sh" }.into()), - ); + let shell = prep.shell_program.unwrap_or_else(|| gix_path::env::shell().into()); + let mut cmd = Command::new(shell); cmd.arg("-c"); if !prep.args.is_empty() { if prep.command.to_str().map_or(true, |cmd| !cmd.contains("$@")) { diff --git a/gix-command/tests/command.rs b/gix-command/tests/command.rs index 118c593d45a..b73a4299632 100644 --- a/gix-command/tests/command.rs +++ b/gix-command/tests/command.rs @@ -222,10 +222,13 @@ mod context { } mod prepare { - #[cfg(windows)] - const SH: &str = "sh"; - #[cfg(not(windows))] - const SH: &str = "/bin/sh"; + use once_cell::sync::Lazy; + + static SH: Lazy<&'static str> = Lazy::new(|| { + gix_path::env::shell() + .to_str() + .expect("`prepare` tests must be run where 'sh' path is valid Unicode") + }); fn quoted(input: &[&str]) -> String { input.iter().map(|s| format!("\"{s}\"")).collect::>().join(" ") @@ -275,7 +278,7 @@ mod prepare { if cfg!(windows) { quoted(&["ls", "first", "second", "third"]) } else { - quoted(&[SH, "-c", "ls first second third", "--"]) + quoted(&[*SH, "-c", "ls first second third", "--"]) }, "with shell, this works as it performs word splitting" ); @@ -311,7 +314,8 @@ mod prepare { if cfg!(windows) { quoted(&["ls", "--foo", "a b", "additional"]) } else { - format!(r#""{SH}" "-c" "ls --foo \"a b\" \"$@\"" "--" "additional""#) + let sh = *SH; + format!(r#""{sh}" "-c" "ls --foo \"a b\" \"$@\"" "--" "additional""#) }, "with shell, this works as it performs word splitting" ); @@ -334,7 +338,7 @@ mod prepare { let cmd = std::process::Command::from( gix_command::prepare("ls --foo=\"a b\"").command_may_be_shell_script_disallow_manual_argument_splitting(), ); - assert_eq!(format!("{cmd:?}"), quoted(&[SH, "-c", r#"ls --foo=\"a b\""#, "--"])); + assert_eq!(format!("{cmd:?}"), quoted(&[*SH, "-c", r#"ls --foo=\"a b\""#, "--"])); } #[test] @@ -347,7 +351,7 @@ mod prepare { ); assert_eq!( format!("{cmd:?}"), - quoted(&[SH, "-c", "ls \\\"$@\\\"", "--", "--foo=a b"]) + quoted(&[*SH, "-c", "ls \\\"$@\\\"", "--", "--foo=a b"]) ); } @@ -362,7 +366,7 @@ mod prepare { ); assert_eq!( format!("{cmd:?}"), - quoted(&[SH, "-c", "\\'ls\\' \\\"$@\\\"", "--", "--foo=a b"]), + quoted(&[*SH, "-c", "\\'ls\\' \\\"$@\\\"", "--", "--foo=a b"]), "looks strange thanks to debug printing, but is the right amount of quotes actually" ); } @@ -379,7 +383,7 @@ mod prepare { assert_eq!( format!("{cmd:?}"), quoted(&[ - SH, + *SH, "-c", "\\'C:\\\\Users\\\\O\\'\\\\\\'\\'Shaughnessy\\\\with space.exe\\' \\\"$@\\\"", "--", @@ -394,9 +398,10 @@ mod prepare { let cmd = std::process::Command::from( gix_command::prepare("ls --foo=~/path").command_may_be_shell_script_allow_manual_argument_splitting(), ); + let sh = *SH; assert_eq!( format!("{cmd:?}"), - format!(r#""{SH}" "-c" "ls --foo=~/path" "--""#), + format!(r#""{sh}" "-c" "ls --foo=~/path" "--""#), "splitting can also handle quotes" ); } @@ -405,9 +410,10 @@ mod prepare { fn tilde_path_and_multiple_arguments_as_part_of_command_with_shell() { let cmd = std::process::Command::from(gix_command::prepare("~/bin/exe --foo \"a b\"").command_may_be_shell_script()); + let sh = *SH; assert_eq!( format!("{cmd:?}"), - format!(r#""{SH}" "-c" "~/bin/exe --foo \"a b\"" "--""#), + format!(r#""{sh}" "-c" "~/bin/exe --foo \"a b\"" "--""#), "this always needs a shell as we need tilde expansion" ); } @@ -419,9 +425,10 @@ mod prepare { .command_may_be_shell_script() .arg("store"), ); + let sh = *SH; assert_eq!( format!("{cmd:?}"), - format!(r#""{SH}" "-c" "echo \"$@\" >&2" "--" "store""#), + format!(r#""{sh}" "-c" "echo \"$@\" >&2" "--" "store""#), "this is how credential helpers have to work as for some reason they don't get '$@' added in Git.\ We deal with it by not doubling the '$@' argument, which seems more flexible." ); @@ -435,9 +442,10 @@ mod prepare { .with_quoted_command() .arg("store"), ); + let sh = *SH; assert_eq!( format!("{cmd:?}"), - format!(r#""{SH}" "-c" "echo \"$@\" >&2" "--" "store""#) + format!(r#""{sh}" "-c" "echo \"$@\" >&2" "--" "store""#) ); } } diff --git a/gix-credentials/tests/program/from_custom_definition.rs b/gix-credentials/tests/program/from_custom_definition.rs index 25e9dd2ae4b..c746e0b48af 100644 --- a/gix-credentials/tests/program/from_custom_definition.rs +++ b/gix-credentials/tests/program/from_custom_definition.rs @@ -1,12 +1,17 @@ use gix_credentials::{helper, program::Kind, Program}; +use once_cell::sync::Lazy; -static GIT: once_cell::sync::Lazy<&'static str> = - once_cell::sync::Lazy::new(|| gix_path::env::exe_invocation().to_str().expect("not illformed")); +static GIT: once_cell::sync::Lazy<&'static str> = once_cell::sync::Lazy::new(|| { + gix_path::env::exe_invocation() + .to_str() + .expect("some `from_custom_definition` tests must be run where 'git' path is valid Unicode") +}); -#[cfg(windows)] -const SH: &str = "sh"; -#[cfg(not(windows))] -const SH: &str = "/bin/sh"; +static SH: Lazy<&'static str> = Lazy::new(|| { + gix_path::env::shell() + .to_str() + .expect("some `from_custom_definition` tests must be run where 'sh' path is valid Unicode") +}); #[test] fn empty() { @@ -47,11 +52,12 @@ fn name_with_args() { fn name_with_special_args() { let input = "name --arg --bar=~/folder/in/home"; let prog = Program::from_custom_definition(input); + let sh = *SH; let git = *GIT; assert!(matches!(&prog.kind, Kind::ExternalName{name_and_args} if name_and_args == input)); assert_eq!( format!("{:?}", prog.to_command(&helper::Action::Store("egal".into()))), - format!(r#""{SH}" "-c" "{git} credential-name --arg --bar=~/folder/in/home \"$@\"" "--" "store""#) + format!(r#""{sh}" "-c" "{git} credential-name --arg --bar=~/folder/in/home \"$@\"" "--" "store""#) ); } @@ -73,12 +79,13 @@ fn path_with_args_that_definitely_need_shell() { let input = "/abs/name --arg --bar=\"a b\""; let prog = Program::from_custom_definition(input); assert!(matches!(&prog.kind, Kind::ExternalPath{path_and_args} if path_and_args == input)); + let sh = *SH; assert_eq!( format!("{:?}", prog.to_command(&helper::Action::Store("egal".into()))), if cfg!(windows) { r#""/abs/name" "--arg" "--bar=a b" "store""#.to_owned() } else { - format!(r#""{SH}" "-c" "/abs/name --arg --bar=\"a b\" \"$@\"" "--" "store""#) + format!(r#""{sh}" "-c" "/abs/name --arg --bar=\"a b\" \"$@\"" "--" "store""#) } ); } @@ -100,12 +107,13 @@ fn path_with_simple_args() { let input = "/abs/name a b"; let prog = Program::from_custom_definition(input); assert!(matches!(&prog.kind, Kind::ExternalPath{path_and_args} if path_and_args == input)); + let sh = *SH; assert_eq!( format!("{:?}", prog.to_command(&helper::Action::Store("egal".into()))), if cfg!(windows) { r#""/abs/name" "a" "b" "store""#.to_owned() } else { - format!(r#""{SH}" "-c" "/abs/name a b \"$@\"" "--" "store""#) + format!(r#""{sh}" "-c" "/abs/name a b \"$@\"" "--" "store""#) }, "a shell is used as there are arguments, and it's generally more flexible, but on windows we split ourselves" ); diff --git a/gix-path/src/env/mod.rs b/gix-path/src/env/mod.rs index 71188fb2a17..78e2da294c4 100644 --- a/gix-path/src/env/mod.rs +++ b/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 = 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() } @@ -50,14 +67,16 @@ pub fn shell() -> &'static OsStr { } /// Return the name of the Git executable to invoke it. +/// /// If it's in the `PATH`, it will always be a short name. /// /// Note that on Windows, we will find the executable in the `PATH` if it exists there, or search it /// in alternative locations which when found yields the full path to it. pub fn exe_invocation() -> &'static Path { if cfg!(windows) { - /// The path to the Git executable as located in the `PATH` or in other locations that it's known to be installed to. - /// It's `None` if environment variables couldn't be read or if no executable could be found. + /// The path to the Git executable as located in the `PATH` or in other locations that it's + /// known to be installed to. It's `None` if environment variables couldn't be read or if + /// no executable could be found. static EXECUTABLE_PATH: Lazy> = Lazy::new(|| { std::env::split_paths(&std::env::var_os("PATH")?) .chain(git::ALTERNATIVE_LOCATIONS.iter().map(Into::into)) @@ -82,11 +101,11 @@ pub fn exe_invocation() -> &'static Path { } } -/// Returns the fully qualified path in the *xdg-home* directory (or equivalent in the home dir) to `file`, -/// accessing `env_var()` to learn where these bases are. +/// Returns the fully qualified path in the *xdg-home* directory (or equivalent in the home dir) to +/// `file`, accessing `env_var()` to learn where these bases are. /// -/// Note that the `HOME` directory should ultimately come from [`home_dir()`] as it handles windows correctly. -/// The same can be achieved by using [`var()`] as `env_var`. +/// Note that the `HOME` directory should ultimately come from [`home_dir()`] as it handles Windows +/// correctly. The same can be achieved by using [`var()`] as `env_var`. pub fn xdg_config(file: &str, env_var: &mut dyn FnMut(&str) -> Option) -> Option { env_var("XDG_CONFIG_HOME") .map(|home| { @@ -136,17 +155,17 @@ pub fn core_dir() -> Option<&'static Path> { GIT_CORE_DIR.as_deref() } -/// Returns the platform dependent system prefix or `None` if it cannot be found (right now only on windows). +/// Returns the platform dependent system prefix or `None` if it cannot be found (right now only on Windows). /// /// ### Performance /// -/// On windows, the slowest part is the launch of the Git executable in the PATH, which only happens when launched -/// from outside of the `msys2` shell. +/// On Windows, the slowest part is the launch of the Git executable in the PATH. This is often +/// avoided by inspecting the environment, when launched from inside a Git Bash MSYS2 shell. /// /// ### When `None` is returned /// -/// This happens only windows if the git binary can't be found at all for obtaining its executable path, or if the git binary -/// wasn't built with a well-known directory structure or environment. +/// This happens only Windows if the git binary can't be found at all for obtaining its executable +/// path, or if the git binary wasn't built with a well-known directory structure or environment. pub fn system_prefix() -> Option<&'static Path> { if cfg!(windows) { static PREFIX: Lazy> = Lazy::new(|| { @@ -177,19 +196,20 @@ pub fn home_dir() -> Option { std::env::var("HOME").map(PathBuf::from).ok() } -/// Tries to obtain the home directory from `HOME` on all platforms, but falls back to [`home::home_dir()`] for -/// more complex ways of obtaining a home directory, particularly useful on Windows. +/// Tries to obtain the home directory from `HOME` on all platforms, but falls back to +/// [`home::home_dir()`] for more complex ways of obtaining a home directory, particularly useful +/// on Windows. /// -/// The reason `HOME` is tried first is to allow Windows users to have a custom location for their linux-style -/// home, as otherwise they would have to accumulate dot files in a directory these are inconvenient and perceived -/// as clutter. +/// The reason `HOME` is tried first is to allow Windows users to have a custom location for their +/// linux-style home, as otherwise they would have to accumulate dot files in a directory these are +/// inconvenient and perceived as clutter. #[cfg(not(target_family = "wasm"))] pub fn home_dir() -> Option { std::env::var_os("HOME").map(Into::into).or_else(home::home_dir) } -/// Returns the contents of an environment variable of `name` with some special handling -/// for certain environment variables (like `HOME`) for platform compatibility. +/// Returns the contents of an environment variable of `name` with some special handling for +/// certain environment variables (like `HOME`) for platform compatibility. pub fn var(name: &str) -> Option { if name == "HOME" { home_dir().map(PathBuf::into_os_string) diff --git a/gix-path/tests/path/env.rs b/gix-path/tests/path/env.rs index 28f23f9cb7b..c3c7019cbce 100644 --- a/gix-path/tests/path/env.rs +++ b/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" ); }