mirror of
https://github.com/openai/codex.git
synced 2026-10-29 19:08:43 +00:00
adjust apply_patch replacement behavior
Normalize existing-target replacement before persisting new content. Add focused regression coverage for the updated write-path behavior and keep the change scoped to the relevant integration boundary. Co-authored-by: Codex <noreply@openai.com>
This commit is contained in:
@@ -397,7 +397,7 @@ async fn apply_hunks_to_files(
|
||||
read_optional_file_text_for_delta(&path_abs, fs, sandbox, &mut delta.exact)
|
||||
.await;
|
||||
try_write!(
|
||||
write_file_with_missing_parent_retry(
|
||||
replace_file_with_missing_parent_retry(
|
||||
fs,
|
||||
&path_abs,
|
||||
contents.clone().into_bytes(),
|
||||
@@ -466,7 +466,7 @@ async fn apply_hunks_to_files(
|
||||
read_optional_file_text_for_delta(&dest_abs, fs, sandbox, &mut delta.exact)
|
||||
.await;
|
||||
try_write!(
|
||||
write_file_with_missing_parent_retry(
|
||||
replace_file_with_missing_parent_retry(
|
||||
fs,
|
||||
&dest_abs,
|
||||
new_contents.clone().into_bytes(),
|
||||
@@ -522,12 +522,13 @@ async fn apply_hunks_to_files(
|
||||
modified.push(affected_path);
|
||||
} else {
|
||||
try_write!(
|
||||
fs.write_file(&path_abs, new_contents.clone().into_bytes(), sandbox)
|
||||
.await
|
||||
.with_context(|| format!(
|
||||
"Failed to write file {}",
|
||||
path_abs.display()
|
||||
))
|
||||
replace_file_with_missing_parent_retry(
|
||||
fs,
|
||||
&path_abs,
|
||||
new_contents.clone().into_bytes(),
|
||||
sandbox,
|
||||
)
|
||||
.await
|
||||
);
|
||||
delta.changes.push(AppliedPatchChange {
|
||||
path: path_abs.into_path_buf(),
|
||||
@@ -645,6 +646,45 @@ async fn write_file_with_missing_parent_retry(
|
||||
}
|
||||
}
|
||||
|
||||
async fn replace_file_with_missing_parent_retry(
|
||||
fs: &dyn ExecutorFileSystem,
|
||||
path_abs: &AbsolutePathBuf,
|
||||
contents: Vec<u8>,
|
||||
sandbox: Option<&FileSystemSandboxContext>,
|
||||
) -> anyhow::Result<()> {
|
||||
match fs.get_metadata(path_abs, sandbox).await {
|
||||
Ok(metadata) => {
|
||||
if metadata.is_directory {
|
||||
return Err(io::Error::new(
|
||||
io::ErrorKind::InvalidInput,
|
||||
"path is a directory",
|
||||
))
|
||||
.with_context(|| format!("Failed to write file {}", path_abs.display()));
|
||||
}
|
||||
if metadata.is_symlink {
|
||||
return write_file_with_missing_parent_retry(fs, path_abs, contents, sandbox).await;
|
||||
}
|
||||
fs.remove(
|
||||
path_abs,
|
||||
RemoveOptions {
|
||||
recursive: false,
|
||||
force: false,
|
||||
},
|
||||
sandbox,
|
||||
)
|
||||
.await
|
||||
.with_context(|| format!("Failed to replace file {}", path_abs.display()))?;
|
||||
}
|
||||
Err(error) if error.kind() == io::ErrorKind::NotFound => {}
|
||||
Err(error) => {
|
||||
return Err(error)
|
||||
.with_context(|| format!("Failed to inspect file {}", path_abs.display()));
|
||||
}
|
||||
}
|
||||
|
||||
write_file_with_missing_parent_retry(fs, path_abs, contents, sandbox).await
|
||||
}
|
||||
|
||||
struct AppliedPatch {
|
||||
original_contents: String,
|
||||
new_contents: String,
|
||||
|
||||
@@ -205,6 +205,52 @@ fn test_apply_patch_cli_add_overwrites_existing_file() -> anyhow::Result<()> {
|
||||
Ok(())
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn test_apply_patch_cli_add_replaces_existing_hard_link() -> anyhow::Result<()> {
|
||||
let tmp = tempdir()?;
|
||||
let workspace = tmp.path().join("workspace");
|
||||
let outside = tmp.path().join("outside.txt");
|
||||
let link = workspace.join("hard-link.txt");
|
||||
fs::create_dir_all(&workspace)?;
|
||||
fs::write(&outside, "outside\n")?;
|
||||
fs::hard_link(&outside, &link)?;
|
||||
|
||||
run_apply_patch_in_dir(
|
||||
&workspace,
|
||||
"*** Begin Patch\n*** Add File: hard-link.txt\n+workspace replacement\n*** End Patch",
|
||||
)?
|
||||
.success()
|
||||
.stdout("Success. Updated the following files:\nA hard-link.txt\n");
|
||||
|
||||
assert_eq!(fs::read_to_string(&outside)?, "outside\n");
|
||||
assert_eq!(fs::read_to_string(&link)?, "workspace replacement\n");
|
||||
|
||||
Ok(())
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn test_apply_patch_cli_update_replaces_existing_hard_link() -> anyhow::Result<()> {
|
||||
let tmp = tempdir()?;
|
||||
let workspace = tmp.path().join("workspace");
|
||||
let outside = tmp.path().join("outside.txt");
|
||||
let link = workspace.join("hard-link.txt");
|
||||
fs::create_dir_all(&workspace)?;
|
||||
fs::write(&outside, "outside\n")?;
|
||||
fs::hard_link(&outside, &link)?;
|
||||
|
||||
run_apply_patch_in_dir(
|
||||
&workspace,
|
||||
"*** Begin Patch\n*** Update File: hard-link.txt\n@@\n-outside\n+workspace replacement\n*** End Patch",
|
||||
)?
|
||||
.success()
|
||||
.stdout("Success. Updated the following files:\nM hard-link.txt\n");
|
||||
|
||||
assert_eq!(fs::read_to_string(&outside)?, "outside\n");
|
||||
assert_eq!(fs::read_to_string(&link)?, "workspace replacement\n");
|
||||
|
||||
Ok(())
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn test_apply_patch_cli_delete_directory_fails() -> anyhow::Result<()> {
|
||||
let tmp = tempdir()?;
|
||||
|
||||
Reference in New Issue
Block a user