diff --git a/benches/ferrflow_benchmarks.rs b/benches/ferrflow_benchmarks.rs index 727ab7e6..1ab72d39 100644 --- a/benches/ferrflow_benchmarks.rs +++ b/benches/ferrflow_benchmarks.rs @@ -182,7 +182,7 @@ fn bench_config_loading(c: &mut Criterion) { let dir = TempDir::new().unwrap(); let config_path = dir.path().join(".ferrflow"); std::fs::write(&config_path, generate_config_json(num_pkgs)).unwrap(); - std::process::Command::new("git") + ferrflow::git_cmd::command() .args(["init", "-q"]) .current_dir(dir.path()) .output() @@ -200,7 +200,7 @@ fn run_git(dir: &std::path::Path, args: &[&str]) -> String { fn run_git_with_stdin(dir: &std::path::Path, args: &[&str], stdin: Option<&[u8]>) -> String { use std::io::Write; - let mut cmd = std::process::Command::new("git"); + let mut cmd = ferrflow::git_cmd::command(); cmd.current_dir(dir).args(args); cmd.env("GIT_AUTHOR_NAME", "bench"); cmd.env("GIT_AUTHOR_EMAIL", "bench@test.com"); @@ -384,7 +384,7 @@ fn bench_validate(c: &mut Criterion) { .unwrap(); } - std::process::Command::new("git") + ferrflow::git_cmd::command() .args(["init", "-q"]) .current_dir(dir.path()) .output() diff --git a/src/bot_token.rs b/src/bot_token.rs index adbbead4..0e99e30c 100644 --- a/src/bot_token.rs +++ b/src/bot_token.rs @@ -318,11 +318,11 @@ fn configure_bot_git_identity_in(repo_dir: &std::path::Path) { .unwrap_or_else(|| DEFAULT_BOT_USER_ID.to_string()); let email = format!("{user_id}+{login}@users.noreply.github.com"); - let _ = std::process::Command::new("git") + let _ = crate::git_cmd::command() .args(["config", "--local", "user.name", &login]) .current_dir(repo_dir) .status(); - let _ = std::process::Command::new("git") + let _ = crate::git_cmd::command() .args(["config", "--local", "user.email", &email]) .current_dir(repo_dir) .status(); @@ -576,7 +576,7 @@ mod tests { } fn read_local_git_config(repo_dir: &std::path::Path, key: &str) -> Option { - let out = std::process::Command::new("git") + let out = crate::test_utils::git_command() .args(["config", "--local", "--get", key]) .current_dir(repo_dir) .output() @@ -589,7 +589,7 @@ mod tests { } fn init_repo(dir: &std::path::Path) { - let ok = std::process::Command::new("git") + let ok = crate::test_utils::git_command() .args(["init", "-q"]) .current_dir(dir) .status() @@ -600,7 +600,7 @@ mod tests { #[test] fn configure_bot_git_identity_uses_hosted_defaults() { - if std::process::Command::new("git") + if crate::test_utils::git_command() .arg("--version") .status() .is_err() @@ -629,7 +629,7 @@ mod tests { #[test] fn configure_bot_git_identity_honours_env_overrides() { - if std::process::Command::new("git") + if crate::test_utils::git_command() .arg("--version") .status() .is_err() @@ -661,7 +661,7 @@ mod tests { #[test] fn configure_bot_git_identity_treats_blank_overrides_as_unset() { - if std::process::Command::new("git") + if crate::test_utils::git_command() .arg("--version") .status() .is_err() diff --git a/src/doctor/checks.rs b/src/doctor/checks.rs index ba60394d..7239ee9d 100644 --- a/src/doctor/checks.rs +++ b/src/doctor/checks.rs @@ -1,5 +1,4 @@ use std::path::Path; -use std::process::Command; use crate::config::{Config, ForgeKind, GENERIC_TOKEN_ENV_VAR}; use crate::forge::{Probe, resolve_forge}; @@ -463,7 +462,7 @@ pub(super) fn identity_check(identity: Option, on_actions: bool) -> Chec } fn dirty_count(root: &Path) -> Option { - let output = Command::new("git") + let output = crate::git_cmd::command() .current_dir(root) .args(["status", "--porcelain"]) .output() diff --git a/src/formats/gomod.rs b/src/formats/gomod.rs index 380f536d..3b8202ae 100644 --- a/src/formats/gomod.rs +++ b/src/formats/gomod.rs @@ -7,7 +7,7 @@ pub struct GoModVersionFile; impl VersionFile for GoModVersionFile { fn read_version(&self, file_path: &Path) -> Result { - let mut cmd = std::process::Command::new("git"); + let mut cmd = crate::git_cmd::command(); cmd.args([ "describe", "--tags", @@ -74,7 +74,6 @@ impl VersionFile for GoModVersionFile { mod tests { use super::*; use std::path::Path; - use std::process::Command; #[test] fn write_version_is_noop() { @@ -96,9 +95,10 @@ mod tests { "config email", ), (vec!["config", "user.name", "Test"], "config name"), + (vec!["config", "tag.gpgsign", "false"], "config tag.gpgsign"), (vec!["commit", "--allow-empty", "-m", "initial"], "commit"), ] { - let out = Command::new("git") + let out = crate::test_utils::git_command() .args(args) .current_dir(repo) .output() @@ -132,7 +132,7 @@ mod tests { let repo = dir.path(); init_repo(repo); - let tag_out = Command::new("git") + let tag_out = crate::test_utils::git_command() .args(["tag", "v1.2.3"]) .current_dir(repo) .output() diff --git a/src/git/fetch.rs b/src/git/fetch.rs index a9a0835c..5b92dfa0 100644 --- a/src/git/fetch.rs +++ b/src/git/fetch.rs @@ -15,7 +15,7 @@ pub fn fetch_tags(repo: &Repository, remote: Remote<'_>) -> Result<()> { let url = get_remote_url(repo, remote_name) .ok_or_else(|| anyhow!("Remote '{remote_name}' not found"))?; - let mut cmd = std::process::Command::new("git"); + let mut cmd = crate::git_cmd::command(); cmd.current_dir(workdir); configure_git_command(&mut cmd, &url, forge); cmd.args(["fetch", "--tags", remote_name]); diff --git a/src/git/identity.rs b/src/git/identity.rs index c6976f10..4670ed69 100644 --- a/src/git/identity.rs +++ b/src/git/identity.rs @@ -61,10 +61,49 @@ fn ensure_commit_identity_with_env( #[cfg(test)] mod tests { use super::*; + use crate::test_utils::ENV_LOCK; + + const INHERITED_IDENTITY_ENV: [&str; 7] = [ + "GIT_AUTHOR_NAME", + "GIT_AUTHOR_EMAIL", + "GIT_COMMITTER_NAME", + "GIT_COMMITTER_EMAIL", + "EMAIL", + "GIT_CONFIG_PARAMETERS", + "GIT_CONFIG_COUNT", + ]; + + struct IdentityEnvGuard { + _lock: std::sync::MutexGuard<'static, ()>, + saved: Vec<(&'static str, std::ffi::OsString)>, + } + + impl IdentityEnvGuard { + fn clear() -> Self { + let lock = ENV_LOCK.lock().unwrap_or_else(|e| e.into_inner()); + let saved = INHERITED_IDENTITY_ENV + .into_iter() + .filter_map(|var| std::env::var_os(var).map(|v| (var, v))) + .collect(); + for var in INHERITED_IDENTITY_ENV { + unsafe { std::env::remove_var(var) }; + } + Self { _lock: lock, saved } + } + } + + impl Drop for IdentityEnvGuard { + fn drop(&mut self) { + for (var, value) in &self.saved { + unsafe { std::env::set_var(var, value) }; + } + } + } struct IsolatedRepo { dir: tempfile::TempDir, global_config: String, + _env: IdentityEnvGuard, } impl IsolatedRepo { @@ -75,6 +114,7 @@ mod tests { let repo = Self { global_config: global.to_string_lossy().into_owned(), dir, + _env: IdentityEnvGuard::clear(), }; repo.git(&["init", "-q"]); repo.git(&["config", "--local", "user.useConfigOnly", "true"]); diff --git a/src/git/push.rs b/src/git/push.rs index 735a44f2..7bbce97e 100644 --- a/src/git/push.rs +++ b/src/git/push.rs @@ -54,7 +54,7 @@ pub(super) fn remote_tag_target_shas( if tags.is_empty() { return Ok(HashMap::new()); } - let mut cmd = std::process::Command::new("git"); + let mut cmd = crate::git_cmd::command(); cmd.current_dir(workdir); configure_git_command(&mut cmd, push_url, forge); cmd.arg("ls-remote").arg("--tags").arg(push_url); @@ -109,7 +109,7 @@ pub fn verify_remote_branch( let url = get_remote_url(repo, remote_name) .ok_or_else(|| anyhow!("Remote '{remote_name}' has no URL"))?; - let mut cmd = std::process::Command::new("git"); + let mut cmd = crate::git_cmd::command(); cmd.current_dir(workdir); configure_git_command(&mut cmd, &url, forge); cmd.args([ @@ -234,7 +234,7 @@ fn push_branch_once( let source = resolve_push_source(repo, branch); let refspec = format!("{}{source}:refs/heads/{branch}", mode.refspec_prefix()); - let mut cmd = std::process::Command::new("git"); + let mut cmd = crate::git_cmd::command(); cmd.current_dir(workdir); configure_git_command(&mut cmd, &push_url, forge); cmd.arg("push").arg(&push_url).arg(&refspec); @@ -275,7 +275,7 @@ pub fn release_branch_foreign_commit( let url = get_remote_url(repo, remote_name) .ok_or_else(|| anyhow!("Remote '{remote_name}' has no URL"))?; - let mut ls = std::process::Command::new("git"); + let mut ls = crate::git_cmd::command(); ls.current_dir(workdir); configure_git_command(&mut ls, &url, forge); ls.args([ @@ -299,7 +299,7 @@ pub fn release_branch_foreign_commit( return Ok(None); } - let mut fetch = std::process::Command::new("git"); + let mut fetch = crate::git_cmd::command(); fetch.current_dir(workdir); configure_git_command(&mut fetch, &url, forge); fetch.args(["fetch", "--quiet", &url, &format!("refs/heads/{branch}")]); @@ -315,7 +315,7 @@ pub fn release_branch_foreign_commit( .error_code(error_code::GIT_INSPECT_RELEASE_BRANCH); } - let mut log = std::process::Command::new("git"); + let mut log = crate::git_cmd::command(); log.current_dir(workdir); log.args(["log", "--format=%s", &format!("{base}..FETCH_HEAD")]); let log_out = log @@ -420,7 +420,7 @@ fn shell_push_tags( } let prefix = if force { "+" } else { "" }; - let mut cmd = std::process::Command::new("git"); + let mut cmd = crate::git_cmd::command(); cmd.current_dir(workdir); configure_git_command(&mut cmd, &push_url, forge); cmd.arg("push").arg(&push_url); @@ -459,7 +459,7 @@ pub fn reset_branch_to_remote(repo: &Repository, remote: Remote<'_>, branch: &st let push_url = get_remote_url(repo, remote_name) .ok_or_else(|| anyhow!("Remote '{remote_name}' has no URL"))?; - let mut fetch_cmd = std::process::Command::new("git"); + let mut fetch_cmd = crate::git_cmd::command(); fetch_cmd.current_dir(workdir); configure_git_command(&mut fetch_cmd, &push_url, forge); fetch_cmd.arg("fetch").arg(remote_name).arg(format!( @@ -582,7 +582,7 @@ pub fn delete_tag_if_unchanged( Some(_) => {} } - let mut cmd = std::process::Command::new("git"); + let mut cmd = crate::git_cmd::command(); cmd.current_dir(workdir); configure_git_command(&mut cmd, &url, forge); cmd.args(["push", "--delete", &url, &format!("refs/tags/{tag}")]); diff --git a/src/git/shell.rs b/src/git/shell.rs index 1344de73..d9f69c06 100644 --- a/src/git/shell.rs +++ b/src/git/shell.rs @@ -1,6 +1,5 @@ use anyhow::{Context, Result, anyhow}; use std::path::Path; -use std::process::Command; use super::auth::scrub_trace_env; @@ -13,7 +12,7 @@ pub(super) fn run_git_with_env( args: &[&str], extra_env: &[(&str, &str)], ) -> Result { - let mut cmd = Command::new("git"); + let mut cmd = crate::git_cmd::command(); cmd.current_dir(workdir); scrub_trace_env(&mut cmd); for (k, v) in extra_env { diff --git a/src/git/validate.rs b/src/git/validate.rs index e4ae0ad4..4bb5f34a 100644 --- a/src/git/validate.rs +++ b/src/git/validate.rs @@ -80,7 +80,7 @@ mod tests { ensure_safe_refname_fragment(bad, "tag").is_err(), "should reject {bad:?}" ); - let accepted = std::process::Command::new("git") + let accepted = crate::test_utils::git_command() .arg("check-ref-format") .arg(format!("refs/tags/{bad}")) .status() diff --git a/src/git_cmd.rs b/src/git_cmd.rs new file mode 100644 index 00000000..8b00892f --- /dev/null +++ b/src/git_cmd.rs @@ -0,0 +1,80 @@ +use std::process::Command; + +const REPO_LOCATION_ENV: [&str; 10] = [ + "GIT_DIR", + "GIT_WORK_TREE", + "GIT_IMPLICIT_WORK_TREE", + "GIT_COMMON_DIR", + "GIT_INDEX_FILE", + "GIT_OBJECT_DIRECTORY", + "GIT_ALTERNATE_OBJECT_DIRECTORIES", + "GIT_SHALLOW_FILE", + "GIT_GRAFT_FILE", + "GIT_PREFIX", +]; + +pub fn command() -> Command { + let mut cmd = Command::new("git"); + for var in REPO_LOCATION_ENV { + cmd.env_remove(var); + } + cmd +} + +#[cfg(all(test, feature = "cli"))] +mod tests { + use crate::test_utils::{ENV_LOCK, git, init_repo}; + use std::path::Path; + + struct GitDirGuard(Option); + + impl GitDirGuard { + fn set(path: &Path) -> Self { + let previous = std::env::var_os("GIT_DIR"); + unsafe { std::env::set_var("GIT_DIR", path) }; + Self(previous) + } + } + + impl Drop for GitDirGuard { + fn drop(&mut self) { + unsafe { + match &self.0 { + Some(v) => std::env::set_var("GIT_DIR", v), + None => std::env::remove_var("GIT_DIR"), + } + } + } + } + + #[test] + fn an_inherited_git_dir_never_redirects_git_away_from_the_temp_repo() { + let _lock = ENV_LOCK.lock().unwrap_or_else(|e| e.into_inner()); + let decoy = tempfile::tempdir().unwrap(); + git(decoy.path(), &["init", "-q"]); + let decoy_config = decoy.path().join(".git/config"); + let config_before = std::fs::read_to_string(&decoy_config).unwrap(); + + { + let _guard = GitDirGuard::set(&decoy.path().join(".git")); + let initialized = init_repo(); + let git_dir = super::command() + .current_dir(initialized.0.path()) + .args(["rev-parse", "--absolute-git-dir"]) + .output() + .unwrap(); + assert!(git_dir.status.success()); + assert_eq!( + Path::new(String::from_utf8_lossy(&git_dir.stdout).trim()) + .canonicalize() + .unwrap(), + initialized.0.path().join(".git").canonicalize().unwrap() + ); + } + + assert_eq!( + std::fs::read_to_string(&decoy_config).unwrap(), + config_before + ); + } +} diff --git a/src/lib.rs b/src/lib.rs index 32575101..959b8a14 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -5,6 +5,7 @@ pub mod config; pub mod conventional_commits; pub mod error_code; pub mod formats; +pub mod git_cmd; pub mod prerelease; pub mod schema; pub mod uri; @@ -35,7 +36,7 @@ pub mod version_diff; pub mod test_utils { use std::path::Path; use std::process::Command; - use std::sync::Mutex; + use std::sync::{Mutex, OnceLock}; pub static CWD_LOCK: Mutex<()> = Mutex::new(()); pub static ENV_LOCK: Mutex<()> = Mutex::new(()); @@ -52,8 +53,27 @@ pub mod test_utils { result } + pub fn git_command() -> Command { + static LOCAL_ENV_VARS: OnceLock> = OnceLock::new(); + let vars = LOCAL_ENV_VARS.get_or_init(|| { + let out = Command::new("git") + .args(["rev-parse", "--local-env-vars"]) + .output() + .expect("git should be on PATH"); + String::from_utf8_lossy(&out.stdout) + .lines() + .map(str::to_owned) + .collect() + }); + let mut cmd = Command::new("git"); + for var in vars { + cmd.env_remove(var); + } + cmd + } + pub fn git(dir: &Path, args: &[&str]) -> String { - let out = Command::new("git") + let out = git_command() .current_dir(dir) .args(args) .output() @@ -67,7 +87,7 @@ pub mod test_utils { } pub fn git_with_env(dir: &Path, args: &[&str], env: &[(&str, &str)]) -> String { - let mut cmd = Command::new("git"); + let mut cmd = git_command(); cmd.current_dir(dir).args(args); for (k, v) in env { cmd.env(k, v); diff --git a/src/main.rs b/src/main.rs index 7ebeaf37..738c2d07 100644 --- a/src/main.rs +++ b/src/main.rs @@ -14,6 +14,7 @@ mod error_report; mod forge; mod formats; mod git; +mod git_cmd; mod hooks; mod http; mod logging; @@ -72,7 +73,7 @@ fn main() { mod test_utils { use std::path::Path; use std::process::Command; - use std::sync::Mutex; + use std::sync::{Mutex, OnceLock}; pub static CWD_LOCK: Mutex<()> = Mutex::new(()); pub static ENV_LOCK: Mutex<()> = Mutex::new(()); @@ -89,8 +90,27 @@ mod test_utils { result } + pub fn git_command() -> Command { + static LOCAL_ENV_VARS: OnceLock> = OnceLock::new(); + let vars = LOCAL_ENV_VARS.get_or_init(|| { + let out = Command::new("git") + .args(["rev-parse", "--local-env-vars"]) + .output() + .expect("git should be on PATH"); + String::from_utf8_lossy(&out.stdout) + .lines() + .map(str::to_owned) + .collect() + }); + let mut cmd = Command::new("git"); + for var in vars { + cmd.env_remove(var); + } + cmd + } + pub fn git(dir: &Path, args: &[&str]) -> String { - let out = Command::new("git") + let out = git_command() .current_dir(dir) .args(args) .output() @@ -104,7 +124,7 @@ mod test_utils { } pub fn git_with_env(dir: &Path, args: &[&str], env: &[(&str, &str)]) -> String { - let mut cmd = Command::new("git"); + let mut cmd = git_command(); cmd.current_dir(dir).args(args); for (k, v) in env { cmd.env(k, v); diff --git a/src/monorepo/release.rs b/src/monorepo/release.rs index f9ff4894..6b39c136 100644 --- a/src/monorepo/release.rs +++ b/src/monorepo/release.rs @@ -161,7 +161,7 @@ fn cleanup_failed_release_attempt( crate::git::collect_all_tags(&repo).into_iter().collect(); if let Some(workdir) = repo.workdir() { for tag in after.difference(pre_attempt_tags) { - let _ = std::process::Command::new("git") + let _ = crate::git_cmd::command() .current_dir(workdir) .args(["tag", "-d", tag]) .output(); @@ -186,7 +186,7 @@ mod tests { use std::collections::HashSet; fn git(dir: &Path, args: &[&str]) -> String { - let out = std::process::Command::new("git") + let out = crate::test_utils::git_command() .current_dir(dir) .args(args) .env("GIT_AUTHOR_NAME", "t") diff --git a/src/monorepo/run/context.rs b/src/monorepo/run/context.rs index 72b93b59..ffffc240 100644 --- a/src/monorepo/run/context.rs +++ b/src/monorepo/run/context.rs @@ -223,11 +223,9 @@ mod tests { use super::*; fn git(dir: &Path, args: &[&str]) { - let status = std::process::Command::new("git") + let status = crate::test_utils::git_command() .current_dir(dir) .args(args) - .env_remove("GIT_DIR") - .env_remove("GIT_WORK_TREE") .stdout(std::process::Stdio::null()) .stderr(std::process::Stdio::null()) .status() diff --git a/src/monorepo/run/execute_tests.rs b/src/monorepo/run/execute_tests.rs index ff5617ed..e2d8ff2d 100644 --- a/src/monorepo/run/execute_tests.rs +++ b/src/monorepo/run/execute_tests.rs @@ -1,6 +1,5 @@ use std::collections::HashMap; use std::path::{Path, PathBuf}; -use std::process::Command; use std::sync::Mutex; use anyhow::Result; @@ -18,7 +17,7 @@ mod floating_tags; mod release_commit; fn git(dir: &Path, args: &[&str]) -> String { - let out = Command::new("git") + let out = crate::test_utils::git_command() .current_dir(dir) .args(args) .output() diff --git a/src/monorepo/run/lock.rs b/src/monorepo/run/lock.rs index 92a946fc..fbe58cb1 100644 --- a/src/monorepo/run/lock.rs +++ b/src/monorepo/run/lock.rs @@ -253,11 +253,9 @@ mod tests { use super::*; fn git(dir: &Path, args: &[&str]) { - let status = std::process::Command::new("git") + let status = crate::test_utils::git_command() .current_dir(dir) .args(args) - .env_remove("GIT_DIR") - .env_remove("GIT_WORK_TREE") .stdout(std::process::Stdio::null()) .stderr(std::process::Stdio::null()) .status() @@ -370,7 +368,7 @@ mod tests { } fn a_dead_pid() -> u32 { - let mut child = std::process::Command::new("git") + let mut child = crate::test_utils::git_command() .arg("--version") .stdout(std::process::Stdio::null()) .stderr(std::process::Stdio::null()) diff --git a/src/monorepo/run/plan.rs b/src/monorepo/run/plan.rs index 574ee8f4..a62525fe 100644 --- a/src/monorepo/run/plan.rs +++ b/src/monorepo/run/plan.rs @@ -1069,7 +1069,7 @@ mod tests { content.len() )); } - let mut child = std::process::Command::new("git") + let mut child = crate::test_utils::git_command() .current_dir(dir) .args(["fast-import", "--quiet"]) .stdin(std::process::Stdio::piped()) diff --git a/src/monorepo/util.rs b/src/monorepo/util.rs index 888c4c95..b53fc598 100644 --- a/src/monorepo/util.rs +++ b/src/monorepo/util.rs @@ -36,7 +36,7 @@ pub(super) fn collect_dirty_files(repo: &crate::git::Repository) -> HashSet p, None => return files, }; - let output = std::process::Command::new("git") + let output = crate::git_cmd::command() .current_dir(workdir) .args(["status", "--porcelain=v1", "-z", "--no-renames"]) .output(); diff --git a/src/shadow.rs b/src/shadow.rs index 022f882d..3bc4dd2e 100644 --- a/src/shadow.rs +++ b/src/shadow.rs @@ -2,7 +2,6 @@ use anyhow::{Context, Result}; use colored::Colorize; use std::collections::BTreeSet; use std::path::{Path, PathBuf}; -use std::process::Command; use crate::config::Config; use crate::error_code::{self, ErrorCodeExt}; @@ -107,7 +106,7 @@ fn tag_set(dir: &Path) -> Result> { } fn git_stdout(dir: &Path, args: &[&str]) -> Result { - let out = Command::new("git") + let out = crate::git_cmd::command() .current_dir(dir) .args(args) .output()