Fixes a bug in PatchUtil after upgrading java-diff-utils

meteorcloudy · copybara-github · commit 555d7e51c980 · 2023-10-06T07:54:19.000-07:00
Apply the chunks separately and in reverse order to workaround an issue in applyFuzzy. See java-diff-utils/java-diff-utils#125 (comment) Fixes #17897 (comment) RELNOTES: None PiperOrigin-RevId: 571336806 Change-Id: I8eb6b859d1ce8fb990d9237f728198af34e0d393
diff --git a/src/main/java/com/google/devtools/build/lib/bazel/repository/PatchUtil.java b/src/main/java/com/google/devtools/build/lib/bazel/repository/PatchUtil.java
@@ -17,12 +17,14 @@
 import static java.nio.charset.StandardCharsets.UTF_8;
 
 import com.github.difflib.UnifiedDiffUtils;
+import com.github.difflib.patch.AbstractDelta;
 import com.github.difflib.patch.Patch;
 import com.github.difflib.patch.PatchFailedException;
 import com.google.common.base.Preconditions;
 import com.google.common.base.Splitter;
 import com.google.common.collect.ImmutableList;
 import com.google.common.collect.Iterables;
+import com.google.common.collect.Lists;
 import com.google.devtools.build.lib.vfs.FileSystemUtils;
 import com.google.devtools.build.lib.vfs.Path;
 import java.io.IOException;
@@ -212,13 +214,20 @@ private static void applyPatchToFile(
       }
     }
 
-    List<String> newContent;
-    try {
-      // By using applyFuzzy, the patch also applies when there is an offset.
-      newContent = patch.applyFuzzy(oldContent, 0);
-    } catch (PatchFailedException e) {
-      throw new PatchFailedException(
-          String.format("in patch applied to %s: %s", oldFile, e.getMessage()));
+    List<String> newContent = new ArrayList<>(oldContent);
+    // Apply the chunks separately and in reverse order to workaround an issue in applyFuzzy.
+    // See https://github.com/java-diff-utils/java-diff-utils/pull/125#issuecomment-1749385825
+    for (AbstractDelta<String> delta : Lists.reverse(patch.getDeltas())) {
+      Patch<String> tmpPatch = new Patch<>();
+      tmpPatch.addDelta(delta);
+      try {
+        newContent = tmpPatch.applyFuzzy(newContent, 0);
+      } catch (PatchFailedException | IndexOutOfBoundsException e) {
+        throw new PatchFailedException(
+            String.format(
+                "in patch applied to %s: %s, error applying change near line %s",
+                oldFile, e.getMessage(), delta.getSource().getPosition() + 1));
+      }
     }
 
     // The file we should write newContent to.
diff --git a/src/test/java/com/google/devtools/build/lib/bazel/repository/PatchUtilTest.java b/src/test/java/com/google/devtools/build/lib/bazel/repository/PatchUtilTest.java
@@ -339,6 +339,65 @@ public void testMatchWithOffset() throws IOException, PatchFailedException {
     assertThat(FileSystemUtils.readLines(foo, UTF_8)).isEqualTo(newFoo);
   }
 
+  @Test
+  // regression test for https://github.com/bazelbuild/bazel/issues/17897#issuecomment-1749389613
+  public void testMultipleChunks() throws IOException, PatchFailedException {
+    Path foo =
+        scratch.file(
+            "/root/foo",
+            "1",
+            "2",
+            "3",
+            "4",
+            "5",
+            "6",
+            "7",
+            "8",
+            "9",
+            "10",
+            "11",
+            "12",
+            "13",
+            "14",
+            "15",
+            "16",
+            "17",
+            "18");
+    Path patchFile =
+        scratch.file(
+            "/root/patchfile",
+            "diff --git a/foo b/foo",
+            "index c20ab12..b83bdb1 100644",
+            "--- a/foo",
+            "+++ b/foo",
+            "@@ -1,12 +1,5 @@",
+            " 1",
+            " 2",
+            "-3",
+            "-4",
+            "-5",
+            "-6",
+            "-7",
+            "-8",
+            "-9",
+            " 10",
+            " 11",
+            " 12",
+            "@@ -15,4 +8,7 @@",
+            " 15",
+            " 16",
+            " 17",
+            "+a",
+            "+b",
+            "+c",
+            " 18");
+    PatchUtil.apply(patchFile, 1, root);
+    ImmutableList<String> newFoo =
+        ImmutableList.of(
+            "1", "2", "10", "11", "12", "13", "14", "15", "16", "17", "a", "b", "c", "18");
+    assertThat(FileSystemUtils.readLines(foo, UTF_8)).isEqualTo(newFoo);
+  }
+
   @Test
   public void testMultipleChunksWithDifferentOffset() throws IOException, PatchFailedException {
     Path foo =
diff --git a/src/test/java/com/google/devtools/build/lib/bazel/repository/starlark/StarlarkRepositoryContextTest.java b/src/test/java/com/google/devtools/build/lib/bazel/repository/starlark/StarlarkRepositoryContextTest.java
@@ -377,7 +377,7 @@ public void testPatchErrorWasThrown() throws Exception {
           .isEqualTo(
               "Error applying patch /outputDir/my.patch: in patch applied to "
                   + "/outputDir/foo: could not apply patch due to"
-                  + " CONTENT_DOES_NOT_MATCH_TARGET");
+                  + " CONTENT_DOES_NOT_MATCH_TARGET, error applying change near line 1");
     }
   }
 

Original file line number	Diff line number	Diff line change
`@@ -377,7 +377,7 @@ public void testPatchErrorWasThrown() throws Exception {`
`377`	`377`	`.isEqualTo(`
`378`	`378`	`"Error applying patch /outputDir/my.patch: in patch applied to "`
`379`	`379`	`+ "/outputDir/foo: could not apply patch due to"`
`380`		`- + " CONTENT_DOES_NOT_MATCH_TARGET");`
	`380`	`+ + " CONTENT_DOES_NOT_MATCH_TARGET, error applying change near line 1");`
`381`	`381`	`}`
`382`	`382`	`}`
`383`	`383`