From 8538d77f30c5815d0055de9cc3964b7076dd4815 Mon Sep 17 00:00:00 2001 From: Jo <10510431+j178@users.noreply.github.com> Date: Sat, 27 Jun 2026 17:34:52 +0800 Subject: [PATCH 1/2] Improve subprocess error messages --- crates/prek-consts/src/env_vars.rs | 1 - crates/prek/src/cli/auto_update/repository.rs | 44 +-- crates/prek/src/cli/hook_impl.rs | 2 +- crates/prek/src/cli/install.rs | 2 +- crates/prek/src/cli/run/keeper.rs | 6 +- crates/prek/src/cli/run/run.rs | 2 +- crates/prek/src/cli/try_repo.rs | 16 +- crates/prek/src/git.rs | 82 ++--- .../check_executables_have_shebangs.rs | 2 +- .../pre_commit_hooks/destroyed_symlinks.rs | 6 +- .../pre_commit_hooks/forbid_new_submodules.rs | 4 +- .../pre_commit_hooks/no_commit_to_branch.rs | 2 +- .../src/hooks/pre_commit_hooks/shebangs.rs | 2 +- crates/prek/src/languages/bun/bun.rs | 6 +- crates/prek/src/languages/bun/installer.rs | 4 +- crates/prek/src/languages/conda.rs | 10 +- crates/prek/src/languages/coursier.rs | 10 +- crates/prek/src/languages/dart.rs | 8 +- crates/prek/src/languages/deno/deno.rs | 6 +- crates/prek/src/languages/deno/installer.rs | 2 +- crates/prek/src/languages/docker.rs | 6 +- crates/prek/src/languages/docker_image.rs | 2 +- crates/prek/src/languages/dotnet/dotnet.rs | 6 +- crates/prek/src/languages/dotnet/installer.rs | 10 +- crates/prek/src/languages/golang/golang.rs | 8 +- crates/prek/src/languages/golang/installer.rs | 8 +- crates/prek/src/languages/haskell.rs | 8 +- crates/prek/src/languages/julia.rs | 8 +- crates/prek/src/languages/lua.rs | 17 +- crates/prek/src/languages/node/installer.rs | 2 +- crates/prek/src/languages/node/node.rs | 6 +- crates/prek/src/languages/perl.rs | 8 +- crates/prek/src/languages/pygrep/pygrep.rs | 6 +- crates/prek/src/languages/python/python.rs | 10 +- crates/prek/src/languages/python/uv.rs | 6 +- crates/prek/src/languages/r.rs | 11 +- crates/prek/src/languages/ruby/gem.rs | 8 +- crates/prek/src/languages/ruby/installer.rs | 2 +- crates/prek/src/languages/ruby/ruby.rs | 4 +- crates/prek/src/languages/rust/installer.rs | 4 +- crates/prek/src/languages/rust/rust.rs | 14 +- crates/prek/src/languages/rust/rustup.rs | 8 +- crates/prek/src/languages/script.rs | 4 +- crates/prek/src/languages/swift.rs | 10 +- crates/prek/src/languages/system.rs | 4 +- crates/prek/src/process.rs | 319 ++++++++++-------- docs/reference/environment-variables.md | 5 - 47 files changed, 373 insertions(+), 348 deletions(-) diff --git a/crates/prek-consts/src/env_vars.rs b/crates/prek-consts/src/env_vars.rs index be837dd31..312430425 100644 --- a/crates/prek-consts/src/env_vars.rs +++ b/crates/prek-consts/src/env_vars.rs @@ -33,7 +33,6 @@ impl EnvVars { pub const PREK_CONTAINER_RUNTIME: &'static str = "PREK_CONTAINER_RUNTIME"; pub const PREK_DOCKER_NO_INIT: &'static str = "PREK_DOCKER_NO_INIT"; pub const PREK_QUIET: &'static str = "PREK_QUIET"; - pub const PREK_LOG_TRUNCATE_LIMIT: &'static str = "PREK_LOG_TRUNCATE_LIMIT"; // PREK internal environment variables pub const PREK_INTERNAL__TEST_DIR: &'static str = "PREK_INTERNAL__TEST_DIR"; diff --git a/crates/prek/src/cli/auto_update/repository.rs b/crates/prek/src/cli/auto_update/repository.rs index 4b5cc2e44..7cad0ad28 100644 --- a/crates/prek/src/cli/auto_update/repository.rs +++ b/crates/prek/src/cli/auto_update/repository.rs @@ -19,7 +19,7 @@ use crate::{config, git}; /// Initializes a temporary git repo and fetches the remote HEAD plus tags. pub(super) async fn setup_and_fetch_repo(repo_url: &str, repo_path: &Path) -> Result<()> { git::init_repo(repo_url, repo_path).await?; - git::git_cmd("git fetch")? + git::git_cmd()? .arg("fetch") .arg("origin") .arg("HEAD") @@ -38,7 +38,7 @@ pub(super) async fn setup_and_fetch_repo(repo_url: &str, repo_path: &Path) -> Re /// Resolves any revision-like string to the underlying commit SHA. pub(super) async fn resolve_revision_to_commit(repo_path: &Path, rev: &str) -> Result { - let output = git::git_cmd("git rev-parse")? + let output = git::git_cmd()? .arg("rev-parse") .arg(format!("{rev}^{{}}")) .check(true) @@ -75,7 +75,7 @@ pub(super) async fn is_commit_present(repo_path: &Path, commit: &str) -> Result< return Ok(CommitPresence::Unknown); } - let output = git::git_cmd("git cat-file")? + let output = git::git_cmd()? .arg("--no-lazy-fetch") .arg("cat-file") .arg("-e") @@ -120,7 +120,7 @@ pub(super) fn get_tags_pointing_at_revision<'a>( /// Resolves the default branch tip to an exact tag when possible, otherwise to a commit SHA. pub(super) async fn resolve_bleeding_edge(repo_path: &Path) -> Result> { - let output = git::git_cmd("git describe")? + let output = git::git_cmd()? .arg("describe") .arg("FETCH_HEAD") .arg("--tags") @@ -134,7 +134,7 @@ pub(super) async fn resolve_bleeding_edge(repo_path: &Path) -> Result Result Result> { - let output = git::git_cmd("git for-each-ref")? + let output = git::git_cmd()? .arg("for-each-ref") .arg("--sort=-creatordate") .arg("--format=%(refname:lstrip=2)\t%(creatordate:unix)\t%(objectname)\t%(*objectname)") @@ -376,7 +376,7 @@ pub(super) async fn checkout_and_validate_manifest( required_hook_ids: &[&str], ) -> Result<()> { if cfg!(windows) { - git::git_cmd("git show")? + git::git_cmd()? .arg("show") .arg(format!("{rev}:{PRE_COMMIT_HOOKS_YAML}")) .current_dir(repo_path) @@ -387,7 +387,7 @@ pub(super) async fn checkout_and_validate_manifest( .await?; } - git::git_cmd("git checkout")? + git::git_cmd()? .arg("checkout") .arg("--quiet") .arg(rev) @@ -439,7 +439,7 @@ mod tests { let tmp = tempfile::tempdir().unwrap(); let repo = tmp.path(); - git::git_cmd("git init") + git::git_cmd() .unwrap() .arg("init") .current_dir(repo) @@ -448,7 +448,7 @@ mod tests { .await .unwrap(); - git::git_cmd("git config") + git::git_cmd() .unwrap() .args(["config", "user.email", "test@test.com"]) .current_dir(repo) @@ -457,7 +457,7 @@ mod tests { .await .unwrap(); - git::git_cmd("git config") + git::git_cmd() .unwrap() .args(["config", "user.name", "Test"]) .current_dir(repo) @@ -466,7 +466,7 @@ mod tests { .await .unwrap(); - git::git_cmd("git commit") + git::git_cmd() .unwrap() .args([ "-c", @@ -482,7 +482,7 @@ mod tests { .await .unwrap(); - git::git_cmd("git branch") + git::git_cmd() .unwrap() .args(["branch", "-M", "trunk"]) .current_dir(repo) @@ -494,8 +494,8 @@ mod tests { tmp } - fn git_cmd(dir: impl AsRef, summary: &str) -> Cmd { - let mut cmd = git::git_cmd(summary).unwrap(); + fn git_cmd(dir: impl AsRef) -> Cmd { + let mut cmd = git::git_cmd().unwrap(); cmd.current_dir(dir) .args(["-c", "commit.gpgsign=false"]) .args(["-c", "tag.gpgsign=false"]); @@ -520,7 +520,7 @@ mod tests { } async fn create_commit(repo: &Path, message: &str) { - git_cmd(repo, "git commit") + git_cmd(repo) .args(["commit", "--allow-empty", "-m", message]) .remove_git_envs() .output() @@ -537,7 +537,7 @@ mod tests { let date_str = format!("{timestamp} +0000"); - git_cmd(repo, "git commit") + git_cmd(repo) .args(["commit", "--allow-empty", "-m", message]) .env("GIT_AUTHOR_DATE", &date_str) .env("GIT_COMMITTER_DATE", &date_str) @@ -548,7 +548,7 @@ mod tests { } async fn create_lightweight_tag(repo: &Path, tag: &str) { - git_cmd(repo, "git tag") + git_cmd(repo) .arg("tag") .arg(tag) .remove_git_envs() @@ -566,7 +566,7 @@ mod tests { let date_str = format!("{timestamp} +0000"); - git_cmd(repo, "git tag") + git_cmd(repo) .arg("tag") .arg(tag) .arg("-m") @@ -607,7 +607,7 @@ mod tests { create_commit(repo, "tagged").await; create_lightweight_tag(repo, "v1.2.3").await; - git::git_cmd("git fetch") + git::git_cmd() .unwrap() .args(["fetch", ".", "HEAD"]) .current_dir(repo) @@ -627,7 +627,7 @@ mod tests { create_commit(repo, "untagged").await; - git::git_cmd("git fetch") + git::git_cmd() .unwrap() .args(["fetch", ".", "HEAD"]) .current_dir(repo) @@ -638,7 +638,7 @@ mod tests { let rev = resolve_bleeding_edge(repo).await.unwrap(); - let head = git::git_cmd("git rev-parse") + let head = git::git_cmd() .unwrap() .args(["rev-parse", "HEAD"]) .current_dir(repo) diff --git a/crates/prek/src/cli/hook_impl.rs b/crates/prek/src/cli/hook_impl.rs index 883fe77a0..2fe41949d 100644 --- a/crates/prek/src/cli/hook_impl.rs +++ b/crates/prek/src/cli/hook_impl.rs @@ -220,7 +220,7 @@ async fn run_legacy( } let entry = resolve_command(vec![legacy_hook.to_string_lossy().into_owned()], None); - let mut cmd = Cmd::new(&entry[0], format!("legacy hook `{}`", hook_type.as_ref())); + let mut cmd = Cmd::new(&entry[0]); cmd.check(false).args(&entry[1..]).args(args); cmd.env(EnvVars::PREK_RUNNING_LEGACY, "1"); diff --git a/crates/prek/src/cli/install.rs b/crates/prek/src/cli/install.rs index d021f782e..25eddd2b9 100644 --- a/crates/prek/src/cli/install.rs +++ b/crates/prek/src/cli/install.rs @@ -506,7 +506,7 @@ pub(crate) async fn init_template_dir( ) .await?; - let output = git_cmd("git config")? + let output = git_cmd()? .arg("config") .arg("init.templateDir") .check(false) diff --git a/crates/prek/src/cli/run/keeper.rs b/crates/prek/src/cli/run/keeper.rs index fe63e84d8..0c9d0c95a 100644 --- a/crates/prek/src/cli/run/keeper.rs +++ b/crates/prek/src/cli/run/keeper.rs @@ -42,11 +42,11 @@ impl IntentToAddRestorer { } // TODO: xargs - git_cmd("git rm")? + git_cmd()? .arg("rm") .arg("--cached") .arg("--") - .args(&files) + .file_args(&files) .check(true) .stdout(std::process::Stdio::null()) .stderr(std::process::Stdio::null()) @@ -88,7 +88,7 @@ impl UnstagedChangesRestorer { async fn clean(root: &Path, patch_dir: &Path) -> Result { let tree = git::write_tree().await?; - let mut cmd = git_cmd("git diff-index")?; + let mut cmd = git_cmd()?; let output = cmd .arg("diff-index") .arg("--ignore-submodules") diff --git a/crates/prek/src/cli/run/run.rs b/crates/prek/src/cli/run/run.rs index 6ce00cd9b..fc3d7690c 100644 --- a/crates/prek/src/cli/run/run.rs +++ b/crates/prek/src/cli/run/run.rs @@ -1098,7 +1098,7 @@ impl<'a> HookRunSession<'a> { } else { "--color=never" }; - git::git_cmd("git diff")? + git::git_cmd()? .arg("--no-pager") .arg("diff") .arg("--no-ext-diff") diff --git a/crates/prek/src/cli/try_repo.rs b/crates/prek/src/cli/try_repo.rs index 037b0e6d4..fbae87814 100644 --- a/crates/prek/src/cli/try_repo.rs +++ b/crates/prek/src/cli/try_repo.rs @@ -18,7 +18,7 @@ use crate::store::Store; use crate::warn_user; async fn get_head_rev(repo: &Path) -> Result { - let head_rev = git::git_cmd("get head rev")? + let head_rev = git::git_cmd()? .arg("rev-parse") .arg("HEAD") .current_dir(repo) @@ -31,13 +31,13 @@ async fn get_head_rev(repo: &Path) -> Result { async fn clone_and_commit(repo_path: &Path, head_rev: &str, tmp_dir: &Path) -> Result { let shadow = tmp_dir.join("shadow-repo"); - git::git_cmd("clone shadow repo")? + git::git_cmd()? .arg("clone") .arg(repo_path) .arg(&shadow) .output() .await?; - git::git_cmd("checkout shadow repo")? + git::git_cmd()? .arg("checkout") .arg(head_rev) .arg("-b") @@ -51,10 +51,10 @@ async fn clone_and_commit(repo_path: &Path, head_rev: &str, tmp_dir: &Path) -> R let staged_files = git::get_staged_files(repo_path).await?; if !staged_files.is_empty() { - git::git_cmd("add staged files to shadow")? + git::git_cmd()? .arg("add") .arg("--") - .args(&staged_files) + .file_args(&staged_files) .current_dir(repo_path) .env("GIT_INDEX_FILE", &index_path) .env("GIT_OBJECT_DIRECTORY", &objects_path) @@ -62,7 +62,7 @@ async fn clone_and_commit(repo_path: &Path, head_rev: &str, tmp_dir: &Path) -> R .await?; } - let mut add_u_cmd = git::git_cmd("add unstaged to shadow")?; + let mut add_u_cmd = git::git_cmd()?; add_u_cmd .arg("add") .arg("--update") // Update tracked files @@ -72,7 +72,7 @@ async fn clone_and_commit(repo_path: &Path, head_rev: &str, tmp_dir: &Path) -> R .output() .await?; - git::git_cmd("git commit")? + git::git_cmd()? .arg("commit") .arg("-m") .arg("Temporary commit by prek try-repo") @@ -108,7 +108,7 @@ async fn prepare_repo_and_rev<'a>( get_head_rev(repo_path).await? } else { // For remote repositories, use ls-remote - let head_rev = git::git_cmd("get head rev")? + let head_rev = git::git_cmd()? .arg("ls-remote") .arg("--exit-code") .arg(repo) diff --git a/crates/prek/src/git.rs b/crates/prek/src/git.rs index d70096c11..75251b50a 100644 --- a/crates/prek/src/git.rs +++ b/crates/prek/src/git.rs @@ -76,8 +76,8 @@ pub(crate) static GIT_ENV_TO_REMOVE: LazyLock> = LazyLock: .collect() }); -pub(crate) fn git_cmd(summary: &str) -> Result { - let mut cmd = Cmd::new(GIT.as_ref().map_err(|&e| Error::GitNotFound(e))?, summary); +pub(crate) fn git_cmd() -> Result { + let mut cmd = Cmd::new(GIT.as_ref().map_err(|&e| Error::GitNotFound(e))?); cmd.arg("-c").arg("core.useBuiltinFSMonitor=false"); Ok(cmd) @@ -105,7 +105,7 @@ fn path_from_git_bytes(bytes: &[u8]) -> Result { } pub(crate) async fn intent_to_add_files(root: &Path) -> Result, Error> { - let output = git_cmd("get intent to add files")? + let output = git_cmd()? .arg("diff") .arg("--no-ext-diff") .arg("--ignore-submodules") @@ -121,7 +121,7 @@ pub(crate) async fn intent_to_add_files(root: &Path) -> Result, Err } pub(crate) async fn get_added_files(root: &Path) -> Result, Error> { - let output = git_cmd("get added files")? + let output = git_cmd()? .current_dir(root) .arg("diff") .arg("--staged") @@ -144,7 +144,7 @@ pub(crate) async fn get_changed_files( root: &Path, ) -> Result, Error> { let build_cmd = |range: String| -> Result { - let mut cmd = git_cmd("get changed files")?; + let mut cmd = git_cmd()?; cmd.arg("diff") .arg("--name-only") .arg("--diff-filter=ACMRT") @@ -176,7 +176,7 @@ pub(crate) async fn get_changed_files( #[instrument(level = "trace")] pub(crate) async fn ls_files(cwd: &Path, path: &Path) -> Result, Error> { - let output = git_cmd("git ls-files")? + let output = git_cmd()? .current_dir(cwd) .arg("ls-files") .arg("-z") @@ -190,7 +190,7 @@ pub(crate) async fn ls_files(cwd: &Path, path: &Path) -> Result, Er } pub(crate) async fn get_git_dir() -> Result { - let output = git_cmd("get git dir")? + let output = git_cmd()? .arg("rev-parse") .arg("--git-dir") .check(true) @@ -202,7 +202,7 @@ pub(crate) async fn get_git_dir() -> Result { } pub(crate) async fn get_git_common_dir() -> Result { - let output = git_cmd("get git common dir")? + let output = git_cmd()? .arg("rev-parse") .arg("--git-common-dir") .check(true) @@ -223,7 +223,7 @@ pub(crate) async fn get_git_hooks_dir() -> Result { // `core.hooksPath`, including local/worktree config, linked worktrees, bare // + worktree layouts, and repo-owned config loaded through `include.path` // / `includeIf`. - let output = git_cmd("get git hooks dir")? + let output = git_cmd()? .arg("rev-parse") .arg("--git-path") .arg("hooks") @@ -250,7 +250,7 @@ pub(crate) async fn get_git_hooks_dir() -> Result { } pub(crate) async fn get_staged_files(root: &Path) -> Result, Error> { - let output = git_cmd("get staged files")? + let output = git_cmd()? .current_dir(root) .arg("diff") .arg("--cached") @@ -265,13 +265,13 @@ pub(crate) async fn get_staged_files(root: &Path) -> Result, Error> } pub(crate) async fn files_not_staged(files: &[&Path]) -> Result> { - let output = git_cmd("git diff")? + let output = git_cmd()? .arg("diff") .arg("--exit-code") .arg("--name-only") .arg("--no-ext-diff") .arg("-z") // Use NUL as line terminator - .args(files) + .file_args(files) .check(false) .output() .await?; @@ -284,7 +284,7 @@ pub(crate) async fn files_not_staged(files: &[&Path]) -> Result> { } pub(crate) async fn has_unmerged_paths() -> Result { - let output = git_cmd("check has unmerged paths")? + let output = git_cmd()? .arg("ls-files") .arg("--unmerged") .check(true) @@ -294,7 +294,7 @@ pub(crate) async fn has_unmerged_paths() -> Result { } pub(crate) async fn has_diff(rev: &str, path: &Path) -> Result { - let status = git_cmd("check diff")? + let status = git_cmd()? .arg("diff") .arg("--quiet") .arg(rev) @@ -311,13 +311,9 @@ pub(crate) async fn is_in_merge_conflict() -> Result { } pub(crate) async fn get_conflicted_files(root: &Path) -> Result, Error> { - let tree = git_cmd("git write-tree")? - .arg("write-tree") - .check(true) - .output() - .await?; + let tree = git_cmd()?.arg("write-tree").check(true).output().await?; - let output = git_cmd("get conflicted files")? + let output = git_cmd()? .arg("diff") .arg("--name-only") .arg("--no-ext-diff") // Disable external diff drivers @@ -357,7 +353,7 @@ async fn parse_merge_msg_for_conflicts() -> Result, Error> { #[instrument(level = "trace")] pub(crate) async fn has_worktree_diff(path: &Path) -> Result { - let mut cmd = git_cmd("check worktree diff")?; + let mut cmd = git_cmd()?; let status = cmd .arg("diff-files") .arg("--quiet") @@ -383,7 +379,7 @@ pub(crate) async fn has_worktree_diff(path: &Path) -> Result { #[instrument(level = "trace")] pub(crate) async fn get_diff(path: &Path) -> Result, Error> { - let output = git_cmd("git diff")? + let output = git_cmd()? .arg("diff") .arg("--no-ext-diff") // Disable external diff drivers .arg("--no-textconv") @@ -414,11 +410,7 @@ pub(crate) async fn get_diff(path: &Path) -> Result, Error> { /// The name of the new tree object is printed to standard output. /// The index must be in a fully merged state. pub(crate) async fn write_tree() -> Result { - let output = git_cmd("git write-tree")? - .arg("write-tree") - .check(true) - .output() - .await?; + let output = git_cmd()?.arg("write-tree").check(true).output().await?; Ok(String::from_utf8_lossy(&output.stdout) .trim_ascii() .to_string()) @@ -434,7 +426,7 @@ pub(crate) fn get_root() -> Result { .output()?; if !output.status.success() { return Err(Error::Command(process::Error::Status { - summary: "get git root".to_string(), + command: format!("{} rev-parse --show-toplevel", git.to_string_lossy()), error: StatusError { status: output.status, output: Some(output), @@ -460,7 +452,7 @@ pub(crate) async fn init_repo(url: &str, path: &Path) -> Result<(), Error> { Cow::Borrowed(url) }; - git_cmd("init git repo")? + git_cmd()? // Unset `extensions.objectFormat` if set, just follow what hash the remote uses. .arg("-c") .arg("init.defaultObjectFormat=") @@ -472,7 +464,7 @@ pub(crate) async fn init_repo(url: &str, path: &Path) -> Result<(), Error> { .output() .await?; - git_cmd("add git remote")? + git_cmd()? .current_dir(path) .arg("remote") .arg("add") @@ -534,7 +526,7 @@ async fn shallow_clone( path: &Path, terminal_prompt: TerminalPrompt, ) -> Result<(), Error> { - git_cmd("git shallow clone")? + git_cmd()? .current_dir(path) .arg("-c") .arg("protocol.version=2") @@ -549,7 +541,7 @@ async fn shallow_clone( .output() .await?; - git_cmd("git checkout")? + git_cmd()? .current_dir(path) .arg("checkout") .arg("FETCH_HEAD") @@ -561,7 +553,7 @@ async fn shallow_clone( .output() .await?; - git_cmd("update git submodules")? + git_cmd()? .current_dir(path) .arg("-c") .arg("protocol.version=2") @@ -581,7 +573,7 @@ async fn shallow_clone( } async fn full_clone(rev: &str, path: &Path, terminal_prompt: TerminalPrompt) -> Result<(), Error> { - git_cmd("git full clone")? + git_cmd()? .current_dir(path) .arg("fetch") .arg("origin") @@ -593,7 +585,7 @@ async fn full_clone(rev: &str, path: &Path, terminal_prompt: TerminalPrompt) -> .output() .await?; - git_cmd("git checkout")? + git_cmd()? .current_dir(path) .arg("checkout") .arg(rev) @@ -605,7 +597,7 @@ async fn full_clone(rev: &str, path: &Path, terminal_prompt: TerminalPrompt) -> .output() .await?; - git_cmd("update git submodules")? + git_cmd()? .current_dir(path) .arg("submodule") .arg("update") @@ -651,7 +643,7 @@ pub(crate) async fn clone_repo( } async fn get_config_value(scope: Option<&str>, key: &str) -> Result>, Error> { - let mut cmd = git_cmd("get git config value")?; + let mut cmd = git_cmd()?; cmd.arg("config").arg("--includes"); if let Some(scope) = scope { cmd.arg(scope); @@ -739,7 +731,7 @@ fn shared_repository_file_mode(value: &str, mode: u32) -> Option { /// Resolve the file mode implied by `core.sharedRepository` for a newly created file. pub(crate) async fn get_shared_repository_file_mode(mode: u32) -> Result { - let output = git_cmd("get shared repository config")? + let output = git_cmd()? .arg("config") .arg("--get") .arg("core.sharedRepository") @@ -762,7 +754,7 @@ pub(crate) async fn get_lfs_files( return Ok(FxHashSet::default()); } - let mut child = git_cmd("git check-attr")? + let mut child = git_cmd()? .current_dir(current_dir) .arg("check-attr") .arg("filter") @@ -796,7 +788,7 @@ pub(crate) async fn get_lfs_files( let status = child.wait().await?; if !status.success() { return Err(Error::Command(process::Error::Status { - summary: "git check-attr".to_string(), + command: "git check-attr -z filter --stdin".to_string(), error: StatusError { status, output: None, @@ -818,7 +810,7 @@ pub(crate) async fn get_lfs_files( /// Check if a git revision exists pub(crate) async fn rev_exists(rev: &str) -> Result { - let output = git_cmd("git cat-file")? + let output = git_cmd()? .arg("cat-file") // Exit with zero status if exists and is a valid object. .arg("-e") @@ -831,7 +823,7 @@ pub(crate) async fn rev_exists(rev: &str) -> Result { /// Check if `ancestor` is an ancestor of `commit`. pub(crate) async fn is_ancestor(ancestor: &str, commit: &str) -> Result { - let mut cmd = git_cmd("check commit ancestry")?; + let mut cmd = git_cmd()?; let status = cmd .arg("merge-base") .arg("--is-ancestor") @@ -857,7 +849,7 @@ pub(crate) async fn get_ancestors_not_in_remote( local_sha: &str, remote_name: &str, ) -> Result, Error> { - let output = git_cmd("get ancestors not in remote")? + let output = git_cmd()? .arg("rev-list") .arg(local_sha) .arg("--topo-order") @@ -876,7 +868,7 @@ pub(crate) async fn get_ancestors_not_in_remote( /// Get root commits (commits with no parents) for the given commit pub(crate) async fn get_root_commits(local_sha: &str) -> Result, Error> { - let output = git_cmd("get root commits")? + let output = git_cmd()? .arg("rev-list") .arg("--max-parents=0") .arg(local_sha) @@ -892,7 +884,7 @@ pub(crate) async fn get_root_commits(local_sha: &str) -> Result Result, Error> { - let output = git_cmd("get parent commit")? + let output = git_cmd()? .arg("rev-parse") .arg(format!("{commit}^")) .check(false) diff --git a/crates/prek/src/hooks/pre_commit_hooks/check_executables_have_shebangs.rs b/crates/prek/src/hooks/pre_commit_hooks/check_executables_have_shebangs.rs index 310db6a57..1b630ceba 100644 --- a/crates/prek/src/hooks/pre_commit_hooks/check_executables_have_shebangs.rs +++ b/crates/prek/src/hooks/pre_commit_hooks/check_executables_have_shebangs.rs @@ -20,7 +20,7 @@ pub(crate) async fn check_executables_have_shebangs( return Ok((0, Vec::new())); } - let stdout = git::git_cmd("get file file mode")? + let stdout = git::git_cmd()? .arg("config") .arg("core.fileMode") .check(true) diff --git a/crates/prek/src/hooks/pre_commit_hooks/destroyed_symlinks.rs b/crates/prek/src/hooks/pre_commit_hooks/destroyed_symlinks.rs index 3954bef92..8841fb486 100644 --- a/crates/prek/src/hooks/pre_commit_hooks/destroyed_symlinks.rs +++ b/crates/prek/src/hooks/pre_commit_hooks/destroyed_symlinks.rs @@ -54,7 +54,7 @@ fn write_reset_command(output: &mut Vec, destroyed_links: &[&Path]) -> Resul } async fn git_status_output(work_dir: &Path) -> Result> { - Ok(git::git_cmd("git status")? + Ok(git::git_cmd()? .current_dir(work_dir) .arg("status") .arg("--porcelain=v2") @@ -191,7 +191,7 @@ async fn is_destroyed_symlink(work_dir: &Path, entry: &OrdinaryChangedEntry<'_>) } async fn git_object_size(work_dir: &Path, object: &str) -> Result { - let output = git::git_cmd("git cat-file")? + let output = git::git_cmd()? .current_dir(work_dir) .arg("cat-file") .arg("-s") @@ -204,7 +204,7 @@ async fn git_object_size(work_dir: &Path, object: &str) -> Result { } async fn git_object_content(work_dir: &Path, object: &str) -> Result> { - Ok(git::git_cmd("git cat-file")? + Ok(git::git_cmd()? .current_dir(work_dir) .arg("cat-file") .arg("-p") diff --git a/crates/prek/src/hooks/pre_commit_hooks/forbid_new_submodules.rs b/crates/prek/src/hooks/pre_commit_hooks/forbid_new_submodules.rs index 0616297e1..d35bac214 100644 --- a/crates/prek/src/hooks/pre_commit_hooks/forbid_new_submodules.rs +++ b/crates/prek/src/hooks/pre_commit_hooks/forbid_new_submodules.rs @@ -20,7 +20,7 @@ pub(crate) async fn forbid_new_submodules( Cow::Borrowed("--staged") }; - let stdout = git::git_cmd("git diff")? + let stdout = git::git_cmd()? .current_dir(hook.work_dir()) .arg("diff") .arg("--relative") @@ -30,7 +30,7 @@ pub(crate) async fn forbid_new_submodules( .arg("-z") .arg(diff_arg.as_ref()) .arg("--") - .args(filenames) + .file_args(filenames) .check(true) .output() .await? diff --git a/crates/prek/src/hooks/pre_commit_hooks/no_commit_to_branch.rs b/crates/prek/src/hooks/pre_commit_hooks/no_commit_to_branch.rs index 483290087..fc8528034 100644 --- a/crates/prek/src/hooks/pre_commit_hooks/no_commit_to_branch.rs +++ b/crates/prek/src/hooks/pre_commit_hooks/no_commit_to_branch.rs @@ -36,7 +36,7 @@ impl Args { pub(crate) async fn no_commit_to_branch(hook: &Hook) -> Result<(i32, Vec)> { let args = Args::try_parse_from(hook.entry.expect_direct().split()?.iter().chain(&hook.args))?; - let output = git_cmd("get current branch")? + let output = git_cmd()? .arg("symbolic-ref") .arg("HEAD") .check(false) diff --git a/crates/prek/src/hooks/pre_commit_hooks/shebangs.rs b/crates/prek/src/hooks/pre_commit_hooks/shebangs.rs index f77c61bb5..40b0ffada 100644 --- a/crates/prek/src/hooks/pre_commit_hooks/shebangs.rs +++ b/crates/prek/src/hooks/pre_commit_hooks/shebangs.rs @@ -14,7 +14,7 @@ pub(super) async fn file_has_shebang(path: &Path) -> Result } pub(super) async fn git_index_stage_output(file_base: &Path) -> Result, anyhow::Error> { - Ok(git::git_cmd("git ls-files")? + Ok(git::git_cmd()? .arg("ls-files") .arg("--stage") .arg("-z") diff --git a/crates/prek/src/languages/bun/bun.rs b/crates/prek/src/languages/bun/bun.rs index d7c6ce82a..c5237fce2 100644 --- a/crates/prek/src/languages/bun/bun.rs +++ b/crates/prek/src/languages/bun/bun.rs @@ -79,7 +79,7 @@ impl LanguageImpl for Bun { // Use BUN_INSTALL to set where global packages are installed // This makes `bun install -g` install to our hook environment - Cmd::new(bun.bun(), "bun install") + Cmd::new(bun.bun()) .arg("install") .arg("-g") .args(&*deps) @@ -133,14 +133,14 @@ impl LanguageImpl for Bun { let entry = hook.entry.resolve(Some(&new_path), store)?; let run = async |batch: &[&Path]| { - let mut output = Cmd::new(&entry[0], "bun hook") + let mut output = Cmd::new(&entry[0]) .current_dir(hook.work_dir()) .args(&entry[1..]) .env(EnvVars::PATH, &new_path) .env(EnvVars::BUN_INSTALL, env_dir) .envs(&hook.env) .args(&hook.args) - .args(batch) + .file_args(batch) .check(false) .stdin(Stdio::null()) .pty_output_with_sink(reporter.output_sink(progress)) diff --git a/crates/prek/src/languages/bun/installer.rs b/crates/prek/src/languages/bun/installer.rs index 3ed1142c4..964f09bff 100644 --- a/crates/prek/src/languages/bun/installer.rs +++ b/crates/prek/src/languages/bun/installer.rs @@ -61,7 +61,7 @@ impl BunResult { } pub(crate) async fn fill_version(mut self) -> Result { - let output = Cmd::new(&self.bun, "bun --version") + let output = Cmd::new(&self.bun) .arg("--version") .check(true) .output() @@ -175,7 +175,7 @@ impl BunInstaller { /// List all versions of Bun available on GitHub releases. async fn list_remote_versions(&self) -> Result> { - let output = git::git_cmd("list bun tags")? + let output = git::git_cmd()? .arg("ls-remote") .arg("--tags") .arg("https://github.com/oven-sh/bun") diff --git a/crates/prek/src/languages/conda.rs b/crates/prek/src/languages/conda.rs index e6b4a2f34..99051fbe6 100644 --- a/crates/prek/src/languages/conda.rs +++ b/crates/prek/src/languages/conda.rs @@ -37,7 +37,7 @@ impl LanguageImpl for Conda { let conda = conda_executable(); if let Some(repo_path) = hook.repo_path() { - Cmd::new(conda, "create conda environment") + Cmd::new(conda) .current_dir(repo_path) .arg("create") .arg("-p") @@ -49,7 +49,7 @@ impl LanguageImpl for Conda { .await .context("Failed to create Conda environment")?; } else { - Cmd::new(conda, "create conda environment") + Cmd::new(conda) .arg("create") .arg("-p") .arg(&info.env_path) @@ -60,7 +60,7 @@ impl LanguageImpl for Conda { } if !hook.additional_dependencies.is_empty() { - let mut install_cmd = Cmd::new(conda, "install conda dependencies"); + let mut install_cmd = Cmd::new(conda); install_cmd .arg("install") .arg("-p") @@ -104,7 +104,7 @@ impl LanguageImpl for Conda { let entry = hook.entry.resolve(Some(&new_path), store)?; let run = async |batch: &[&Path]| { - let mut output = Cmd::new(&entry[0], "run conda hook") + let mut output = Cmd::new(&entry[0]) .current_dir(hook.work_dir()) .args(&entry[1..]) .env(EnvVars::PATH, &new_path) @@ -113,7 +113,7 @@ impl LanguageImpl for Conda { .env_remove(EnvVars::VIRTUAL_ENV) .envs(&hook.env) .args(&hook.args) - .args(batch) + .file_args(batch) .check(false) .stdin(Stdio::null()) .pty_output_with_sink(reporter.output_sink(progress)) diff --git a/crates/prek/src/languages/coursier.rs b/crates/prek/src/languages/coursier.rs index 2c483a9dd..10a04bd78 100644 --- a/crates/prek/src/languages/coursier.rs +++ b/crates/prek/src/languages/coursier.rs @@ -92,7 +92,7 @@ impl LanguageImpl for Coursier { if let Some(channel_apps) = collect_channel_apps(&channel_dir)? { has_channel_apps = true; for app in channel_apps { - Cmd::new(&cs, "coursier install") + Cmd::new(&cs) .current_dir(repo_path) .arg("install") .arg("--dir") @@ -112,7 +112,7 @@ impl LanguageImpl for Coursier { } if !dependencies.is_empty() { - let mut fetch_cmd = Cmd::new(&cs, "coursier fetch"); + let mut fetch_cmd = Cmd::new(&cs); fetch_cmd .arg("fetch") .args(&dependencies) @@ -125,7 +125,7 @@ impl LanguageImpl for Coursier { format!("Failed to fetch coursier app `{}`", dependencies.join(" ")) })?; - let mut install_cmd = Cmd::new(&cs, "coursier install"); + let mut install_cmd = Cmd::new(&cs); install_cmd .arg("install") .arg("--dir") @@ -176,12 +176,12 @@ impl LanguageImpl for Coursier { let entry = hook.entry.resolve(Some(&path_env), store)?; let run = async |batch: &[&Path]| { - let mut output = Cmd::new(&entry[0], "run coursier hook") + let mut output = Cmd::new(&entry[0]) .current_dir(hook.work_dir()) .args(&entry[1..]) .envs(&hook.env) .args(&hook.args) - .args(batch) + .file_args(batch) .check(false) .stdin(Stdio::null()) .env(EnvVars::PATH, &path_env) diff --git a/crates/prek/src/languages/dart.rs b/crates/prek/src/languages/dart.rs index cc9148309..66f207a82 100644 --- a/crates/prek/src/languages/dart.rs +++ b/crates/prek/src/languages/dart.rs @@ -180,14 +180,14 @@ impl LanguageImpl for Dart { } let run = async |batch: &[&Path]| { - let mut output = Cmd::new(&entry[0], "run dart command") + let mut output = Cmd::new(&entry[0]) .current_dir(hook.work_dir()) .args(&entry[1..]) .env(EnvVars::PATH, &new_path) .env(EnvVars::PUB_CACHE, env_dir) .envs(&hook.env) .args(&hook.args) - .args(batch) + .file_args(batch) .check(false) .stdin(Stdio::null()) .pty_output_with_sink(reporter.output_sink(progress)) @@ -304,7 +304,7 @@ async fn compile_executables( output = output_path.display(), ); - Cmd::new(dart, "dart compile exe") + Cmd::new(dart) .arg("compile") .arg("exe") .arg(format!("--packages={}", packages_path.display())) @@ -372,7 +372,7 @@ async fn install_package_config( let pubspec_path = env_path.join(PUBSPEC_YAML); fs_err::tokio::write(&pubspec_path, pubspec_content).await?; - Cmd::new(dart, "dart pub get") + Cmd::new(dart) .current_dir(env_path) .env(EnvVars::PUB_CACHE, env_path) .arg("pub") diff --git a/crates/prek/src/languages/deno/deno.rs b/crates/prek/src/languages/deno/deno.rs index 2ec0733f6..1d5a2ee4d 100644 --- a/crates/prek/src/languages/deno/deno.rs +++ b/crates/prek/src/languages/deno/deno.rs @@ -127,7 +127,7 @@ impl LanguageImpl for Deno { for spec in &hook.additional_dependencies { let (dep, name) = parse_install_dependency(spec); - let mut install_cmd = Cmd::new(deno.deno(), "deno install dependency"); + let mut install_cmd = Cmd::new(deno.deno()); install_cmd .current_dir(install_dir) .env(EnvVars::DENO_DIR, &deno_cache_dir) @@ -197,7 +197,7 @@ impl LanguageImpl for Deno { let entry = hook.entry.resolve(Some(&new_path), store)?; let run = async |batch: &[&Path]| { - let mut cmd = Cmd::new(&entry[0], "deno hook"); + let mut cmd = Cmd::new(&entry[0]); let mut output = cmd .current_dir(hook.work_dir()) .env(EnvVars::PATH, &new_path) @@ -206,7 +206,7 @@ impl LanguageImpl for Deno { .envs(&hook.env) .args(&entry[1..]) .args(&hook.args) - .args(batch) + .file_args(batch) .check(false) .stdin(Stdio::null()) .pty_output_with_sink(reporter.output_sink(progress)) diff --git a/crates/prek/src/languages/deno/installer.rs b/crates/prek/src/languages/deno/installer.rs index f9505f0ba..5c27e1dea 100644 --- a/crates/prek/src/languages/deno/installer.rs +++ b/crates/prek/src/languages/deno/installer.rs @@ -61,7 +61,7 @@ impl DenoResult { } pub(crate) async fn fill_version(mut self) -> Result { - let output = Cmd::new(&self.deno, "deno --version") + let output = Cmd::new(&self.deno) .env(EnvVars::DENO_NO_UPDATE_CHECK, "1") .arg("--version") .check(true) diff --git a/crates/prek/src/languages/docker.rs b/crates/prek/src/languages/docker.rs index 62bd91f6f..3323fe333 100644 --- a/crates/prek/src/languages/docker.rs +++ b/crates/prek/src/languages/docker.rs @@ -342,7 +342,7 @@ impl Docker { }; let tag = Self::docker_tag(install_info); - let mut cmd = Cmd::new(CONTAINER_RUNTIME.cmd(), "build docker image"); + let mut cmd = Cmd::new(CONTAINER_RUNTIME.cmd()); let cmd = cmd .arg("build") .arg("--tag") @@ -371,7 +371,7 @@ impl Docker { } pub(crate) fn docker_run_cmd(work_dir: &Path) -> Cmd { - let mut command = Cmd::new(CONTAINER_RUNTIME.cmd(), "run container"); + let mut command = Cmd::new(CONTAINER_RUNTIME.cmd()); command.arg("run").arg("--rm"); if *USE_COLOR { @@ -491,7 +491,7 @@ impl LanguageImpl for Docker { .arg(&docker_tag) .args(&entry[1..]) .args(&hook.args) - .args(batch) + .file_args(batch) .check(false) .stdin(Stdio::null()) .output_with_sink(reporter.output_sink(progress)) diff --git a/crates/prek/src/languages/docker_image.rs b/crates/prek/src/languages/docker_image.rs index 47abc713d..36449b0ba 100644 --- a/crates/prek/src/languages/docker_image.rs +++ b/crates/prek/src/languages/docker_image.rs @@ -53,7 +53,7 @@ impl LanguageImpl for DockerImage { .args(&env_args) .args(&entry[..]) .args(&hook.args) - .args(batch) + .file_args(batch) .check(false) .stdin(Stdio::null()) .output_with_sink(reporter.output_sink(progress)) diff --git a/crates/prek/src/languages/dotnet/dotnet.rs b/crates/prek/src/languages/dotnet/dotnet.rs index c845b2c99..08d6af170 100644 --- a/crates/prek/src/languages/dotnet/dotnet.rs +++ b/crates/prek/src/languages/dotnet/dotnet.rs @@ -132,14 +132,14 @@ impl LanguageImpl for Dotnet { let entry = hook.entry.resolve(Some(&new_path), store)?; let run = async |batch: &[&Path]| { - let mut output = Cmd::new(&entry[0], "run dotnet hook") + let mut output = Cmd::new(&entry[0]) .current_dir(hook.work_dir()) .args(&entry[1..]) .env(EnvVars::PATH, &new_path) .env(EnvVars::DOTNET_ROOT, &dotnet_root) .envs(&hook.env) .args(&hook.args) - .args(batch) + .file_args(batch) .check(false) .stdin(Stdio::null()) .pty_output_with_sink(reporter.output_sink(progress)) @@ -181,7 +181,7 @@ async fn install_tool(dotnet: &Path, tool_dir: &Path, dependency: &str) -> Resul }); let tool_cmd = |action: &str| { - let mut cmd = Cmd::new(dotnet, format!("dotnet tool {action}")); + let mut cmd = Cmd::new(dotnet); cmd.arg("tool") .arg(action) .arg("--tool-path") diff --git a/crates/prek/src/languages/dotnet/installer.rs b/crates/prek/src/languages/dotnet/installer.rs index caf0a28ad..af05d3a60 100644 --- a/crates/prek/src/languages/dotnet/installer.rs +++ b/crates/prek/src/languages/dotnet/installer.rs @@ -69,13 +69,13 @@ impl DotnetResult { } /// Builds a command that runs this `dotnet` executable. - pub(crate) fn cmd(&self, summary: &str) -> Cmd { - Cmd::new(&self.dotnet, summary) + pub(crate) fn cmd(&self) -> Cmd { + Cmd::new(&self.dotnet) } /// Fills the SDK version by running `dotnet --version`. pub(crate) async fn fill_version(mut self) -> Result { - let mut cmd = self.cmd("get dotnet version"); + let mut cmd = self.cmd(); if let Some(parent) = self.dotnet.parent() { cmd.current_dir(parent); } @@ -271,7 +271,7 @@ impl DotnetInstaller { install_dir: &Path, request: &DotnetRequest, ) -> Result<()> { - let mut cmd = Cmd::new("bash", "dotnet-install.sh"); + let mut cmd = Cmd::new("bash"); cmd.arg(script_path) .arg("--no-path") .arg("--install-dir") @@ -299,7 +299,7 @@ impl DotnetInstaller { install_dir: &Path, request: &DotnetRequest, ) -> Result<()> { - let mut cmd = Cmd::new("powershell.exe", "dotnet-install.ps1"); + let mut cmd = Cmd::new("powershell.exe"); cmd.arg("-NoProfile") .arg("-ExecutionPolicy") .arg("Bypass") diff --git a/crates/prek/src/languages/golang/golang.rs b/crates/prek/src/languages/golang/golang.rs index a0c57931a..30ad68904 100644 --- a/crates/prek/src/languages/golang/golang.rs +++ b/crates/prek/src/languages/golang/golang.rs @@ -69,13 +69,13 @@ impl LanguageImpl for Golang { let go_install_cmd = || { if go.is_from_system() { - let mut cmd = go.cmd("go install"); + let mut cmd = go.cmd(); cmd.arg("install") .env(EnvVars::GOTOOLCHAIN, "local") .env(EnvVars::GOBIN, bin_dir(&info.env_path)); cmd } else { - let mut cmd = go.cmd("go install"); + let mut cmd = go.cmd(); cmd.arg("install") .env(EnvVars::GOTOOLCHAIN, "local") .env(EnvVars::GOROOT, go_root) @@ -145,7 +145,7 @@ impl LanguageImpl for Golang { let entry = hook.entry.resolve(Some(&new_path), store)?; let run = async |batch: &[&Path]| { - let mut output = Cmd::new(&entry[0], "go hook") + let mut output = Cmd::new(&entry[0]) .current_dir(hook.work_dir()) .args(&entry[1..]) .env(EnvVars::PATH, &new_path) @@ -155,7 +155,7 @@ impl LanguageImpl for Golang { .envs(go_envs.iter().copied()) .envs(&hook.env) .args(&hook.args) - .args(batch) + .file_args(batch) .check(false) .stdin(Stdio::null()) .pty_output_with_sink(reporter.output_sink(progress)) diff --git a/crates/prek/src/languages/golang/installer.rs b/crates/prek/src/languages/golang/installer.rs index 969cd2867..8bb6b1d28 100644 --- a/crates/prek/src/languages/golang/installer.rs +++ b/crates/prek/src/languages/golang/installer.rs @@ -82,8 +82,8 @@ impl GoResult { self.from_system } - pub(crate) fn cmd(&self, summary: &str) -> Cmd { - Cmd::new(&self.path, summary) + pub(crate) fn cmd(&self) -> Cmd { + Cmd::new(&self.path) } pub(crate) fn with_version(mut self, version: GoVersion) -> Self { @@ -93,7 +93,7 @@ impl GoResult { pub(crate) async fn fill_version(mut self) -> Result { let output = self - .cmd("go version") + .cmd() .arg("version") .env(EnvVars::GOTOOLCHAIN, "local") .check(true) @@ -191,7 +191,7 @@ impl GoInstaller { } async fn resolve_version(&self, req: &GoRequest) -> Result { - let output = git::git_cmd("list go tags")? + let output = git::git_cmd()? .arg("ls-remote") .arg("--tags") .arg("https://github.com/golang/go") diff --git a/crates/prek/src/languages/haskell.rs b/crates/prek/src/languages/haskell.rs index 82bb22413..5446b28f5 100644 --- a/crates/prek/src/languages/haskell.rs +++ b/crates/prek/src/languages/haskell.rs @@ -72,7 +72,7 @@ impl LanguageImpl for Haskell { // `cabal update` is slow, so only run it once per process. CABAL_UPDATE_ONCE .get_or_try_init(async || { - Cmd::new("cabal", "update cabal package database") + Cmd::new("cabal") .arg("update") .check(true) .output() @@ -84,7 +84,7 @@ impl LanguageImpl for Haskell { } // cabal v2-install --installdir (default install-method is copy) - Cmd::new("cabal", "install haskell dependencies") + Cmd::new("cabal") .current_dir(search_path) .arg("v2-install") .arg("--installdir") @@ -125,13 +125,13 @@ impl LanguageImpl for Haskell { let entry = hook.entry.resolve(Some(&new_path), store)?; let run = async |batch: &[&Path]| { - let mut output = Cmd::new(&entry[0], "run haskell hook") + let mut output = Cmd::new(&entry[0]) .current_dir(hook.work_dir()) .args(&entry[1..]) .env(EnvVars::PATH, &new_path) .envs(&hook.env) .args(&hook.args) - .args(batch) + .file_args(batch) .check(false) .stdin(Stdio::null()) .pty_output_with_sink(reporter.output_sink(progress)) diff --git a/crates/prek/src/languages/julia.rs b/crates/prek/src/languages/julia.rs index 5ee618504..578a5b465 100644 --- a/crates/prek/src/languages/julia.rs +++ b/crates/prek/src/languages/julia.rs @@ -65,7 +65,7 @@ impl LanguageImpl for Julia { end "}; - Cmd::new("julia", "instantiate julia environment") + Cmd::new("julia") .current_dir(search_path) .arg("--startup-file=no") .arg(format!("--project={}", info.env_path.display())) @@ -89,7 +89,7 @@ impl LanguageImpl for Julia { } async fn check_health(&self, _info: &InstallInfo) -> Result<()> { - Cmd::new("julia", "check julia version") + Cmd::new("julia") .arg("--version") .check(true) .output() @@ -118,14 +118,14 @@ impl LanguageImpl for Julia { } let run = async |batch: &[&Path]| { - let mut output = Cmd::new("julia", "run julia hook") + let mut output = Cmd::new("julia") .current_dir(hook.work_dir()) .arg("--startup-file=no") .arg(format!("--project={}", env_dir.display())) .args(&entry) .envs(&hook.env) .args(&hook.args) - .args(batch) + .file_args(batch) .check(false) .stdin(Stdio::null()) .pty_output_with_sink(reporter.output_sink(progress)) diff --git a/crates/prek/src/languages/lua.rs b/crates/prek/src/languages/lua.rs index e9e2ba570..29b47c550 100644 --- a/crates/prek/src/languages/lua.rs +++ b/crates/prek/src/languages/lua.rs @@ -25,12 +25,7 @@ pub(crate) struct LuaInfo { } pub(crate) async fn query_lua_info() -> Result { - let stdout = Cmd::new("lua", "get lua version") - .arg("-v") - .check(true) - .output() - .await? - .stdout; + let stdout = Cmd::new("lua").arg("-v").check(true).output().await?.stdout; // Lua 5.4.8 Copyright (C) 1994-2025 Lua.org, PUC-Rio let version = String::from_utf8_lossy(&stdout) .split_whitespace() @@ -39,7 +34,7 @@ pub(crate) async fn query_lua_info() -> Result { .parse::() .context("Failed to parse Lua version")?; - let stdout = Cmd::new("luarocks", "get lua executable") + let stdout = Cmd::new("luarocks") .arg("config") .arg("variables.LUA") .check(true) @@ -147,7 +142,7 @@ impl LanguageImpl for Lua { let lua_cpath = Lua::get_lua_cpath(env_dir, &version); let run = async |batch: &[&Path]| { - let mut output = Cmd::new(&entry[0], "run lua command") + let mut output = Cmd::new(&entry[0]) .current_dir(hook.work_dir()) .args(&entry[1..]) .env(EnvVars::PATH, &new_path) @@ -155,7 +150,7 @@ impl LanguageImpl for Lua { .env(EnvVars::LUA_CPATH, &lua_cpath) .envs(&hook.env) .args(&hook.args) - .args(batch) + .file_args(batch) .check(false) .stdin(Stdio::null()) .pty_output_with_sink(reporter.output_sink(progress)) @@ -186,7 +181,7 @@ impl LanguageImpl for Lua { impl Lua { async fn install_rockspec(env_path: &Path, root_path: &Path, rockspec: &Path) -> Result<()> { - Cmd::new("luarocks", "luarocks make rockspec") + Cmd::new("luarocks") .current_dir(root_path) .arg("--tree") .arg(env_path) @@ -200,7 +195,7 @@ impl Lua { } async fn install_dependency(env_path: &Path, dependency: &str) -> Result<()> { - Cmd::new("luarocks", "luarocks install dependency") + Cmd::new("luarocks") .arg("--tree") .arg(env_path) .arg("install") diff --git a/crates/prek/src/languages/node/installer.rs b/crates/prek/src/languages/node/installer.rs index dec84cc34..d28866f4d 100644 --- a/crates/prek/src/languages/node/installer.rs +++ b/crates/prek/src/languages/node/installer.rs @@ -67,7 +67,7 @@ impl NodeResult { pub(crate) async fn fill_version(mut self) -> Result { // https://nodejs.org/api/process.html#processrelease - let output = Cmd::new(&self.node, "node -p") + let output = Cmd::new(&self.node) .arg("-p") .arg("JSON.stringify({version: process.version, lts: process.release.lts || false})") .check(true) diff --git a/crates/prek/src/languages/node/node.rs b/crates/prek/src/languages/node/node.rs index c315df1c9..92432a7e8 100644 --- a/crates/prek/src/languages/node/node.rs +++ b/crates/prek/src/languages/node/node.rs @@ -107,7 +107,7 @@ impl LanguageImpl for Node { let new_path = prepend_paths(&[&bin_dir, node_bin]).context("Failed to join PATH")?; let npm_cache = store.cache_path(CacheBucket::Npm); - let mut cmd = Cmd::new(node.npm(), "npm install"); + let mut cmd = Cmd::new(node.npm()); cmd.arg("install") .arg("-g") .arg("--no-progress") @@ -167,7 +167,7 @@ impl LanguageImpl for Node { let npm_cache = store.cache_path(CacheBucket::Npm); let run = async |batch: &[&Path]| { - let mut cmd = Cmd::new(&entry[0], "node hook"); + let mut cmd = Cmd::new(&entry[0]); cmd.current_dir(hook.work_dir()) .args(&entry[1..]) .env(EnvVars::PATH, &new_path) @@ -176,7 +176,7 @@ impl LanguageImpl for Node { apply_npm_config_env(&mut cmd, env_dir, &npm_cache); let mut output = cmd .args(&hook.args) - .args(batch) + .file_args(batch) .check(false) .stdin(Stdio::null()) .pty_output_with_sink(reporter.output_sink(progress)) diff --git a/crates/prek/src/languages/perl.rs b/crates/prek/src/languages/perl.rs index f0cc119d1..bb44373fc 100644 --- a/crates/prek/src/languages/perl.rs +++ b/crates/prek/src/languages/perl.rs @@ -41,7 +41,7 @@ impl LanguageImpl for Perl { )?; if let Some(repo_path) = hook.repo_path() { - Cmd::new(&cpan, "install perl dependencies") + Cmd::new(&cpan) .current_dir(repo_path) .arg("-T") .arg(".") @@ -52,7 +52,7 @@ impl LanguageImpl for Perl { .await .context("Failed to install Perl dependencies")?; } else if !hook.additional_dependencies.is_empty() { - Cmd::new(&cpan, "install perl dependencies") + Cmd::new(&cpan) .arg("-T") .args(&hook.additional_dependencies) .envs(perl_env(&info.env_path)?) @@ -90,14 +90,14 @@ impl LanguageImpl for Perl { let entry = hook.entry.resolve(Some(&new_path), store)?; let run = async |batch: &[&Path]| { - let mut output = Cmd::new(&entry[0], "run perl hook") + let mut output = Cmd::new(&entry[0]) .current_dir(hook.work_dir()) .args(&entry[1..]) .env(EnvVars::PATH, &new_path) .envs(perl_env(env_dir)?) .envs(&hook.env) .args(&hook.args) - .args(batch) + .file_args(batch) .check(false) .stdin(Stdio::null()) .pty_output_with_sink(reporter.output_sink(progress)) diff --git a/crates/prek/src/languages/pygrep/pygrep.rs b/crates/prek/src/languages/pygrep/pygrep.rs index a5e055157..6aa0e1269 100644 --- a/crates/prek/src/languages/pygrep/pygrep.rs +++ b/crates/prek/src/languages/pygrep/pygrep.rs @@ -111,7 +111,7 @@ impl LanguageImpl for Pygrep { // 2) If not found, try to find a system installed Python (system or system uv managed). debug!("No managed Python interpreter found, trying to find a system installed one"); let mut output = uv - .cmd("uv python find", store) + .cmd(store) .arg("python") .arg("find") .arg("--python-preference") @@ -133,7 +133,7 @@ impl LanguageImpl for Pygrep { // 3) If still not found, try to download a Python interpreter. debug!("No Python interpreter found, trying to install one"); output = uv - .cmd("uv python install", store) + .cmd(store) .arg("python") .arg("install") .arg(INSTALL_PYTHON_VERSION) @@ -200,7 +200,7 @@ impl LanguageImpl for Pygrep { .context("Failed to write Python script")?; let args = Args::parse(&hook.args).context("Failed to parse `args`")?; - let mut cmd = Cmd::new(&info.toolchain, "python script") + let mut cmd = Cmd::new(&info.toolchain) .current_dir(hook.work_dir()) .envs(&hook.env) .arg("-I") // Isolate mode. diff --git a/crates/prek/src/languages/python/python.rs b/crates/prek/src/languages/python/python.rs index 40fc22b90..0bdbb03af 100644 --- a/crates/prek/src/languages/python/python.rs +++ b/crates/prek/src/languages/python/python.rs @@ -61,7 +61,7 @@ async fn query_python_info(python: &Path) -> Result print(json.dumps(info)) "#}; - let stdout = Cmd::new(python, "python -c") + let stdout = Cmd::new(python) .arg("-I") .arg("-c") .arg(QUERY_PYTHON_INFO) @@ -197,7 +197,7 @@ impl LanguageImpl for Python { let entry = hook.entry.resolve(Some(&new_path), store)?; let run = async |batch: &[&Path]| { - let mut output = Cmd::new(&entry[0], "python hook") + let mut output = Cmd::new(&entry[0]) .current_dir(hook.work_dir()) .args(&entry[1..]) .env(EnvVars::VIRTUAL_ENV, env_dir) @@ -205,7 +205,7 @@ impl LanguageImpl for Python { .env_remove(EnvVars::PYTHONHOME) .envs(&hook.env) .args(&hook.args) - .args(batch) + .file_args(batch) .check(false) .stdin(Stdio::null()) .pty_output_with_sink(reporter.output_sink(progress)) @@ -262,7 +262,7 @@ impl Python { } fn pip_install_command(uv: &Uv, store: &Store, env_path: &Path) -> Cmd { - let mut cmd = uv.cmd("uv pip", store); + let mut cmd = uv.cmd(store); cmd.arg("pip") .arg("install") // Explicitly set project to root to avoid uv searching for project-level configs. @@ -333,7 +333,7 @@ impl Python { set_install_dir: bool, allow_downloads: bool, ) -> Cmd { - let mut cmd = uv.cmd("create venv", store); + let mut cmd = uv.cmd(store); cmd.arg("venv") .arg(&info.env_path) .args(["--python-preference", "managed"]) diff --git a/crates/prek/src/languages/python/uv.rs b/crates/prek/src/languages/python/uv.rs index 9bd40a7d0..18807047b 100644 --- a/crates/prek/src/languages/python/uv.rs +++ b/crates/prek/src/languages/python/uv.rs @@ -370,7 +370,7 @@ impl InstallSource { async fn install_from_pip(&self, target: &Path) -> Result<()> { // When running `pip install` in multiple threads, it can fail // without extracting files properly. - Cmd::new("python3", "pip install uv") + Cmd::new("python3") .arg("-m") .arg("pip") .arg("install") @@ -415,8 +415,8 @@ impl Uv { Self { path } } - pub(crate) fn cmd(&self, summary: &str, store: &Store) -> Cmd { - let mut cmd = Cmd::new(&self.path, summary); + pub(crate) fn cmd(&self, store: &Store) -> Cmd { + let mut cmd = Cmd::new(&self.path); cmd.env(EnvVars::UV_CACHE_DIR, store.cache_path(CacheBucket::Uv)); cmd } diff --git a/crates/prek/src/languages/r.rs b/crates/prek/src/languages/r.rs index fd790505e..247309e0a 100644 --- a/crates/prek/src/languages/r.rs +++ b/crates/prek/src/languages/r.rs @@ -53,7 +53,6 @@ impl LanguageImpl for R { // renv and choose the project library instead of overriding .libPaths. run_r_code( &rscript, - "install R hook environment", &renv_project_install_code(repo_path), &hook.additional_dependencies, &info.env_path, @@ -83,7 +82,6 @@ impl LanguageImpl for R { run_r_code( &rscript, - "install R additional dependencies", &additional_dependency_install_code(&info.env_path), &hook.additional_dependencies, hook.work_dir(), @@ -140,7 +138,7 @@ impl LanguageImpl for R { let entry = r_hook_entry(hook)?; let run = async |batch: &[&Path]| { - let mut cmd = Cmd::new(&entry[0], "run R hook"); + let mut cmd = Cmd::new(&entry[0]); cmd.current_dir(hook.work_dir()) .args(&entry[1..]) .env_remove(EnvVars::RENV_PROJECT) @@ -149,7 +147,7 @@ impl LanguageImpl for R { cmd.envs(&hook.env) .args(&hook.args) - .args(batch) + .file_args(batch) .check(false); let mut output = cmd @@ -181,7 +179,6 @@ impl LanguageImpl for R { async fn run_r_code( rscript: &Path, - summary: &str, code: &str, args: &FxHashSet, cwd: &Path, @@ -200,7 +197,7 @@ async fn run_r_code( ) .await?; - Cmd::new(rscript, summary) + Cmd::new(rscript) .current_dir(cwd) .arg("--vanilla") .arg(script_path) @@ -213,7 +210,7 @@ async fn run_r_code( } async fn query_r_version(rscript: &Path) -> Result { - let output = Cmd::new(rscript, "get R version") + let output = Cmd::new(rscript) .arg("--vanilla") .arg("-e") .arg("cat(as.character(getRversion()))") diff --git a/crates/prek/src/languages/ruby/gem.rs b/crates/prek/src/languages/ruby/gem.rs index 8d0422eed..fca2b46b3 100644 --- a/crates/prek/src/languages/ruby/gem.rs +++ b/crates/prek/src/languages/ruby/gem.rs @@ -58,7 +58,7 @@ async fn build_gemspec(ruby: &RubyResult, gemspec_path: &Path) -> Result, ) -> Result> { - let mut cmd = Cmd::new(ruby.ruby_bin(), "gem install --explain"); + let mut cmd = Cmd::new(ruby.ruby_bin()); cmd.arg("-S") .arg("gem") .arg("install") @@ -224,7 +224,7 @@ async fn install_single_gem( gem: &ResolvedGem, local_path: Option<&Path>, ) -> Result<()> { - let mut cmd = Cmd::new(ruby.ruby_bin(), format!("gem install {}", gem.name)); + let mut cmd = Cmd::new(ruby.ruby_bin()); cmd.arg("-S") .arg("gem") .arg("install") @@ -259,7 +259,7 @@ async fn install_gems_sequential( gem_files: &[PathBuf], additional_dependencies: &FxHashSet, ) -> Result<()> { - let mut cmd = Cmd::new(ruby.ruby_bin(), "gem install"); + let mut cmd = Cmd::new(ruby.ruby_bin()); cmd.arg("-S") .arg("gem") .arg("install") diff --git a/crates/prek/src/languages/ruby/installer.rs b/crates/prek/src/languages/ruby/installer.rs index d58d37911..32f7ff58c 100644 --- a/crates/prek/src/languages/ruby/installer.rs +++ b/crates/prek/src/languages/ruby/installer.rs @@ -559,7 +559,7 @@ fn find_gem_for_ruby(ruby_path: &Path) -> Result { pub(crate) async fn query_ruby_info(ruby_path: &Path) -> Result<(semver::Version, String)> { let script = "puts RUBY_ENGINE; puts RUBY_VERSION"; - let output = Cmd::new(ruby_path, "query ruby version") + let output = Cmd::new(ruby_path) .arg("-e") .arg(script) .check(true) diff --git a/crates/prek/src/languages/ruby/ruby.rs b/crates/prek/src/languages/ruby/ruby.rs index 145b49ab8..838e03329 100644 --- a/crates/prek/src/languages/ruby/ruby.rs +++ b/crates/prek/src/languages/ruby/ruby.rs @@ -157,7 +157,7 @@ impl LanguageImpl for Ruby { // Execute in batches let run = async |batch: &[&Path]| { - let mut output = Cmd::new(&entry[0], "ruby hook") + let mut output = Cmd::new(&entry[0]) .current_dir(hook.work_dir()) .env(EnvVars::PATH, &new_path) .env(EnvVars::GEM_HOME, &gem_home) @@ -167,7 +167,7 @@ impl LanguageImpl for Ruby { .envs(&hook.env) .args(&entry[1..]) .args(&hook.args) - .args(batch) + .file_args(batch) .check(false) .stdin(Stdio::null()) .pty_output_with_sink(reporter.output_sink(progress)) diff --git a/crates/prek/src/languages/rust/installer.rs b/crates/prek/src/languages/rust/installer.rs index fff0d2187..76f10ce5a 100644 --- a/crates/prek/src/languages/rust/installer.rs +++ b/crates/prek/src/languages/rust/installer.rs @@ -53,7 +53,7 @@ impl RustResult { .join("rustc") .with_extension(std::env::consts::EXE_EXTENSION); - let output = Cmd::new(rustc, "rustc --version") + let output = Cmd::new(rustc) .arg("--version") .env(EnvVars::RUSTUP_AUTO_INSTALL, "0") .check(true) @@ -168,7 +168,7 @@ impl RustInstaller { | RustRequest::MajorMinor(_, _) | RustRequest::MajorMinorPatch(_, _, _) | RustRequest::Range(_, _) => { - let output = crate::git::git_cmd("list rust tags")? + let output = crate::git::git_cmd()? .arg("ls-remote") .arg("--tags") .arg("https://github.com/rust-lang/rust") diff --git a/crates/prek/src/languages/rust/rust.rs b/crates/prek/src/languages/rust/rust.rs index 308943d71..04c6bb1cb 100644 --- a/crates/prek/src/languages/rust/rust.rs +++ b/crates/prek/src/languages/rust/rust.rs @@ -139,7 +139,7 @@ async fn find_package_dir( ) -> anyhow::Result> { let cargo = cargo.unwrap_or(Path::new("cargo")); - let mut cmd = Cmd::new(cargo, "cargo metadata"); + let mut cmd = Cmd::new(cargo); if let Some(new_path) = new_path { cmd.env(EnvVars::PATH, new_path); } @@ -285,7 +285,7 @@ async fn install_local_project( if lib_deps.is_empty() { // For packages without lib deps, use `cargo install` directly - Cmd::new(cargo, "install local") + Cmd::new(cargo) .args(["install", "--bins", "--root"]) .arg(&info.env_path) .args(["--path", ".", "--locked"]) @@ -333,7 +333,7 @@ async fn install_local_project( } // Run cargo add on the copied manifest - let mut cmd = Cmd::new(cargo, "add dependencies"); + let mut cmd = Cmd::new(cargo); cmd.arg("add"); for dep in lib_deps { cmd.arg(format_cargo_dependency(dep.as_str())); @@ -349,7 +349,7 @@ async fn install_local_project( // Build using cargo build with --manifest-path pointing to modified manifest // but source files come from original package_dir let target_dir = info.env_path.join("target"); - let mut cmd = Cmd::new(cargo, "build local with deps"); + let mut cmd = Cmd::new(cargo); cmd.args(["build", "--bins", "--release"]) .arg("--manifest-path") .arg(&dst_manifest) @@ -389,7 +389,7 @@ async fn install_cli_dependency( ) -> anyhow::Result<()> { let dep = CargoCliDependency::from_str(cli_dep)?; - let mut cmd = Cmd::new(cargo, "install cli dep"); + let mut cmd = Cmd::new(cargo); cmd.args(["install", "--bins", "--root"]) .arg(&info.env_path) .args(dep.to_cargo_args()) @@ -529,7 +529,7 @@ impl LanguageImpl for Rust { let entry = hook.entry.resolve(Some(&new_path), store)?; let run = async |batch: &[&Path]| { - let mut output = Cmd::new(&entry[0], "rust hook") + let mut output = Cmd::new(&entry[0]) .current_dir(hook.work_dir()) .args(&entry[1..]) .env(EnvVars::PATH, &new_path) @@ -537,7 +537,7 @@ impl LanguageImpl for Rust { .env(EnvVars::RUSTUP_AUTO_INSTALL, "0") .envs(&hook.env) .args(&hook.args) - .args(batch) + .file_args(batch) .check(false) .stdin(Stdio::null()) .pty_output_with_sink(reporter.output_sink(progress)) diff --git a/crates/prek/src/languages/rust/rustup.rs b/crates/prek/src/languages/rust/rustup.rs index 960ef17e8..c99a37359 100644 --- a/crates/prek/src/languages/rust/rustup.rs +++ b/crates/prek/src/languages/rust/rustup.rs @@ -155,7 +155,7 @@ impl Rustup { } pub(crate) async fn install_toolchain(&self, toolchain: &str) -> Result { - let output = Cmd::new(&self.bin, "rustup toolchain install") + let output = Cmd::new(&self.bin) .env(EnvVars::RUSTUP_HOME, &self.rustup_home) .env(EnvVars::RUSTUP_AUTO_INSTALL, "0") .arg("toolchain") @@ -194,7 +194,7 @@ impl Rustup { /// List installed toolchains managed by prek. pub(crate) async fn list_installed_toolchains(&self) -> Result> { - let output = Cmd::new(&self.bin, "rustup list toolchains") + let output = Cmd::new(&self.bin) .arg("toolchain") .arg("list") .arg("-v") @@ -228,7 +228,7 @@ impl Rustup { /// List system-installed Rust toolchains. pub(crate) async fn list_system_toolchains(&self) -> Result> { - let output = Cmd::new(&self.bin, "rustup toolchain list") + let output = Cmd::new(&self.bin) .arg("toolchain") .arg("list") .arg("-v") @@ -288,7 +288,7 @@ async fn toolchain_info(name: String, toolchain_dir: PathBuf) -> Result Result { // macOS: "swift-driver version: X.Y.Z Apple Swift version X.Y.Z ..." // Linux/Windows: "Swift version X.Y.Z ..." - let stdout = Cmd::new("swift", "get swift version") + let stdout = Cmd::new("swift") .arg("--version") .check(true) .output() @@ -111,7 +111,7 @@ impl LanguageImpl for Swift { if repo_path.join("Package.swift").exists() { debug!(%hook, "Building Swift package"); let build_path = build_dir(&info.env_path); - Cmd::new("swift", "swift build") + Cmd::new("swift") .arg("build") .arg("-c") .arg("release") @@ -125,7 +125,7 @@ impl LanguageImpl for Swift { .context("Failed to build Swift package")?; // Get the actual bin path (includes target triple, e.g., .build/arm64-apple-macosx/release) - let bin_path_output = Cmd::new("swift", "get bin path") + let bin_path_output = Cmd::new("swift") .arg("build") .arg("-c") .arg("release") @@ -193,13 +193,13 @@ impl LanguageImpl for Swift { let entry = hook.entry.resolve(Some(&new_path), store)?; let run = async |batch: &[&Path]| { - let mut output = Cmd::new(&entry[0], "swift hook") + let mut output = Cmd::new(&entry[0]) .current_dir(hook.work_dir()) .args(&entry[1..]) .env(EnvVars::PATH, &new_path) .envs(&hook.env) .args(&hook.args) - .args(batch) + .file_args(batch) .check(false) .stdin(Stdio::null()) .pty_output_with_sink(reporter.output_sink(progress)) diff --git a/crates/prek/src/languages/system.rs b/crates/prek/src/languages/system.rs index 44c15766f..07e717bc3 100644 --- a/crates/prek/src/languages/system.rs +++ b/crates/prek/src/languages/system.rs @@ -41,12 +41,12 @@ impl LanguageImpl for System { let entry = hook.entry.resolve(None, store)?; let run = async |batch: &[&Path]| { - let mut output = Cmd::new(&entry[0], "run system command") + let mut output = Cmd::new(&entry[0]) .current_dir(hook.work_dir()) .envs(&hook.env) .args(&entry[1..]) .args(&hook.args) - .args(batch) + .file_args(batch) .check(false) .stdin(Stdio::null()) .pty_output_with_sink(reporter.output_sink(progress)) diff --git a/crates/prek/src/process.rs b/crates/prek/src/process.rs index bf9c132d3..637d206aa 100644 --- a/crates/prek/src/process.rs +++ b/crates/prek/src/process.rs @@ -30,38 +30,28 @@ use std::fmt::Display; use std::path::Path; use std::process::Output; use std::process::{CommandArgs, CommandEnvs, ExitStatus, Stdio}; -use std::sync::LazyLock; use owo_colors::OwoColorize; -use prek_consts::env_vars::EnvVars; use thiserror::Error; use tokio::io::AsyncReadExt; use tracing::trace; use crate::git::GIT; -static LOG_TRUNCATE_LIMIT: LazyLock = LazyLock::new(|| { - EnvVars::var(EnvVars::PREK_LOG_TRUNCATE_LIMIT) - .ok() - .and_then(|limit| limit.parse::().ok()) - .filter(|limit| *limit > 0) - .unwrap_or(120) -}); - -/// An error from executing a Command +/// An error from executing a command. #[derive(Debug, Error)] pub enum Error { - /// The command fundamentally failed to execute (usually means it didn't exist) - #[error("Run command `{summary}` failed")] + /// The command could not be started or monitored to completion. + #[error("Failed to run `{command}`")] Exec { - /// Summary of what the Command was trying to do - summary: String, - /// What failed + /// The command that failed. + command: String, + /// What failed. #[source] cause: std::io::Error, }, - #[error("Command `{summary}` exited with an error:\n{error}")] - Status { summary: String, error: StatusError }, + #[error("Command `{command}` exited with an error:\n{error}")] + Status { command: String, error: StatusError }, #[cfg(not(windows))] #[error("Failed to open pty")] Pty(#[from] prek_pty::Error), @@ -69,8 +59,7 @@ pub enum Error { PtySetup(#[from] std::io::Error), } -/// The command ran but signaled some kind of error condition -/// (assuming the exit code is used for that) +/// The command ran but signaled an error condition through its exit status. #[derive(Debug)] pub struct StatusError { pub status: ExitStatus, @@ -115,9 +104,9 @@ fn write_trimmed_output_section( /// A fancier Command, see the crate's top-level docs! pub struct Cmd { - /// The inner Command, in case you need to access it + /// The inner command, in case you need to access it. pub inner: tokio::process::Command, - summary: String, + file_args_start: Option, check_status: bool, } @@ -132,12 +121,12 @@ fn write_output_chunk(output: &mut Vec, sink: &mut impl OutputSink, chunk: & /// Constructors impl Cmd { - /// Create a new Command with an additional "summary" of what this is trying to do - pub fn new(command: impl AsRef, summary: impl Into) -> Self { + /// Create a new command. + pub fn new(command: impl AsRef) -> Self { let inner = tokio::process::Command::new(command); Self { - summary: summary.into(), inner, + file_args_start: None, check_status: true, } } @@ -156,15 +145,14 @@ impl Cmd { self } - /// Set whether `Status::success` should be checked after executions - /// (except `spawn`, which doesn't yet have a Status to check). + /// Set whether `ExitStatus::success` should be checked after executions + /// (except `spawn`, which doesn't yet have an exit status to check). /// /// Defaults to `true`. /// - /// If true, an Err will be produced by those execution commands. + /// If true, a non-zero exit status will produce an error. /// - /// Executions which produce status will pass them to [`Cmd::maybe_check_status`][], - /// which uses this setting. + /// Execution methods that return or capture an exit status use this setting. pub fn check(&mut self, checked: bool) -> &mut Self { self.check_status = checked; self @@ -173,31 +161,29 @@ impl Cmd { /// Execution APIs impl Cmd { - /// Equivalent to [`Cmd::status`][], + /// Equivalent to [`Cmd::status`], /// but doesn't bother returning the actual status code (because it's captured in the Result) pub async fn run(&mut self) -> Result<(), Error> { self.status().await?; Ok(()) } - /// Equivalent to [`std::process::Command::spawn`][], + /// Equivalent to [`std::process::Command::spawn`], /// but logged and with the error wrapped. pub fn spawn(&mut self) -> Result { self.log_command(); - self.inner.spawn().map_err(|cause| Error::Exec { - summary: self.summary.clone(), - cause, - }) + self.inner.spawn().map_err(|cause| self.exec_error(cause)) } - /// Equivalent to [`std::process::Command::output`][], + /// Equivalent to [`std::process::Command::output`], /// but logged, with the error wrapped, and status checked (by default) pub async fn output(&mut self) -> Result { self.log_command(); - let output = self.inner.output().await.map_err(|cause| Error::Exec { - summary: self.summary.clone(), - cause, - })?; + let output = self + .inner + .output() + .await + .map_err(|cause| self.exec_error(cause))?; self.maybe_check_output(&output)?; Ok(output) } @@ -214,10 +200,7 @@ impl Cmd { self.inner.stdout(Stdio::piped()); self.inner.stderr(Stdio::piped()); - let mut child = self.inner.spawn().map_err(|cause| Error::Exec { - summary: self.summary.clone(), - cause, - })?; + let mut child = self.inner.spawn().map_err(|cause| self.exec_error(cause))?; let mut stdout = child .stdout @@ -241,10 +224,7 @@ impl Cmd { Ok(0) => stdout_done = true, Ok(n) => write_output_chunk(&mut stdout_output, &mut sink, &stdout_buffer[..n]), Err(cause) => { - return Err(Error::Exec { - summary: self.summary.clone(), - cause, - }); + return Err(self.exec_error(cause)); } } } @@ -253,10 +233,7 @@ impl Cmd { Ok(0) => stderr_done = true, Ok(n) => write_output_chunk(&mut stderr_output, &mut sink, &stderr_buffer[..n]), Err(cause) => { - return Err(Error::Exec { - summary: self.summary.clone(), - cause, - }); + return Err(self.exec_error(cause)); } } } @@ -265,10 +242,7 @@ impl Cmd { // For regular pipes, EOF on both streams is the point where output capture is complete. // Waiting earlier must not make us return before trailing pipe bytes are read. - let status = child.wait().await.map_err(|cause| Error::Exec { - summary: self.summary.clone(), - cause, - })?; + let status = child.wait().await.map_err(|cause| self.exec_error(cause))?; let output = Output { status, stdout: stdout_output, @@ -336,26 +310,17 @@ impl Cmd { tokio::select! { read_result = pty.read(&mut buffer) => { match read_result { - Ok(0) => break child.wait().await.map_err(|cause| Error::Exec { - summary: self.summary.clone(), - cause, - })?, + Ok(0) => break child.wait().await.map_err(|cause| self.exec_error(cause))?, Ok(n) => write_output_chunk(&mut output, &mut sink, &buffer[..n]), // Linux reports PTY master EOF as EIO after all slave handles close. Err(err) if err.raw_os_error() == Some(libc::EIO) => { - break child.wait().await.map_err(|cause| Error::Exec { - summary: self.summary.clone(), - cause, - })?; + break child.wait().await.map_err(|cause| self.exec_error(cause))?; } Err(err) => return Err(Error::PtySetup(err)), } } status = child.wait() => { - let status = status.map_err(|cause| Error::Exec { - summary: self.summary.clone(), - cause, - })?; + let status = status.map_err(|cause| self.exec_error(cause))?; // Child exit can be observed before the PTY read future is woken. Drain any // bytes already available so fast commands do not lose their final output. loop { @@ -387,38 +352,58 @@ impl Cmd { Ok(output) } - /// Equivalent to [`std::process::Command::status`][] + /// Equivalent to [`std::process::Command::status`] /// but logged, with the error wrapped, and status checked (by default) pub async fn status(&mut self) -> Result { self.log_command(); - let status = self.inner.status().await.map_err(|cause| Error::Exec { - summary: self.summary.clone(), - cause, - })?; + let status = self + .inner + .status() + .await + .map_err(|cause| self.exec_error(cause))?; self.maybe_check_status(status)?; Ok(status) } } -/// Transparently forwarded [`std::process::Command`][] APIs +/// Selected forwarded [`std::process::Command`] APIs. impl Cmd { - /// Forwards to [`std::process::Command::arg`][] + /// Forwards to [`std::process::Command::arg`]. pub fn arg>(&mut self, arg: S) -> &mut Self { + debug_assert!( + self.file_args_start.is_none(), + "regular arguments must be added before file-list arguments" + ); self.inner.arg(arg); self } - /// Forwards to [`std::process::Command::args`][] + /// Forwards to [`std::process::Command::args`]. pub fn args(&mut self, args: I) -> &mut Self where I: IntoIterator, S: AsRef, { + for arg in args { + self.arg(arg); + } + self + } + + /// Append trailing file-list arguments without showing them in the display command. + pub fn file_args(&mut self, args: I) -> &mut Self + where + I: IntoIterator, + S: AsRef, + { + if self.file_args_start.is_none() { + self.file_args_start = Some(self.get_args().count()); + } self.inner.args(args); self } - /// Forwards to [`std::process::Command::env`][] + /// Forwards to [`std::process::Command::env`]. pub fn env(&mut self, key: K, val: V) -> &mut Self where K: AsRef, @@ -428,7 +413,7 @@ impl Cmd { self } - /// Forwards to [`std::process::Command::envs`][] + /// Forwards to [`std::process::Command::envs`]. pub fn envs(&mut self, vars: I) -> &mut Self where I: IntoIterator, @@ -439,58 +424,58 @@ impl Cmd { self } - /// Forwards to [`std::process::Command::env_remove`][] + /// Forwards to [`std::process::Command::env_remove`]. pub fn env_remove>(&mut self, key: K) -> &mut Self { self.inner.env_remove(key); self } - /// Forwards to [`std::process::Command::env_clear`][] + /// Forwards to [`std::process::Command::env_clear`]. pub fn env_clear(&mut self) -> &mut Self { self.inner.env_clear(); self } - /// Forwards to [`std::process::Command::current_dir`][] + /// Forwards to [`std::process::Command::current_dir`]. pub fn current_dir>(&mut self, dir: P) -> &mut Self { self.inner.current_dir(dir); self } - /// Forwards to [`std::process::Command::stdin`][] + /// Forwards to [`std::process::Command::stdin`]. pub fn stdin>(&mut self, cfg: T) -> &mut Self { self.inner.stdin(cfg); self } - /// Forwards to [`std::process::Command::stdout`][] + /// Forwards to [`std::process::Command::stdout`]. pub fn stdout>(&mut self, cfg: T) -> &mut Self { self.inner.stdout(cfg); self } - /// Forwards to [`std::process::Command::stderr`][] + /// Forwards to [`std::process::Command::stderr`]. pub fn stderr>(&mut self, cfg: T) -> &mut Self { self.inner.stderr(cfg); self } - /// Forwards to [`std::process::Command::get_program`][] + /// Forwards to [`std::process::Command::get_program`]. pub fn get_program(&self) -> &OsStr { self.inner.as_std().get_program() } - /// Forwards to [`std::process::Command::get_args`][] + /// Forwards to [`std::process::Command::get_args`]. pub fn get_args(&self) -> CommandArgs<'_> { self.inner.as_std().get_args() } - /// Forwards to [`std::process::Command::get_envs`][] + /// Forwards to [`std::process::Command::get_envs`]. pub fn get_envs(&self) -> CommandEnvs<'_> { self.inner.as_std().get_envs() } - /// Forwards to [`std::process::Command::get_current_dir`][] + /// Forwards to [`std::process::Command::get_current_dir`]. pub fn get_current_dir(&self) -> Option<&Path> { self.inner.as_std().get_current_dir() } @@ -504,38 +489,41 @@ impl Cmd { } } -/// Diagnostic APIs (used internally, but available for yourself) +/// Diagnostic APIs used by execution methods and direct child-process callers. impl Cmd { - /// Check `Status::success`, producing a contextual Error if it's `false`. + fn exec_error(&self, cause: std::io::Error) -> Error { + Error::Exec { + command: self.display_command(), + cause, + } + } + + fn status_error(&self, status: ExitStatus, output: Option) -> Error { + Error::Status { + command: self.display_command(), + error: StatusError { status, output }, + } + } + + /// Check `ExitStatus::success`, producing a contextual error if it's `false`. pub fn check_status(&self, status: ExitStatus) -> Result<(), Error> { if status.success() { Ok(()) } else { - Err(Error::Status { - summary: self.summary.clone(), - error: StatusError { - status, - output: None, - }, - }) + Err(self.status_error(status, None)) } } + /// Check `Output::status`, producing a contextual error if it's not successful. pub fn check_output(&self, output: &Output) -> Result<(), Error> { if output.status.success() { Ok(()) } else { - Err(Error::Status { - summary: self.summary.clone(), - error: StatusError { - status: output.status, - output: Some(output.clone()), - }, - }) + Err(self.status_error(output.status, Some(output.clone()))) } } - /// Invoke [`Cmd::check_status`][] if [`Cmd::check`][] is `true` + /// Invoke [`Cmd::check_status`] if [`Cmd::check`] is `true` /// (defaults to `true`). pub fn maybe_check_status(&self, status: ExitStatus) -> Result<(), Error> { if self.check_status { @@ -544,7 +532,7 @@ impl Cmd { Ok(()) } - /// Invoke [`Cmd::check_status`][] if [`Cmd::check`][] is `true` + /// Invoke [`Cmd::check_output`] if [`Cmd::check`] is `true` /// (defaults to `true`). pub fn maybe_check_output(&self, output: &Output) -> Result<(), Error> { if self.check_status { @@ -553,11 +541,27 @@ impl Cmd { Ok(()) } - /// Log the current Command using the method specified by [`Cmd::log`][] - /// (defaults to [`tracing::info!`][]). + /// Log the current command with [`tracing::trace!`]. pub fn log_command(&self) { trace!("Executing `{self}`"); } + + fn display_arg_count(&self) -> usize { + self.file_args_start + .unwrap_or_else(|| self.get_args().count()) + } + + fn display_command(&self) -> String { + let mut command = String::new(); + write_display_command( + &mut command, + None, + self.get_program(), + self.get_args().take(self.display_arg_count()), + ) + .expect("writing to a string cannot fail"); + command + } } /// Returns the number of arguments to skip. @@ -583,42 +587,53 @@ fn skip_args(cmd: &OsStr, cur: &OsStr, next: Option<&&OsStr>) -> usize { 0 } -/// Simplified Command Debug output, with args truncated if they're too long. +/// Simplified command output, omitting trailing file-list arguments. impl Display for Cmd { fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { - if let Some(cwd) = self.get_current_dir() { - write!(f, "cd {} && ", cwd.to_string_lossy())?; - } - let program = self.get_program(); - let mut args = self.get_args().peekable(); + write_display_command( + f, + self.get_current_dir(), + self.get_program(), + self.get_args().take(self.display_arg_count()), + ) + } +} - write!(f, "{}", program.to_string_lossy())?; - if args.peek().is_some_and(|arg| *arg == program) { - args.next(); // Skip the program if it's repeated - } +fn write_display_command<'a>( + f: &mut impl std::fmt::Write, + cwd: Option<&Path>, + program: &OsStr, + args: impl IntoIterator, +) -> std::fmt::Result { + if let Some(cwd) = cwd { + write!(f, "cd {} && ", cwd.to_string_lossy())?; + } - let mut len = 0; - while let Some(arg) = args.next() { - let skip = skip_args(program, arg, args.peek()); - if skip > 0 { - for _ in 1..skip { - args.next(); - } - continue; - } - write!(f, " {}", arg.to_string_lossy())?; - len += arg.len() + 1; - if len > *LOG_TRUNCATE_LIMIT { - write!(f, " [...]")?; - break; + let program_display = program.to_string_lossy(); + write!(f, "{program_display}")?; + let mut args = args.into_iter().peekable(); + if args.peek().is_some_and(|arg| *arg == program) { + args.next(); // Skip the program if it's repeated + } + + while let Some(arg) = args.next() { + let skip = skip_args(program, arg, args.peek()); + if skip > 0 { + for _ in 1..skip { + args.next(); } + continue; } - Ok(()) + + write!(f, " {}", arg.to_string_lossy())?; } + + Ok(()) } #[cfg(all(test, not(windows)))] mod tests { + use std::error::Error as _; use std::sync::{Arc, Mutex}; use super::{Cmd, OutputSink}; @@ -634,10 +649,42 @@ mod tests { } } + #[tokio::test] + async fn status_reports_missing_executable_name() { + let err = Cmd::new("__prek_missing_command__") + .status() + .await + .expect_err("command should not exist"); + + assert_eq!(err.to_string(), "Failed to run `__prek_missing_command__`"); + let source = err.source().expect("missing executable error has source"); + let io_error = source + .downcast_ref::() + .expect("source is an io error"); + assert_eq!(io_error.kind(), std::io::ErrorKind::NotFound); + } + + #[test] + fn display_command_omits_file_args() { + let mut cmd = Cmd::new("prek"); + cmd.arg("run") + .arg("hook-id") + .file_args(["file-0.rs", "file-1.rs"]); + + assert_eq!(cmd.display_command(), "prek run hook-id"); + assert_eq!(cmd.to_string(), "prek run hook-id"); + assert_eq!( + cmd.get_args() + .map(|arg| arg.to_string_lossy().into_owned()) + .collect::>(), + ["run", "hook-id", "file-0.rs", "file-1.rs"] + ); + } + #[tokio::test] async fn output_with_sink_streams_piped_stdout_and_stderr() { let chunks = Arc::new(Mutex::new(0)); - let output = Cmd::new("/bin/sh", "piped streaming output test") + let output = Cmd::new("/bin/sh") .arg("-c") .arg("printf 'OUT\\n'; printf 'ERR\\n' >&2") .check(false) @@ -658,7 +705,7 @@ mod tests { #[tokio::test] async fn pty_output_captures_trailing_output_after_fast_exit() { for _ in 0..20 { - let output = Cmd::new("/bin/sh", "pty trailing output test") + let output = Cmd::new("/bin/sh") .arg("-c") .arg("printf 'FINAL\\n'") .check(false) diff --git a/docs/reference/environment-variables.md b/docs/reference/environment-variables.md index fae9b536d..7e6e13346 100644 --- a/docs/reference/environment-variables.md +++ b/docs/reference/environment-variables.md @@ -91,11 +91,6 @@ Disable passing the runtime's `--init` flag when running `docker` and `docker_im This is a compatibility escape hatch for container environments that cannot run the init helper. Disabling `--init` can leave containers running after Ctrl-C if the container's PID 1 does not handle forwarded signals. -### `PREK_LOG_TRUNCATE_LIMIT` - -Control the truncation limit for command lines shown in trace logs (`Executing ...`). -Defaults to `120` characters of arguments; set a larger value to reduce truncation. - ### `PREK_RUBY_MIRROR` Override the Ruby installer base URL used for downloaded Ruby toolchains (for example, when using mirrors or air-gapped CI environments). From deb7054f57d100ebf560cc3fc92aa9a5fa8e9773 Mon Sep 17 00:00:00 2001 From: Jo <10510431+j178@users.noreply.github.com> Date: Sat, 27 Jun 2026 17:42:55 +0800 Subject: [PATCH 2/2] Address subprocess review comments --- crates/prek/src/cli/run/keeper.rs | 4 +- crates/prek/src/cli/run/run.rs | 2 +- crates/prek/src/git.rs | 31 +-- .../pre_commit_hooks/forbid_new_submodules.rs | 2 +- crates/prek/src/languages/python/python.rs | 34 ++- crates/prek/src/process.rs | 250 ++++++++++++------ crates/prek/tests/languages/python.rs | 14 +- crates/prek/tests/run.rs | 21 +- 8 files changed, 228 insertions(+), 130 deletions(-) diff --git a/crates/prek/src/cli/run/keeper.rs b/crates/prek/src/cli/run/keeper.rs index 0c9d0c95a..4484bbe95 100644 --- a/crates/prek/src/cli/run/keeper.rs +++ b/crates/prek/src/cli/run/keeper.rs @@ -91,11 +91,9 @@ impl UnstagedChangesRestorer { let mut cmd = git_cmd()?; let output = cmd .arg("diff-index") - .arg("--ignore-submodules") .arg("--binary") .arg("--exit-code") - .arg("--no-color") - .arg("--no-ext-diff") + .hidden_args(["--ignore-submodules", "--no-color", "--no-ext-diff"]) .arg(tree) .arg("--") .arg(root) diff --git a/crates/prek/src/cli/run/run.rs b/crates/prek/src/cli/run/run.rs index fc3d7690c..c064c8fbb 100644 --- a/crates/prek/src/cli/run/run.rs +++ b/crates/prek/src/cli/run/run.rs @@ -1101,7 +1101,7 @@ impl<'a> HookRunSession<'a> { git::git_cmd()? .arg("--no-pager") .arg("diff") - .arg("--no-ext-diff") + .hidden_args(["--no-ext-diff"]) .arg(color) .arg("--") .arg(workspace.root()) diff --git a/crates/prek/src/git.rs b/crates/prek/src/git.rs index 75251b50a..4b742d054 100644 --- a/crates/prek/src/git.rs +++ b/crates/prek/src/git.rs @@ -78,7 +78,7 @@ pub(crate) static GIT_ENV_TO_REMOVE: LazyLock> = LazyLock: pub(crate) fn git_cmd() -> Result { let mut cmd = Cmd::new(GIT.as_ref().map_err(|&e| Error::GitNotFound(e))?); - cmd.arg("-c").arg("core.useBuiltinFSMonitor=false"); + cmd.hidden_args(["-c", "core.useBuiltinFSMonitor=false"]); Ok(cmd) } @@ -107,8 +107,7 @@ fn path_from_git_bytes(bytes: &[u8]) -> Result { pub(crate) async fn intent_to_add_files(root: &Path) -> Result, Error> { let output = git_cmd()? .arg("diff") - .arg("--no-ext-diff") - .arg("--ignore-submodules") + .hidden_args(["--no-ext-diff", "--ignore-submodules"]) .arg("--diff-filter=A") .arg("--name-only") .arg("-z") @@ -148,7 +147,7 @@ pub(crate) async fn get_changed_files( cmd.arg("diff") .arg("--name-only") .arg("--diff-filter=ACMRT") - .arg("--no-ext-diff") // Disable external diff drivers + .hidden_args(["--no-ext-diff"]) .arg("-z") // Use NUL as line terminator .arg(range) .arg("--") @@ -256,7 +255,7 @@ pub(crate) async fn get_staged_files(root: &Path) -> Result, Error> .arg("--cached") .arg("--name-only") .arg("--diff-filter=ACMRTUXB") // Everything except for D - .arg("--no-ext-diff") // Disable external diff drivers + .hidden_args(["--no-ext-diff"]) .arg("-z") // Use NUL as line terminator .check(true) .output() @@ -269,7 +268,7 @@ pub(crate) async fn files_not_staged(files: &[&Path]) -> Result> { .arg("diff") .arg("--exit-code") .arg("--name-only") - .arg("--no-ext-diff") + .hidden_args(["--no-ext-diff"]) .arg("-z") // Use NUL as line terminator .file_args(files) .check(false) @@ -316,7 +315,7 @@ pub(crate) async fn get_conflicted_files(root: &Path) -> Result, Er let output = git_cmd()? .arg("diff") .arg("--name-only") - .arg("--no-ext-diff") // Disable external diff drivers + .hidden_args(["--no-ext-diff"]) .arg("-z") // Use NUL as line terminator .arg("-m") // Show diffs for merge commits in the default format. .arg(String::from_utf8_lossy(&tree.stdout).trim_ascii()) @@ -357,9 +356,7 @@ pub(crate) async fn has_worktree_diff(path: &Path) -> Result { let status = cmd .arg("diff-files") .arg("--quiet") - .arg("--no-ext-diff") - .arg("--no-textconv") - .arg("--ignore-submodules") + .hidden_args(["--no-ext-diff", "--no-textconv", "--ignore-submodules"]) .arg("--") .arg(path) .check(false) @@ -381,9 +378,7 @@ pub(crate) async fn has_worktree_diff(path: &Path) -> Result { pub(crate) async fn get_diff(path: &Path) -> Result, Error> { let output = git_cmd()? .arg("diff") - .arg("--no-ext-diff") // Disable external diff drivers - .arg("--no-textconv") - .arg("--ignore-submodules") + .hidden_args(["--no-ext-diff", "--no-textconv", "--ignore-submodules"]) .arg("--") .arg(path) // This diff is only used as a best-effort before/after snapshot of @@ -434,9 +429,7 @@ pub(crate) fn get_root() -> Result { })); } - Ok(PathBuf::from( - String::from_utf8_lossy(&output.stdout).trim_ascii(), - )) + path_from_git_bytes(output.stdout.trim_ascii()).map_err(Error::from) } pub(crate) async fn init_repo(url: &str, path: &Path) -> Result<(), Error> { @@ -528,8 +521,7 @@ async fn shallow_clone( ) -> Result<(), Error> { git_cmd()? .current_dir(path) - .arg("-c") - .arg("protocol.version=2") + .hidden_args(["-c", "protocol.version=2"]) .arg("fetch") .arg("origin") .arg(rev) @@ -555,8 +547,7 @@ async fn shallow_clone( git_cmd()? .current_dir(path) - .arg("-c") - .arg("protocol.version=2") + .hidden_args(["-c", "protocol.version=2"]) .arg("submodule") .arg("update") .arg("--init") diff --git a/crates/prek/src/hooks/pre_commit_hooks/forbid_new_submodules.rs b/crates/prek/src/hooks/pre_commit_hooks/forbid_new_submodules.rs index d35bac214..5eff94fe6 100644 --- a/crates/prek/src/hooks/pre_commit_hooks/forbid_new_submodules.rs +++ b/crates/prek/src/hooks/pre_commit_hooks/forbid_new_submodules.rs @@ -25,7 +25,7 @@ pub(crate) async fn forbid_new_submodules( .arg("diff") .arg("--relative") .arg("--diff-filter=A") - .arg("--no-ext-diff") + .hidden_args(["--no-ext-diff"]) .arg("--raw") .arg("-z") .arg(diff_arg.as_ref()) diff --git a/crates/prek/src/languages/python/python.rs b/crates/prek/src/languages/python/python.rs index 0bdbb03af..513d7a06b 100644 --- a/crates/prek/src/languages/python/python.rs +++ b/crates/prek/src/languages/python/python.rs @@ -334,13 +334,7 @@ impl Python { allow_downloads: bool, ) -> Cmd { let mut cmd = uv.cmd(store); - cmd.arg("venv") - .arg(&info.env_path) - .args(["--python-preference", "managed"]) - // Avoid discovering a project or workspace - .arg("--no-project") - // Explicitly set project to root to avoid uv searching for project-level configs - .args(["--project", "/"]); + cmd.arg("venv").arg(&info.env_path); Self::remove_uv_python_override_envs(&mut cmd); if set_install_dir { cmd.env( @@ -348,15 +342,27 @@ impl Python { store.tools_path(ToolBucket::Python), ); } - if allow_downloads { - cmd.arg("--allow-python-downloads"); - } else { - cmd.arg("--no-python-downloads"); - } - if let Some(python) = to_uv_python_request(python_request) { - cmd.arg("--python").arg(python); + let download_arg = if allow_downloads { + "--allow-python-downloads" + } else { + "--no-python-downloads" + }; + let python = to_uv_python_request(python_request); + let mut hidden_args = vec![ + "--python-preference", + "managed", + // Avoid discovering a project or workspace. + "--no-project", + // Explicitly set project to root to avoid uv searching for project-level configs. + "--project", + "/", + download_arg, + ]; + if let Some(python) = &python { + hidden_args.extend(["--python", python.as_str()]); } + cmd.hidden_args(hidden_args); cmd } diff --git a/crates/prek/src/process.rs b/crates/prek/src/process.rs index 637d206aa..3c8f78f75 100644 --- a/crates/prek/src/process.rs +++ b/crates/prek/src/process.rs @@ -27,6 +27,7 @@ /// Adapt [axoprocess] to use [`tokio::process::Process`] instead of [`std::process::Command`]. use std::ffi::OsStr; use std::fmt::Display; +use std::ops::Range; use std::path::Path; use std::process::Output; use std::process::{CommandArgs, CommandEnvs, ExitStatus, Stdio}; @@ -34,9 +35,7 @@ use std::process::{CommandArgs, CommandEnvs, ExitStatus, Stdio}; use owo_colors::OwoColorize; use thiserror::Error; use tokio::io::AsyncReadExt; -use tracing::trace; - -use crate::git::GIT; +use tracing::{enabled, trace}; /// An error from executing a command. #[derive(Debug, Error)] @@ -106,7 +105,8 @@ fn write_trimmed_output_section( pub struct Cmd { /// The inner command, in case you need to access it. pub inner: tokio::process::Command, - file_args_start: Option, + hidden_arg_ranges: Vec>, + file_arg_boundary: usize, check_status: bool, } @@ -126,7 +126,8 @@ impl Cmd { let inner = tokio::process::Command::new(command); Self { inner, - file_args_start: None, + hidden_arg_ranges: Vec::new(), + file_arg_boundary: usize::MAX, check_status: true, } } @@ -184,8 +185,7 @@ impl Cmd { .output() .await .map_err(|cause| self.exec_error(cause))?; - self.maybe_check_output(&output)?; - Ok(output) + self.maybe_check_output(output) } /// Like [`Cmd::output`], but streams stdout and stderr chunks into `sink` as @@ -249,8 +249,7 @@ impl Cmd { stderr: stderr_output, }; - self.maybe_check_output(&output)?; - Ok(output) + self.maybe_check_output(output) } #[cfg(windows)] @@ -348,8 +347,7 @@ impl Cmd { stderr: Vec::new(), }; - self.maybe_check_output(&output)?; - Ok(output) + self.maybe_check_output(output) } /// Equivalent to [`std::process::Command::status`] @@ -370,10 +368,6 @@ impl Cmd { impl Cmd { /// Forwards to [`std::process::Command::arg`]. pub fn arg>(&mut self, arg: S) -> &mut Self { - debug_assert!( - self.file_args_start.is_none(), - "regular arguments must be added before file-list arguments" - ); self.inner.arg(arg); self } @@ -384,22 +378,50 @@ impl Cmd { I: IntoIterator, S: AsRef, { - for arg in args { - self.arg(arg); + self.inner.args(args); + self + } + + /// Append arguments without showing them in display and error messages. + pub fn hidden_args(&mut self, args: I) -> &mut Self + where + I: IntoIterator, + S: AsRef, + { + let start = self.get_args().count(); + self.inner.args(args); + let end = self.get_args().count(); + if start < end { + if let Some(last) = self.hidden_arg_ranges.last_mut() + && last.end >= start + { + last.end = last.end.max(end); + return self; + } + self.hidden_arg_ranges.push(start..end); } self } - /// Append trailing file-list arguments without showing them in the display command. + /// Append trailing file-list arguments without showing them in display, error messages, or logs. pub fn file_args(&mut self, args: I) -> &mut Self where I: IntoIterator, S: AsRef, { - if self.file_args_start.is_none() { - self.file_args_start = Some(self.get_args().count()); + if self.file_arg_boundary != usize::MAX { + self.inner.args(args); + return self; + } + + let mut args = args.into_iter().peekable(); + if args.peek().is_none() { + return self; } + + let start = self.get_args().count(); self.inner.args(args); + self.file_arg_boundary = start; self } @@ -492,15 +514,16 @@ impl Cmd { /// Diagnostic APIs used by execution methods and direct child-process callers. impl Cmd { fn exec_error(&self, cause: std::io::Error) -> Error { - Error::Exec { - command: self.display_command(), - cause, - } + let mut command = String::new(); + let _ = write_command_line(&mut command, None, self.get_program(), self.display_args()); + Error::Exec { command, cause } } fn status_error(&self, status: ExitStatus, output: Option) -> Error { + let mut command = String::new(); + let _ = write_command_line(&mut command, None, self.get_program(), self.display_args()); Error::Status { - command: self.display_command(), + command, error: StatusError { status, output }, } } @@ -515,11 +538,11 @@ impl Cmd { } /// Check `Output::status`, producing a contextual error if it's not successful. - pub fn check_output(&self, output: &Output) -> Result<(), Error> { + pub fn check_output(&self, output: Output) -> Result { if output.status.success() { - Ok(()) + Ok(output) } else { - Err(self.status_error(output.status, Some(output.clone()))) + Err(self.status_error(output.status, Some(output))) } } @@ -534,72 +557,64 @@ impl Cmd { /// Invoke [`Cmd::check_output`] if [`Cmd::check`] is `true` /// (defaults to `true`). - pub fn maybe_check_output(&self, output: &Output) -> Result<(), Error> { + pub fn maybe_check_output(&self, output: Output) -> Result { if self.check_status { - self.check_output(output)?; + self.check_output(output) + } else { + Ok(output) } - Ok(()) } /// Log the current command with [`tracing::trace!`]. pub fn log_command(&self) { - trace!("Executing `{self}`"); - } - - fn display_arg_count(&self) -> usize { - self.file_args_start - .unwrap_or_else(|| self.get_args().count()) - } + if !enabled!(tracing::Level::TRACE) { + return; + } - fn display_command(&self) -> String { let mut command = String::new(); - write_display_command( + let _ = write_command_line( &mut command, - None, + self.get_current_dir(), self.get_program(), - self.get_args().take(self.display_arg_count()), - ) - .expect("writing to a string cannot fail"); - command + self.non_file_args(), + ); + trace!("Executing `{command}`"); } -} -/// Returns the number of arguments to skip. -fn skip_args(cmd: &OsStr, cur: &OsStr, next: Option<&&OsStr>) -> usize { - if GIT.as_ref().is_ok_and(|git| cmd == git) { - if cur == "-c" { - if let Some(flag) = next { - let flag = flag.as_encoded_bytes(); - if flag.starts_with(b"core.useBuiltinFSMonitor") - || flag.starts_with(b"protocol.version") - { - return 2; - } + fn display_args(&self) -> impl Iterator { + self.non_file_args().enumerate().filter_map(|(index, arg)| { + if self.is_hidden_arg(index) { + None + } else { + Some(arg) } - } else if cur == "--no-ext-diff" - || cur == "--no-textconv" - || cur == "--ignore-submodules" - || cur == "--no-color" - { - return 1; - } + }) + } + + fn is_hidden_arg(&self, index: usize) -> bool { + self.hidden_arg_ranges + .iter() + .any(|range| range.contains(&index)) + } + + fn non_file_args(&self) -> impl Iterator { + self.get_args().take(self.file_arg_boundary) } - 0 } -/// Simplified command output, omitting trailing file-list arguments. +/// Simplified command output, omitting hidden arguments and file-list arguments. impl Display for Cmd { fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { - write_display_command( + write_command_line( f, self.get_current_dir(), self.get_program(), - self.get_args().take(self.display_arg_count()), + self.display_args(), ) } } -fn write_display_command<'a>( +fn write_command_line<'a>( f: &mut impl std::fmt::Write, cwd: Option<&Path>, program: &OsStr, @@ -616,15 +631,7 @@ fn write_display_command<'a>( args.next(); // Skip the program if it's repeated } - while let Some(arg) = args.next() { - let skip = skip_args(program, arg, args.peek()); - if skip > 0 { - for _ in 1..skip { - args.next(); - } - continue; - } - + for arg in args { write!(f, " {}", arg.to_string_lossy())?; } @@ -636,7 +643,7 @@ mod tests { use std::error::Error as _; use std::sync::{Arc, Mutex}; - use super::{Cmd, OutputSink}; + use super::{Cmd, OutputSink, write_command_line}; #[derive(Default)] struct RecordingSink { @@ -649,6 +656,17 @@ mod tests { } } + fn command_log_string(cmd: &Cmd) -> String { + let mut command = String::new(); + let _ = write_command_line( + &mut command, + cmd.get_current_dir(), + cmd.get_program(), + cmd.non_file_args(), + ); + command + } + #[tokio::test] async fn status_reports_missing_executable_name() { let err = Cmd::new("__prek_missing_command__") @@ -665,20 +683,88 @@ mod tests { } #[test] - fn display_command_omits_file_args() { + fn display_and_log_commands_omit_file_args() { let mut cmd = Cmd::new("prek"); cmd.arg("run") .arg("hook-id") - .file_args(["file-0.rs", "file-1.rs"]); + .file_args(["file-0.rs"]) + .file_args(["file-1.rs"]); - assert_eq!(cmd.display_command(), "prek run hook-id"); assert_eq!(cmd.to_string(), "prek run hook-id"); + assert_eq!(command_log_string(&cmd), "prek run hook-id"); assert_eq!( cmd.get_args() .map(|arg| arg.to_string_lossy().into_owned()) .collect::>(), ["run", "hook-id", "file-0.rs", "file-1.rs"] ); + assert!(cmd.hidden_arg_ranges.is_empty()); + assert_eq!(cmd.file_arg_boundary, 2); + } + + #[test] + fn display_command_omits_hidden_args() { + let mut cmd = Cmd::new("git"); + cmd.hidden_args(["-c", "core.useBuiltinFSMonitor=false"]) + .arg("diff") + .arg("--name-only") + .hidden_args(["--no-ext-diff", "--ignore-submodules"]) + .arg("HEAD"); + + assert_eq!(cmd.to_string(), "git diff --name-only HEAD"); + assert_eq!( + cmd.get_args() + .map(|arg| arg.to_string_lossy().into_owned()) + .collect::>(), + [ + "-c", + "core.useBuiltinFSMonitor=false", + "diff", + "--name-only", + "--no-ext-diff", + "--ignore-submodules", + "HEAD" + ] + ); + } + + #[test] + fn command_log_includes_hidden_args() { + let mut cmd = Cmd::new("git"); + cmd.hidden_args(["-c", "core.useBuiltinFSMonitor=false"]) + .arg("diff") + .arg("--name-only") + .hidden_args(["--no-ext-diff", "--ignore-submodules"]) + .arg("HEAD"); + + assert_eq!(cmd.to_string(), "git diff --name-only HEAD"); + assert_eq!( + command_log_string(&cmd), + "git -c core.useBuiltinFSMonitor=false diff --name-only --no-ext-diff --ignore-submodules HEAD" + ); + } + + #[test] + fn command_log_skips_repeated_program_arg() { + let mut cmd = Cmd::new("python"); + cmd.arg("python").arg("-m").arg("module"); + + assert_eq!(cmd.to_string(), "python -m module"); + assert_eq!(command_log_string(&cmd), "python -m module"); + } + + #[test] + fn hidden_args_merges_adjacent_ranges() { + let mut cmd = Cmd::new("uv"); + cmd.arg("venv") + .arg("/tmp/python-abc") + .hidden_args(["--python-preference", "managed"]) + .hidden_args(["--no-project"]) + .hidden_args(["--project", "/"]); + + assert_eq!(cmd.to_string(), "uv venv /tmp/python-abc"); + assert_eq!(cmd.hidden_arg_ranges, vec![2..7]); + assert_eq!(cmd.file_arg_boundary, usize::MAX); } #[tokio::test] diff --git a/crates/prek/tests/languages/python.rs b/crates/prek/tests/languages/python.rs index b779f47b0..f48ed2ef2 100644 --- a/crates/prek/tests/languages/python.rs +++ b/crates/prek/tests/languages/python.rs @@ -195,10 +195,14 @@ fn can_not_download() { let mut filters = context .filters() .into_iter() - .chain([( - "managed installations, search path, or registry", - "managed installations or search path", - )]) + .chain([ + ( + "managed installations, search path, or registry", + "managed installations or search path", + ), + (r"Command `[^`]*uv(?:\.exe)? venv", "Command `[UV] venv"), + (r"python-[[:alnum:]]{20}", "python-[HASH]"), + ]) .collect::>(); if cfg!(windows) { // Unix uses "exit status", Windows uses "exit code" @@ -213,7 +217,7 @@ fn can_not_download() { ----- stderr ----- error: Failed to install hook `less-than-3.6` caused by: Failed to create Python virtual environment - caused by: Command `create venv` exited with an error: + caused by: Command `[UV] venv [HOME]/hooks/python-[HASH]` exited with an error: [status] exit status: 2 diff --git a/crates/prek/tests/run.rs b/crates/prek/tests/run.rs index 7db65033a..1f93c19b2 100644 --- a/crates/prek/tests/run.rs +++ b/crates/prek/tests/run.rs @@ -181,6 +181,10 @@ fn run_in_non_git_repo() { let mut filters = context.filters(); filters.push((r"exit code: ", "exit status: ")); + filters.push(( + r"Command `[^`]*git(?:\.exe)? rev-parse --show-toplevel`", + "Command `[GIT] rev-parse --show-toplevel`", + )); cmd_snapshot!(filters, context.run(), @r" success: false @@ -188,7 +192,7 @@ fn run_in_non_git_repo() { ----- stdout ----- ----- stderr ----- - error: Command `get git root` exited with an error: + error: Command `[GIT] rev-parse --show-toplevel` exited with an error: [status] exit status: 128 @@ -2094,6 +2098,10 @@ fn init_nonexistent_repo() { .into_iter() .chain([ (r"exit code: ", "exit status: "), + ( + r"Command `[^`]*git(?:\.exe)? fetch origin --tags`", + "Command `[GIT] fetch origin --tags`", + ), // Normalize Git error message to handle environment-specific variations ( r"fatal: unable to access 'https://notexistentatallnevergonnahappen\.com/nonexistent/repo/':.*", @@ -2110,7 +2118,7 @@ fn init_nonexistent_repo() { ----- stderr ----- error: Failed to init hooks caused by: Failed to clone repo `https://notexistentatallnevergonnahappen.com/nonexistent/repo` - caused by: Command `git full clone` exited with an error: + caused by: Command `[GIT] fetch origin --tags` exited with an error: [status] exit status: 128 @@ -2267,7 +2275,12 @@ fn unmatched_skip_does_not_suppress_remote_clone() { let filters = context .filters() .into_iter() - .chain([(r"exit code: ", "exit status: "), + .chain([ + (r"exit code: ", "exit status: "), + ( + r"Command `[^`]*git(?:\.exe)? fetch origin --tags`", + "Command `[GIT] fetch origin --tags`", + ), // Normalize Git error message to handle environment-specific variations ( r"fatal: unable to access 'https://notexistentatallnevergonnahappen\.com/nonexistent/repo/':.*", @@ -2287,7 +2300,7 @@ fn unmatched_skip_does_not_suppress_remote_clone() { ----- stderr ----- error: Failed to init hooks caused by: Failed to clone repo `https://notexistentatallnevergonnahappen.com/nonexistent/repo` - caused by: Command `git full clone` exited with an error: + caused by: Command `[GIT] fetch origin --tags` exited with an error: [status] exit status: 128