mirror of
https://github.com/openai/codex.git
synced 2026-10-21 17:48:44 +00:00
fix(git): preserve patch parsing compatibility
This commit is contained in:
@@ -48,12 +48,12 @@ pub struct ApplyGitResult {
|
||||
pub fn apply_git_patch(req: &ApplyGitRequest) -> io::Result<ApplyGitResult> {
|
||||
let git = GitRunner::for_cwd_io(&req.cwd)?;
|
||||
let mut cfg_parts = configured_git_config_parts();
|
||||
ensure_no_worktree_primary_config_sources(&git, &req.cwd)?;
|
||||
let requested_cwd = std::fs::canonicalize(&req.cwd)?;
|
||||
let git_root = resolve_git_root(&git, &req.cwd, &cfg_parts)?;
|
||||
if git_root != requested_cwd {
|
||||
ensure_no_worktree_primary_config_sources(&git, &git_root)?;
|
||||
}
|
||||
let expected_root = crate::get_git_repo_root(&requested_cwd)
|
||||
.ok_or_else(|| io::Error::other("not a Git repository"))
|
||||
.and_then(std::fs::canonicalize)?;
|
||||
ensure_no_worktree_primary_config_sources(&git, &expected_root)?;
|
||||
let git_root = resolve_git_root(&git, &expected_root, &cfg_parts)?;
|
||||
|
||||
// Write unified diff into a temporary file
|
||||
let (tmpdir, patch_path) = write_temp_patch(&req.diff)?;
|
||||
@@ -359,6 +359,72 @@ mod tests {
|
||||
assert!(root.join("hello.txt").exists());
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn apply_resolves_relative_primary_config_from_repository_root() {
|
||||
let _g = env_lock().lock().unwrap();
|
||||
if std::env::var_os("CODEX_GIT_UTILS_APPLY_ENV_CHILD").is_none() {
|
||||
run_isolated_test(
|
||||
"apply::tests::apply_resolves_relative_primary_config_from_repository_root",
|
||||
&[("GIT_CONFIG_GLOBAL", OsStr::new("../external/config"))],
|
||||
);
|
||||
return;
|
||||
}
|
||||
|
||||
let fixture = tempfile::tempdir().expect("fixture");
|
||||
let root = fixture.path().join("repo");
|
||||
let nested_cwd = root.join("nested");
|
||||
let external_config = fixture.path().join("external/config");
|
||||
let mismatched_nested_config = root.join("external/config");
|
||||
std::fs::create_dir_all(&nested_cwd).expect("nested cwd");
|
||||
std::fs::create_dir_all(external_config.parent().expect("config parent"))
|
||||
.expect("external config directory");
|
||||
std::fs::create_dir_all(
|
||||
mismatched_nested_config
|
||||
.parent()
|
||||
.expect("mismatched config parent"),
|
||||
)
|
||||
.expect("mismatched config directory");
|
||||
std::fs::write(&external_config, "[codex]\n\tprobe = loaded\n").expect("external config");
|
||||
std::fs::write(&mismatched_nested_config, "[invalid\n")
|
||||
.expect("mismatched nested-cwd config");
|
||||
let (init_code, _, init_err) = run(&root, &["git", "init"]);
|
||||
assert_eq!(init_code, 0, "init repository: {init_err}");
|
||||
|
||||
let result = apply_git_patch(&ApplyGitRequest {
|
||||
cwd: nested_cwd,
|
||||
diff: "diff --git a/hello.txt b/hello.txt\nnew file mode 100644\n--- /dev/null\n+++ b/hello.txt\n@@ -0,0 +1 @@\n+hello\n".to_string(),
|
||||
revert: false,
|
||||
preflight: false,
|
||||
})
|
||||
.expect("apply with root-relative primary config");
|
||||
assert_eq!(result.exit_code, 0, "apply result: {result:?}");
|
||||
assert_eq!(read_file_normalized(&root.join("hello.txt")), "hello\n");
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn numstat_path_discovery_does_not_preempt_apply_whitespace_result() {
|
||||
let _g = env_lock().lock().unwrap();
|
||||
let repo = init_repo();
|
||||
let root = repo.path();
|
||||
let (config_code, _, config_err) =
|
||||
run(root, &["git", "config", "apply.whitespace", "error"]);
|
||||
assert_eq!(config_code, 0, "configure whitespace policy: {config_err}");
|
||||
|
||||
let result = apply_git_patch(&ApplyGitRequest {
|
||||
cwd: root.to_path_buf(),
|
||||
diff: "diff --git a/trailing.txt b/trailing.txt\nnew file mode 100644\n--- /dev/null\n+++ b/trailing.txt\n@@ -0,0 +1 @@\n+trailing \n".to_string(),
|
||||
revert: false,
|
||||
preflight: false,
|
||||
})
|
||||
.expect("path discovery must leave apply failure in the structured result");
|
||||
assert_ne!(result.exit_code, 0);
|
||||
assert!(
|
||||
result.stderr.contains("trailing whitespace"),
|
||||
"apply result: {result:?}"
|
||||
);
|
||||
assert!(!root.join("trailing.txt").exists());
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn apply_uses_cwd_repo_despite_inherited_repository_selectors() {
|
||||
let _g = env_lock().lock().unwrap();
|
||||
|
||||
@@ -11,6 +11,10 @@ use crate::git_command::GitRunner;
|
||||
use crate::git_config::path_is_within;
|
||||
use crate::git_config_sources::ensure_no_worktree_primary_config_sources;
|
||||
|
||||
// Path discovery must not turn a repository's whitespace policy into an
|
||||
// API-level parser failure before the real apply can report its status.
|
||||
const APPLY_NUMSTAT_ARGS: [&str; 3] = ["--numstat", "--whitespace=nowarn", "-z"];
|
||||
|
||||
/// Extract paths with Git from a cwd whose config sources have already been
|
||||
/// authorized for `git_config_args`.
|
||||
pub(crate) fn extract_effective_paths_from_patch(
|
||||
@@ -63,11 +67,18 @@ fn extract_paths_from_patch_from_cwd(diff_text: &str, cwd: &Path) -> Vec<String>
|
||||
};
|
||||
let paths = (|| -> io::Result<Vec<String>> {
|
||||
let git = GitRunner::for_cwd_io(cwd)?;
|
||||
let git_root = crate::get_git_repo_root(cwd)
|
||||
.ok_or_else(|| io::Error::other("not a Git repository"))?;
|
||||
let git_root = std::fs::canonicalize(git_root)?;
|
||||
ensure_no_worktree_primary_config_sources(&git, &git_root)?;
|
||||
extract_effective_paths_from_patch(&git, &git_root, &patch_path, /*revert*/ false, &[])
|
||||
let authorized_cwd = crate::get_git_repo_root(cwd)
|
||||
.map(std::fs::canonicalize)
|
||||
.transpose()?
|
||||
.unwrap_or(std::fs::canonicalize(cwd)?);
|
||||
ensure_no_worktree_primary_config_sources(&git, &authorized_cwd)?;
|
||||
extract_effective_paths_from_patch(
|
||||
&git,
|
||||
&authorized_cwd,
|
||||
&patch_path,
|
||||
/*revert*/ false,
|
||||
&[],
|
||||
)
|
||||
})()
|
||||
.unwrap_or_default();
|
||||
drop(tmpdir);
|
||||
@@ -83,7 +94,7 @@ fn git_apply_numstat_paths(
|
||||
) -> io::Result<Vec<String>> {
|
||||
let mut cmd = git.command_for_cwd(authorized_cwd)?;
|
||||
cmd.args(git_config_args);
|
||||
cmd.args(["apply", "--numstat", "-z"]);
|
||||
cmd.arg("apply").args(APPLY_NUMSTAT_ARGS);
|
||||
if revert {
|
||||
cmd.arg("-R");
|
||||
}
|
||||
|
||||
@@ -75,8 +75,8 @@ fn effective_paths(diff: &str, revert: bool) -> io::Result<Vec<String>> {
|
||||
}
|
||||
|
||||
fn best_effort_paths(diff: &str) -> Vec<String> {
|
||||
let repo = init_repo();
|
||||
extract_paths_from_patch_from_cwd(diff, repo.path())
|
||||
let cwd = tempfile::tempdir().expect("non-repository cwd");
|
||||
extract_paths_from_patch_from_cwd(diff, cwd.path())
|
||||
}
|
||||
|
||||
#[test]
|
||||
|
||||
Reference in New Issue
Block a user