From 51bbb8646287f0a6dfa9ff0fd3852c25e001aaf0 Mon Sep 17 00:00:00 2001 From: Sebastian Thiel Date: Sat, 11 Jan 2025 17:07:06 +0100 Subject: [PATCH 1/5] fix: rename `env::login_shell()` to `shell()` and explain what it is. This assures we don't suggest the usage of actual login shells to ever be used to execute hooks. Note that this is not marked as breaking change as no release was made with the old name yet. --- gix-path/src/env/mod.rs | 22 +++++++++++++--------- gix-path/tests/path/env.rs | 12 +++++------- 2 files changed, 18 insertions(+), 16 deletions(-) diff --git a/gix-path/src/env/mod.rs b/gix-path/src/env/mod.rs index cdc092dcb25..32c6e8991ba 100644 --- a/gix-path/src/env/mod.rs +++ b/gix-path/src/env/mod.rs @@ -1,4 +1,4 @@ -use std::ffi::OsString; +use std::ffi::{OsStr, OsString}; use std::path::{Path, PathBuf}; use bstr::{BString, ByteSlice}; @@ -28,21 +28,25 @@ pub fn installation_config_prefix() -> Option<&'static Path> { installation_config().map(git::config_to_base_path) } -/// Return the shell that Git would prefer as login shell, the shell to execute Git commands from. +/// Return the shell that Git would use, the shell to execute commands from. /// -/// On Windows, this is the `bash.exe` bundled with it, and on Unix it's the shell specified by `SHELL`, -/// or `None` if it is truly unspecified. -pub fn login_shell() -> Option<&'static Path> { - static PATH: Lazy> = Lazy::new(|| { +/// On Windows, this is the full path to `sh.exe` bundled with it, 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. +pub fn shell() -> &'static OsStr { + static PATH: Lazy> = Lazy::new(|| { if cfg!(windows) { installation_config_prefix() .and_then(|p| p.parent()) - .map(|p| p.join("usr").join("bin").join("bash.exe")) + .map(|p| p.join("usr").join("bin").join("sh.exe")) + .map(Into::into) } else { - std::env::var_os("SHELL").map(PathBuf::from) + Some("/bin/sh".into()) } }); - PATH.as_deref() + PATH.as_deref().unwrap_or(OsStr::new("sh")) } /// Return the name of the Git executable to invoke it. diff --git a/gix-path/tests/path/env.rs b/gix-path/tests/path/env.rs index d2c4f9fd265..9b503a095f3 100644 --- a/gix-path/tests/path/env.rs +++ b/gix-path/tests/path/env.rs @@ -8,13 +8,11 @@ fn exe_invocation() { } #[test] -fn login_shell() { - // On CI, the $SHELL variable isn't necessarily set. Maybe other ways to get the login shell should be used then. - if !gix_testtools::is_ci::cached() { - assert!(gix_path::env::login_shell() - .expect("There should always be the notion of a shell used by git") - .exists()); - } +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" + ); } #[test] From 73ee27afb1871cea7117982f41a999efa3167186 Mon Sep 17 00:00:00 2001 From: Sebastian Thiel Date: Sat, 11 Jan 2025 18:13:50 +0100 Subject: [PATCH 2/5] feat: add `env::core_dir()` --- gix-path/src/env/mod.rs | 47 +++++++++++++++++++++++++------------- gix-path/tests/path/env.rs | 8 +++++++ 2 files changed, 39 insertions(+), 16 deletions(-) diff --git a/gix-path/src/env/mod.rs b/gix-path/src/env/mod.rs index 32c6e8991ba..e3ad4b83a8c 100644 --- a/gix-path/src/env/mod.rs +++ b/gix-path/src/env/mod.rs @@ -106,6 +106,36 @@ pub fn xdg_config(file: &str, env_var: &mut dyn FnMut(&str) -> Option) }) } +static GIT_CORE_DIR: Lazy> = Lazy::new(|| { + let mut cmd = std::process::Command::new(exe_invocation()); + + #[cfg(windows)] + { + use std::os::windows::process::CommandExt; + const CREATE_NO_WINDOW: u32 = 0x08000000; + cmd.creation_flags(CREATE_NO_WINDOW); + } + let output = cmd.arg("--exec-path").output().ok()?; + + if !output.status.success() { + return None; + } + + BString::new(output.stdout) + .trim_with(|b| b.is_ascii_whitespace()) + .to_path() + .ok()? + .to_owned() + .into() +}); + +/// Return the directory obtained by calling `git --exec-path`. +/// +/// Returns `None` if Git could not be found or if it returned an error. +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). /// /// ### Performance @@ -129,22 +159,7 @@ pub fn system_prefix() -> Option<&'static Path> { } } - let mut cmd = std::process::Command::new(exe_invocation()); - #[cfg(windows)] - { - use std::os::windows::process::CommandExt; - const CREATE_NO_WINDOW: u32 = 0x08000000; - cmd.creation_flags(CREATE_NO_WINDOW); - } - cmd.arg("--exec-path").stderr(std::process::Stdio::null()); - gix_trace::debug!(cmd = ?cmd, "invoking git to get system prefix/exec path"); - let path = cmd.output().ok()?.stdout; - let path = BString::new(path) - .trim_with(|b| b.is_ascii_whitespace()) - .to_path() - .ok()? - .to_owned(); - + let path = GIT_CORE_DIR.as_deref()?; let one_past_prefix = path.components().enumerate().find_map(|(idx, c)| { matches!(c,std::path::Component::Normal(name) if name.to_str() == Some("libexec")).then_some(idx) })?; diff --git a/gix-path/tests/path/env.rs b/gix-path/tests/path/env.rs index 9b503a095f3..157c22e3477 100644 --- a/gix-path/tests/path/env.rs +++ b/gix-path/tests/path/env.rs @@ -24,6 +24,14 @@ fn installation_config() { ); } +#[test] +fn core_dir() { + assert!( + gix_path::env::core_dir().expect("Git is always in PATH").is_dir(), + "The core directory is a valid directory" + ); +} + #[test] fn system_prefix() { assert_ne!( From 75d689f6a195f301e4a3000e79f58e5ec1c20557 Mon Sep 17 00:00:00 2001 From: Sebastian Thiel Date: Sat, 11 Jan 2025 17:34:50 +0100 Subject: [PATCH 3/5] feat: add `gix env` to print paths relevant to the Git installation. --- gitoxide-core/src/lib.rs | 46 +++++++++++++++++++++++++++++++++++++ src/plumbing/main.rs | 9 ++++++++ src/plumbing/options/mod.rs | 1 + 3 files changed, 56 insertions(+) diff --git a/gitoxide-core/src/lib.rs b/gitoxide-core/src/lib.rs index 9ff7ab73760..3b0237d1e52 100644 --- a/gitoxide-core/src/lib.rs +++ b/gitoxide-core/src/lib.rs @@ -30,6 +30,7 @@ #![deny(rust_2018_idioms)] #![forbid(unsafe_code)] +use anyhow::bail; use std::str::FromStr; #[derive(Debug, Eq, PartialEq, Hash, Clone, Copy)] @@ -82,6 +83,51 @@ pub mod repository; mod discover; pub use discover::discover; +pub fn env(mut out: impl std::io::Write, format: OutputFormat) -> anyhow::Result<()> { + if format != OutputFormat::Human { + bail!("JSON output isn't supported"); + }; + + let width = 15; + writeln!( + out, + "{field:>width$}: {}", + std::path::Path::new(gix::path::env::shell()).display(), + field = "shell", + )?; + writeln!( + out, + "{field:>width$}: {:?}", + gix::path::env::installation_config_prefix(), + field = "config prefix", + )?; + writeln!( + out, + "{field:>width$}: {:?}", + gix::path::env::installation_config(), + field = "config", + )?; + writeln!( + out, + "{field:>width$}: {}", + gix::path::env::exe_invocation().display(), + field = "git exe", + )?; + writeln!( + out, + "{field:>width$}: {:?}", + gix::path::env::system_prefix(), + field = "system prefix", + )?; + writeln!( + out, + "{field:>width$}: {:?}", + gix::path::env::core_dir(), + field = "core dir", + )?; + Ok(()) +} + #[cfg(all(feature = "async-client", feature = "blocking-client"))] compile_error!("Cannot set both 'blocking-client' and 'async-client' features as they are mutually exclusive"); diff --git a/src/plumbing/main.rs b/src/plumbing/main.rs index 625f9733268..577b23208d6 100644 --- a/src/plumbing/main.rs +++ b/src/plumbing/main.rs @@ -146,6 +146,15 @@ pub fn main() -> Result<()> { } match cmd { + Subcommands::Env => prepare_and_run( + "env", + trace, + verbose, + progress, + progress_keep_open, + None, + move |_progress, out, _err| core::env(out, format), + ), Subcommands::Merge(merge::Platform { cmd }) => match cmd { merge::SubCommands::File { resolve_with, diff --git a/src/plumbing/options/mod.rs b/src/plumbing/options/mod.rs index a1f37b08e13..5439b600f42 100644 --- a/src/plumbing/options/mod.rs +++ b/src/plumbing/options/mod.rs @@ -145,6 +145,7 @@ pub enum Subcommands { Corpus(corpus::Platform), MergeBase(merge_base::Command), Merge(merge::Platform), + Env, Diff(diff::Platform), Log(log::Platform), Worktree(worktree::Platform), From 0737c41ab921a8b3228252b5809cb0a0f064bfa6 Mon Sep 17 00:00:00 2001 From: Sebastian Thiel Date: Sat, 11 Jan 2025 17:30:08 +0100 Subject: [PATCH 4/5] More robust localization of `sh.exe` on Windows Here is what @EliahKagan wrote: > While it sometimes works, I don't think this technique is a reliable way of finding the `bash.exe` associated with Git for Windows. If I recall correctly, `installation_config_prefix()` is based on the location of the highest scoped non-local non-worktree configuration file of those that supply at least one configuration variable, except in the case of the `EXEPATH` optimization, which in practice only works in a Git Bash environment (and not always reliably). > > Git for Windows installations usually have variables configured in the system scope, but this is not guaranteed. That scope may also be suppressed by `GIT_CONFIG_NOSYSTEM` or have its associated configuration file set to a custom path by `GIT_CONFIG_SYSTEM`. In any of those cases, we may get no path but, currently, we are more likely to get a different path that would not be correct here, because while the local and worktree scopes are excluded, the global scope is not. > > Although it is valuable to perform fewer subprocess invocations on Windows where they can be slow, I don't think a technique along these lines for finding the `bash.exe` associated with a Git for Windows installation can be made reliable. The only reliable approach I know of is to infer the path from the output of `git --exec-path`, as in #1712. > > (If it is necessary to eke out a little extra performance, then it might be possible to attempt another approach while carefully covering cases where it does not work and checking for signs that it may be inaccurate, while still falling back to an `--exec-path`-based way.) Note that there is no attempt to eke out more performance yet. --- gix-path/src/env/mod.rs | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/gix-path/src/env/mod.rs b/gix-path/src/env/mod.rs index e3ad4b83a8c..fe4fd8e21aa 100644 --- a/gix-path/src/env/mod.rs +++ b/gix-path/src/env/mod.rs @@ -36,17 +36,17 @@ pub fn installation_config_prefix() -> Option<&'static Path> { /// as it could possibly be found in `PATH`. /// Note that the returned path might not be a path on disk. pub fn shell() -> &'static OsStr { - static PATH: Lazy> = Lazy::new(|| { + static PATH: Lazy = Lazy::new(|| { if cfg!(windows) { - installation_config_prefix() - .and_then(|p| p.parent()) + core_dir() + .and_then(|p| p.ancestors().nth(3) /* skip mingw64/libexec/git-core */) .map(|p| p.join("usr").join("bin").join("sh.exe")) - .map(Into::into) + .map_or_else(|| OsString::from("sh"), Into::into) } else { - Some("/bin/sh".into()) + "/bin/sh".into() } }); - PATH.as_deref().unwrap_or(OsStr::new("sh")) + PATH.as_ref() } /// Return the name of the Git executable to invoke it. From 5400320cb8bfa94181abb94fe08321ae188f8555 Mon Sep 17 00:00:00 2001 From: Sebastian Thiel Date: Sun, 12 Jan 2025 08:47:19 +0100 Subject: [PATCH 5/5] Apply suggestions from code review Co-authored-by: Eliah Kagan --- gix-path/src/env/mod.rs | 6 +++--- gix-path/tests/path/env.rs | 4 +++- 2 files changed, 6 insertions(+), 4 deletions(-) diff --git a/gix-path/src/env/mod.rs b/gix-path/src/env/mod.rs index fe4fd8e21aa..71188fb2a17 100644 --- a/gix-path/src/env/mod.rs +++ b/gix-path/src/env/mod.rs @@ -30,7 +30,7 @@ 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 it, and on +/// 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`. @@ -39,7 +39,7 @@ pub fn shell() -> &'static OsStr { static PATH: Lazy = Lazy::new(|| { if cfg!(windows) { core_dir() - .and_then(|p| p.ancestors().nth(3) /* skip mingw64/libexec/git-core */) + .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) } else { @@ -122,7 +122,7 @@ static GIT_CORE_DIR: Lazy> = Lazy::new(|| { } BString::new(output.stdout) - .trim_with(|b| b.is_ascii_whitespace()) + .strip_suffix(b"\n")? .to_path() .ok()? .to_owned() diff --git a/gix-path/tests/path/env.rs b/gix-path/tests/path/env.rs index 157c22e3477..28f23f9cb7b 100644 --- a/gix-path/tests/path/env.rs +++ b/gix-path/tests/path/env.rs @@ -27,7 +27,9 @@ fn installation_config() { #[test] fn core_dir() { assert!( - gix_path::env::core_dir().expect("Git is always in PATH").is_dir(), + gix_path::env::core_dir() + .expect("Git is always in PATH when we run tests") + .is_dir(), "The core directory is a valid directory" ); }