fix: eliminate runtime dependency on patch(1) for apply_patch

codex-rs/apply-patch/src/lib.rs

@@ -86,6 +86,8 @@ pub enum ApplyPatchFileChange {
     Update {
         unified_diff: String,
         move_path: Option<PathBuf>,
+        /// new_content that will result after the unified_diff is applied.
+        new_content: String,
     },
 }
 
@@ -126,7 +128,10 @@ pub fn maybe_parse_apply_patch_verified(argv: &[String]) -> MaybeApplyPatchVerif
                         move_path,
                         chunks,
                     } => {
-                        let unified_diff = match unified_diff_from_chunks(&path, &chunks) {
+                        let ApplyPatchFileUpdate {
+                            unified_diff,
+                            content: contents,
+                        } = match unified_diff_from_chunks(&path, &chunks) {
                             Ok(diff) => diff,
                             Err(e) => {
                                 return MaybeApplyPatchVerified::CorrectnessError(e);
@@ -137,6 +142,7 @@ pub fn maybe_parse_apply_patch_verified(argv: &[String]) -> MaybeApplyPatchVerif
                             ApplyPatchFileChange::Update {
                                 unified_diff,
                                 move_path,
+                                new_content: contents,
                             },
                         );
                     }
@@ -516,10 +522,17 @@ fn apply_replacements(
     lines
 }
 
+/// Intended result of a file update for apply_patch.
+#[derive(Debug, Eq, PartialEq)]
+pub struct ApplyPatchFileUpdate {
+    unified_diff: String,
+    content: String,
+}
+
 pub fn unified_diff_from_chunks(
     path: &Path,
     chunks: &[UpdateFileChunk],
-) -> std::result::Result<String, ApplyPatchError> {
+) -> std::result::Result<ApplyPatchFileUpdate, ApplyPatchError> {
     unified_diff_from_chunks_with_context(path, chunks, 1)
 }
 
@@ -527,13 +540,17 @@ pub fn unified_diff_from_chunks_with_context(
     path: &Path,
     chunks: &[UpdateFileChunk],
     context: usize,
-) -> std::result::Result<String, ApplyPatchError> {
+) -> std::result::Result<ApplyPatchFileUpdate, ApplyPatchError> {
     let AppliedPatch {
         original_contents,
         new_contents,
     } = derive_new_contents_from_chunks(path, chunks)?;
     let text_diff = TextDiff::from_lines(&original_contents, &new_contents);
-    Ok(text_diff.unified_diff().context_radius(context).to_string())
+    let unified_diff = text_diff.unified_diff().context_radius(context).to_string();
+    Ok(ApplyPatchFileUpdate {
+        unified_diff,
+        content: new_contents,
+    })
 }
 
 /// Print the summary of changes in git-style format.
@@ -898,7 +915,11 @@ PATCH"#,
 -qux
 +QUX
 "#;
-        assert_eq!(expected_diff, diff);
+        let expected = ApplyPatchFileUpdate {
+            unified_diff: expected_diff.to_string(),
+            content: "foo\nBAR\nbaz\nQUX\n".to_string(),
+        };
+        assert_eq!(expected, diff);
     }
 
     #[test]
@@ -930,7 +951,11 @@ PATCH"#,
 +FOO
  bar
 "#;
-        assert_eq!(expected_diff, diff);
+        let expected = ApplyPatchFileUpdate {
+            unified_diff: expected_diff.to_string(),
+            content: "FOO\nbar\nbaz\n".to_string(),
+        };
+        assert_eq!(expected, diff);
     }
 
     #[test]
@@ -963,7 +988,11 @@ PATCH"#,
 -baz
 +BAZ
 "#;
-        assert_eq!(expected_diff, diff);
+        let expected = ApplyPatchFileUpdate {
+            unified_diff: expected_diff.to_string(),
+            content: "foo\nbar\nBAZ\n".to_string(),
+        };
+        assert_eq!(expected, diff);
     }
 
     #[test]
@@ -993,7 +1022,11 @@ PATCH"#,
  baz
 +quux
 "#;
-        assert_eq!(expected_diff, diff);
+        let expected = ApplyPatchFileUpdate {
+            unified_diff: expected_diff.to_string(),
+            content: "foo\nbar\nbaz\nquux\n".to_string(),
+        };
+        assert_eq!(expected, diff);
     }
 
     #[test]
@@ -1032,7 +1065,7 @@ PATCH"#,
 
         let diff = unified_diff_from_chunks(&path, chunks).unwrap();
 
-        let expected = r#"@@ -1,6 +1,7 @@
+        let expected_diff = r#"@@ -1,6 +1,7 @@
  a
 -b
 +B
@@ -1044,6 +1077,11 @@ PATCH"#,
 +g
 "#;
 
+        let expected = ApplyPatchFileUpdate {
+            unified_diff: expected_diff.to_string(),
+            content: "a\nB\nc\nd\nE\nf\ng\n".to_string(),
+        };
+
         assert_eq!(expected, diff);
 
         let mut stdout = Vec::new();

codex-rs/core/src/codex.rs

@@ -3,8 +3,6 @@ use std::collections::HashSet;
 use std::io::Write;
 use std::path::Path;
 use std::path::PathBuf;
-use std::process::Command;
-use std::process::Stdio;
 use std::sync::Arc;
 use std::sync::Mutex;
 
@@ -1346,6 +1344,7 @@ fn convert_apply_patch_to_protocol(
             ApplyPatchFileChange::Update {
                 unified_diff,
                 move_path,
+                new_content: _new_content,
             } => FileChange::Update {
                 unified_diff: unified_diff.clone(),
                 move_path: move_path.clone(),
@@ -1400,28 +1399,10 @@ fn apply_changes_from_apply_patch(
                 deleted.push(path.clone());
             }
             ApplyPatchFileChange::Update {
-                unified_diff,
+                unified_diff: _unified_diff,
                 move_path,
+                new_content,
             } => {
-                // TODO(mbolin): `patch` is not guaranteed to be available.
-                // Allegedly macOS provides it, but minimal Linux installs
-                // might omit it.
-                Command::new("patch")
-                    .arg(path)
-                    .arg("-p0")
-                    .stdout(Stdio::null())
-                    .stderr(Stdio::null())
-                    .stdin(Stdio::piped())
-                    .spawn()
-                    .and_then(|mut child| {
-                        let mut stdin = child.stdin.take().unwrap();
-                        stdin.write_all(unified_diff.as_bytes())?;
-                        stdin.flush()?;
-                        // Drop stdin to send EOF.
-                        drop(stdin);
-                        child.wait()
-                    })
-                    .with_context(|| format!("Failed to apply patch to {}", path.display()))?;
                 if let Some(move_path) = move_path {
                     if let Some(parent) = move_path.parent() {
                         if !parent.as_os_str().is_empty() {
@@ -1433,11 +1414,14 @@ fn apply_changes_from_apply_patch(
                             })?;
                         }
                     }
+
                     std::fs::rename(path, move_path)
                         .with_context(|| format!("Failed to rename file {}", path.display()))?;
+                    std::fs::write(move_path, new_content)?;
                     modified.push(move_path.clone());
                     deleted.push(path.clone());
                 } else {
+                    std::fs::write(path, new_content)?;
                     modified.push(path.clone());
                 }
             }