Git binary diff (#6292)

* WIP

* Use new binary patch output format

* Fix incubating version value

* Fully integrate binary patch support

* Revert back to latest.release for jgit

* Enable being able to specify the remote charset, but when absent fallback to ISO-8859-1

---------

Co-authored-by: Jammy Louie <[email protected]>

Author: shanman190

rewrite-core/src/main/java/org/openrewrite/Result.java

@@ -208,21 +208,11 @@ public String diff(@Nullable Path relativeTo, PrintOutputCapture.@Nullable Marke
 
     @Incubating(since = "7.34.0")
     public String diff(@Nullable Path relativeTo, PrintOutputCapture.@Nullable MarkerPrinter markerPrinter, @Nullable Boolean ignoreAllWhitespace) {
-        Path beforePath = before == null ? null : before.getSourcePath();
-        Path afterPath = null;
-        if (before == null && after == null) {
-            afterPath = (relativeTo == null ? Paths.get(".") : relativeTo).resolve("partial-" + System.nanoTime());
-        } else if (after != null) {
-            afterPath = after.getSourcePath();
-        }
-
-        PrintOutputCapture<Integer> out = markerPrinter == null ?
-                new PrintOutputCapture<>(0) :
-                new PrintOutputCapture<>(0, markerPrinter);
-
-        FileMode beforeMode = before != null && before.getFileAttributes() != null && before.getFileAttributes().isExecutable() ? FileMode.EXECUTABLE_FILE : FileMode.REGULAR_FILE;
-        FileMode afterMode = after != null && after.getFileAttributes() != null && after.getFileAttributes().isExecutable() ? FileMode.EXECUTABLE_FILE : FileMode.REGULAR_FILE;
+        return diff(relativeTo, markerPrinter, ignoreAllWhitespace, false);
+    }
 
+    @Incubating(since = "8.69.0")
+    public String diff(@Nullable Path relativeTo, PrintOutputCapture.@Nullable MarkerPrinter markerPrinter, @Nullable Boolean ignoreAllWhitespace, boolean binaryPatch) {
         Set<Recipe> recipeSet = new HashSet<>(recipes.size());
         for (List<Recipe> rs : recipes) {
             if (!rs.isEmpty()) {
@@ -231,14 +221,12 @@ public String diff(@Nullable Path relativeTo, PrintOutputCapture.@Nullable Marke
         }
 
         try (InMemoryDiffEntry diffEntry = new InMemoryDiffEntry(
-                beforePath,
-                afterPath,
+                before,
+                after,
                 relativeTo,
-                before == null ? "" : before.printAll(out),
-                after == null ? "" : after.printAll(out.clone()),
+                markerPrinter,
                 recipeSet,
-                beforeMode,
-                afterMode
+                binaryPatch
         )) {
             return diffEntry.getDiff(ignoreAllWhitespace);
         }

rewrite-core/src/main/java/org/openrewrite/internal/InMemoryDiffEntry.java

@@ -15,24 +15,29 @@
  */
 package org.openrewrite.internal;
 
+import lombok.EqualsAndHashCode;
+import lombok.Value;
+import lombok.experimental.NonFinal;
 import org.jspecify.annotations.Nullable;
+import org.openrewrite.Incubating;
+import org.openrewrite.PrintOutputCapture;
 import org.openrewrite.Recipe;
+import org.openrewrite.SourceFile;
+import org.openrewrite.jgit.attributes.AttributesNodeProvider;
 import org.openrewrite.jgit.diff.DiffEntry;
 import org.openrewrite.jgit.diff.DiffFormatter;
 import org.openrewrite.jgit.diff.RawTextComparator;
-import org.openrewrite.jgit.internal.storage.dfs.DfsRepositoryDescription;
-import org.openrewrite.jgit.internal.storage.dfs.InMemoryRepository;
+import org.openrewrite.jgit.internal.storage.dfs.*;
 import org.openrewrite.jgit.lib.*;
+import org.openrewrite.marker.GitTreeEntry;
+import org.openrewrite.quark.Quark;
 
 import java.io.ByteArrayOutputStream;
 import java.io.IOException;
 import java.io.UncheckedIOException;
 import java.nio.charset.StandardCharsets;
 import java.nio.file.Path;
-import java.util.Arrays;
-import java.util.LinkedHashSet;
-import java.util.Set;
-import java.util.StringJoiner;
+import java.util.*;
 import java.util.concurrent.atomic.AtomicBoolean;
 
 import static java.util.stream.Collectors.joining;
@@ -42,28 +47,105 @@ public class InMemoryDiffEntry extends DiffEntry implements AutoCloseable {
     static final AbbreviatedObjectId A_ZERO = AbbreviatedObjectId
             .fromObjectId(ObjectId.zeroId());
 
-    private final InMemoryRepository repo;
+    private final VirtualInMemoryRepository repo;
     private final Set<Recipe> recipesThatMadeChanges;
 
+    private final boolean binaryPatch;
+
     public InMemoryDiffEntry(@Nullable Path originalFilePath, @Nullable Path filePath, @Nullable Path relativeTo, String oldSource,
                              String newSource, Set<Recipe> recipesThatMadeChanges) {
         this(originalFilePath, filePath, relativeTo, oldSource, newSource, recipesThatMadeChanges, FileMode.REGULAR_FILE, FileMode.REGULAR_FILE);
     }
 
     public InMemoryDiffEntry(@Nullable Path originalFilePath, @Nullable Path filePath, @Nullable Path relativeTo, String oldSource,
                              String newSource, Set<Recipe> recipesThatMadeChanges, FileMode oldMode, FileMode newMode) {
+        this(originalFilePath, filePath, relativeTo, oldSource.getBytes(StandardCharsets.UTF_8), newSource.getBytes(StandardCharsets.UTF_8), recipesThatMadeChanges, oldMode, newMode, false);
+    }
+
+    @Incubating(since = "8.69.0")
+    public InMemoryDiffEntry(@Nullable SourceFile before, @Nullable SourceFile after, @Nullable Path relativeTo, PrintOutputCapture.@Nullable MarkerPrinter markerPrinter, Set<Recipe> recipesThatMadeChanges, boolean binaryPatch) {
+        if (before == null && after == null) {
+            throw new NullPointerException("before and after can't be null");
+        }
 
         this.recipesThatMadeChanges = recipesThatMadeChanges;
+        this.binaryPatch = binaryPatch;
 
         try {
-            this.repo = new InMemoryRepository.Builder()
-                    .setRepositoryDescription(new DfsRepositoryDescription())
-                    .build();
+            this.repo = new VirtualInMemoryRepository(new InMemoryRepository.Builder()
+                    .setRepositoryDescription(new DfsRepositoryDescription()));
+
+            try (VirtualObjectDatabase database = repo.getObjectDatabase()) {
+                try (ObjectInserter inserter = repo.getObjectDatabase().newInserter()) {
+                    if (before == null) {
+                        this.oldId = A_ZERO;
+                        this.oldMode = FileMode.MISSING;
+                        this.oldPath = DEV_NULL;
+                    } else {
+                        Optional<GitTreeEntry> maybeGitTreeEntry = before.getMarkers().findFirst(GitTreeEntry.class);
+                        if (maybeGitTreeEntry.isPresent()) {
+                            GitTreeEntry entry = maybeGitTreeEntry.get();
+                            this.oldId = database.insertVirtual(ObjectId.fromString(entry.getObjectId()), printAllAsBytes(before, markerPrinter)).abbreviate(40);
+                            this.oldMode = FileMode.fromBits(entry.getFileMode());
+                        } else {
+                            this.oldId = inserter.insert(Constants.OBJ_BLOB, printAllAsBytes(before, markerPrinter)).abbreviate(40);
+                            this.oldMode = before.getFileAttributes() != null && before.getFileAttributes().isExecutable() ? FileMode.EXECUTABLE_FILE : FileMode.REGULAR_FILE;
+                        }
+                        this.oldPath = (relativeTo == null ? before.getSourcePath() : relativeTo.relativize(before.getSourcePath())).toString().replace("\\", "/");
+                    }
+
+                    if (after == null) {
+                        this.newId = A_ZERO;
+                        this.newMode = FileMode.MISSING;
+                        this.newPath = DEV_NULL;
+                    } else {
+                        this.newId = inserter.insert(Constants.OBJ_BLOB, printAllAsBytes(after, markerPrinter)).abbreviate(40);
+                        this.newMode = after.getMarkers().findFirst(GitTreeEntry.class)
+                                .map(entry -> FileMode.fromBits(entry.getFileMode()))
+                                .orElseGet(() -> after.getFileAttributes() != null && after.getFileAttributes().isExecutable() ? FileMode.EXECUTABLE_FILE : FileMode.REGULAR_FILE);
+                        this.newPath = (relativeTo == null ? after.getSourcePath() : relativeTo.relativize(after.getSourcePath())).toString().replace("\\", "/");
+                    }
+                    inserter.flush();
+                }
+            }
+        } catch (IOException e) {
+            throw new UncheckedIOException(e);
+        }
+
+        if (this.oldMode == FileMode.MISSING && this.newMode != FileMode.MISSING) {
+            this.changeType = ChangeType.ADD;
+        } else if (this.oldMode != FileMode.MISSING && this.newMode == FileMode.MISSING) {
+            this.changeType = ChangeType.DELETE;
+        } else if (!oldPath.equals(newPath)) {
+            this.changeType = ChangeType.RENAME;
+        } else {
+            this.changeType = ChangeType.MODIFY;
+        }
+    }
+
+    public InMemoryDiffEntry(
+            @Nullable Path originalFilePath,
+            @Nullable Path filePath,
+            @Nullable Path relativeTo,
+            byte[] oldSource,
+            byte[] newSource,
+            Set<Recipe> recipesThatMadeChanges,
+            FileMode oldMode,
+            FileMode newMode,
+            boolean binaryPatch
+    ) {
+
+        this.recipesThatMadeChanges = recipesThatMadeChanges;
+        this.binaryPatch = binaryPatch;
+
+        try {
+            this.repo = new VirtualInMemoryRepository(new InMemoryRepository.Builder()
+                    .setRepositoryDescription(new DfsRepositoryDescription()));
 
             try (ObjectInserter inserter = repo.getObjectDatabase().newInserter()) {
 
                 if (originalFilePath != null) {
-                    this.oldId = inserter.insert(Constants.OBJ_BLOB, oldSource.getBytes(StandardCharsets.UTF_8)).abbreviate(40);
+                    this.oldId = inserter.insert(Constants.OBJ_BLOB, oldSource).abbreviate(40);
                     this.oldMode = oldMode;
                     this.oldPath = (relativeTo == null ? originalFilePath : relativeTo.relativize(originalFilePath)).toString().replace("\\", "/");
                 } else {
@@ -73,7 +155,7 @@ public InMemoryDiffEntry(@Nullable Path originalFilePath, @Nullable Path filePat
                 }
 
                 if (filePath != null) {
-                    this.newId = inserter.insert(Constants.OBJ_BLOB, newSource.getBytes(StandardCharsets.UTF_8)).abbreviate(40);
+                    this.newId = inserter.insert(Constants.OBJ_BLOB, newSource).abbreviate(40);
                     this.newMode = newMode;
                     this.newPath = (relativeTo == null ? filePath : relativeTo.relativize(filePath)).toString().replace("\\", "/");
                 } else {
@@ -115,6 +197,7 @@ public String getDiff(@Nullable Boolean ignoreAllWhitespace) {
         try (DiffFormatter formatter = new DiffFormatter(patch)) {
             formatter.setDiffComparator(ignoreAllWhitespace ? RawTextComparator.WS_IGNORE_ALL : RawTextComparator.DEFAULT);
             formatter.setRepository(repo);
+            formatter.setBinary(binaryPatch);
             formatter.format(this);
         } catch (IOException e) {
             throw new UncheckedIOException(e);
@@ -124,7 +207,8 @@ public String getDiff(@Nullable Boolean ignoreAllWhitespace) {
 
         AtomicBoolean addedComment = new AtomicBoolean(false);
         // NOTE: String.lines() would remove empty lines which we don't want
-        return Arrays.stream(diff.split("\n"))
+        // Use split with limit -1 to preserve trailing empty strings (important for binary patches)
+        return Arrays.stream(diff.split("\n", -1))
                        .map(l -> {
                            if (!addedComment.get() && l.startsWith("@@") && l.endsWith("@@")) {
                                addedComment.set(true);
@@ -142,11 +226,148 @@ public String getDiff(@Nullable Boolean ignoreAllWhitespace) {
                            }
                            return l;
                        })
-                       .collect(joining("\n")) + "\n";
+                       .collect(joining("\n"));
     }
 
     @Override
     public void close() {
         this.repo.close();
     }
+
+    private static byte[] printAllAsBytes(@Nullable SourceFile sourceFile, PrintOutputCapture.@Nullable MarkerPrinter markerPrinter) {
+        if (sourceFile == null || sourceFile instanceof Quark) {
+            return new byte[0];
+        }
+
+        PrintOutputCapture<Integer> out = markerPrinter == null ?
+                new PrintOutputCapture<>(0) :
+                new PrintOutputCapture<>(0, markerPrinter);
+
+        return sourceFile.printAllAsBytes(out);
+    }
+
+    @Value
+    @EqualsAndHashCode(callSuper = false)
+    private static class VirtualInMemoryRepository extends Repository {
+        InMemoryRepository repo;
+        VirtualObjectDatabase objdb;
+
+        public VirtualInMemoryRepository(InMemoryRepository.Builder builder) throws IOException {
+            super(builder);
+            repo = builder.build();
+            objdb = new VirtualObjectDatabase(repo.getObjectDatabase());
+        }
+
+        @Override
+        public void create(boolean bare) throws IOException {
+            repo.create(bare);
+        }
+
+        @Override
+        public String getIdentifier() {
+            return repo.getIdentifier();
+        }
+
+        @Override
+        public VirtualObjectDatabase getObjectDatabase() {
+            return objdb;
+        }
+
+        @Override
+        public RefDatabase getRefDatabase() {
+            return repo.getRefDatabase();
+        }
+
+        @Override
+        public StoredConfig getConfig() {
+            return repo.getConfig();
+        }
+
+        @Override
+        public AttributesNodeProvider createAttributesNodeProvider() {
+            return repo.createAttributesNodeProvider();
+        }
+
+        @Override
+        public void scanForRepoChanges() throws IOException {
+            repo.scanForRepoChanges();
+        }
+
+        @Override
+        public void notifyIndexChanged(boolean internal) {
+            repo.notifyIndexChanged(internal);
+        }
+
+        @Override
+        public ReflogReader getReflogReader(String refName) throws IOException {
+            return repo.getReflogReader(refName);
+        }
+    }
+
+    @Value
+    @EqualsAndHashCode(callSuper = false)
+    private static class VirtualObjectDatabase extends ObjectDatabase implements AutoCloseable {
+        ObjectDatabase delegate;
+        Map<ObjectId, byte[]> virtualObjects = new HashMap<>();
+
+        public ObjectId insertVirtual(ObjectId id, byte[] content) {
+            virtualObjects.put(id, content);
+            return id;
+        }
+
+        @Override
+        public ObjectInserter newInserter() {
+            return delegate.newInserter();
+        }
+
+        @Override
+        public VirtualObjectReader newReader() {
+            return new VirtualObjectReader(virtualObjects, delegate.newReader());
+        }
+
+        @Override
+        public void close() {
+            delegate.close();
+        }
+    }
+
+    @Value
+    @EqualsAndHashCode(callSuper = false)
+    private static class VirtualObjectReader extends ObjectReader {
+        Map<ObjectId, byte[]> virtualObjects;
+        ObjectReader delegate;
+
+        @Override
+        public ObjectReader newReader() {
+            return delegate.newReader();
+        }
+
+        @Override
+        public Collection<ObjectId> resolve(AbbreviatedObjectId id) throws IOException {
+            return delegate.resolve(id);
+        }
+
+        @Override
+        public ObjectLoader open(AnyObjectId objectId, int typeHint) throws IOException {
+            byte[] virtual = virtualObjects.get(objectId.toObjectId());
+            if (virtual != null) {
+                return new ObjectLoader.SmallObject(typeHint, virtual);
+            }
+            return delegate.open(objectId, typeHint);
+        }
+
+        @Override
+        public Set<ObjectId> getShallowCommits() {
+            try {
+                return delegate.getShallowCommits();
+            } catch (IOException e) {
+                throw new UncheckedIOException(e);
+            }
+        }
+
+        @Override
+        public void close() {
+            delegate.close();
+        }
+    }
 }

rewrite-core/src/main/java/org/openrewrite/remote/Remote.java

@@ -73,7 +73,7 @@ default <T extends SourceFile> T withChecksum(@Nullable Checksum checksum) {
     default <P> String printAll(P p) {
         ExecutionContext ctx = p instanceof ExecutionContext ? (ExecutionContext) p :
                 new InMemoryExecutionContext();
-        return StringUtils.readFully(getInputStream(ctx), StandardCharsets.UTF_8);
+        return StringUtils.readFully(getInputStream(ctx), getCharset() != null ? getCharset() : StandardCharsets.ISO_8859_1);
     }
 
     @Override
@@ -108,7 +108,7 @@ public Tree visit(@Nullable Tree tree, PrintOutputCapture<P> p) {
                 SourceFile sourceFile = (SourceFile) requireNonNull(tree);
                 ExecutionContext ctx = p.getContext() instanceof ExecutionContext ? (ExecutionContext) p.getContext() :
                         new InMemoryExecutionContext();
-                p.append(StringUtils.readFully(getInputStream(ctx), StandardCharsets.UTF_8));
+                p.append(StringUtils.readFully(getInputStream(ctx), getCharset() != null ? getCharset() : StandardCharsets.ISO_8859_1));
                 return sourceFile;
             }
         };