mirror of
https://github.com/openai/codex.git
synced 2026-10-24 18:18:41 +00:00
git-utils: gate selected merge drivers before three-way apply
This commit is contained in:
Chris Bookholt
@@ -13,6 +13,7 @@ use std::path::PathBuf;
|
||||
use crate::FsmonitorOverride;
|
||||
use crate::apply_output::parse_git_apply_output;
|
||||
use crate::git_command::GitRunner;
|
||||
use crate::merge_driver::ensure_no_selected_merge_drivers;
|
||||
use crate::patch_paths::extract_effective_paths_from_patch;
|
||||
use crate::patch_paths::stage_effective_paths;
|
||||
use crate::safe_git::DISABLED_HOOKS_PATH;
|
||||
@@ -57,20 +58,8 @@ pub fn apply_git_patch(req: &ApplyGitRequest) -> io::Result<ApplyGitResult> {
|
||||
let patch_paths = extract_effective_paths_from_patch(&git, &patch_path, req.revert)?;
|
||||
ensure_no_selected_executable_git_filters(&git, &git_root, &patch_paths, &cfg_parts)?;
|
||||
|
||||
if req.revert && !req.preflight {
|
||||
// Stage WT paths first to avoid index mismatch on revert.
|
||||
stage_effective_paths(&git, &git_root, &patch_paths)?;
|
||||
}
|
||||
|
||||
// Build git args
|
||||
let mut args: Vec<String> = vec!["apply".into(), "--3way".into()];
|
||||
if req.revert {
|
||||
args.push("-R".into());
|
||||
}
|
||||
|
||||
cfg_parts.extend(safe_git_config_parts());
|
||||
|
||||
args.push(patch_path.to_string_lossy().to_string());
|
||||
let patch_arg = patch_path.to_string_lossy().to_string();
|
||||
|
||||
// Optional preflight: dry-run only; do not modify working tree
|
||||
if req.preflight {
|
||||
@@ -78,7 +67,7 @@ pub fn apply_git_patch(req: &ApplyGitRequest) -> io::Result<ApplyGitResult> {
|
||||
if req.revert {
|
||||
check_args.push("-R".to_string());
|
||||
}
|
||||
check_args.push(patch_path.to_string_lossy().to_string());
|
||||
check_args.push(patch_arg);
|
||||
let rendered = render_command_for_log(&git_root, &cfg_parts, &check_args);
|
||||
let (c_code, c_out, c_err) = run_git(&git, &git_root, &cfg_parts, &check_args)?;
|
||||
let (mut applied_paths, mut skipped_paths, mut conflicted_paths) =
|
||||
@@ -100,6 +89,35 @@ pub fn apply_git_patch(req: &ApplyGitRequest) -> io::Result<ApplyGitResult> {
|
||||
});
|
||||
}
|
||||
|
||||
// Avoid three-way machinery entirely when the patch applies cleanly.
|
||||
// A selected merge driver is relevant only to the three-way fallback.
|
||||
let mut plain_check_args = vec![
|
||||
"apply".to_string(),
|
||||
"--check".to_string(),
|
||||
"--index".to_string(),
|
||||
];
|
||||
if req.revert {
|
||||
plain_check_args.push("-R".to_string());
|
||||
}
|
||||
plain_check_args.push(patch_arg.clone());
|
||||
let (plain_check_code, _, _) = run_git(&git, &git_root, &cfg_parts, &plain_check_args)?;
|
||||
|
||||
let mut args = vec!["apply".to_string()];
|
||||
if plain_check_code != 0 {
|
||||
ensure_no_selected_merge_drivers(&git, &git_root, &patch_paths, &cfg_parts)?;
|
||||
if req.revert {
|
||||
// Stage WT paths first to avoid index mismatch on three-way revert.
|
||||
stage_effective_paths(&git, &git_root, &patch_paths)?;
|
||||
}
|
||||
args.push("--3way".to_string());
|
||||
} else {
|
||||
args.push("--index".to_string());
|
||||
}
|
||||
if req.revert {
|
||||
args.push("-R".to_string());
|
||||
}
|
||||
args.push(patch_arg);
|
||||
|
||||
let cmd_for_log = render_command_for_log(&git_root, &cfg_parts, &args);
|
||||
let (code, stdout, stderr) = run_git(&git, &git_root, &cfg_parts, &args)?;
|
||||
|
||||
|
||||
@@ -8,6 +8,7 @@ mod git_command;
|
||||
mod git_config;
|
||||
mod info;
|
||||
mod local_only;
|
||||
mod merge_driver;
|
||||
mod operations;
|
||||
mod patch_paths;
|
||||
mod platform;
|
||||
|
||||
@@ -0,0 +1,178 @@
|
||||
use std::collections::BTreeMap;
|
||||
use std::collections::BTreeSet;
|
||||
use std::io;
|
||||
use std::io::Seek;
|
||||
use std::io::Write;
|
||||
use std::path::Path;
|
||||
use std::process::Stdio;
|
||||
|
||||
use crate::git_command::GitRunner;
|
||||
use crate::git_config::GitConfigEntry;
|
||||
#[cfg(test)]
|
||||
use crate::safe_git::isolate_git_command_environment;
|
||||
use crate::safe_git::read_effective_config_with_fallback;
|
||||
|
||||
const MERGE_CONFIG_PATTERN: &str = r"^(merge\.default|merge\..*\.driver)$";
|
||||
|
||||
pub(crate) fn ensure_no_selected_merge_drivers(
|
||||
git: &GitRunner,
|
||||
cwd: &Path,
|
||||
paths: &[String],
|
||||
git_config_args: &[String],
|
||||
) -> io::Result<()> {
|
||||
let entries = read_merge_config(git, cwd, git_config_args)?;
|
||||
let attributes = read_merge_attributes(git, cwd, paths, git_config_args)?;
|
||||
if let Some((driver, path)) = untrusted_driver_selection(&entries, &attributes)? {
|
||||
return Err(io::Error::new(
|
||||
io::ErrorKind::Unsupported,
|
||||
format!(
|
||||
"refusing to run an internal Git three-way apply with merge driver {driver:?} selected for {path:?}"
|
||||
),
|
||||
));
|
||||
}
|
||||
Ok(())
|
||||
}
|
||||
|
||||
fn read_merge_config(
|
||||
git: &GitRunner,
|
||||
cwd: &Path,
|
||||
git_config_args: &[String],
|
||||
) -> io::Result<BTreeMap<String, GitConfigEntry>> {
|
||||
read_effective_config_with_fallback(git, cwd, git_config_args, MERGE_CONFIG_PATTERN, "merge")
|
||||
}
|
||||
|
||||
fn read_merge_attributes(
|
||||
git: &GitRunner,
|
||||
cwd: &Path,
|
||||
paths: &[String],
|
||||
git_config_args: &[String],
|
||||
) -> io::Result<BTreeMap<String, String>> {
|
||||
if paths.is_empty() {
|
||||
return Err(io::Error::new(
|
||||
io::ErrorKind::InvalidInput,
|
||||
"refusing to inspect merge attributes for an empty patch path set",
|
||||
));
|
||||
}
|
||||
let mut input = tempfile::tempfile()?;
|
||||
for path in paths {
|
||||
input.write_all(path.as_bytes())?;
|
||||
input.write_all(&[0])?;
|
||||
}
|
||||
input.rewind()?;
|
||||
|
||||
let mut command = git.command();
|
||||
command
|
||||
.env("GIT_OPTIONAL_LOCKS", "0")
|
||||
.args(git_config_args)
|
||||
.args(["check-attr", "--stdin", "-z", "merge"])
|
||||
.current_dir(cwd)
|
||||
.stdin(Stdio::from(input));
|
||||
let output = git.output(command)?;
|
||||
if !output.status.success() {
|
||||
return Err(io::Error::other(format!(
|
||||
"git merge attribute probe failed with status {}: {}",
|
||||
output.status,
|
||||
String::from_utf8_lossy(&output.stderr).trim()
|
||||
)));
|
||||
}
|
||||
parse_merge_attributes(&output.stdout, paths)
|
||||
}
|
||||
|
||||
fn parse_merge_attributes(
|
||||
output: &[u8],
|
||||
expected_paths: &[String],
|
||||
) -> io::Result<BTreeMap<String, String>> {
|
||||
let Some(body) = output.strip_suffix(&[0]) else {
|
||||
return Err(invalid_attribute_output(
|
||||
"unterminated Git attribute output",
|
||||
));
|
||||
};
|
||||
let fields = body.split(|byte| *byte == 0).collect::<Vec<_>>();
|
||||
if fields.len() % 3 != 0 {
|
||||
return Err(invalid_attribute_output("incomplete Git attribute record"));
|
||||
}
|
||||
let expected = expected_paths
|
||||
.iter()
|
||||
.map(String::as_str)
|
||||
.collect::<BTreeSet<_>>();
|
||||
let mut attributes = BTreeMap::new();
|
||||
for record in fields.chunks_exact(3) {
|
||||
let path = std::str::from_utf8(record[0])
|
||||
.map_err(|_| invalid_attribute_output("non-UTF-8 Git attribute path"))?;
|
||||
let attribute = std::str::from_utf8(record[1])
|
||||
.map_err(|_| invalid_attribute_output("non-UTF-8 Git attribute name"))?;
|
||||
let value = std::str::from_utf8(record[2])
|
||||
.map_err(|_| invalid_attribute_output("non-UTF-8 Git attribute value"))?;
|
||||
if !expected.contains(path) || attribute != "merge" {
|
||||
return Err(invalid_attribute_output(
|
||||
"unexpected Git merge attribute record",
|
||||
));
|
||||
}
|
||||
if attributes
|
||||
.insert(path.to_string(), value.to_string())
|
||||
.is_some()
|
||||
{
|
||||
return Err(invalid_attribute_output(
|
||||
"duplicate Git merge attribute record",
|
||||
));
|
||||
}
|
||||
}
|
||||
if attributes.len() != expected.len() {
|
||||
return Err(invalid_attribute_output(
|
||||
"missing Git merge attribute record",
|
||||
));
|
||||
}
|
||||
Ok(attributes)
|
||||
}
|
||||
|
||||
fn untrusted_driver_selection(
|
||||
entries: &BTreeMap<String, GitConfigEntry>,
|
||||
attributes: &BTreeMap<String, String>,
|
||||
) -> io::Result<Option<(String, String)>> {
|
||||
let mut drivers = BTreeMap::new();
|
||||
for entry in entries.values() {
|
||||
if entry.key == "merge.default" {
|
||||
continue;
|
||||
}
|
||||
let name = entry
|
||||
.key
|
||||
.strip_prefix("merge.")
|
||||
.and_then(|key| key.strip_suffix(".driver"))
|
||||
.ok_or_else(|| invalid_config_entry("malformed merge driver key"))?;
|
||||
drivers.insert(name.to_string(), entry.value.as_str());
|
||||
}
|
||||
|
||||
let default = entries
|
||||
.get("merge.default")
|
||||
.map(|entry| entry.value.as_str());
|
||||
for (path, attribute) in attributes {
|
||||
for name in candidate_driver_names(attribute, default) {
|
||||
if drivers.get(name).is_some_and(|value| !value.is_empty()) {
|
||||
return Ok(Some((name.to_string(), path.clone())));
|
||||
}
|
||||
}
|
||||
}
|
||||
Ok(None)
|
||||
}
|
||||
|
||||
fn candidate_driver_names<'a>(attribute: &'a str, default: Option<&'a str>) -> BTreeSet<&'a str> {
|
||||
let mut names = BTreeSet::from([attribute]);
|
||||
if attribute == "unspecified"
|
||||
&& let Some(default) = default
|
||||
{
|
||||
names.insert(default);
|
||||
}
|
||||
names
|
||||
}
|
||||
|
||||
fn invalid_attribute_output(message: &str) -> io::Error {
|
||||
io::Error::new(io::ErrorKind::InvalidData, message)
|
||||
}
|
||||
|
||||
fn invalid_config_entry(message: &str) -> io::Error {
|
||||
io::Error::new(io::ErrorKind::InvalidData, message)
|
||||
}
|
||||
|
||||
#[cfg(test)]
|
||||
#[path = "merge_driver_tests.rs"]
|
||||
mod tests;
|
||||
@@ -0,0 +1,567 @@
|
||||
use super::*;
|
||||
use crate::apply::ApplyGitRequest;
|
||||
use crate::apply::apply_git_patch;
|
||||
use crate::git_config::GitConfigScope;
|
||||
use std::ffi::OsStr;
|
||||
|
||||
#[test]
|
||||
fn apply_allows_unused_global_merge_driver() {
|
||||
if std::env::var_os("CODEX_GIT_UTILS_MERGE_ENV_CHILD").is_none() {
|
||||
let config_dir = tempfile::tempdir().expect("config tempdir");
|
||||
let global_config = config_dir.path().join("global.gitconfig");
|
||||
let system_config = config_dir.path().join("system.gitconfig");
|
||||
std::fs::write(
|
||||
&global_config,
|
||||
"[merge \"unused\"]\n\tdriver = git config codex.mergeran true && false\n",
|
||||
)
|
||||
.expect("write global config");
|
||||
std::fs::write(&system_config, "").expect("write system config");
|
||||
run_isolated_merge_test(
|
||||
"merge_driver::tests::apply_allows_unused_global_merge_driver",
|
||||
&[
|
||||
("GIT_CONFIG_GLOBAL", global_config.as_os_str()),
|
||||
("GIT_CONFIG_SYSTEM", system_config.as_os_str()),
|
||||
],
|
||||
);
|
||||
return;
|
||||
}
|
||||
|
||||
let repo = init_repo();
|
||||
let root = repo.path();
|
||||
std::fs::write(root.join("file.txt"), "old\n").expect("write file");
|
||||
let (add_code, _, add_err) = run(root, &["git", "add", "file.txt"]);
|
||||
assert_eq!(add_code, 0, "add file: {add_err}");
|
||||
let (commit_code, _, commit_err) = run(root, &["git", "commit", "-m", "file"]);
|
||||
assert_eq!(commit_code, 0, "commit file: {commit_err}");
|
||||
|
||||
let result = apply_git_patch(&ApplyGitRequest {
|
||||
cwd: root.to_path_buf(),
|
||||
diff: "diff --git a/file.txt b/file.txt\n--- a/file.txt\n+++ b/file.txt\n@@ -1 +1 @@\n-old\n+new\n".to_string(),
|
||||
revert: false,
|
||||
preflight: false,
|
||||
})
|
||||
.expect("allow unrelated global merge driver");
|
||||
assert_eq!(result.exit_code, 0);
|
||||
assert_eq!(
|
||||
std::fs::read_to_string(root.join("file.txt")).expect("read file"),
|
||||
"new\n"
|
||||
);
|
||||
let (marker_code, _, _) = run(root, &["git", "config", "--get", "codex.mergeran"]);
|
||||
assert_ne!(marker_code, 0, "unused merge driver must not run");
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn apply_rejects_global_merge_driver_before_three_way() {
|
||||
if std::env::var_os("CODEX_GIT_UTILS_MERGE_ENV_CHILD").is_none() {
|
||||
let config_dir = tempfile::tempdir().expect("config tempdir");
|
||||
let global_config = config_dir.path().join("global.gitconfig");
|
||||
let system_config = config_dir.path().join("system.gitconfig");
|
||||
std::fs::write(
|
||||
&global_config,
|
||||
"[merge \"codex-test\"]\n\tdriver = git config codex.mergeran true && false\n",
|
||||
)
|
||||
.expect("write global config");
|
||||
std::fs::write(&system_config, "").expect("write system config");
|
||||
run_isolated_merge_test(
|
||||
"merge_driver::tests::apply_rejects_global_merge_driver_before_three_way",
|
||||
&[
|
||||
("GIT_CONFIG_GLOBAL", global_config.as_os_str()),
|
||||
("GIT_CONFIG_SYSTEM", system_config.as_os_str()),
|
||||
],
|
||||
);
|
||||
return;
|
||||
}
|
||||
|
||||
let repo = init_repo();
|
||||
let root = repo.path();
|
||||
std::fs::write(root.join(".gitattributes"), "file.txt merge=codex-test\n")
|
||||
.expect("write attributes");
|
||||
std::fs::write(root.join("file.txt"), "base\n").expect("write base");
|
||||
let (add_code, _, add_err) = run(root, &["git", "add", "."]);
|
||||
assert_eq!(add_code, 0, "add base: {add_err}");
|
||||
let (commit_code, _, commit_err) = run(root, &["git", "commit", "-m", "base"]);
|
||||
assert_eq!(commit_code, 0, "commit base: {commit_err}");
|
||||
let (base_code, base, base_err) = run(root, &["git", "rev-parse", "HEAD"]);
|
||||
assert_eq!(base_code, 0, "resolve base: {base_err}");
|
||||
|
||||
std::fs::write(root.join("file.txt"), "theirs\n").expect("write theirs");
|
||||
let (add_code, _, add_err) = run(root, &["git", "add", "file.txt"]);
|
||||
assert_eq!(add_code, 0, "add theirs: {add_err}");
|
||||
let (commit_code, _, commit_err) = run(root, &["git", "commit", "-m", "theirs"]);
|
||||
assert_eq!(commit_code, 0, "commit theirs: {commit_err}");
|
||||
let (diff_code, diff, diff_err) = run(
|
||||
root,
|
||||
&[
|
||||
"git",
|
||||
"diff",
|
||||
"--full-index",
|
||||
base.trim(),
|
||||
"HEAD",
|
||||
"--",
|
||||
"file.txt",
|
||||
],
|
||||
);
|
||||
assert_eq!(diff_code, 0, "create full-index patch: {diff_err}");
|
||||
|
||||
let (checkout_code, _, checkout_err) =
|
||||
run(root, &["git", "checkout", "-b", "ours", base.trim()]);
|
||||
assert_eq!(checkout_code, 0, "checkout base: {checkout_err}");
|
||||
std::fs::write(root.join("file.txt"), "ours\n").expect("write ours");
|
||||
let (commit_code, _, commit_err) = run(root, &["git", "commit", "-am", "ours"]);
|
||||
assert_eq!(commit_code, 0, "commit ours: {commit_err}");
|
||||
|
||||
let error = apply_git_patch(&ApplyGitRequest {
|
||||
cwd: root.to_path_buf(),
|
||||
diff,
|
||||
revert: false,
|
||||
preflight: false,
|
||||
})
|
||||
.expect_err("reject global merge driver");
|
||||
assert_eq!(error.kind(), io::ErrorKind::Unsupported);
|
||||
let (marker_code, _, _) = run(root, &["git", "config", "--get", "codex.mergeran"]);
|
||||
assert_ne!(marker_code, 0, "merge driver must not run");
|
||||
assert_eq!(
|
||||
std::fs::read_to_string(root.join("file.txt")).expect("read ours"),
|
||||
"ours\n"
|
||||
);
|
||||
let (status_code, status, status_err) = run(root, &["git", "status", "--porcelain"]);
|
||||
assert_eq!(status_code, 0, "status: {status_err}");
|
||||
assert!(status.is_empty(), "worktree/index changed: {status}");
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn three_way_apply_allows_unrelated_local_merge_driver() {
|
||||
let repo = init_repo();
|
||||
let root = repo.path();
|
||||
std::fs::write(root.join("file.txt"), "base\n").expect("write base");
|
||||
let (add_code, _, add_err) = run(root, &["git", "add", "file.txt"]);
|
||||
assert_eq!(add_code, 0, "add base: {add_err}");
|
||||
let (commit_code, _, commit_err) = run(root, &["git", "commit", "-m", "base"]);
|
||||
assert_eq!(commit_code, 0, "commit base: {commit_err}");
|
||||
|
||||
std::fs::write(root.join("file.txt"), "base\npatched\n").expect("write patch side");
|
||||
let (diff_code, diff, diff_err) = run(root, &["git", "diff", "--full-index", "--", "file.txt"]);
|
||||
assert_eq!(diff_code, 0, "create patch: {diff_err}");
|
||||
let (restore_code, _, restore_err) = run(root, &["git", "checkout", "--", "file.txt"]);
|
||||
assert_eq!(restore_code, 0, "restore base: {restore_err}");
|
||||
|
||||
std::fs::write(root.join("file.txt"), "current\nbase\n").expect("write current side");
|
||||
let (stage_code, _, stage_err) = run(root, &["git", "add", "file.txt"]);
|
||||
assert_eq!(stage_code, 0, "stage current side: {stage_err}");
|
||||
let (config_code, _, config_err) = run(
|
||||
root,
|
||||
&[
|
||||
"git",
|
||||
"config",
|
||||
"merge.unused.driver",
|
||||
"git config codex.mergeran true && false",
|
||||
],
|
||||
);
|
||||
assert_eq!(
|
||||
config_code, 0,
|
||||
"configure unused local driver: {config_err}"
|
||||
);
|
||||
|
||||
let result = apply_git_patch(&ApplyGitRequest {
|
||||
cwd: root.to_path_buf(),
|
||||
diff,
|
||||
revert: false,
|
||||
preflight: false,
|
||||
})
|
||||
.expect("allow unrelated local merge driver during three-way fallback");
|
||||
assert_eq!(result.exit_code, 0, "three-way apply: {}", result.stderr);
|
||||
assert!(result.cmd_for_log.contains("--3way"));
|
||||
assert_eq!(
|
||||
std::fs::read_to_string(root.join("file.txt"))
|
||||
.expect("read merged file")
|
||||
.replace("\r\n", "\n"),
|
||||
"current\nbase\npatched\n"
|
||||
);
|
||||
let (marker_code, _, _) = run(root, &["git", "config", "--get", "codex.mergeran"]);
|
||||
assert_ne!(marker_code, 0, "unused merge driver must not run");
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn apply_allows_clean_patch_with_selected_merge_driver() {
|
||||
let repo = init_repo();
|
||||
let root = repo.path();
|
||||
std::fs::write(root.join(".gitattributes"), "file.txt merge=codex-test\n")
|
||||
.expect("write attributes");
|
||||
std::fs::write(root.join("file.txt"), "old\n").expect("write file");
|
||||
let (add_code, _, add_err) = run(root, &["git", "add", "."]);
|
||||
assert_eq!(add_code, 0, "add fixture: {add_err}");
|
||||
let (commit_code, _, commit_err) = run(root, &["git", "commit", "-m", "fixture"]);
|
||||
assert_eq!(commit_code, 0, "commit fixture: {commit_err}");
|
||||
let (config_code, _, config_err) = run(
|
||||
root,
|
||||
&[
|
||||
"git",
|
||||
"config",
|
||||
"merge.codex-test.driver",
|
||||
"git config codex.mergeran true && false",
|
||||
],
|
||||
);
|
||||
assert_eq!(config_code, 0, "configure merge driver: {config_err}");
|
||||
|
||||
let result = apply_git_patch(&ApplyGitRequest {
|
||||
cwd: root.to_path_buf(),
|
||||
diff: "diff --git a/file.txt b/file.txt\n--- a/file.txt\n+++ b/file.txt\n@@ -1 +1 @@\n-old\n+new\n".to_string(),
|
||||
revert: false,
|
||||
preflight: false,
|
||||
})
|
||||
.expect("allow clean patch with selected merge driver");
|
||||
|
||||
assert_eq!(result.exit_code, 0);
|
||||
assert!(!result.cmd_for_log.contains("--3way"));
|
||||
let contents = std::fs::read_to_string(root.join("file.txt")).expect("read file");
|
||||
assert!(
|
||||
matches!(contents.as_str(), "new\n" | "new\r\n"),
|
||||
"expected the patched contents with a platform line ending, got {contents:?}"
|
||||
);
|
||||
let (marker_code, _, _) = run(root, &["git", "config", "--get", "codex.mergeran"]);
|
||||
assert_ne!(marker_code, 0, "merge driver must not run");
|
||||
let (status_code, status, status_err) = run(root, &["git", "status", "--porcelain"]);
|
||||
assert_eq!(status_code, 0, "status: {status_err}");
|
||||
assert_eq!(
|
||||
status.trim(),
|
||||
"M file.txt",
|
||||
"patch should update the index"
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn reverse_apply_allows_clean_patch_with_selected_merge_driver() {
|
||||
let repo = init_repo();
|
||||
let root = repo.path();
|
||||
std::fs::write(root.join(".gitattributes"), "file.txt merge=codex-test\n")
|
||||
.expect("write attributes");
|
||||
std::fs::write(root.join("file.txt"), "old\n").expect("write file");
|
||||
let (add_code, _, add_err) = run(root, &["git", "add", "."]);
|
||||
assert_eq!(add_code, 0, "add fixture: {add_err}");
|
||||
let (commit_code, _, commit_err) = run(root, &["git", "commit", "-m", "fixture"]);
|
||||
assert_eq!(commit_code, 0, "commit fixture: {commit_err}");
|
||||
let (config_code, _, config_err) = run(
|
||||
root,
|
||||
&[
|
||||
"git",
|
||||
"config",
|
||||
"merge.codex-test.driver",
|
||||
"git config codex.mergeran true && false",
|
||||
],
|
||||
);
|
||||
assert_eq!(config_code, 0, "configure merge driver: {config_err}");
|
||||
let diff = "diff --git a/file.txt b/file.txt\n--- a/file.txt\n+++ b/file.txt\n@@ -1 +1 @@\n-old\n+new\n";
|
||||
let patch_dir = tempfile::tempdir().expect("patch tempdir");
|
||||
let patch = patch_dir.path().join("change.diff");
|
||||
std::fs::write(&patch, diff).expect("write patch");
|
||||
let (apply_code, _, apply_err) = run(
|
||||
root,
|
||||
&[
|
||||
"git",
|
||||
"apply",
|
||||
"--index",
|
||||
patch.to_str().expect("patch path"),
|
||||
],
|
||||
);
|
||||
assert_eq!(apply_code, 0, "prepare forward apply: {apply_err}");
|
||||
|
||||
let result = apply_git_patch(&ApplyGitRequest {
|
||||
cwd: root.to_path_buf(),
|
||||
diff: diff.to_string(),
|
||||
revert: true,
|
||||
preflight: false,
|
||||
})
|
||||
.expect("allow clean reverse patch with selected merge driver");
|
||||
|
||||
assert_eq!(result.exit_code, 0);
|
||||
assert!(!result.cmd_for_log.contains("--3way"));
|
||||
let contents = std::fs::read_to_string(root.join("file.txt")).expect("read file");
|
||||
assert!(
|
||||
matches!(contents.as_str(), "old\n" | "old\r\n"),
|
||||
"expected the reverted contents with a platform line ending, got {contents:?}"
|
||||
);
|
||||
let (marker_code, _, _) = run(root, &["git", "config", "--get", "codex.mergeran"]);
|
||||
assert_ne!(marker_code, 0, "merge driver must not run");
|
||||
let (status_code, status, status_err) = run(root, &["git", "status", "--porcelain"]);
|
||||
assert_eq!(status_code, 0, "status: {status_err}");
|
||||
assert!(
|
||||
status.is_empty(),
|
||||
"reverse apply should restore HEAD: {status}"
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn preflight_does_not_probe_or_run_selected_merge_driver() {
|
||||
let repo = init_repo();
|
||||
let root = repo.path();
|
||||
std::fs::write(root.join(".gitattributes"), "file.txt merge=codex-test\n")
|
||||
.expect("write attributes");
|
||||
std::fs::write(root.join("file.txt"), "old\n").expect("write file");
|
||||
let (add_code, _, add_err) = run(root, &["git", "add", "."]);
|
||||
assert_eq!(add_code, 0, "add fixture: {add_err}");
|
||||
let (commit_code, _, commit_err) = run(root, &["git", "commit", "-m", "fixture"]);
|
||||
assert_eq!(commit_code, 0, "commit fixture: {commit_err}");
|
||||
let (config_code, _, config_err) = run(
|
||||
root,
|
||||
&[
|
||||
"git",
|
||||
"config",
|
||||
"merge.codex-test.driver",
|
||||
"git config codex.mergeran true && false",
|
||||
],
|
||||
);
|
||||
assert_eq!(config_code, 0, "configure merge driver: {config_err}");
|
||||
|
||||
let result = apply_git_patch(&ApplyGitRequest {
|
||||
cwd: root.to_path_buf(),
|
||||
diff: "diff --git a/file.txt b/file.txt\n--- a/file.txt\n+++ b/file.txt\n@@ -1 +1 @@\n-old\n+new\n".to_string(),
|
||||
revert: false,
|
||||
preflight: true,
|
||||
})
|
||||
.expect("preflight must not probe merge drivers");
|
||||
assert_eq!(result.exit_code, 0);
|
||||
assert_eq!(
|
||||
std::fs::read_to_string(root.join("file.txt")).expect("read file"),
|
||||
"old\n"
|
||||
);
|
||||
let (marker_code, _, _) = run(root, &["git", "config", "--get", "codex.mergeran"]);
|
||||
assert_ne!(marker_code, 0, "preflight must not run merge driver");
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn empty_name_driver_follows_effective_git_selection() {
|
||||
let repo = init_repo();
|
||||
let root = repo.path();
|
||||
std::fs::write(root.join("file.txt"), "base\n").expect("write file");
|
||||
std::fs::write(root.join(".gitattributes"), "file.txt merge=other\n")
|
||||
.expect("write attributes");
|
||||
let (config_code, _, config_err) = run(
|
||||
root,
|
||||
&["git", "config", "merge..driver", "empty-name helper"],
|
||||
);
|
||||
assert_eq!(config_code, 0, "configure empty-name driver: {config_err}");
|
||||
let git = GitRunner::for_cwd_io(root).expect("trusted Git");
|
||||
let paths = ["file.txt".to_string()];
|
||||
|
||||
ensure_no_selected_merge_drivers(&git, root, &paths, &[])
|
||||
.expect("allow unused empty-name driver");
|
||||
|
||||
std::fs::write(root.join(".gitattributes"), "file.txt merge=\n")
|
||||
.expect("select empty-name driver");
|
||||
let selected_error = ensure_no_selected_merge_drivers(&git, root, &paths, &[])
|
||||
.expect_err("reject selected empty-name driver");
|
||||
assert_eq!(selected_error.kind(), io::ErrorKind::Unsupported);
|
||||
|
||||
std::fs::write(root.join(".gitattributes"), "").expect("clear attributes");
|
||||
let (default_code, _, default_err) = run(root, &["git", "config", "merge.default", ""]);
|
||||
assert_eq!(
|
||||
default_code, 0,
|
||||
"configure empty merge.default: {default_err}"
|
||||
);
|
||||
let default_error = ensure_no_selected_merge_drivers(&git, root, &paths, &[])
|
||||
.expect_err("reject empty-name driver selected by empty default");
|
||||
assert_eq!(default_error.kind(), io::ErrorKind::Unsupported);
|
||||
|
||||
let (empty_code, _, empty_err) = run(root, &["git", "config", "merge..driver", ""]);
|
||||
assert_eq!(empty_code, 0, "empty effective driver: {empty_err}");
|
||||
ensure_no_selected_merge_drivers(&git, root, &paths, &[])
|
||||
.expect("allow selected empty command through empty default");
|
||||
std::fs::write(root.join(".gitattributes"), "file.txt merge=\n")
|
||||
.expect("select empty-name driver");
|
||||
ensure_no_selected_merge_drivers(&git, root, &paths, &[])
|
||||
.expect("allow explicitly selected empty command");
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn selected_driver_policy_allows_unused_global_and_rejects_selected() {
|
||||
let mut entries = config_entries([
|
||||
("merge.unused.driver", "true"),
|
||||
("merge.selected.driver", "helper %A %B"),
|
||||
("merge..driver", "empty-name helper"),
|
||||
("merge.default", ""),
|
||||
]);
|
||||
let unused = BTreeMap::from([("file.txt".to_string(), "other".to_string())]);
|
||||
assert_eq!(
|
||||
untrusted_driver_selection(&entries, &unused).expect("unused selection"),
|
||||
None
|
||||
);
|
||||
|
||||
let selected = BTreeMap::from([("file.txt".to_string(), "selected".to_string())]);
|
||||
assert_eq!(
|
||||
untrusted_driver_selection(&entries, &selected).expect("selected driver"),
|
||||
Some(("selected".to_string(), "file.txt".to_string()))
|
||||
);
|
||||
|
||||
let selected_empty_name = BTreeMap::from([("file.txt".to_string(), String::new())]);
|
||||
assert_eq!(
|
||||
untrusted_driver_selection(&entries, &selected_empty_name)
|
||||
.expect("selected empty-name driver"),
|
||||
Some((String::new(), "file.txt".to_string()))
|
||||
);
|
||||
|
||||
let selected_by_empty_default =
|
||||
BTreeMap::from([("file.txt".to_string(), "unspecified".to_string())]);
|
||||
assert_eq!(
|
||||
untrusted_driver_selection(&entries, &selected_by_empty_default)
|
||||
.expect("empty default selection"),
|
||||
Some((String::new(), "file.txt".to_string()))
|
||||
);
|
||||
|
||||
entries
|
||||
.get_mut("merge..driver")
|
||||
.expect("empty-name driver")
|
||||
.value
|
||||
.clear();
|
||||
assert_eq!(
|
||||
untrusted_driver_selection(&entries, &selected_empty_name).expect("empty selected command"),
|
||||
None
|
||||
);
|
||||
assert_eq!(
|
||||
untrusted_driver_selection(&entries, &selected_by_empty_default)
|
||||
.expect("empty defaulted command"),
|
||||
None
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn selected_driver_policy_handles_default_and_sentinel_ambiguity() {
|
||||
let mut entries = config_entries([
|
||||
("merge.default", "defaulted"),
|
||||
("merge.defaulted.driver", "helper"),
|
||||
("merge.set.driver", "set helper"),
|
||||
("merge.unset.driver", "unset helper"),
|
||||
("merge.unspecified.driver", "unspecified helper"),
|
||||
]);
|
||||
for value in ["set", "unset", "unspecified"] {
|
||||
let attributes = BTreeMap::from([("file.txt".to_string(), value.to_string())]);
|
||||
assert!(
|
||||
untrusted_driver_selection(&entries, &attributes)
|
||||
.expect("sentinel selection")
|
||||
.is_some(),
|
||||
"{value}"
|
||||
);
|
||||
}
|
||||
|
||||
entries
|
||||
.get_mut("merge.unspecified.driver")
|
||||
.expect("unspecified driver")
|
||||
.value
|
||||
.clear();
|
||||
let attributes = BTreeMap::from([("file.txt".to_string(), "unspecified".to_string())]);
|
||||
assert_eq!(
|
||||
untrusted_driver_selection(&entries, &attributes).expect("merge.default selection"),
|
||||
Some(("defaulted".to_string(), "file.txt".to_string()))
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn selected_driver_policy_ignores_scope_until_driver_is_selected() {
|
||||
for scope in [
|
||||
GitConfigScope::System,
|
||||
GitConfigScope::Global,
|
||||
GitConfigScope::Local,
|
||||
GitConfigScope::Worktree,
|
||||
GitConfigScope::Command,
|
||||
] {
|
||||
let mut entries = config_entries([("merge.scoped.driver", "helper")]);
|
||||
entries
|
||||
.get_mut("merge.scoped.driver")
|
||||
.expect("scoped driver")
|
||||
.scope = scope;
|
||||
let attributes = BTreeMap::from([("file.txt".to_string(), "other".to_string())]);
|
||||
assert_eq!(
|
||||
untrusted_driver_selection(&entries, &attributes).expect("unused scoped driver"),
|
||||
None,
|
||||
"{scope:?}"
|
||||
);
|
||||
let attributes = BTreeMap::from([("file.txt".to_string(), "scoped".to_string())]);
|
||||
assert_eq!(
|
||||
untrusted_driver_selection(&entries, &attributes).expect("selected scoped driver"),
|
||||
Some(("scoped".to_string(), "file.txt".to_string())),
|
||||
"{scope:?}"
|
||||
);
|
||||
}
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn merge_attribute_parser_is_strict() {
|
||||
let paths = vec!["a.txt".to_string(), "b.txt".to_string()];
|
||||
let parsed = parse_merge_attributes(b"a.txt\0merge\0unspecified\0b.txt\0merge\0\0", &paths)
|
||||
.expect("parse attributes");
|
||||
assert_eq!(parsed.get("a.txt").map(String::as_str), Some("unspecified"));
|
||||
assert_eq!(parsed.get("b.txt").map(String::as_str), Some(""));
|
||||
|
||||
for malformed in [
|
||||
b"a.txt\0merge\0unspecified".as_slice(),
|
||||
b"a.txt\0filter\0unspecified\0b.txt\0merge\0custom\0".as_slice(),
|
||||
b"a.txt\0merge\0unspecified\0".as_slice(),
|
||||
b"a.txt\0merge\0unspecified\0a.txt\0merge\0custom\0".as_slice(),
|
||||
] {
|
||||
assert!(
|
||||
parse_merge_attributes(malformed, &paths).is_err(),
|
||||
"{malformed:?}"
|
||||
);
|
||||
}
|
||||
}
|
||||
|
||||
fn config_entries<const N: usize>(values: [(&str, &str); N]) -> BTreeMap<String, GitConfigEntry> {
|
||||
values
|
||||
.into_iter()
|
||||
.map(|(key, value)| {
|
||||
(
|
||||
key.to_string(),
|
||||
GitConfigEntry {
|
||||
scope: GitConfigScope::Global,
|
||||
origin: "file:/tmp/global.gitconfig".to_string(),
|
||||
key: key.to_string(),
|
||||
value: value.to_string(),
|
||||
},
|
||||
)
|
||||
})
|
||||
.collect()
|
||||
}
|
||||
|
||||
fn run(cwd: &Path, args: &[&str]) -> (i32, String, String) {
|
||||
let mut command = std::process::Command::new(args[0]);
|
||||
isolate_git_command_environment(&mut command);
|
||||
let output = command
|
||||
.args(&args[1..])
|
||||
.current_dir(cwd)
|
||||
.output()
|
||||
.expect("run command");
|
||||
(
|
||||
output.status.code().unwrap_or(-1),
|
||||
String::from_utf8_lossy(&output.stdout).into_owned(),
|
||||
String::from_utf8_lossy(&output.stderr).into_owned(),
|
||||
)
|
||||
}
|
||||
|
||||
fn run_isolated_merge_test(test_name: &str, env: &[(&str, &OsStr)]) {
|
||||
let mut command = std::process::Command::new(std::env::current_exe().expect("test binary"));
|
||||
isolate_git_command_environment(&mut command);
|
||||
command
|
||||
.arg(test_name)
|
||||
.arg("--exact")
|
||||
.arg("--nocapture")
|
||||
.env("CODEX_GIT_UTILS_MERGE_ENV_CHILD", "1")
|
||||
.env("RUST_TEST_THREADS", "1");
|
||||
for (name, value) in env {
|
||||
command.env(name, value);
|
||||
}
|
||||
let output = command.output().expect("run isolated test process");
|
||||
assert!(
|
||||
output.status.success(),
|
||||
"isolated test {test_name} failed:\nstdout:\n{}\nstderr:\n{}",
|
||||
String::from_utf8_lossy(&output.stdout),
|
||||
String::from_utf8_lossy(&output.stderr)
|
||||
);
|
||||
}
|
||||
|
||||
fn init_repo() -> tempfile::TempDir {
|
||||
let repo = tempfile::tempdir().expect("repo tempdir");
|
||||
let root = repo.path();
|
||||
let _ = run(root, &["git", "init"]);
|
||||
let _ = run(root, &["git", "config", "user.email", "codex@example.com"]);
|
||||
let _ = run(root, &["git", "config", "user.name", "Codex"]);
|
||||
repo
|
||||
}
|
||||
Reference in New Issue
Block a user