Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
4 changes: 2 additions & 2 deletions src/git/error.rs
Original file line number Diff line number Diff line change
Expand Up @@ -219,14 +219,14 @@ impl CommandError {
/// Build from the captured `Output` of a non-zero exit.
pub fn from_failed_output(
program: impl Into<String>,
args: &[&str],
args: &[impl AsRef<str>],
output: &std::process::Output,
) -> Self {
let stderr = String::from_utf8_lossy(&output.stderr).replace('\r', "\n");
let stdout = String::from_utf8_lossy(&output.stdout).replace('\r', "\n");
Self {
program: program.into(),
args: args.iter().map(|&s| s.to_string()).collect(),
args: args.iter().map(|s| s.as_ref().to_string()).collect(),
stderr,
stdout,
exit_code: output.status.code(),
Expand Down
52 changes: 46 additions & 6 deletions src/git/repository/config.rs
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,8 @@ use color_print::cformat;

use crate::config::ProjectConfig;

use crate::git::CommandError;

use super::{DefaultBranchName, GitError, Repository};

impl Repository {
Expand Down Expand Up @@ -67,15 +69,15 @@ impl Repository {
/// Removes the canonical form (matching what git emits) to stay in
/// sync with `set_config_value` and `config_last`.
pub(super) fn unset_config_value(&self, key: &str) -> anyhow::Result<bool> {
let output = self.run_command_output(&["config", "--unset", key])?;
let args = ["config", "--unset", key];
let output = self.run_command_output(&args)?;
let existed = if output.status.success() {
true
} else if output.status.code() == Some(5) {
// --unset exit code 5 = key didn't exist
false
} else {
let stderr = String::from_utf8_lossy(&output.stderr);
anyhow::bail!("git config --unset {}: {}", key, stderr.trim());
return Err(CommandError::from_failed_output("git", &args, &output).into());
};
if let Some(lock) = self.cache.all_config.get() {
// `shift_remove` preserves remaining order (swap_remove would
Expand All @@ -94,15 +96,15 @@ impl Repository {
/// surfaced as `Err`). Use this instead of `run_command` + `.unwrap_or_default()`,
/// which conflates the two.
pub fn get_config_regexp(&self, pattern: &str) -> anyhow::Result<String> {
let output = self.run_command_output(&["config", "--get-regexp", pattern])?;
let args = ["config", "--get-regexp", pattern];
let output = self.run_command_output(&args)?;
if output.status.success() {
Ok(String::from_utf8_lossy(&output.stdout).into_owned())
} else if output.status.code() == Some(1) {
// Exit 1 = no keys matched the pattern
Ok(String::new())
} else {
let stderr = String::from_utf8_lossy(&output.stderr);
anyhow::bail!("git config --get-regexp {}: {}", pattern, stderr.trim());
Err(CommandError::from_failed_output("git", &args, &output).into())
}
}

Expand Down Expand Up @@ -824,6 +826,44 @@ mod tests {
assert_eq!(output, "");
}

#[test]
fn test_get_config_regexp_failure_is_command_error() {
// A real failure (invalid pattern, exit 6) must surface as a typed
// `CommandError` — unlike exit 1, which means "no keys matched".
let test = TestRepo::with_initial_commit();
let repo = Repository::at(test.root_path()).unwrap();

let err = repo.get_config_regexp("(").unwrap_err();
let cmd_err = CommandError::find_in(&err).expect("error should carry a CommandError");
assert_eq!(cmd_err.command_string(), "git config --get-regexp (");
}

#[test]
fn test_unset_config_failure_is_command_error() {
// A real failure (invalid key, exit 1) must surface as a typed
// `CommandError` — unlike exit 5, which means "key didn't exist".
let test = TestRepo::with_initial_commit();
let repo = Repository::at(test.root_path()).unwrap();

let err = repo.unset_config("inva lid.key").unwrap_err();
let cmd_err = CommandError::find_in(&err).expect("error should carry a CommandError");
assert_eq!(cmd_err.command_string(), "git config --unset inva lid.key");
}

#[test]
fn test_config_read_failure_is_command_error() {
// Corrupting the config after the repository is open (the bulk map
// populates lazily) makes `git config --list -z` fail — the failure
// must surface as a typed `CommandError`.
let test = TestRepo::with_initial_commit();
let repo = Repository::at(test.root_path()).unwrap();
std::fs::write(test.root_path().join(".git/config"), "[bad\n").unwrap();

let err = repo.config_value("user.name").unwrap_err();
let cmd_err = CommandError::find_in(&err).expect("error should carry a CommandError");
assert_eq!(cmd_err.command_string(), "git config --list -z");
}

#[test]
fn test_get_config_regexp_returns_matches() {
let test = TestRepo::with_initial_commit();
Expand Down
9 changes: 6 additions & 3 deletions src/git/repository/diff.rs
Original file line number Diff line number Diff line change
Expand Up @@ -270,15 +270,18 @@ impl Repository {
}

// Exit codes: 0 = found, 1 = no common ancestor, 128+ = invalid ref
let output = self.run_command_output(&["merge-base", sha1, sha2])?;
let args = ["merge-base", sha1, sha2];
let output = self.run_command_output(&args)?;

let result = if output.status.success() {
Some(String::from_utf8_lossy(&output.stdout).trim().to_owned())
} else if output.status.code() == Some(1) {
None
} else {
let stderr = String::from_utf8_lossy(&output.stderr);
bail!("git merge-base failed for {sha1} {sha2}: {}", stderr.trim());
return Err(crate::git::CommandError::from_failed_output(
"git", &args, &output,
)
.into());
};

super::sha_cache::put_merge_base(self, sha1, sha2, &result);
Expand Down
34 changes: 30 additions & 4 deletions src/git/repository/integration.rs
Original file line number Diff line number Diff line change
Expand Up @@ -291,15 +291,17 @@ impl Repository {
/// than success is an error; exit 0 → [`MergeTreeOutcome::Clean`] with the
/// resulting tree SHA (first line of stdout).
fn compute_merge_tree_outcome(&self, a: &str, b: &str) -> anyhow::Result<MergeTreeOutcome> {
// Exit codes: 0 = clean merge, 1 = conflicts, 128+ = error (invalid ref, corrupt repo)
let output = self.run_command_output(&["merge-tree", "--write-tree", a, b])?;
// Exit codes: 0 = clean merge, 1 = conflicts (git also uses 1 for
// unresolvable args — callers pass pre-resolved commit SHAs, see
// `run_merge_tree`), anything else = error (corrupt repo, bad usage)
let args = ["merge-tree", "--write-tree", a, b];
let output = self.run_command_output(&args)?;

if output.status.code() == Some(1) {
return Ok(MergeTreeOutcome::Conflict);
}
if !output.status.success() {
let stderr = String::from_utf8_lossy(&output.stderr);
anyhow::bail!("git merge-tree failed for {a} {b}: {}", stderr.trim());
return Err(crate::git::CommandError::from_failed_output("git", &args, &output).into());
}

// Clean merge — first line of stdout is the resulting tree SHA.
Expand Down Expand Up @@ -1129,6 +1131,30 @@ mod patch_id_tests {
}
}

#[cfg(test)]
mod merge_tree_error_tests {
use super::*;
use crate::testing::TestRepo;

/// A hard `git merge-tree` failure (here: a fatal config error, exit
/// 128) must surface as a typed `CommandError`, distinct from exit 1,
/// which means "merged with conflicts".
#[test]
fn hard_failure_is_command_error() {
let test = TestRepo::with_initial_commit();
let repo = Repository::at(test.root_path()).unwrap();
std::fs::write(test.root_path().join(".git/config"), "[bad\n").unwrap();

let err = repo.compute_merge_tree_outcome("HEAD", "HEAD").unwrap_err();
let cmd_err =
crate::git::CommandError::find_in(&err).expect("error should carry a CommandError");
assert_eq!(
cmd_err.command_string(),
"git merge-tree --write-tree HEAD HEAD"
);
}
}

#[cfg(test)]
mod merge_tree_cache_tests {
use super::*;
Expand Down
18 changes: 11 additions & 7 deletions src/git/repository/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -118,7 +118,7 @@ use color_print::cformat;
use dashmap::DashMap;
use once_cell::sync::OnceCell;

use anyhow::{Context, bail};
use anyhow::Context;
use dunce::canonicalize;

use crate::config::{LoadError, ProjectConfig, ResolvedConfig, UserConfig};
Expand Down Expand Up @@ -891,16 +891,18 @@ impl Repository {
// captures the surrounding canonicalize + cache-insert work too.
let _span = crate::trace::Span::new("resolve_git_common_dir");

let args = ["rev-parse", "--git-common-dir"];
let output = Cmd::new("git")
.args(["rev-parse", "--git-common-dir"])
.args(args)
.current_dir(discovery_path)
.context(path_to_logging_context(discovery_path))
.run()
.context("Failed to execute: git rev-parse --git-common-dir")?;

if !output.status.success() {
let stderr = String::from_utf8_lossy(&output.stderr);
bail!("{}", stderr.trim());
return Err(
super::error::CommandError::from_failed_output("git", &args, &output).into(),
);
}

let stdout = String::from_utf8_lossy(&output.stdout);
Expand Down Expand Up @@ -1189,15 +1191,17 @@ impl Repository {
&self,
) -> anyhow::Result<&std::sync::RwLock<indexmap::IndexMap<String, Vec<String>>>> {
self.cache.all_config.get_or_try_init(|| {
let args = ["config", "--list", "-z"];
let output = Cmd::new("git")
.args(["config", "--list", "-z"])
.args(args)
.current_dir(&self.git_common_dir)
.context(path_to_logging_context(&self.git_common_dir))
.run()
.context("failed to read git config")?;
if !output.status.success() {
let stderr = String::from_utf8_lossy(&output.stderr);
bail!("git config --list failed: {}", stderr.trim());
return Err(
super::error::CommandError::from_failed_output("git", &args, &output).into(),
);
}
Ok(std::sync::RwLock::new(parse_config_list_z(&output.stdout)))
})
Expand Down
25 changes: 21 additions & 4 deletions src/git/repository/working_tree.rs
Original file line number Diff line number Diff line change
Expand Up @@ -518,14 +518,12 @@ impl<'a> WorkingTree<'a> {
"--".to_string(),
];
args.extend(paths);
let command = args.join(" ");
let output = idx
.git(args)
.git(&args)
.run()
.context("Failed to compute untracked diff stats")?;
if !output.status.success() {
let stderr = String::from_utf8_lossy(&output.stderr);
anyhow::bail!("git {} failed: {}", command, stderr.trim());
return Err(CommandError::from_failed_output("git", &args, &output).into());
}

let mut stats = LineDiff::default();
Expand Down Expand Up @@ -932,6 +930,25 @@ mod tests {
);
}

#[test]
fn untracked_diff_stats_unborn_head_is_command_error() {
// With an unborn HEAD the untracked files stage fine into the temp
// index, but `git diff --cached --numstat HEAD` cannot resolve HEAD —
// the failure must surface as a typed `CommandError`.
let test = TestRepo::new();
std::fs::write(test.root_path().join("new.txt"), "hello\n").unwrap();
let repo = Repository::at(test.root_path()).unwrap();

let err = repo.current_worktree().untracked_diff_stats().unwrap_err();
let cmd_err =
crate::git::CommandError::find_in(&err).expect("error should carry a CommandError");
assert!(
cmd_err
.command_string()
.starts_with("git diff --cached --numstat HEAD")
);
}

#[test]
fn temp_index_tolerates_missing_real_index() {
// A worktree whose `<gitdir>/index` file is absent must not error
Expand Down
Loading
Loading