Skip to content

Improve subprocess error messages #2257

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
j178 merged 2 commits into from
Jun 27, 2026
Merged

Improve subprocess error messages #2257

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
8538d77
Improve subprocess error messages
j178 Jun 27, 2026
deb7054
Address subprocess review comments
j178 Jun 27, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 0 additions & 1 deletion crates/prek-consts/src/env_vars.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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";
Expand Down
44 changes: 22 additions & 22 deletions crates/prek/src/cli/auto_update/repository.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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")
Expand All @@ -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<String> {
let output = git::git_cmd("git rev-parse")?
let output = git::git_cmd()?
.arg("rev-parse")
.arg(format!("{rev}^{{}}"))
.check(true)
Expand Down Expand Up @@ -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")
Expand Down Expand Up @@ -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<Option<String>> {
let output = git::git_cmd("git describe")?
let output = git::git_cmd()?
.arg("describe")
.arg("FETCH_HEAD")
.arg("--tags")
Expand All @@ -134,7 +134,7 @@ pub(super) async fn resolve_bleeding_edge(repo_path: &Path) -> Result<Option<Str
String::from_utf8_lossy(&output.stdout).trim().to_string()
} else {
debug!("No matching tag for `FETCH_HEAD`, using rev-parse instead");
let output = git::git_cmd("git rev-parse")?
let output = git::git_cmd()?
.arg("rev-parse")
.arg("FETCH_HEAD")
.check(true)
Expand All @@ -154,7 +154,7 @@ pub(super) async fn resolve_bleeding_edge(repo_path: &Path) -> Result<Option<Str
/// Within groups of tags sharing the same timestamp, semver-parseable tags
/// are sorted highest version first; non-semver tags sort after them.
pub(super) async fn list_tag_metadata(repo: &Path) -> Result<Vec<TagTimestamp>> {
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)")
Expand Down Expand Up @@ -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)
Expand All @@ -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)
Expand Down Expand Up @@ -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)
Expand All @@ -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)
Expand All @@ -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)
Expand All @@ -466,7 +466,7 @@ mod tests {
.await
.unwrap();

git::git_cmd("git commit")
git::git_cmd()
.unwrap()
.args([
"-c",
Expand All @@ -482,7 +482,7 @@ mod tests {
.await
.unwrap();

git::git_cmd("git branch")
git::git_cmd()
.unwrap()
.args(["branch", "-M", "trunk"])
.current_dir(repo)
Expand All @@ -494,8 +494,8 @@ mod tests {
tmp
}

fn git_cmd(dir: impl AsRef<Path>, summary: &str) -> Cmd {
let mut cmd = git::git_cmd(summary).unwrap();
fn git_cmd(dir: impl AsRef<Path>) -> Cmd {
let mut cmd = git::git_cmd().unwrap();
cmd.current_dir(dir)
.args(["-c", "commit.gpgsign=false"])
.args(["-c", "tag.gpgsign=false"]);
Expand All @@ -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()
Expand All @@ -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)
Expand All @@ -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()
Expand All @@ -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")
Expand Down Expand Up @@ -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)
Expand All @@ -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)
Expand All @@ -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)
Expand Down
2 changes: 1 addition & 1 deletion crates/prek/src/cli/hook_impl.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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");

Expand Down
2 changes: 1 addition & 1 deletion crates/prek/src/cli/install.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
10 changes: 4 additions & 6 deletions crates/prek/src/cli/run/keeper.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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())
Expand Down Expand Up @@ -88,14 +88,12 @@ impl UnstagedChangesRestorer {
async fn clean(root: &Path, patch_dir: &Path) -> Result<Self> {
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")
.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)
Expand Down
4 changes: 2 additions & 2 deletions crates/prek/src/cli/run/run.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1098,10 +1098,10 @@ impl<'a> HookRunSession<'a> {
} else {
"--color=never"
};
git::git_cmd("git diff")?
git::git_cmd()?
.arg("--no-pager")
.arg("diff")
.arg("--no-ext-diff")
.hidden_args(["--no-ext-diff"])
.arg(color)
.arg("--")
.arg(workspace.root())
Expand Down
16 changes: 8 additions & 8 deletions crates/prek/src/cli/try_repo.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ use crate::store::Store;
use crate::warn_user;

async fn get_head_rev(repo: &Path) -> Result<String> {
let head_rev = git::git_cmd("get head rev")?
let head_rev = git::git_cmd()?
.arg("rev-parse")
.arg("HEAD")
.current_dir(repo)
Expand All @@ -31,13 +31,13 @@ async fn get_head_rev(repo: &Path) -> Result<String> {

async fn clone_and_commit(repo_path: &Path, head_rev: &str, tmp_dir: &Path) -> Result<PathBuf> {
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")
Expand All @@ -51,18 +51,18 @@ 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)
.output()
.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
Expand All @@ -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")
Expand Down Expand Up @@ -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()?
Comment thread
j178 marked this conversation as resolved.
.arg("ls-remote")
.arg("--exit-code")
.arg(repo)
Expand Down
Loading
Loading