diff --git a/cli/adopt/BUILD.bazel b/cli/adopt/BUILD.bazel index f3b55edaa9..f7333165bd 100644 --- a/cli/adopt/BUILD.bazel +++ b/cli/adopt/BUILD.bazel @@ -114,6 +114,7 @@ rust_library( "//cli/ide:dx_ide", "//cli/merge:dx_merge", "//cli/path:dx_path", + "//cli/process:dx_process", "//libs/families:dx_families", "//libs/gomod:dx_gomod", "//libs/module_bazel:dx_module_bazel", @@ -249,6 +250,7 @@ rust_test( "//cli/ide:dx_ide", "//cli/merge:dx_merge", "//cli/path:dx_path", + "//cli/process:dx_process", "//cli/test_scratch:dx_test_scratch", "//libs/families:dx_families", "//libs/gomod:dx_gomod", diff --git a/cli/adopt/src/build_output.rs b/cli/adopt/src/build_output.rs index f29b51c819..2ad123426e 100644 --- a/cli/adopt/src/build_output.rs +++ b/cli/adopt/src/build_output.rs @@ -201,19 +201,19 @@ struct Git<'a> { } impl Git<'_> { - fn run(&self, args: &[&str]) -> Option { - let mut cmd = Command::new("git"); - cmd.arg("-C").arg(self.root).args(args); + fn run(&self, args: &[&str]) -> Option { + let mut cmd = dx_process::capture::git(self.root); + cmd.args(args); (self.configure)(&mut cmd); - cmd.output().ok() + dx_process::capture::run_captured(&mut cmd, dx_process::capture::DEFAULT_TIMEOUT).ok() } fn ok_text(&self, args: &[&str]) -> Option { let output = self.run(args)?; - if !output.status.success() { + if !output.success() { return None; } - let text = String::from_utf8_lossy(&output.stdout).trim().to_owned(); + let text = output.stdout_text().trim().to_owned(); (!text.is_empty()).then_some(text) } @@ -221,10 +221,10 @@ impl Git<'_> { let output = self .run(args) .ok_or_else(|| "git did not start".to_owned())?; - if output.status.success() { + if output.success() { return Ok(output.stdout); } - let stderr = String::from_utf8_lossy(&output.stderr); + let stderr = output.stderr_text(); let line = stderr.lines().next().unwrap_or("git failed").trim(); Err(line.to_owned()) } @@ -272,7 +272,7 @@ fn scan_with(root: &Path, configure: &dyn Fn(&mut Command)) -> Result = Vec::new(); @@ -489,10 +489,8 @@ mod tests { } fn git(root: &Path, args: &[&str]) { - let mut cmd = Command::new("git"); - cmd.arg("-C") - .arg(root) - .args([ + let mut cmd = dx_process::capture::git(root); + cmd.args([ "-c", "user.name=t", "-c", @@ -502,8 +500,8 @@ mod tests { ]) .args(args); dx_test_scratch::isolate_git(&mut cmd, sandbox_of(root)); - let status = cmd.status().expect("git"); - assert!(status.success(), "{args:?}"); + dx_process::capture::run_checked(&mut cmd, dx_process::capture::DEFAULT_TIMEOUT) + .unwrap_or_else(|error| panic!("{args:?}: {error}")); } fn write(root: &Path, rel: &str, body: &[u8]) { @@ -651,11 +649,11 @@ mod tests { #[test] fn scan_outside_git_finds_nothing() { let root = temp(); - let version = Command::new("git") - .arg("--version") - .output() - .expect("git runs"); - assert!(version.status.success(), "git is installed"); + dx_process::capture::run_checked( + dx_process::capture::git(&root).arg("--version"), + dx_process::capture::DEFAULT_TIMEOUT, + ) + .expect("git is installed"); write(&root, "target/x", b"x"); assert_eq!(scan_isolated(&root), Ok(ScanReport::default())); } diff --git a/cli/adopt/src/families.rs b/cli/adopt/src/families.rs index f30de9a751..50da2f373a 100644 --- a/cli/adopt/src/families.rs +++ b/cli/adopt/src/families.rs @@ -228,16 +228,7 @@ fn git_files(root: &Path) -> Option> { if !root.join(".git").exists() { return None; } - let mut git = std::process::Command::new("git"); - for var in [ - "GIT_DIR", - "GIT_WORK_TREE", - "GIT_INDEX_FILE", - "GIT_COMMON_DIR", - "GIT_OBJECT_DIRECTORY", - ] { - git.env_remove(var); - } + let mut git = dx_process::capture::git(root); if let Some(parent) = root .canonicalize() .ok() @@ -245,22 +236,18 @@ fn git_files(root: &Path) -> Option> { { git.env("GIT_CEILING_DIRECTORIES", parent); } - let out = git - .arg("-C") - .arg(root) - .args([ + let out = dx_process::capture::run_checked( + git.args([ "ls-files", "-z", "--cached", "--others", "--exclude-standard", - ]) - .output() - .ok()?; - if !out.status.success() { - return None; - } - let text = String::from_utf8_lossy(&out.stdout); + ]), + dx_process::capture::DEFAULT_TIMEOUT, + ) + .ok()?; + let text = out.stdout_text(); Some( text.split('\0') .filter(|path| !path.is_empty()) @@ -338,13 +325,11 @@ mod tests { } fn git(dir: &Path) { - let ok = std::process::Command::new("git") - .arg("-C") - .arg(dir) - .args(["init", "-q"]) - .status() - .is_ok_and(|status| status.success()); - assert!(ok, "git init"); + dx_process::capture::run_checked( + dx_process::capture::git(dir).args(["init", "-q"]), + dx_process::capture::DEFAULT_TIMEOUT, + ) + .expect("git init"); } fn workspace(module: &str, files: &[&str], with_git: bool) -> dx_test_scratch::TempDir { diff --git a/cli/adopt/src/git_changes.rs b/cli/adopt/src/git_changes.rs index 76e4882fe4..dc3417f9ff 100644 --- a/cli/adopt/src/git_changes.rs +++ b/cli/adopt/src/git_changes.rs @@ -98,16 +98,11 @@ pub fn is_dx_state(path: &str) -> bool { /// True when git ignores the generated `.dx` files, so they would not be committed. pub fn dx_config_git_ignored(root: &Path) -> bool { - std::process::Command::new("git") - .arg("-C") - .arg(root) - .args(["check-ignore", "-q", ".dx/BUILD.bazel"]) - .stdin(std::process::Stdio::null()) - .stdout(std::process::Stdio::null()) - .stderr(std::process::Stdio::null()) - .status() - .map(|status| status.code() == Some(0)) - .unwrap_or(false) + dx_process::capture::run_captured( + dx_process::capture::git(root).args(["check-ignore", "-q", ".dx/BUILD.bazel"]), + dx_process::capture::DEFAULT_TIMEOUT, + ) + .is_ok_and(|output| output.success()) } /// Sorts changes by path and drops duplicate paths, keeping rename sources. diff --git a/cli/adopt/src/preset.rs b/cli/adopt/src/preset.rs index 73a4918dff..6b6850c37f 100644 --- a/cli/adopt/src/preset.rs +++ b/cli/adopt/src/preset.rs @@ -208,11 +208,11 @@ fn ignore_with_missing(text: Option) -> Option { /// Whether git lists files under `.dx/bazelrc`; false when git is missing or this is no repository. fn layers_tracked(root: &Path) -> bool { - std::process::Command::new("git") - .args(["ls-files", "--", CONSUMER_DIR]) - .current_dir(root) - .output() - .is_ok_and(|out| out.status.success() && !out.stdout.is_empty()) + dx_process::capture::run_checked( + dx_process::capture::git(root).args(["ls-files", "--", CONSUMER_DIR]), + dx_process::capture::DEFAULT_TIMEOUT, + ) + .is_ok_and(|out| !out.stdout.is_empty()) } fn tracked_note() -> String { @@ -471,10 +471,11 @@ mod tests { } fn git(root: &Path, sandbox: &Path, args: &[&str]) { - let mut cmd = std::process::Command::new("git"); - cmd.arg("-C").arg(root).args(args); + let mut cmd = dx_process::capture::git(root); + cmd.args(args); dx_test_scratch::isolate_git(&mut cmd, sandbox); - assert!(cmd.status().expect("git").success(), "{args:?}"); + dx_process::capture::run_checked(&mut cmd, dx_process::capture::DEFAULT_TIMEOUT) + .unwrap_or_else(|error| panic!("{args:?}: {error}")); } #[test] diff --git a/cli/adopt/src/scaffold.rs b/cli/adopt/src/scaffold.rs index 17798032fe..a467548270 100644 --- a/cli/adopt/src/scaffold.rs +++ b/cli/adopt/src/scaffold.rs @@ -271,14 +271,11 @@ impl CiProvider { if root.join(".github/workflows/ci.yml").exists() { return Self::Github; } - let gitlab_remote = std::process::Command::new("git") - .arg("-C") - .arg(root) - .args(["remote", "get-url", "origin"]) - .output() - .is_ok_and(|out| { - out.status.success() && String::from_utf8_lossy(&out.stdout).contains("gitlab") - }); + let gitlab_remote = dx_process::capture::run_checked( + dx_process::capture::git(root).args(["remote", "get-url", "origin"]), + dx_process::capture::DEFAULT_TIMEOUT, + ) + .is_ok_and(|out| out.stdout_text().contains("gitlab")); if gitlab_remote { Self::Gitlab } else { @@ -292,16 +289,12 @@ pub const DEFAULT_BRANCH_FALLBACK: &str = "main"; /// The repo's default branch: `origin/HEAD`, then `init.defaultBranch`, then `main`. pub fn detect_default_branch(root: &Path) -> String { let git = |args: &[&str]| -> Option { - let out = std::process::Command::new("git") - .arg("-C") - .arg(root) - .args(args) - .output() - .ok()?; - if !out.status.success() { - return None; - } - Some(String::from_utf8_lossy(&out.stdout).trim().to_owned()) + let out = dx_process::capture::run_checked( + dx_process::capture::git(root).args(args), + dx_process::capture::DEFAULT_TIMEOUT, + ) + .ok()?; + Some(out.stdout_text().trim().to_owned()) }; let from_origin = git(&["symbolic-ref", "--short", "refs/remotes/origin/HEAD"]) .and_then(|name| name.strip_prefix("origin/").map(str::to_owned)) @@ -1611,13 +1604,11 @@ mod tests { let dir = std::env::temp_dir().join(format!("dx-adopt-branch-{}", std::process::id())); std::fs::create_dir_all(&dir).expect("temp dir"); let git = |args: &[&str]| { - std::process::Command::new("git") - .arg("-C") - .arg(&dir) - .args(args) - .output() - .map(|o| o.status.success()) - .unwrap_or(false) + dx_process::capture::run_checked( + dx_process::capture::git(&dir).args(args), + dx_process::capture::DEFAULT_TIMEOUT, + ) + .is_ok() }; if git(&["init", "-q"]) && git(&["config", "init.defaultBranch", "trunk"]) { assert_eq!(super::detect_default_branch(&dir), "trunk"); @@ -1636,13 +1627,11 @@ mod tests { let _ = std::fs::remove_dir_all(&dir); std::fs::create_dir_all(&dir).expect("temp dir"); let git = |args: &[&str]| { - std::process::Command::new("git") - .arg("-C") - .arg(&dir) - .args(args) - .output() - .map(|o| o.status.success()) - .unwrap_or(false) + dx_process::capture::run_checked( + dx_process::capture::git(&dir).args(args), + dx_process::capture::DEFAULT_TIMEOUT, + ) + .is_ok() }; if git(&["init", "-q"]) && git(&["config", "init.defaultBranch", "trunk"]) diff --git a/cli/audit/BUILD.bazel b/cli/audit/BUILD.bazel index 8104d518e8..96a23aad70 100644 --- a/cli/audit/BUILD.bazel +++ b/cli/audit/BUILD.bazel @@ -93,6 +93,7 @@ rust_library( ) + [ "//cli/digest:dx_digest", "//cli/dx_config", + "//cli/process:dx_process", "//cli/sbom:dx_sbom", "//libs/gomod:dx_gomod", "//libs/module_bazel:dx_module_bazel", @@ -189,6 +190,7 @@ rust_test( ) + [ "//cli/digest:dx_digest", "//cli/dx_config", + "//cli/process:dx_process", "//cli/sbom:dx_sbom", "//cli/sbom/schema:dx_sbom_schema", "//libs/gomod:dx_gomod", diff --git a/cli/audit/src/image_scan.rs b/cli/audit/src/image_scan.rs index 4b85a11575..a8247bbd42 100644 --- a/cli/audit/src/image_scan.rs +++ b/cli/audit/src/image_scan.rs @@ -10,6 +10,8 @@ use serde::Deserialize; use crate::advisory::{AdvisoryPin, CODE_ADVISORY_REFRESH_FAILED, PIN_MAX_AGE_DAYS, TRIVY_FAMILY}; use crate::exception::RiskException; +const FETCH_TIMEOUT: std::time::Duration = std::time::Duration::from_secs(900); + pub const SEVERITIES: [&str; 4] = ["LOW", "MEDIUM", "HIGH", "CRITICAL"]; /// The exception set that covers every image. @@ -184,13 +186,11 @@ fn fetch_trivy_pin(pin: &AdvisoryPin, cache: &Path, offline: bool) -> Result<(), 1, ) .map_err(|error| error.to_string())?; - let done = Command::new(&plan.program) - .args(&plan.argv) - .status() - .map_err(|error| format!("{}: {error}", plan.program))?; - if !done.success() { - return Err(format!("{}: {} {done}", pin.url, plan.program)); - } + dx_process::capture::run_checked( + Command::new(&plan.program).args(&plan.argv), + FETCH_TIMEOUT, + ) + .map_err(|error| format!("{}: {error}", pin.url))?; let bytes = std::fs::read(&archive).map_err(|error| format!("{}: {error}", pin.url)); let _ = std::fs::remove_file(&archive); bytes? diff --git a/cli/cli/src/adopt/config.rs b/cli/cli/src/adopt/config.rs index bf3895fe92..afacf01288 100644 --- a/cli/cli/src/adopt/config.rs +++ b/cli/cli/src/adopt/config.rs @@ -152,19 +152,12 @@ fn tool_config(tool: &str) -> Result<&'static ToolConfig, String> { } pub(super) fn blob_sha(workspace: &Path, base: &str) -> Result { - let output = std::process::Command::new("git") - .arg("-C") - .arg(workspace) - .args(["hash-object", "--", base]) - .output() - .map_err(|error| format!("cannot run git hash-object: {error}"))?; - if !output.status.success() { - return Err(format!( - "git hash-object {base} failed: {}", - String::from_utf8_lossy(&output.stderr).trim() - )); - } - Ok(String::from_utf8_lossy(&output.stdout).trim().to_owned()) + let output = dx_process::capture::run_checked( + dx_process::capture::git(workspace).args(["hash-object", "--", base]), + dx_process::capture::DEFAULT_TIMEOUT, + ) + .map_err(|error| error.to_string())?; + Ok(output.stdout_text().trim().to_owned()) } /// Copies the base to the tool's root file behind a `dx: override of base ` comment and @@ -394,18 +387,16 @@ mod tests { fn override_copies_the_base_behind_a_header() { let scratch = workspace(FILES); let git = |args: &[&str]| { - std::process::Command::new("git") - .arg("-C") - .arg(scratch.path()) - .args(args) - .output() - .expect("git") + dx_process::capture::run_checked( + dx_process::capture::git(scratch.path()).args(args), + dx_process::capture::DEFAULT_TIMEOUT, + ) + .expect("git") }; git(&["init", "-q"]); let (code, out) = run(scratch.path(), &["config", "override", "hlint"]); assert_eq!(code, 0, "{out}"); - let sha = String::from_utf8(git(&["hash-object", ".dx/config/hlint/base.yaml"]).stdout) - .expect("utf8"); + let sha = git(&["hash-object", ".dx/config/hlint/base.yaml"]).stdout_text(); let text = std::fs::read_to_string(scratch.path().join(".hlint.yaml")).expect("root"); assert_eq!( text, @@ -421,13 +412,11 @@ mod tests { #[test] fn diff_of_a_fresh_override_shows_no_header() { let scratch = workspace(FILES); - let init = std::process::Command::new("git") - .arg("-C") - .arg(scratch.path()) - .args(["init", "-q"]) - .status() - .expect("git"); - assert!(init.success()); + dx_process::capture::run_checked( + dx_process::capture::git(scratch.path()).args(["init", "-q"]), + dx_process::capture::DEFAULT_TIMEOUT, + ) + .expect("git init"); assert_eq!(run(scratch.path(), &["config", "override", "hlint"]).0, 0); let (code, out) = run(scratch.path(), &["config", "diff", "hlint"]); assert_eq!(code, 0, "{out}"); diff --git a/cli/cli/src/adopt/init.rs b/cli/cli/src/adopt/init.rs index 7d65edc17f..17ace9be63 100644 --- a/cli/cli/src/adopt/init.rs +++ b/cli/cli/src/adopt/init.rs @@ -808,13 +808,11 @@ mod tests { fn init_notes_a_gitignore_that_hides_dx_config() { let scratch = dx_test_scratch::scratch("dx-adopt-init-gitignore-config-"); let root = scratch.path().to_path_buf(); - let git = std::process::Command::new("git") - .arg("-C") - .arg(&root) - .args(["init", "-q"]) - .status() - .expect("git"); - assert!(git.success()); + dx_process::capture::run_checked( + dx_process::capture::git(&root).args(["init", "-q"]), + dx_process::capture::DEFAULT_TIMEOUT, + ) + .expect("git init"); std::fs::write(root.join(".gitignore"), ".dx/\n").expect("seed"); let (code, _out, err) = run(&invocation(&["init", "--apply"]), &root); assert_eq!(code, 0, "{err}"); diff --git a/cli/cli/src/adopt/upgrade.rs b/cli/cli/src/adopt/upgrade.rs index 0243cd69b1..565b302364 100644 --- a/cli/cli/src/adopt/upgrade.rs +++ b/cli/cli/src/adopt/upgrade.rs @@ -455,22 +455,14 @@ fn dx_snapshot(workspace: &Path) -> BTreeMap> { } fn git(workspace: &Path, args: &[&str], input: Option<&[u8]>) -> Option> { - let mut child = std::process::Command::new("git") - .arg("-C") - .arg(workspace) - .args(args) - .stdin(std::process::Stdio::piped()) - .stdout(std::process::Stdio::piped()) - .stderr(std::process::Stdio::null()) - .spawn() - .ok()?; - let mut stdin = child.stdin.take()?; - if let Some(bytes) = input { - stdin.write_all(bytes).ok()?; - } - drop(stdin); - let output = child.wait_with_output().ok()?; - output.status.success().then_some(output.stdout) + dx_process::capture::run_captured_with_input( + dx_process::capture::git(workspace).args(args), + input, + dx_process::capture::DEFAULT_TIMEOUT, + ) + .and_then(dx_process::capture::Captured::into_success) + .ok() + .map(|output| output.stdout) } /// The old base an override copy was taken from: the pre-upgrade base when its blob sha @@ -788,13 +780,11 @@ mod tests { let js = "a();\n// eslint-disable-next-line no-console\nb();\n// eslint-disable-next-line no-undef\n"; std::fs::write(root.join("web/a.js"), js).expect("a.js"); let git = |args: &[&str]| { - let status = std::process::Command::new("git") - .arg("-C") - .arg(&root) - .args(args) - .status() - .expect("git"); - assert!(status.success()); + dx_process::capture::run_checked( + dx_process::capture::git(&root).args(args), + dx_process::capture::DEFAULT_TIMEOUT, + ) + .expect("git"); }; git(&["init", "-q"]); git(&["add", "web/a.js"]); @@ -811,16 +801,12 @@ mod tests { const TYPOS_BASE: &str = ".dx/config/typos/base.toml"; fn blob(root: &Path, path: &str) -> String { - let out = std::process::Command::new("git") - .arg("-C") - .arg(root) - .args(["hash-object", "--", path]) - .output() - .expect("git"); - String::from_utf8(out.stdout) - .expect("utf8") - .trim() - .to_owned() + let out = dx_process::capture::run_checked( + dx_process::capture::git(root).args(["hash-object", "--", path]), + dx_process::capture::DEFAULT_TIMEOUT, + ) + .expect("git"); + out.stdout_text().trim().to_owned() } /// A synced workspace whose typos base gained a line after the override was diff --git a/cli/cli/src/exec/audit.rs b/cli/cli/src/exec/audit.rs index 05883c223c..1773b8f7c3 100644 --- a/cli/cli/src/exec/audit.rs +++ b/cli/cli/src/exec/audit.rs @@ -436,16 +436,16 @@ fn fetch_pinned(url: &str) -> Result, String> { 1, ) .map_err(|error| error.to_string())?; - let done = std::process::Command::new(&plan.program) - .args(&plan.argv) - .status() - .map_err(|error| format!("{}: {error}", plan.program))?; - if !done.success() { - return Err(format!("{url}: {} {done}", plan.program)); - } + dx_process::capture::run_checked( + std::process::Command::new(&plan.program).args(&plan.argv), + FETCH_TIMEOUT, + ) + .map_err(|error| format!("{url}: {error}"))?; std::fs::read(&file).map_err(|error| format!("{url}: {error}")) } +const FETCH_TIMEOUT: std::time::Duration = std::time::Duration::from_secs(900); + const NO_DEPENDENCY_SETS: &str = "no dependency sets: this workspace declares no hubs in MODULE.bazel. Add one, for example:\n\ncrate = use_extension(\"@rules_rust//crate_universe:extensions.bzl\", \"crate\")\ncrate.from_cargo(\n name = \"crates\",\n cargo_lockfile = \"//:Cargo.lock\",\n manifests = [\"//:Cargo.toml\"],\n)"; /// Which lockfile each parsed package was read from. diff --git a/cli/cli/src/exec/audit_live.rs b/cli/cli/src/exec/audit_live.rs index 3d4c8b2a79..235bc52a4a 100644 --- a/cli/cli/src/exec/audit_live.rs +++ b/cli/cli/src/exec/audit_live.rs @@ -1267,12 +1267,13 @@ struct RealGit; impl dx_plan::resolve::QueryRunner for RealGit { fn run_query(&self, argv: &[String], cwd: &Path) -> io::Result { assert_eq!(argv[0], "git", "unexpected query {argv:?}"); - let output = std::process::Command::new("git") - .args(&argv[1..]) - .current_dir(cwd) - .output()?; + let output = dx_process::capture::run_captured( + dx_process::capture::git(cwd).args(&argv[1..]), + dx_process::capture::DEFAULT_TIMEOUT, + ) + .map_err(io::Error::other)?; Ok(dx_plan::resolve::QueryResult { - code: output.status.code(), + code: output.code, stdout: output.stdout, stderr: output.stderr, }) @@ -1280,14 +1281,14 @@ impl dx_plan::resolve::QueryRunner for RealGit { } fn git(root: &Path, args: &[&str]) { - let status = std::process::Command::new("git") - .args(["-c", "user.name=dx", "-c", "user.email=dx@example.com"]) - .args(["-c", "commit.gpgsign=false"]) - .args(args) - .current_dir(root) - .status() - .expect("run git"); - assert!(status.success(), "git {args:?} failed"); + dx_process::capture::run_checked( + dx_process::capture::git(root) + .args(["-c", "user.name=dx", "-c", "user.email=dx@example.com"]) + .args(["-c", "commit.gpgsign=false"]) + .args(args), + dx_process::capture::DEFAULT_TIMEOUT, + ) + .unwrap_or_else(|error| panic!("{error}")); } fn real_secret_paths(harness: &Harness) -> (Vec, Option) { diff --git a/cli/cli/src/exec/bench.rs b/cli/cli/src/exec/bench.rs index 0fa7cdd2c8..888c49403f 100644 --- a/cli/cli/src/exec/bench.rs +++ b/cli/cli/src/exec/bench.rs @@ -130,18 +130,12 @@ fn load_baseline( } fn gh_output(gh: &Path, args: &[&str]) -> Result { - let output = std::process::Command::new(gh) - .args(args) - .output() - .map_err(|error| format!("cannot run gh: {error}"))?; - if !output.status.success() { - return Err(format!( - "gh {} failed: {}", - args.join(" "), - String::from_utf8_lossy(&output.stderr).trim() - )); - } - Ok(String::from_utf8_lossy(&output.stdout).into_owned()) + dx_process::capture::run_checked( + std::process::Command::new(gh).args(args), + dx_process::capture::DEFAULT_TIMEOUT, + ) + .map(|output| output.stdout_text()) + .map_err(|error| error.to_string()) } /// Downloads the `dx-bench-` artifact of the latest run on a ref. diff --git a/cli/cli/src/exec/changed_coverage_tests.rs b/cli/cli/src/exec/changed_coverage_tests.rs index 5aef38df65..0ce569a08a 100644 --- a/cli/cli/src/exec/changed_coverage_tests.rs +++ b/cli/cli/src/exec/changed_coverage_tests.rs @@ -350,20 +350,21 @@ fn real_git_counts_renames_untracked_files_and_a_nested_workspace() { } fn run_git(root: &Path, args: &[&str]) { - let status = std::process::Command::new("git") - .args([ - "-c", - "user.name=dx", - "-c", - "user.email=dx@example.com", - "-c", - "commit.gpgsign=false", - ]) - .args(args) - .current_dir(root) - .status() - .expect("git"); - assert!(status.success(), "git {args:?}"); + dx_process::capture::run_checked( + dx_process::capture::git(root) + .args([ + "-c", + "user.name=dx", + "-c", + "user.email=dx@example.com", + "-c", + "commit.gpgsign=false", + ]) + .args(args) + , + dx_process::capture::DEFAULT_TIMEOUT, + ) + .unwrap_or_else(|error| panic!("git {args:?}: {error}")); } #[test] diff --git a/cli/cli/src/exec/generated_drift.rs b/cli/cli/src/exec/generated_drift.rs index 15fab88a27..f631fc4207 100644 --- a/cli/cli/src/exec/generated_drift.rs +++ b/cli/cli/src/exec/generated_drift.rs @@ -115,14 +115,14 @@ mod tests { } fn git(root: &Path, args: &[&str]) { - let status = Command::new("git") - .args(["-c", "user.name=dx", "-c", "user.email=dx@example.com"]) - .args(["-c", "commit.gpgsign=false"]) - .args(args) - .current_dir(root) - .status() - .expect("git runs"); - assert!(status.success(), "git {args:?}"); + dx_process::capture::run_checked( + dx_process::capture::git(root) + .args(["-c", "user.name=dx", "-c", "user.email=dx@example.com"]) + .args(["-c", "commit.gpgsign=false"]) + .args(args), + dx_process::capture::DEFAULT_TIMEOUT, + ) + .unwrap_or_else(|error| panic!("{error}")); } /// A workspace with a python file whose generated `.dx` is committed. diff --git a/cli/cli/src/exec/startup_defaults_launcher_tests.rs b/cli/cli/src/exec/startup_defaults_launcher_tests.rs index 7dfab8acd0..37eb4c9e00 100644 --- a/cli/cli/src/exec/startup_defaults_launcher_tests.rs +++ b/cli/cli/src/exec/startup_defaults_launcher_tests.rs @@ -387,12 +387,11 @@ fn bad_build_slots_exit_two_from_the_binary() { #[test] fn worktree_release_relative_path_uses_the_working_directory() { let ws = sample_workspace("dx-startup-wt-release-ws-"); - let status = std::process::Command::new("git") - .args(["init", "--quiet"]) - .current_dir(ws.path()) - .status() - .expect("git runs"); - assert!(status.success(), "git init the workspace"); + dx_process::capture::run_checked( + dx_process::capture::git(ws.path()).args(["init", "--quiet"]), + dx_process::capture::DEFAULT_TIMEOUT, + ) + .expect("git init the workspace"); let runfiles = dx_test_scratch::scratch("dx-startup-wt-release-runfiles-"); let outside = dx_test_scratch::scratch("dx-startup-wt-release-outside-"); let cache = dx_test_scratch::scratch("dx-startup-wt-release-cache-"); diff --git a/cli/cli/src/exec/umbrella.rs b/cli/cli/src/exec/umbrella.rs index f42a9f64fc..c9698db3da 100644 --- a/cli/cli/src/exec/umbrella.rs +++ b/cli/cli/src/exec/umbrella.rs @@ -1298,16 +1298,11 @@ pub(super) fn lint_commits( } fn git(workspace: &Path, args: &[&str]) -> Option { - let output = std::process::Command::new("git") - .arg("-C") - .arg(workspace) - .args(args) - .output() - .ok()?; - output - .status - .success() - .then(|| String::from_utf8_lossy(&output.stdout).into_owned()) + let mut command = dx_process::capture::git(workspace); + command.args(args); + dx_process::capture::run_checked(&mut command, dx_process::capture::DEFAULT_TIMEOUT) + .ok() + .map(|output| output.stdout_text()) } #[cfg(test)] @@ -1761,13 +1756,11 @@ mod tests { fn check_rejects_build_output_and_names_the_file() { let harness = umbrella_clean("umbrella-check-build-output"); let git = |args: &[&str]| { - let status = std::process::Command::new("git") - .arg("-C") - .arg(&harness.workspace) - .args(args) - .status() - .expect("git"); - assert!(status.success()); + dx_process::capture::run_checked( + dx_process::capture::git(&harness.workspace).args(args), + dx_process::capture::DEFAULT_TIMEOUT, + ) + .unwrap_or_else(|error| panic!("{error}")); }; git(&["init", "-q"]); harness.write_source("cli/target/debug/dx", "binary"); @@ -1780,13 +1773,11 @@ mod tests { fn dirty_build_output(name: &str) -> Harness { let harness = umbrella_clean(name); - let status = std::process::Command::new("git") - .arg("-C") - .arg(&harness.workspace) - .args(["init", "-q"]) - .status() - .expect("git"); - assert!(status.success()); + dx_process::capture::run_checked( + dx_process::capture::git(&harness.workspace).args(["init", "-q"]), + dx_process::capture::DEFAULT_TIMEOUT, + ) + .unwrap_or_else(|error| panic!("{error}")); harness.write_source("cli/target/debug/dx", "binary"); harness } @@ -1879,14 +1870,13 @@ mod tests { fn check_ignores_source_named_target_committed_on_main() { let harness = umbrella_clean("umbrella-check-source-target"); let git = |args: &[&str]| { - let status = std::process::Command::new("git") - .arg("-C") - .arg(&harness.workspace) - .args(["-c", "user.name=t", "-c", "user.email=t@t"]) - .args(args) - .status() - .expect("git"); - assert!(status.success()); + dx_process::capture::run_checked( + dx_process::capture::git(&harness.workspace) + .args(["-c", "user.name=t", "-c", "user.email=t@t"]) + .args(args), + dx_process::capture::DEFAULT_TIMEOUT, + ) + .unwrap_or_else(|error| panic!("{error}")); }; git(&["init", "-q", "-b", "main"]); harness.write_source("src/x/target/Foo.java", "class Foo {}"); @@ -1901,13 +1891,11 @@ mod tests { fn check_reports_a_git_failure_from_the_build_output_check() { let harness = umbrella_clean("umbrella-check-git-failed"); let git = |args: &[&str]| { - let status = std::process::Command::new("git") - .arg("-C") - .arg(&harness.workspace) - .args(args) - .status() - .expect("git"); - assert!(status.success()); + dx_process::capture::run_checked( + dx_process::capture::git(&harness.workspace).args(args), + dx_process::capture::DEFAULT_TIMEOUT, + ) + .unwrap_or_else(|error| panic!("{error}")); }; git(&["init", "-q"]); std::fs::write(harness.workspace.join(".git/index"), b"garbage").expect("index"); @@ -3941,15 +3929,14 @@ mod tests { fn commits_harness(name: &str, config: &str, subject: &str) -> (Harness, Invocation) { let harness = umbrella_clean(name); let git = |args: &[&str]| { - let output = std::process::Command::new("git") - .arg("-C") - .arg(&harness.workspace) - .args(["-c", "user.name=t", "-c", "user.email=t@example.com"]) - .args(["-c", "commit.gpgsign=false"]) - .args(args) - .output() - .expect("git runs"); - assert!(output.status.success(), "{args:?}"); + let output = dx_process::capture::run_checked( + dx_process::capture::git(&harness.workspace) + .args(["-c", "user.name=t", "-c", "user.email=t@example.com"]) + .args(["-c", "commit.gpgsign=false"]) + .args(args), + dx_process::capture::DEFAULT_TIMEOUT, + ) + .unwrap_or_else(|error| panic!("{args:?}: {error}")); String::from_utf8_lossy(&output.stdout).trim().to_owned() }; git(&["init", "-q"]); diff --git a/cli/cli/src/exec/update_outdated.rs b/cli/cli/src/exec/update_outdated.rs index c42ea25d3c..5c5a32862c 100644 --- a/cli/cli/src/exec/update_outdated.rs +++ b/cli/cli/src/exec/update_outdated.rs @@ -193,6 +193,8 @@ fn run(invocation: &Invocation, env: Env<'_>, fetch: &dyn Fetch) -> i32 { exit } +const BAZEL_TIMEOUT: std::time::Duration = std::time::Duration::from_secs(1800); + /// Finds the pinned opam-repository snapshot checkout through the pinned opam wrapper. fn opam_snapshot_dir(workspace: &Path) -> Result { if let Some(dir) = std::env::var_os(OPAM_SNAPSHOT_ENV) { @@ -200,15 +202,19 @@ fn opam_snapshot_dir(workspace: &Path) -> Result { } let mut argv = dx_process::BazelHead::default().argv("run"); argv.extend(["@rules_dx//ocaml/tools:opam", "--", "snapshot-dir"].map(str::to_owned)); - let output = std::process::Command::new(&argv[0]) - .args(&argv[1..]) - .current_dir(workspace) - .output() - .map_err(|error| format!("cannot run bazel for the opam snapshot: {error}"))?; - if !output.status.success() { - return Err("bazel could not provide the opam-repository snapshot".to_owned()); - } - let text = String::from_utf8_lossy(&output.stdout); + let output = dx_process::capture::run_checked( + std::process::Command::new(&argv[0]) + .args(&argv[1..]) + .current_dir(workspace), + BAZEL_TIMEOUT, + ) + .map_err(|error| match error { + dx_process::capture::RunError::Spawn { source, .. } => { + format!("cannot run bazel for the opam snapshot: {source}") + } + _ => "bazel could not provide the opam-repository snapshot".to_owned(), + })?; + let text = output.stdout_text(); match text.lines().last().map(str::trim).filter(|l| !l.is_empty()) { Some(dir) => Ok(PathBuf::from(dir)), None => Err("the opam wrapper printed no snapshot directory".to_owned()), diff --git a/cli/cli/src/exec/update_pr.rs b/cli/cli/src/exec/update_pr.rs index 5eb5e93a89..dcd76bda5d 100644 --- a/cli/cli/src/exec/update_pr.rs +++ b/cli/cli/src/exec/update_pr.rs @@ -325,21 +325,21 @@ fn emit(out: &mut dyn Write, outcomes: &[Outcome], json: bool, exit: i32) -> Res Ok(()) } +const NETWORK_TIMEOUT: std::time::Duration = std::time::Duration::from_secs(600); + pub(super) fn program(program: &Path, root: &Path, args: &[&str]) -> Result { - let output = Command::new(program) - .args(args) - .current_dir(root) - .output() - .map_err(|error| format!("{} {}: {error}", program.display(), args.join(" ")))?; - if !output.status.success() { - return Err(format!( - "{} {} failed: {}", - program.display(), - args.join(" "), - tail(&String::from_utf8_lossy(&output.stderr)) - )); - } - Ok(String::from_utf8_lossy(&output.stdout).into_owned()) + let label = format!("{} {}", program.display(), args.join(" ")); + dx_process::capture::run_checked( + Command::new(program).args(args).current_dir(root), + NETWORK_TIMEOUT, + ) + .map(|output| output.stdout_text()) + .map_err(|error| match error { + dx_process::capture::RunError::Failed { stderr, .. } => { + format!("{label} failed: {}", tail(&stderr)) + } + other => format!("{label}: {other}"), + }) } pub(super) const WORKFLOW_SKIP: &str = "skipped: token cannot push workflow files"; @@ -364,24 +364,21 @@ fn refused_workflow(stderr: &str) -> bool { pub(super) fn push(tools: &Tools, root: &Path, branch: &str) -> Result { let args = ["push", "--force-with-lease", "origin", branch]; - let output = Command::new(&tools.git) - .args(args) - .current_dir(root) - .output() - .map_err(|error| format!("{} {}: {error}", tools.git.display(), args.join(" ")))?; - if output.status.success() { - return Ok(true); - } - let stderr = String::from_utf8_lossy(&output.stderr); - if refused_workflow(&stderr) { - return Ok(false); - } - Err(format!( - "{} {} failed: {}", - tools.git.display(), - args.join(" "), - tail(&stderr) - )) + let label = format!("{} {}", tools.git.display(), args.join(" ")); + let result = dx_process::capture::run_checked( + Command::new(&tools.git).args(args).current_dir(root), + NETWORK_TIMEOUT, + ); + match result { + Ok(_) => Ok(true), + Err(dx_process::capture::RunError::Failed { stderr, .. }) => { + if refused_workflow(&stderr) { + return Ok(false); + } + Err(format!("{label} failed: {}", tail(&stderr))) + } + Err(other) => Err(format!("{label}: {other}")), + } } #[allow(clippy::too_many_arguments)] diff --git a/cli/cli/tests/agents/BUILD.bazel b/cli/cli/tests/agents/BUILD.bazel index fc07f00991..e7520a8b02 100644 --- a/cli/cli/tests/agents/BUILD.bazel +++ b/cli/cli/tests/agents/BUILD.bazel @@ -26,6 +26,7 @@ rust_test( "no-sandbox", ], deps = [ + "//cli/process:dx_process", "//cli/test_scratch:dx_test_scratch", "//tools/testing:dx_testing", ], diff --git a/cli/cli/tests/agents/tests/agents.rs b/cli/cli/tests/agents/tests/agents.rs index ba0d5db191..e8274bb153 100644 --- a/cli/cli/tests/agents/tests/agents.rs +++ b/cli/cli/tests/agents/tests/agents.rs @@ -39,20 +39,21 @@ fn write(path: &Path, text: &str) { } fn git(root: &Path, args: &[&str]) { - let status = Command::new("git") - .args([ - "-c", - "user.name=dx", - "-c", - "user.email=dx@example.com", - "-c", - "commit.gpgsign=false", - ]) - .args(args) - .current_dir(root) - .status() - .unwrap_or_else(|error| panic!("git: {error}")); - assert!(status.success(), "git {args:?}"); + dx_process::capture::run_checked( + dx_process::capture::git(root) + .args([ + "-c", + "user.name=dx", + "-c", + "user.email=dx@example.com", + "-c", + "commit.gpgsign=false", + ]) + .args(args) + , + dx_process::capture::DEFAULT_TIMEOUT, + ) + .unwrap_or_else(|error| panic!("git {args:?}: {error}")); } struct Env { diff --git a/cli/plan/src/platform.rs b/cli/plan/src/platform.rs index 05b95edbb0..e95984f708 100644 --- a/cli/plan/src/platform.rs +++ b/cli/plan/src/platform.rs @@ -93,11 +93,13 @@ fn read_sysctl(key: &str) -> Option { let path = std::env::var_os("PATH").unwrap_or_default(); let mut dirs: Vec<_> = std::env::split_paths(&path).collect(); dirs.extend(["/usr/sbin".into(), "/sbin".into()]); - let output = std::process::Command::new("sysctl") - .args(["-n", key]) - .env("PATH", std::env::join_paths(dirs).ok()?) - .output() - .ok()?; + let output = dx_process::capture::run_captured( + std::process::Command::new("sysctl") + .args(["-n", key]) + .env("PATH", std::env::join_paths(dirs).ok()?), + std::time::Duration::from_secs(5), + ) + .ok()?; String::from_utf8(output.stdout).ok() } diff --git a/cli/process/BUILD.bazel b/cli/process/BUILD.bazel index f8f82aad26..359ce1dc21 100644 --- a/cli/process/BUILD.bazel +++ b/cli/process/BUILD.bazel @@ -10,6 +10,7 @@ rust_library( name = "dx_process", srcs = [ "src/bazel_sh.rs", + "src/capture.rs", "src/groups.rs", "src/lib.rs", "src/lifecycle.rs", @@ -47,6 +48,8 @@ rust_test( srcs = [ "src/bazel_sh.rs", "src/bazel_sh_tests.rs", + "src/capture.rs", + "src/capture_tests.rs", "src/groups.rs", "src/groups_tests.rs", "src/lib.rs", diff --git a/cli/process/src/capture.rs b/cli/process/src/capture.rs new file mode 100644 index 0000000000..027c16ea9d --- /dev/null +++ b/cli/process/src/capture.rs @@ -0,0 +1,222 @@ +//! Bounded subprocess capture and the shared git launcher. + +use std::ffi::OsStr; +use std::fmt; +use std::io; +use std::path::Path; +use std::process::Command; +use std::time::Duration; + +use crate::lifecycle::{ + CapturePolicy, ChildOutcome, CleanupBudget, Descendants, Exit, run_command_with_input, +}; + +/// Bound applied by callers that have no better figure for a short query. +pub const DEFAULT_TIMEOUT: Duration = Duration::from_secs(120); + +const MAX_CAPTURE: usize = 64 << 20; + +const GIT_ENV: [&str; 5] = [ + "GIT_DIR", + "GIT_WORK_TREE", + "GIT_INDEX_FILE", + "GIT_COMMON_DIR", + "GIT_OBJECT_DIRECTORY", +]; + +/// A git command in `cwd` without inherited repository selection or prompts. +pub fn git(cwd: &Path) -> Command { + let mut command = Command::new("git"); + command.current_dir(cwd); + for name in GIT_ENV { + command.env_remove(name); + } + command + .env("GIT_TERMINAL_PROMPT", "0") + .env("GIT_OPTIONAL_LOCKS", "0"); + command +} + +/// What a finished child wrote. +#[derive(Debug, Clone, PartialEq, Eq)] +pub struct Captured { + /// The command line, for messages. + pub command: String, + /// The exit code, or `None` when a signal ended the child. + pub code: Option, + pub stdout: Vec, + pub stderr: Vec, +} + +impl Captured { + pub fn success(&self) -> bool { + self.code == Some(0) + } + + pub fn stdout_text(&self) -> String { + String::from_utf8_lossy(&self.stdout).into_owned() + } + + pub fn stderr_text(&self) -> String { + String::from_utf8_lossy(&self.stderr).trim().to_owned() + } + + /// Turns a non-zero exit into `RunError::Failed`. + pub fn into_success(self) -> Result { + if self.success() { + return Ok(self); + } + Err(RunError::Failed { + stderr: self.stderr_text(), + command: self.command, + code: self.code, + }) + } +} + +/// Why a captured run produced no usable result. +#[derive(Debug)] +pub enum RunError { + Spawn { + command: String, + source: io::Error, + }, + TimedOut { + command: String, + timeout: Duration, + }, + OutputTooLarge { + command: String, + limit: usize, + }, + Failed { + command: String, + code: Option, + stderr: String, + }, +} + +impl RunError { + /// The captured stderr of a failed run, empty for the other kinds. + pub fn stderr(&self) -> &str { + match self { + RunError::Failed { stderr, .. } => stderr, + _ => "", + } + } +} + +impl fmt::Display for RunError { + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { + match self { + RunError::Spawn { command, source } => write!(f, "cannot run {command}: {source}"), + RunError::TimedOut { command, timeout } => { + write!(f, "{command} timed out after {}s", timeout.as_secs()) + } + RunError::OutputTooLarge { command, limit } => { + write!(f, "{command} wrote more than {limit} bytes") + } + RunError::Failed { + command, + code, + stderr, + } => { + let status = match code { + Some(code) => format!("exit {code}"), + None => "a signal".to_owned(), + }; + if stderr.is_empty() { + write!(f, "{command} failed with {status}") + } else { + write!(f, "{command} failed: {stderr}") + } + } + } + } +} + +impl std::error::Error for RunError { + fn source(&self) -> Option<&(dyn std::error::Error + 'static)> { + match self { + RunError::Spawn { source, .. } => Some(source), + _ => None, + } + } +} + +fn describe(command: &Command) -> String { + let program = Path::new(command.get_program()) + .file_name() + .map_or_else(|| command.get_program(), OsStr::new); + std::iter::once(program) + .chain(command.get_args()) + .map(|part| part.to_string_lossy()) + .collect::>() + .join(" ") +} + +/// Runs `command` with stdin null and both output streams captured. +/// +/// The child and its descendants stop once `timeout` passes. A non-zero exit +/// is a result; use `Captured::into_success` or `run_checked` to reject it. +pub fn run_captured(command: &mut Command, timeout: Duration) -> Result { + run_captured_with_input(command, None, timeout) +} + +/// Like `run_captured`, and writes `input` to the child's stdin. +pub fn run_captured_with_input( + command: &mut Command, + input: Option<&[u8]>, + timeout: Duration, +) -> Result { + let label = describe(command); + let capture = CapturePolicy { + max_bytes: MAX_CAPTURE, + }; + let outcome = run_command_with_input( + command, + input, + &capture, + timeout, + Descendants::Owned, + CleanupBudget::STANDARD, + &|| None, + ) + .map_err(|source| RunError::Spawn { + command: label.clone(), + source, + })?; + match outcome { + ChildOutcome::Finished { + exit, + stdout, + stderr, + } => Ok(Captured { + command: label, + code: match exit { + Exit::Code(code) => Some(code), + Exit::Signal(_) => None, + }, + stdout, + stderr, + }), + ChildOutcome::TimedOut => Err(RunError::TimedOut { + command: label, + timeout, + }), + ChildOutcome::OutputTooLarge { limit } => Err(RunError::OutputTooLarge { + command: label, + limit, + }), + } +} + +/// Like `run_captured`, and a non-zero exit is `RunError::Failed`. +pub fn run_checked(command: &mut Command, timeout: Duration) -> Result { + run_captured(command, timeout)?.into_success() +} + +#[cfg(test)] +#[allow(clippy::disallowed_methods)] +#[path = "capture_tests.rs"] +mod capture_tests; diff --git a/cli/process/src/capture_tests.rs b/cli/process/src/capture_tests.rs new file mode 100644 index 0000000000..4dd287bb79 --- /dev/null +++ b/cli/process/src/capture_tests.rs @@ -0,0 +1,133 @@ +use super::*; +use std::time::Instant; + +fn probe(flags: &[&str]) -> Command { + let mut command = Command::new(dx_testing::process_probe()); + command.args(flags); + command +} + +#[test] +fn captures_both_streams_and_the_exit_code() { + let captured = run_captured( + &mut probe(&["--stdout-text=out", "--stderr-text=err", "--exit-code=3"]), + Duration::from_secs(30), + ) + .expect("runs"); + assert_eq!(captured.code, Some(3)); + assert!(!captured.success()); + assert_eq!(captured.stdout_text(), "out"); + assert_eq!(captured.stderr_text(), "err"); +} + +#[test] +fn checked_run_reports_stderr_of_a_failure() { + let error = run_checked( + &mut probe(&["--stderr-text= boom\n", "--exit-code=2"]), + Duration::from_secs(30), + ) + .expect_err("non-zero exit fails"); + assert_eq!(error.stderr(), "boom"); + let text = error.to_string(); + assert!(text.ends_with("failed: boom"), "{text}"); +} + +#[test] +fn a_failure_without_stderr_names_the_exit_code() { + let error = run_checked(&mut probe(&["--exit-code=4"]), Duration::from_secs(30)) + .expect_err("non-zero exit fails"); + assert!(error.to_string().ends_with("failed with exit 4"), "{error}"); +} + +#[test] +fn checked_run_returns_output_of_a_success() { + let captured = run_checked(&mut probe(&["--stdout-text=ok"]), Duration::from_secs(30)) + .expect("zero exit passes"); + assert_eq!(captured.stdout_text(), "ok"); +} + +#[test] +fn a_sleeping_child_times_out_and_is_stopped() { + let started = Instant::now(); + let error = run_captured( + &mut probe(&["--sleep-ms=60000"]), + Duration::from_millis(300), + ) + .expect_err("a sleeping child must time out"); + assert!(matches!(error, RunError::TimedOut { .. }), "{error}"); + assert!(error.to_string().contains("timed out after 0s"), "{error}"); + assert!(started.elapsed() < Duration::from_secs(30)); +} + +#[test] +fn a_missing_program_is_a_spawn_error() { + let error = run_captured( + &mut Command::new("/nonexistent-dx-program"), + Duration::from_secs(5), + ) + .expect_err("cannot spawn"); + assert!(matches!(error, RunError::Spawn { .. }), "{error}"); + assert!( + error + .to_string() + .starts_with("cannot run nonexistent-dx-program: "), + "{error}" + ); + assert!(std::error::Error::source(&error).is_some()); +} + +#[test] +fn output_past_the_bound_is_rejected() { + let error = run_captured( + &mut probe(&["--stdout-bytes=70000000"]), + Duration::from_secs(60), + ) + .expect_err("too large"); + assert!(matches!(error, RunError::OutputTooLarge { .. }), "{error}"); + assert!(error.to_string().contains("wrote more than"), "{error}"); +} + +#[test] +fn git_drops_repository_selection_and_disables_prompts() { + let command = git(Path::new("/")); + let env: std::collections::BTreeMap<_, _> = command + .get_envs() + .map(|(key, value)| (key.to_string_lossy().into_owned(), value.is_some())) + .collect(); + for name in GIT_ENV { + assert_eq!(env.get(name), Some(&false), "{name}"); + } + assert_eq!(env.get("GIT_TERMINAL_PROMPT"), Some(&true)); + assert_eq!(env.get("GIT_OPTIONAL_LOCKS"), Some(&true)); + assert_eq!(command.get_current_dir(), Some(Path::new("/"))); +} + +#[test] +fn git_runs_in_the_given_directory() { + let scratch = dx_test_scratch::scratch("dx-process-git-"); + let mut command = git(scratch.path()); + command.args(["init", "-q"]); + dx_test_scratch::isolate_git(&mut command, scratch.path()); + run_checked(&mut command, DEFAULT_TIMEOUT).expect("init"); + assert!(scratch.path().join(".git").is_dir()); +} + +#[test] +fn input_reaches_stdin_of_the_child() { + let scratch = dx_test_scratch::scratch("dx-process-input-"); + let mut init = git(scratch.path()); + init.args(["init", "-q"]); + dx_test_scratch::isolate_git(&mut init, scratch.path()); + run_checked(&mut init, DEFAULT_TIMEOUT).expect("init"); + let mut hash = git(scratch.path()); + hash.args(["hash-object", "--stdin"]); + dx_test_scratch::isolate_git(&mut hash, scratch.path()); + let captured = run_captured_with_input(&mut hash, Some(b"hello\n"), DEFAULT_TIMEOUT) + .expect("hash") + .into_success() + .expect("zero exit"); + assert_eq!( + captured.stdout_text().trim(), + "ce013625030ba8dba906f756967f9e9ca394464a" + ); +} diff --git a/cli/process/src/lib.rs b/cli/process/src/lib.rs index 926bcdc52e..24eb47f059 100644 --- a/cli/process/src/lib.rs +++ b/cli/process/src/lib.rs @@ -15,6 +15,7 @@ use std::process::Command; use std::sync::atomic::{AtomicI32, Ordering}; pub mod bazel_sh; +pub mod capture; pub mod groups; pub mod lifecycle; #[cfg(unix)] diff --git a/cli/process/src/lifecycle.rs b/cli/process/src/lifecycle.rs index 68afde2c86..c64cf475b3 100644 --- a/cli/process/src/lifecycle.rs +++ b/cli/process/src/lifecycle.rs @@ -189,12 +189,37 @@ pub fn run_command_until( descendants: Descendants, cleanup: CleanupBudget, stop: &dyn Fn() -> Option, +) -> io::Result { + run_command_with_input(command, None, capture, timeout, descendants, cleanup, stop) +} + +/// Like `run_command_until`, and feeds `input` to the child's stdin, then closes it. +/// Without input, stdin is null. A child that stops reading early is not an error. +pub fn run_command_with_input( + command: &mut Command, + input: Option<&[u8]>, + capture: &CapturePolicy, + timeout: Duration, + descendants: Descendants, + cleanup: CleanupBudget, + stop: &dyn Fn() -> Option, ) -> io::Result { command - .stdin(Stdio::null()) + .stdin(if input.is_some() { + Stdio::piped() + } else { + Stdio::null() + }) .stdout(Stdio::piped()) .stderr(Stdio::piped()); let mut owned = backend::spawn_owned(command, descendants)?; + if let (Some(input), Some(mut stdin)) = (input, owned.take_stdin()) { + let input = input.to_vec(); + std::thread::spawn(move || { + use std::io::Write as _; + let _ = stdin.write_all(&input); + }); + } let limit = capture.max_bytes; let stdout_rx = drain_pipe(owned.take_stdout(), limit); let stderr_rx = drain_pipe(owned.take_stderr(), limit); diff --git a/cli/process/src/lifecycle_unix.rs b/cli/process/src/lifecycle_unix.rs index 6971648315..640ed1d097 100644 --- a/cli/process/src/lifecycle_unix.rs +++ b/cli/process/src/lifecycle_unix.rs @@ -22,6 +22,10 @@ impl OwnedChild { self.child.try_wait() } + pub fn take_stdin(&mut self) -> Option { + self.child.stdin.take() + } + pub fn take_stdout(&mut self) -> Option { self.child.stdout.take() } diff --git a/cli/process/src/lifecycle_windows.rs b/cli/process/src/lifecycle_windows.rs index 3510b1a112..fbcb09aa0d 100644 --- a/cli/process/src/lifecycle_windows.rs +++ b/cli/process/src/lifecycle_windows.rs @@ -328,6 +328,10 @@ impl OwnedChild { self.child.try_wait() } + pub fn take_stdin(&mut self) -> Option { + self.child.stdin.take() + } + pub fn take_stdout(&mut self) -> Option { self.child.stdout.take() } diff --git a/cli/tool_resolve/BUILD.bazel b/cli/tool_resolve/BUILD.bazel index 2e300719da..ba010202f3 100644 --- a/cli/tool_resolve/BUILD.bazel +++ b/cli/tool_resolve/BUILD.bazel @@ -18,7 +18,7 @@ rust_library( deps = all_crate_deps( package_name = "cli/dx_tool_resolve", normal = True, - ), + ) + ["//cli/process:dx_process"], ) rust_test( @@ -39,7 +39,7 @@ rust_test( package_name = "cli/dx_tool_resolve", normal = True, normal_dev = True, - ), + ) + ["//cli/process:dx_process"], ) rust_binary( diff --git a/cli/tool_resolve/src/lib.rs b/cli/tool_resolve/src/lib.rs index 5c829db86f..f5ccce9767 100644 --- a/cli/tool_resolve/src/lib.rs +++ b/cli/tool_resolve/src/lib.rs @@ -54,6 +54,17 @@ pub struct CommandBazel { pub dir: PathBuf, } +const BAZEL_TIMEOUT: std::time::Duration = std::time::Duration::from_secs(1800); + +fn run_detail(bin: &Path, error: &dx_process::capture::RunError) -> String { + match error { + dx_process::capture::RunError::Spawn { source, .. } => { + format!("could not run {}: {source}", bin.display()) + } + other => other.to_string(), + } +} + impl Bazel for CommandBazel { fn cquery_files(&self, label: &str, args: &[String]) -> Result { let mut argv = vec![ @@ -62,21 +73,21 @@ impl Bazel for CommandBazel { label.to_owned(), ]; argv.extend(args.iter().cloned()); - let output = Command::new(&self.bin) - .args(&argv) - .current_dir(&self.dir) - .output() - .map_err(|error| ResolveError::CqueryFailed { - label: label.to_owned(), - detail: format!("could not run {}: {error}", self.bin.display()), - })?; - if !output.status.success() { + let output = dx_process::capture::run_captured( + Command::new(&self.bin).args(&argv).current_dir(&self.dir), + BAZEL_TIMEOUT, + ) + .map_err(|error| ResolveError::CqueryFailed { + label: label.to_owned(), + detail: run_detail(&self.bin, &error), + })?; + if !output.success() { return Err(ResolveError::CqueryFailed { label: label.to_owned(), - detail: exit_detail(output.status.code(), &output.stderr), + detail: exit_detail(output.code, &output.stderr), }); } - Ok(String::from_utf8_lossy(&output.stdout).into_owned()) + Ok(output.stdout_text()) } fn output_base(&self) -> Result { @@ -94,15 +105,17 @@ impl CommandBazel { key: key.to_owned(), detail, }; - let output = Command::new(&self.bin) - .args(["info", key]) - .current_dir(&self.dir) - .output() - .map_err(|error| failed(format!("could not run {}: {error}", self.bin.display())))?; - if !output.status.success() { - return Err(failed(exit_detail(output.status.code(), &output.stderr))); + let output = dx_process::capture::run_captured( + Command::new(&self.bin) + .args(["info", key]) + .current_dir(&self.dir), + BAZEL_TIMEOUT, + ) + .map_err(|error| failed(run_detail(&self.bin, &error)))?; + if !output.success() { + return Err(failed(exit_detail(output.code, &output.stderr))); } - let value = String::from_utf8_lossy(&output.stdout).trim().to_owned(); + let value = output.stdout_text().trim().to_owned(); if value.is_empty() { return Err(failed(format!("bazel info {key} printed nothing"))); } diff --git a/cli/worktree/src/git.rs b/cli/worktree/src/git.rs index 743b343c75..43b4fb359b 100644 --- a/cli/worktree/src/git.rs +++ b/cli/worktree/src/git.rs @@ -1,6 +1,6 @@ use std::ffi::OsStr; use std::path::{Path, PathBuf}; -use std::process::{Command, Stdio}; +use std::process::Command; use std::time::Duration; use dx_process::lifecycle::{ @@ -9,45 +9,34 @@ use dx_process::lifecycle::{ use super::WorktreeError; -const GIT_ENV: [&str; 4] = [ - "GIT_DIR", - "GIT_WORK_TREE", - "GIT_INDEX_FILE", - "GIT_COMMON_DIR", -]; +/// Bound for slot-changing git runs such as checkouts of large trees. +const WRITE_TIMEOUT: Duration = Duration::from_secs(1800); /// Bound for the per-slot status reads. A timeout keeps the slot. const READ_TIMEOUT: Duration = Duration::from_secs(120); fn command(cwd: &Path) -> Command { - let mut command = Command::new("git"); - command.current_dir(cwd); - for name in GIT_ENV { - command.env_remove(name); - } - command - .env("GIT_TERMINAL_PROMPT", "0") - .env("GIT_OPTIONAL_LOCKS", "0") - .stdin(Stdio::null()); // Tests must not see the developer's global hooks, `insteadOf` or signing config. #[cfg(test)] - super::tests::isolate(&mut command); - command + { + let mut command = dx_process::capture::git(cwd); + super::tests::isolate(&mut command); + command + } + #[cfg(not(test))] + dx_process::capture::git(cwd) } /// Runs git and returns stdout, or the failure with its stderr. pub(super) fn run>(cwd: &Path, args: &[S]) -> Result { - let output = command(cwd) - .args(args) - .output() - .map_err(|error| WorktreeError::git(args, &format!("cannot start git: {error}")))?; - if output.status.success() { - return Ok(String::from_utf8_lossy(&output.stdout).into_owned()); + let mut cmd = command(cwd); + cmd.args(args); + let output = dx_process::capture::run_captured(&mut cmd, WRITE_TIMEOUT) + .map_err(|error| WorktreeError::git(args, &error.to_string()))?; + if output.success() { + return Ok(output.stdout_text()); } - Err(WorktreeError::git( - args, - String::from_utf8_lossy(&output.stderr).trim(), - )) + Err(WorktreeError::git(args, &output.stderr_text())) } /// Like `run`, for reads only: stops the child once `stop` returns a signal @@ -98,46 +87,25 @@ pub(super) fn line(cwd: &Path, args: &[&str]) -> Result { /// Reports whether git exits zero. pub(super) fn succeeds(cwd: &Path, args: &[&str]) -> bool { - command(cwd) - .args(args) - .stdout(Stdio::null()) - .stderr(Stdio::null()) - .status() - .map(|status| status.success()) - .unwrap_or(false) + let mut cmd = command(cwd); + cmd.args(args); + dx_process::capture::run_captured(&mut cmd, WRITE_TIMEOUT).is_ok_and(|output| output.success()) } -/// Runs git with data on stdin and returns stdout. Feeds stdin concurrently so large input cannot deadlock. +/// Runs git with data on stdin and returns stdout. A zero or one exit is a result. pub(super) fn run_with_input( cwd: &Path, args: &[&str], input: &[u8], ) -> Result { - use std::io::Write as _; - let mut child = command(cwd) - .args(args) - .stdin(Stdio::piped()) - .stdout(Stdio::piped()) - .stderr(Stdio::piped()) - .spawn() - .map_err(|error| WorktreeError::git(args, &format!("cannot start git: {error}")))?; - let stdin = child.stdin.take(); - let output = std::thread::scope(|scope| { - scope.spawn(move || { - if let Some(mut stdin) = stdin { - let _ = stdin.write_all(input); - } - }); - child.wait_with_output() - }) - .map_err(|error| WorktreeError::git(args, &error.to_string()))?; - if output.status.success() || output.status.code() == Some(1) { - return Ok(String::from_utf8_lossy(&output.stdout).into_owned()); + let mut cmd = command(cwd); + cmd.args(args); + let output = dx_process::capture::run_captured_with_input(&mut cmd, Some(input), WRITE_TIMEOUT) + .map_err(|error| WorktreeError::git(args, &error.to_string()))?; + if output.success() || output.code == Some(1) { + return Ok(output.stdout_text()); } - Err(WorktreeError::git( - args, - String::from_utf8_lossy(&output.stderr).trim(), - )) + Err(WorktreeError::git(args, &output.stderr_text())) } /// One entry of `git worktree list --porcelain`. diff --git a/cli/worktree/src/tests.rs b/cli/worktree/src/tests.rs index bf9e73910a..e62c38c6b3 100644 --- a/cli/worktree/src/tests.rs +++ b/cli/worktree/src/tests.rs @@ -337,20 +337,15 @@ pub(super) fn isolate(cmd: &mut std::process::Command) { } fn git(dir: &Path, args: &[&str]) -> String { - let mut command = std::process::Command::new("git"); + let mut command = dx_process::capture::git(dir); command - .current_dir(dir) .args(["-c", "user.name=t", "-c", "user.email=t@example.com"]) .args(["-c", "commit.gpgsign=false", "-c", "tag.gpgsign=false"]) .args(args); isolate(&mut command); - let output = command.output().expect("git runs"); - assert!( - output.status.success(), - "git {args:?}: {}", - String::from_utf8_lossy(&output.stderr) - ); - String::from_utf8_lossy(&output.stdout).trim().to_owned() + let output = dx_process::capture::run_checked(&mut command, dx_process::capture::DEFAULT_TIMEOUT) + .unwrap_or_else(|error| panic!("git {args:?}: {error}")); + output.stdout_text().trim().to_owned() } struct Fixture { @@ -5853,12 +5848,12 @@ fn a_hostile_global_git_config_does_not_break_add_or_release() { ) .expect("config"); // Only this process-wide-free check: the isolated command ignores the hostile file. - let mut command = std::process::Command::new("git"); + let mut command = dx_process::capture::git(&fixture.main); command.args(["config", "--get", "commit.gpgsign"]); - command.current_dir(&fixture.main); command.env("GIT_CONFIG_GLOBAL", &config); isolate(&mut command); - let output = command.output().expect("git"); + let output = dx_process::capture::run_captured(&mut command, dx_process::capture::DEFAULT_TIMEOUT) + .expect("git"); assert!(output.stdout.is_empty(), "isolation overrides the config"); let pool = fixture.pool(); let host = FakeHost::new(); diff --git a/deploy/release/BUILD.bazel b/deploy/release/BUILD.bazel index c7a19a0f06..ca221ef56a 100644 --- a/deploy/release/BUILD.bazel +++ b/deploy/release/BUILD.bazel @@ -76,6 +76,7 @@ rust_library( "//cli/audit:dx_audit", "//cli/digest:dx_digest", "//cli/fingerprint:dx_fingerprint", + "//cli/process:dx_process", "//cli/sbom:dx_sbom", "//deploy/rules:dx_deploy_tools", "//libs/binfmt:dx_binfmt", @@ -131,6 +132,7 @@ rust_test( "//cli/audit:dx_audit", "//cli/digest:dx_digest", "//cli/fingerprint:dx_fingerprint", + "//cli/process:dx_process", "//cli/sbom:dx_sbom", "//cli/sbom/schema:dx_sbom_schema", "//deploy/rules:dx_deploy_tools", @@ -200,6 +202,7 @@ rust_test( ) + [ ":dx_release_tools", "//cli/digest:dx_digest", + "//cli/process:dx_process", "//deploy/rules:dx_deploy_tools", ], ) @@ -339,6 +342,7 @@ rust_binary( ) + [ ":dx_release_tools", "//cli/digest:dx_digest", + "//cli/process:dx_process", ], ) diff --git a/deploy/release/candidate_test.rs b/deploy/release/candidate_test.rs index 8f47f15069..00e358c092 100644 --- a/deploy/release/candidate_test.rs +++ b/deploy/release/candidate_test.rs @@ -1,5 +1,4 @@ use std::path::{Path, PathBuf}; -use std::process::Command; use std::sync::OnceLock; use dx_release_tools::candidate::{ @@ -44,7 +43,8 @@ fn git_home() -> &'static Path { fn git(dir: &Path, args: &[&str]) -> String { let home = git_home(); - let out = Command::new("git") + let out = dx_process::capture::run_checked( + dx_process::capture::git(dir) .env("HOME", home) .env("GIT_CONFIG_NOSYSTEM", "1") .env("GIT_CONFIG_GLOBAL", home.join(".gitconfig")) @@ -61,15 +61,10 @@ fn git(dir: &Path, args: &[&str]) -> String { "-c", "init.defaultBranch=main", ]) - .args(args) - .current_dir(dir) - .output() - .expect("spawn git"); - assert!( - out.status.success(), - "git {args:?} failed: {}", - String::from_utf8_lossy(&out.stderr) - ); + .args(args), + dx_process::capture::DEFAULT_TIMEOUT, + ) + .unwrap_or_else(|error| panic!("git {args:?}: {error}")); String::from_utf8(out.stdout).expect("utf8") } diff --git a/deploy/release/src/bin_bump_version.rs b/deploy/release/src/bin_bump_version.rs index eed2d622d0..9b04cddbff 100644 --- a/deploy/release/src/bin_bump_version.rs +++ b/deploy/release/src/bin_bump_version.rs @@ -10,7 +10,7 @@ use std::fs; use std::path::{Path, PathBuf}; -use std::process::Command; +use dx_process::capture::{DEFAULT_TIMEOUT, run_checked}; const USAGE: &str = "usage: bazel run //deploy/release:bump_version -- [--workflow-ref ]\n\nSets the rules_dx version and the starter workflow pin in every file that carries them, including each families/*/MODULE.bazel, and refreshes tools/ci/starter_pin.sha256 from the workflows in the checkout.\n--workflow-ref defaults to origin/main and must have the affected_on_pr input."; const CONSUMER_WORKFLOW: &str = ".github/workflows/reusable-consumer.yml"; @@ -201,20 +201,12 @@ fn bump(root: &Path, version: &str, commit: &str) -> Result { } fn git(root: &Path, args: &[&str]) -> Result { - let out = Command::new("git") - .arg("-C") - .arg(root) - .args(args) - .output() - .map_err(|e| format!("git: {e}"))?; - if !out.status.success() { - return Err(format!( - "git {}: {}", - args.join(" "), - String::from_utf8_lossy(&out.stderr).trim() - )); - } - Ok(String::from_utf8_lossy(&out.stdout).trim().to_owned()) + let out = run_checked( + dx_process::capture::git(root).args(args), + DEFAULT_TIMEOUT, + ) + .map_err(|e| format!("git {}: {e}", args.join(" ")))?; + Ok(out.stdout_text().trim().to_owned()) } fn has_affected_on_pr(workflow: &str) -> bool { diff --git a/deploy/release/src/candidate.rs b/deploy/release/src/candidate.rs index 8539121be6..12ea6e3462 100644 --- a/deploy/release/src/candidate.rs +++ b/deploy/release/src/candidate.rs @@ -3,7 +3,9 @@ use std::collections::BTreeMap; use std::io::{self, BufRead, Read, Write}; use std::path::{Path, PathBuf}; -use std::process::{Command, Output, Stdio}; +use std::process::Stdio; + +use dx_process::capture::{self, Captured, DEFAULT_TIMEOUT, RunError, run_captured}; use crate::{ BCR_WANT_MODULE, GitState, RELEASE_REMOTE, RELEASE_REPOSITORY, bcr_candidate_version_error, @@ -85,16 +87,19 @@ pub struct SourceArchive { } /// Returns stdout of a git command, or its failure as an error. -pub fn git_output_of(args: &[&str], output: io::Result) -> Result { +pub fn git_output_of(args: &[&str], output: Result) -> Result { let command = format!("git {}", args.join(" ")); match output { - Ok(out) if out.status.success() => { + Ok(out) if out.success() => { String::from_utf8(out.stdout).map_err(|_| format!("{command} printed non-UTF-8 output")) } Ok(out) => { - let stderr = String::from_utf8_lossy(&out.stderr).trim().to_owned(); + let stderr = out.stderr_text(); let detail = if stderr.is_empty() { - out.status.to_string() + match out.code { + Some(code) => format!("exit status: {code}"), + None => "terminated by signal".to_owned(), + } } else { stderr }; @@ -106,12 +111,9 @@ pub fn git_output_of(args: &[&str], output: io::Result) -> Result, args: &[&str]) -> Result { - let mut cmd = Command::new("git"); + let mut cmd = capture::git(dir.unwrap_or(Path::new("."))); cmd.args(args); - if let Some(dir) = dir { - cmd.current_dir(dir); - } - git_output_of(args, cmd.output()) + git_output_of(args, run_captured(&mut cmd, DEFAULT_TIMEOUT)) } pub fn ls_remote_output( @@ -214,9 +216,8 @@ fn git_pipe( read: impl FnOnce(io::BufReader) -> Result, ) -> Result { let command = format!("git {}", args.join(" ")); - let mut child = Command::new("git") + let mut child = capture::git(repo) .args(args) - .current_dir(repo) .stdin(Stdio::piped()) .stdout(Stdio::piped()) .stderr(Stdio::piped()) @@ -1181,36 +1182,32 @@ mod tests { ); } - #[cfg(unix)] #[test] fn git_failure_is_an_error_not_clean() { - use std::os::unix::process::ExitStatusExt; - let failed = Output { - status: std::process::ExitStatus::from_raw(256), - stdout: Vec::new(), - stderr: b"fatal: not a git repository\n".to_vec(), + let captured = |code: Option, stdout: Vec, stderr: &[u8]| Captured { + command: "git".to_owned(), + code, + stdout, + stderr: stderr.to_vec(), }; + let failed = captured(Some(1), Vec::new(), b"fatal: not a git repository\n"); assert_eq!( git_output_of(&["status", "--porcelain"], Ok(failed)), Err("git status --porcelain failed: fatal: not a git repository".to_owned()) ); - let silent = Output { - status: std::process::ExitStatus::from_raw(256), - stdout: Vec::new(), - stderr: Vec::new(), - }; + let silent = captured(Some(1), Vec::new(), b""); let err = git_output_of(&["tag"], Ok(silent)).expect_err("silent failure"); assert!(err.starts_with("git tag failed: "), "{err}"); - let spawn = io::Error::new(io::ErrorKind::NotFound, "no git"); - assert_eq!( - git_output_of(&["tag"], Err(spawn)), - Err("git tag failed: no git".to_owned()) - ); - let binary = Output { - status: std::process::ExitStatus::from_raw(0), - stdout: vec![0xff], - stderr: Vec::new(), + let spawn = RunError::Spawn { + command: "git tag".to_owned(), + source: io::Error::new(io::ErrorKind::NotFound, "no git"), }; + assert!( + git_output_of(&["tag"], Err(spawn)) + .expect_err("spawn failure") + .starts_with("git tag failed: ") + ); + let binary = captured(Some(0), vec![0xff], b""); assert!(git_output_of(&["status"], Ok(binary)).is_err()); } diff --git a/deploy/release/tests/BUILD.bazel b/deploy/release/tests/BUILD.bazel index c1335d8912..e2beb5ca16 100644 --- a/deploy/release/tests/BUILD.bazel +++ b/deploy/release/tests/BUILD.bazel @@ -32,6 +32,7 @@ rust_test( # The consumer fetches the rust and GCC toolchains and bcr modules. tags = NESTED_BAZEL_NETWORK_TAGS, deps = [ + "//cli/process:dx_process", "//deploy/release:dx_release_tools", "//deploy/rules:dx_deploy_tools", "//tools/testing:dx_testing", diff --git a/deploy/release/tests/candidate_consumer_test.rs b/deploy/release/tests/candidate_consumer_test.rs index b242574048..85953bb0d8 100644 --- a/deploy/release/tests/candidate_consumer_test.rs +++ b/deploy/release/tests/candidate_consumer_test.rs @@ -2,7 +2,6 @@ //! registry in a nested Bazel workspace, with no override. use std::path::{Path, PathBuf}; -use std::process::Command; use dx_nested::{self as nested, Case}; use dx_release_tools::candidate::{ @@ -48,7 +47,8 @@ fn write(path: &Path, bytes: &[u8]) { /// Runs git with a fixed identity and no user or system config. fn git(dir: &Path, args: &[&str]) -> String { - let output = Command::new("git") + let output = dx_process::capture::run_checked( + dx_process::capture::git(dir) .args([ "-c", "commit.gpgsign=false", @@ -58,18 +58,11 @@ fn git(dir: &Path, args: &[&str]) -> String { "user.email=candidate@example.com", ]) .args(args) - .current_dir(dir) - .env("GIT_CONFIG_NOSYSTEM", "1") - // Keep `git status` from refreshing and rewriting the checkout's index. - .env("GIT_OPTIONAL_LOCKS", "0") - .output() - .unwrap_or_else(|error| panic!("git {args:?}: {error}")); - assert!( - output.status.success(), - "git {args:?} in {dir:?}: {}", - String::from_utf8_lossy(&output.stderr) - ); - String::from_utf8_lossy(&output.stdout).trim().to_owned() + .env("GIT_CONFIG_NOSYSTEM", "1"), + dx_process::capture::DEFAULT_TIMEOUT, + ) + .unwrap_or_else(|error| panic!("git {args:?} in {dir:?}: {error}")); + output.stdout_text().trim().to_owned() } /// What the checkout looks like to git, to prove the snapshot leaves it alone. diff --git a/deploy/rules/BUILD.bazel b/deploy/rules/BUILD.bazel index 1551542f82..86254d5cae 100644 --- a/deploy/rules/BUILD.bazel +++ b/deploy/rules/BUILD.bazel @@ -216,6 +216,7 @@ rust_library( "//cli/audit:dx_audit", "//cli/digest:dx_digest", "//cli/path:dx_path", + "//cli/process:dx_process", "//cli/sbom:dx_sbom", "//cli/update:dx_update", "//libs/binfmt:dx_binfmt", @@ -336,7 +337,10 @@ rust_test( package_name = "deploy/dx_deploy_tools", normal = True, normal_dev = True, - ) + ["//tools/testing:dx_testing"], + ) + [ + "//cli/process:dx_process", + "//tools/testing:dx_testing", + ], ) exports_files(["Cargo.toml"]) diff --git a/deploy/rules/src/api_compat.rs b/deploy/rules/src/api_compat.rs index a0dc64e686..24527fbc1f 100644 --- a/deploy/rules/src/api_compat.rs +++ b/deploy/rules/src/api_compat.rs @@ -220,16 +220,12 @@ pub fn workspace_dir() -> std::path::PathBuf { /// Returns the highest `v` tag below `next` in the repository at `workspace`. pub fn previous_git_tag(workspace: &std::path::Path, next: &str) -> Result, String> { - let output = std::process::Command::new("git") - .arg("-C") - .arg(workspace) - .args(["tag", "--list", "v*"]) - .output() - .map_err(|error| format!("git: {error}"))?; - if !output.status.success() { - return Err("git tag --list failed".to_owned()); - } - Ok(previous_tag(&String::from_utf8_lossy(&output.stdout), next)) + let output = dx_process::capture::run_checked( + dx_process::capture::git(workspace).args(["tag", "--list", "v*"]), + dx_process::capture::DEFAULT_TIMEOUT, + ) + .map_err(|error| error.to_string())?; + Ok(previous_tag(&output.stdout_text(), next)) } /// Downloads `url` up to `limit` bytes; None when the server answers 404 or 410. @@ -266,20 +262,12 @@ pub fn export_git_dir( marker: &str, ) -> Result { let git = |args: &[&str]| -> Result, String> { - let output = std::process::Command::new("git") - .arg("-C") - .arg(workspace) - .args(args) - .output() - .map_err(|error| format!("git: {error}"))?; - if !output.status.success() { - return Err(format!( - "git {} failed: {}", - args.join(" "), - String::from_utf8_lossy(&output.stderr).trim() - )); - } - Ok(output.stdout) + dx_process::capture::run_checked( + dx_process::capture::git(workspace).args(args), + dx_process::capture::DEFAULT_TIMEOUT, + ) + .map(|output| output.stdout) + .map_err(|error| error.to_string()) }; let dir = rel_dir.trim_matches('/'); let spec = if dir.is_empty() { "." } else { dir }; @@ -475,13 +463,13 @@ mod tests { } fn git_in(dir: &std::path::Path, args: &[&str]) { - let status = std::process::Command::new("git") - .args(["-c", "user.name=t", "-c", "user.email=t@example.com"]) - .args(args) - .current_dir(dir) - .status() - .expect("git"); - assert!(status.success(), "git {args:?}"); + dx_process::capture::run_checked( + dx_process::capture::git(dir) + .args(["-c", "user.name=t", "-c", "user.email=t@example.com"]) + .args(args), + dx_process::capture::DEFAULT_TIMEOUT, + ) + .unwrap_or_else(|error| panic!("git {args:?}: {error}")); } #[test] @@ -523,11 +511,11 @@ mod tests { export_git_dir(&repo, "nope", "v1.0.0", &missing, "Cargo.toml"), Ok(false) ); - let status = std::process::Command::new("git") - .args(["status", "--porcelain"]) - .current_dir(&repo) - .output() - .expect("status"); + let status = dx_process::capture::run_checked( + dx_process::capture::git(&repo).args(["status", "--porcelain"]), + dx_process::capture::DEFAULT_TIMEOUT, + ) + .expect("status"); assert!(status.stdout.is_empty(), "{:?}", status.stdout); let _ = std::fs::remove_dir_all(&root); } diff --git a/deploy/rules/src/crates.rs b/deploy/rules/src/crates.rs index 542a835c92..00e88934b3 100644 --- a/deploy/rules/src/crates.rs +++ b/deploy/rules/src/crates.rs @@ -910,12 +910,11 @@ mod tests { ], vec!["tag", "v1.2.0"], ] { - let status = std::process::Command::new("git") - .args(&args) - .current_dir(&repo) - .status() - .expect("git"); - assert!(status.success(), "git {args:?}"); + dx_process::capture::run_checked( + dx_process::capture::git(&repo).args(&args), + dx_process::capture::DEFAULT_TIMEOUT, + ) + .unwrap_or_else(|error| panic!("git {args:?}: {error}")); } let files = nested_crate(&scratch.path().join("in/pkg")); let mapped = crates_package_paths(&files).expect("paths"); @@ -1015,12 +1014,11 @@ mod tests { ], vec!["tag", "v1.2.0"], ] { - let status = std::process::Command::new("git") - .args(&args) - .current_dir(&repo) - .status() - .expect("git"); - assert!(status.success(), "git {args:?}"); + dx_process::capture::run_checked( + dx_process::capture::git(&repo).args(&args), + dx_process::capture::DEFAULT_TIMEOUT, + ) + .unwrap_or_else(|error| panic!("git {args:?}: {error}")); } let current = scratch.path().join("in/pkg"); let files = vec![ diff --git a/deploy/rules/src/github.rs b/deploy/rules/src/github.rs index a4f6794850..c2b50624b1 100644 --- a/deploy/rules/src/github.rs +++ b/deploy/rules/src/github.rs @@ -50,16 +50,12 @@ fn release_command(tag: &str, asset_bases: &[String], with_notes: bool) -> Strin } fn git_output(workspace: Option<&std::ffi::OsString>, args: &[&str]) -> Result { - let mut cmd = std::process::Command::new("git"); - cmd.args(args); - if let Some(dir) = workspace { - cmd.current_dir(dir); - } - let out = cmd.output().map_err(|error| format!("git: {error}"))?; - if !out.status.success() { - let stderr = String::from_utf8_lossy(&out.stderr); - return Err(format!("git {} failed: {}", args[0], stderr.trim())); - } + let dir = workspace.map_or_else(|| std::path::PathBuf::from("."), std::path::PathBuf::from); + let out = dx_process::capture::run_checked( + dx_process::capture::git(&dir).args(args), + dx_process::capture::DEFAULT_TIMEOUT, + ) + .map_err(|error| error.to_string())?; Ok(String::from_utf8_lossy(&out.stdout).into_owned()) } diff --git a/deploy/rules/src/go_api.rs b/deploy/rules/src/go_api.rs index 4e3c4d732d..75f4b4ae5b 100644 --- a/deploy/rules/src/go_api.rs +++ b/deploy/rules/src/go_api.rs @@ -98,20 +98,12 @@ const GO_CHILD_ENV_EXTRA: &[&str] = &[ ]; fn git(workspace: &Path, args: &[&str]) -> Result { - let output = std::process::Command::new("git") - .arg("-C") - .arg(workspace) - .args(args) - .output() - .map_err(|error| format!("git: {error}"))?; - if !output.status.success() { - return Err(format!( - "git {} failed: {}", - args.join(" "), - String::from_utf8_lossy(&output.stderr).trim() - )); - } - Ok(String::from_utf8_lossy(&output.stdout).into_owned()) + dx_process::capture::run_checked( + dx_process::capture::git(workspace).args(args), + dx_process::capture::DEFAULT_TIMEOUT, + ) + .map(|output| output.stdout_text()) + .map_err(|error| error.to_string()) } /// Returns the breaking changes of the module in `new` against the module in `old`. @@ -336,12 +328,11 @@ mod tests { ], vec!["tag", "v1.2.0"], ] { - let status = std::process::Command::new("git") - .args(&args) - .current_dir(&repo) - .status() - .expect("git"); - assert!(status.success(), "git {args:?}"); + dx_process::capture::run_checked( + dx_process::capture::git(&repo).args(&args), + dx_process::capture::DEFAULT_TIMEOUT, + ) + .unwrap_or_else(|error| panic!("git {args:?}: {error}")); } repo } diff --git a/deploy/rules/src/npm.rs b/deploy/rules/src/npm.rs index 108bd5289e..566b078354 100644 --- a/deploy/rules/src/npm.rs +++ b/deploy/rules/src/npm.rs @@ -788,12 +788,11 @@ mod tests { let unused = dir.join("api-extractor"); let repo = dir.join("repo"); std::fs::create_dir_all(&repo).expect("repo"); - let status = std::process::Command::new("git") - .args(["init", "-q"]) - .current_dir(&repo) - .status() - .expect("git"); - assert!(status.success()); + dx_process::capture::run_checked( + dx_process::capture::git(&repo).args(["init", "-q"]), + dx_process::capture::DEFAULT_TIMEOUT, + ) + .expect("git init"); let repo_dir = repo.display().to_string(); let check = NpmApiCheck { baseline: Baseline::Git, diff --git a/deploy/rules/src/pypi.rs b/deploy/rules/src/pypi.rs index e2c9c81ab3..6c9f5447c0 100644 --- a/deploy/rules/src/pypi.rs +++ b/deploy/rules/src/pypi.rs @@ -710,12 +710,11 @@ mod tests { ], vec!["tag", "v1.2.0"], ] { - let status = std::process::Command::new("git") - .args(&args) - .current_dir(&repo) - .status() - .expect("git"); - assert!(status.success(), "git {args:?}"); + dx_process::capture::run_checked( + dx_process::capture::git(&repo).args(&args), + dx_process::capture::DEFAULT_TIMEOUT, + ) + .unwrap_or_else(|error| panic!("git {args:?}: {error}")); } }); let args_log = scratch.path().join("args");