Improve artifact comparison delta results

Currently only the text is written as detailed information into the artifactcomparision directory. This is often not enough information to find the actual difference, to mitigate this the handling of the writing is now delegated to the delta and the following improvements are made: - class file comparison writes the class file as well as the disassemble - zip comparator writes the baseline and build jars
eclipse-tycho · Dec 22, 2023 · 0fe3631 · 0fe3631
1 parent 7e8005b
commit 0fe3631
Show file tree

Hide file tree

Showing 6 changed files with 93 additions and 23 deletions.
diff --git a/...-artifactcomparator/src/main/java/org/eclipse/tycho/artifactcomparator/ArtifactDelta.java b/...-artifactcomparator/src/main/java/org/eclipse/tycho/artifactcomparator/ArtifactDelta.java
@@ -12,6 +12,9 @@
  *******************************************************************************/
 package org.eclipse.tycho.artifactcomparator;
 
+import java.io.File;
+import java.io.IOException;
+
 import org.eclipse.tycho.zipcomparator.internal.SimpleArtifactDelta;
 
 /**
@@ -49,4 +52,12 @@ public interface ArtifactDelta {
      */
     public String getDetailedMessage();
 
+    /**
+     * Writes some details about this delta to the given destination
+     * 
+     * @param destination
+     * @throws IOException
+     */
+    void writeDetails(File destination) throws IOException;
+
 }
diff --git a/...omparator/src/main/java/org/eclipse/tycho/zipcomparator/internal/ClassfileComparator.java b/...omparator/src/main/java/org/eclipse/tycho/zipcomparator/internal/ClassfileComparator.java
@@ -12,6 +12,7 @@
  *******************************************************************************/
 package org.eclipse.tycho.zipcomparator.internal;
 
+import java.io.File;
 import java.io.IOException;
 import java.io.PrintWriter;
 import java.io.StringWriter;
@@ -47,7 +48,7 @@ public ArtifactDelta getDelta(ComparatorInputStream baseline, ComparatorInputStr
             if (baselineDisassemble.equals(reactorDisassemble)) {
                 return ArtifactDelta.NO_DIFFERENCE;
             }
-            return new SimpleArtifactDelta("different", baselineDisassemble, reactorDisassemble);
+            return new ClassfileArtifactDelta(baselineDisassemble, reactorDisassemble, baseline, reactor);
         } catch (RuntimeException e) {
             return baseline.compare(reactor);
         }
@@ -57,7 +58,7 @@ public ArtifactDelta getDelta(ComparatorInputStream baseline, ComparatorInputStr
     private String disassemble(byte[] bytes) {
         ClassReader reader = new ClassReader(bytes);
         ClassNode clazz = new ClassNode();
-        reader.accept(clazz, Opcodes.ASM9 | ClassReader.SKIP_DEBUG | ClassReader.SKIP_FRAMES);
+        reader.accept(clazz, Opcodes.ASM9 | ClassReader.SKIP_FRAMES);
 
         // inner class list gets reordered during pack200 normalization
         if (clazz.innerClasses != null && !clazz.innerClasses.isEmpty()) {
@@ -79,4 +80,26 @@ private String disassemble(byte[] bytes) {
     public boolean matches(String extension) {
         return TYPE.equalsIgnoreCase(extension);
     }
+
+    private static final class ClassfileArtifactDelta extends SimpleArtifactDelta {
+
+        private ComparatorInputStream baselineStream;
+        private ComparatorInputStream reactorStream;
+
+        public ClassfileArtifactDelta(String baseline, String reactor, ComparatorInputStream baselineStream,
+                ComparatorInputStream reactorStream) {
+            super("different", baseline, reactor);
+            this.baselineStream = baselineStream;
+            this.reactorStream = reactorStream;
+        }
+
+        @Override
+        public void writeDetails(File destination) throws IOException {
+            super.writeDetails(destination);
+            File basedir = destination.getParentFile();
+            writeFile(basedir, destination.getName() + "-baseline.class", baselineStream);
+            writeFile(basedir, destination.getName() + "-build.class", reactorStream);
+        }
+
+    }
 }
diff --git a/...parator/src/main/java/org/eclipse/tycho/zipcomparator/internal/CompoundArtifactDelta.java b/...parator/src/main/java/org/eclipse/tycho/zipcomparator/internal/CompoundArtifactDelta.java
@@ -14,7 +14,6 @@
 
 import java.io.File;
 import java.io.IOException;
-import java.nio.file.Files;
 import java.util.Collections;
 import java.util.LinkedHashMap;
 import java.util.Map;
@@ -62,26 +61,12 @@ private void indent(StringBuilder message, int indent) {
         }
     }
 
+    @Override
     public void writeDetails(File basedir) throws IOException {
         for (Map.Entry<String, ArtifactDelta> member : members.entrySet()) {
             ArtifactDelta memberDelta = member.getValue();
-            if (memberDelta instanceof CompoundArtifactDelta compoundDelta) {
-                compoundDelta.writeDetails(new File(basedir, member.getKey()));
-            } else if (memberDelta instanceof SimpleArtifactDelta delta) {
-                if (delta.getBaseline() != null) {
-                    writeFile(basedir, member.getKey() + "-baseline", delta.getBaseline());
-                }
-                if (delta.getReactor() != null) {
-                    writeFile(basedir, member.getKey() + "-build", delta.getReactor());
-                }
-            }
+            memberDelta.writeDetails(new File(basedir, member.getKey()));
         }
     }
 
-    private void writeFile(File basedir, String path, String data) throws IOException {
-        File file = new File(basedir, path).getAbsoluteFile();
-        file.getParentFile().mkdirs();
-        Files.writeString(file.toPath(), data);
-    }
-
 }
diff --git a/...omparator/src/main/java/org/eclipse/tycho/zipcomparator/internal/SimpleArtifactDelta.java b/...omparator/src/main/java/org/eclipse/tycho/zipcomparator/internal/SimpleArtifactDelta.java
@@ -12,6 +12,11 @@
  *******************************************************************************/
 package org.eclipse.tycho.zipcomparator.internal;
 
+import java.io.File;
+import java.io.IOException;
+import java.io.InputStream;
+import java.nio.file.Files;
+
 import org.eclipse.tycho.artifactcomparator.ArtifactDelta;
 
 public class SimpleArtifactDelta implements ArtifactDelta {
@@ -56,4 +61,26 @@ public String getBaseline() {
     public String getReactor() {
         return reactor;
     }
+
+    @Override
+    public void writeDetails(File destination) throws IOException {
+        if (getBaseline() != null) {
+            writeFile(destination.getParentFile(), destination.getName() + "-baseline", getBaseline());
+        }
+        if (getReactor() != null) {
+            writeFile(destination.getParentFile(), destination.getName() + "-build", getReactor());
+        }
+    }
+
+    protected static void writeFile(File basedir, String path, String data) throws IOException {
+        File file = new File(basedir, path).getAbsoluteFile();
+        file.getParentFile().mkdirs();
+        Files.writeString(file.toPath(), data);
+    }
+
+    protected static void writeFile(File basedir, String path, InputStream data) throws IOException {
+        File file = new File(basedir, path).getAbsoluteFile();
+        file.getParentFile().mkdirs();
+        Files.copy(data, file.toPath());
+    }
 }
diff --git a/...tcomparator/src/main/java/org/eclipse/tycho/zipcomparator/internal/ZipComparatorImpl.java b/...tcomparator/src/main/java/org/eclipse/tycho/zipcomparator/internal/ZipComparatorImpl.java
@@ -16,6 +16,7 @@
 import java.io.File;
 import java.io.IOException;
 import java.io.InputStream;
+import java.nio.file.Files;
 import java.util.Arrays;
 import java.util.Collection;
 import java.util.HashSet;
@@ -82,7 +83,27 @@ public ArtifactDelta getDelta(File baseline, File reactor, ComparisonData data)
             }
             return ArtifactDelta.DEFAULT;
         }
-        return !result.isEmpty() ? new CompoundArtifactDelta("different", result) : null;
+        return !result.isEmpty() ? new ZipArtifactDelta(result, baseline, reactor) : null;
+    }
+
+    private static final class ZipArtifactDelta extends CompoundArtifactDelta {
+
+        private File baseline;
+        private File reactor;
+
+        public ZipArtifactDelta(Map<String, ? extends ArtifactDelta> members, File baseline, File reactor) {
+            super("different", members);
+            this.baseline = baseline;
+            this.reactor = reactor;
+        }
+
+        @Override
+        public void writeDetails(File basedir) throws IOException {
+            super.writeDetails(basedir);
+            Files.copy(baseline.toPath(), basedir.toPath().resolve("baseline-" + baseline.getName()));
+            Files.copy(reactor.toPath(), basedir.toPath().resolve("build-" + reactor.getName()));
+        }
+
     }
 
     private ArtifactDelta getDelta(String name, Map<String, ZipEntry> baselineMap, Map<String, ZipEntry> reactorMap,

diff --git a/tycho-p2-plugin/src/main/java/org/eclipse/tycho/plugins/p2/BaselineValidator.java b/tycho-p2-plugin/src/main/java/org/eclipse/tycho/plugins/p2/BaselineValidator.java
@@ -60,6 +60,11 @@ public String getMessage() {
         public String getDetailedMessage() {
             return getMessage();
         }
+
+        @Override
+        public void writeDetails(File destination) throws IOException {
+
+        }
     }
 
     @Requirement
@@ -97,9 +102,7 @@ public Map<String, IP2Artifact> validateAndReplace(MavenProject project, Compari
                         File logdir = new File(project.getBuild().getDirectory(), "artifactcomparison");
                         log.info("Artifact comparison detailed log directory " + logdir.getAbsolutePath());
                         for (Map.Entry<String, ArtifactDelta> classifier : delta.getMembers().entrySet()) {
-                            if (classifier.getValue() instanceof CompoundArtifactDelta compoundDelta) {
-                                compoundDelta.writeDetails(new File(logdir, classifier.getKey()));
-                            }
+                            classifier.getValue().writeDetails(new File(logdir, classifier.getKey()));
                         }
                     }
                     if (baselineMode == fail || (baselineMode == failCommon && !isMissingOnlyDelta(delta))) {