Merge jar files without unpacking them

private/tools/java/rules/jvm/external/jar/DuplicateEntryStrategy.java

@@ -1,45 +1,36 @@
 package rules.jvm.external.jar;
 
-import rules.jvm.external.ByteStreams;
-
 import java.io.IOException;
-import java.io.InputStream;
-import java.io.OutputStream;
-import java.nio.file.Files;
-import java.nio.file.Path;
-import java.security.MessageDigest;
-import java.security.NoSuchAlgorithmException;
 import java.util.Arrays;
 
-import static java.nio.file.StandardOpenOption.TRUNCATE_EXISTING;
 import static java.util.stream.Collectors.joining;
 
 enum DuplicateEntryStrategy {
 
   LAST_IN_WINS("last-wins") {
     @Override
-    public void resolve(Path current, InputStream inputStreamForDuplicate) throws IOException {
-      try (OutputStream os = Files.newOutputStream(current, TRUNCATE_EXISTING)) {
-        ByteStreams.copy(inputStreamForDuplicate, os);
-      }
+    public boolean isReplacingCurrent(String name, byte[] originalHash, byte[] newHash) {
+      return true;
     }
   },
   FIRST_IN_WINS("first-wins") {
     @Override
-    public void resolve(Path current, InputStream inputStreamForDuplicate) {
-      // No need to do anything.
+    public boolean isReplacingCurrent(String name, byte[] originalHash, byte[] newHash) {
+      return originalHash == null;
     }
   },
   IS_ERROR("are-errors") {
     @Override
-    public void resolve(Path current, InputStream inputStreamForDuplicate) throws IOException {
-      byte[] first = hash(Files.readAllBytes(current));
-      byte[] second = hash(ByteStreams.toByteArray(inputStreamForDuplicate));
+    public boolean isReplacingCurrent(String name, byte[] originalHash, byte[] newHash) throws IOException {
+      if (originalHash == null) {
+        return true;
+      }
 
-      if (Arrays.equals(first, second)) {
-        return;
+      if (Arrays.equals(originalHash, newHash)) {
+        return false;
       }
-      throw new IOException("Attempt to write different duplicate file for: " + current);
+
+      throw new IOException("Attempt to write different duplicate file for: " + name);
     }
   };
 
@@ -65,15 +56,13 @@ public String toString() {
     return shortName;
   }
 
-  public abstract void resolve(Path current, InputStream inputStreamForDuplicate) throws IOException;
-
-  protected byte[] hash(byte[] bytes) {
-    try {
-      MessageDigest digest = MessageDigest.getInstance("SHA-1");
-      digest.update(bytes);
-      return digest.digest();
-    } catch (NoSuchAlgorithmException e) {
-      throw new RuntimeException(e);
-    }
-  }
+  /**
+   * Whether the current version of {@code name} (as identified by {@code originalHash}) should be
+   * replaced by the version identified by {@code newHash}. Both hashes should be generated using
+   * a {@link java.security.MessageDigest}, and the algorithm used for both should be the same.
+   *
+   * @param originalHash Generated hash, which may be null.
+   * @param newHash Generated hash, which must not be null.
+   */
+  public abstract boolean isReplacingCurrent(String name, byte[] originalHash, byte[] newHash) throws IOException;
 }

private/tools/java/rules/jvm/external/jar/MergeJars.java

@@ -25,27 +25,29 @@
 import java.io.IOException;
 import java.io.InputStream;
 import java.io.OutputStream;
-import java.io.UncheckedIOException;
 import java.nio.file.Files;
 import java.nio.file.Path;
 import java.nio.file.Paths;
 import java.nio.file.attribute.FileTime;
+import java.security.MessageDigest;
+import java.security.NoSuchAlgorithmException;
 import java.time.Instant;
 import java.util.Arrays;
 import java.util.Comparator;
+import java.util.HashMap;
 import java.util.HashSet;
 import java.util.LinkedHashSet;
 import java.util.Map;
 import java.util.Objects;
 import java.util.Set;
-import java.util.SortedMap;
 import java.util.TreeMap;
 import java.util.TreeSet;
 import java.util.jar.Attributes;
 import java.util.jar.JarEntry;
 import java.util.jar.JarOutputStream;
 import java.util.jar.Manifest;
 import java.util.zip.ZipEntry;
+import java.util.zip.ZipFile;
 import java.util.zip.ZipInputStream;
 
 import static java.util.zip.Deflater.BEST_COMPRESSION;
@@ -110,17 +112,23 @@ public static void main(String[] args) throws IOException {
     // Remove any jars from sources that we've been told to exclude
     sources.removeIf(excludes::contains);
 
-    // To keep things simple, we expand all the inputs jars into a single directory,
-    // merge the manifests, and then create our own zip.
-    Path temp = Files.createTempDirectory("mergejars");
+    // We would love to keep things simple by expanding all the input jars into
+    // a single directory, but this isn't possible since one jar may contain a
+    // file with the same name as a directory in another. *sigh* Instead, what
+    // we'll do is create a list of contents from each jar, and where we should
+    // pull the contents from. Whee!
 
     Manifest manifest = new Manifest();
     manifest.getMainAttributes().put(Attributes.Name.MANIFEST_VERSION, "1.0");
 
     Map<String, Set<String>> allServices = new TreeMap<>();
-    Map<Path, SortedMap<Path, Path>> allPaths = new TreeMap<>();
     Set<String> excludedPaths = readExcludedFileNames(excludes);
 
+    // Ultimately, we want the entries in the output zip to be sorted
+    // so that we have a deterministic output.
+    Map<String, Path> fileToSourceJar = new TreeMap<>();
+    Map<String, byte[]> fileHashCodes = new HashMap<>();
+
     for (Path source : sources) {
       try (InputStream fis = Files.newInputStream(source);
            ZipInputStream zis = new ZipInputStream(fis)) {
@@ -133,8 +141,8 @@ public static void main(String[] args) throws IOException {
             continue;
           }
 
-          if (entry.isDirectory() ||
-            (!entry.getName().startsWith("META-INF/") && excludedPaths.contains(entry.getName()))) {
+          if ("META-INF/".equals(entry.getName()) ||
+                  (!entry.getName().startsWith("META-INF/") && excludedPaths.contains(entry.getName()))) {
             continue;
           }
 
@@ -146,36 +154,34 @@ public static void main(String[] args) throws IOException {
             continue;
           }
 
-          Path outPath = temp.resolve(entry.getName()).normalize();
-          if (!outPath.startsWith(temp)) {
-            throw new IllegalStateException("Attempt to write jar entry somewhere weird: " + outPath);
-          }
-          if (!Files.exists(outPath.getParent())) {
-            Files.createDirectories(outPath.getParent());
-          }
-
-          if (Files.exists(outPath)) {
-            // Write the file out to a temporary location, and then figure out what to do
-            onDuplicate.resolve(outPath, zis);
+          if (entry.isDirectory()) {
+            // Duplicate directory names are fine
+            fileToSourceJar.put(entry.getName(), source);
           } else {
-            try (OutputStream os = Files.newOutputStream(outPath)) {
-              ByteStreams.copy(zis, os);
+            // Duplicate files, however may not be. We need the hash to determine
+            // whether we should do anything.
+            byte[] hash = hash(zis);
+
+            if (!fileToSourceJar.containsKey(entry.getName())) {
+              fileToSourceJar.put(entry.getName(), source);
+              fileHashCodes.put(entry.getName(), hash);
+            } else {
+              byte[] originalHashCode = fileHashCodes.get(entry.getName());
+              boolean replace = onDuplicate.isReplacingCurrent(entry.getName(), originalHashCode, hash);
+              if (replace) {
+                fileToSourceJar.put(entry.getName(), source);
+                fileHashCodes.put(entry.getName(), hash);
+              }
             }
-
-            SortedMap<Path, Path> dirPaths = allPaths.computeIfAbsent(
-              temp.relativize(outPath.getParent()),
-              path -> new TreeMap<>());
-            dirPaths.put(temp.relativize(outPath), outPath);
           }
         }
       }
     }
 
     manifest.getMainAttributes().put(new Attributes.Name("Created-By"), "mergejars");
 
-    Set<String> seen = new HashSet<>(Arrays.asList("META-INF/", "META-INF/MANIFEST.MF"));
-
     // Now create the output jar
+    Files.createDirectories(out.getParent());
     try (OutputStream os = Files.newOutputStream(out);
          JarOutputStream jos = new JarOutputStream(os)) {
       jos.setMethod(DEFLATED);
@@ -213,36 +219,41 @@ public static void main(String[] args) throws IOException {
         }
       }
 
-      allPaths.forEach((dir, entries) -> {
-        try {
-          String name = dir.toString().replace('\\', '/') + "/";
-          if (seen.add(name)) {
-            JarEntry je = new JarEntry(name);
-            je = resetTime(je);
-            jos.putNextEntry(je);
-            jos.closeEntry();
-          }
+      Path previousSource = sources.isEmpty() ? null : sources.iterator().next();
+      ZipFile source = previousSource == null ? null : new ZipFile(previousSource.toFile());
 
-          for (Map.Entry<Path, Path> me : entries.entrySet()) {
-            name = me.getKey().toString().replace('\\', '/');
+      // We should never enter this loop without there being any sources
+      for (Map.Entry<String, Path> pathAndSource : fileToSourceJar.entrySet()) {
+        // Get the original entry
+        JarEntry je = new JarEntry(pathAndSource.getKey());
+        je = resetTime(je);
+        jos.putNextEntry(je);
 
-            if (seen.add(name)) {
-              JarEntry je = new JarEntry(name);
-              je = resetTime(je);
-              jos.putNextEntry(je);
-              try (InputStream fis = Files.newInputStream(me.getValue())) {
-                ByteStreams.copy(fis, jos);
-              }
-              jos.closeEntry();
-            }
-          }
-        } catch (IOException e) {
-          throw new UncheckedIOException(e);
+        if (je.isDirectory()) {
+          jos.closeEntry();
+          continue;
         }
-      });
-    }
 
-    delete(temp);
+        if (!Objects.equals(previousSource, pathAndSource.getValue())) {
+          source.close();
+          source = new ZipFile(pathAndSource.getValue().toFile());
+          previousSource = pathAndSource.getValue();
+        }
+
+        ZipEntry original = source.getEntry(pathAndSource.getKey());
+        if (original == null) {
+          continue;
+        }
+
+        try (InputStream is = source.getInputStream(original)) {
+          ByteStreams.copy(is, jos);
+        }
+        jos.closeEntry();
+      }
+      if (source != null) {
+        source.close();
+      }
+    }
   }
 
   private static Set<String> readExcludedFileNames(Set<Path> excludes) throws IOException {
@@ -316,4 +327,20 @@ private static Manifest merge(Manifest into, Manifest from) {
 
     return into;
   }
+
+  private static byte[] hash(InputStream inputStream) {
+    try {
+      MessageDigest digest = MessageDigest.getInstance("SHA-1");
+
+      byte[] buf = new byte[100 * 1024];
+      int read;
+
+      while ((read = inputStream.read(buf)) != -1) {
+        digest.update(buf, 0, read);
+      }
+     return digest.digest();
+    } catch (IOException | NoSuchAlgorithmException e) {
+      throw new RuntimeException(e);
+    }
+  }
 }

tests/com/jvm/external/jar/MergeJarsTest.java

@@ -232,6 +232,28 @@ public void shouldNotIncludeManifestOrMetaInfEntriesFromExclusions() throws IOEx
     assertEquals("Hello, World!", contents.get("META-INF/foo"));
   }
 
+  @Test
+  public void canMergeJarsWhereADirectoryAndFileShareTheSamePath() throws IOException {
+    Path inputOne = temp.newFile("one.jar").toPath();
+    createJar(inputOne, ImmutableMap.of("example/file.txt", "Yellow!"));
+
+    Path inputTwo = temp.newFile("two.jar").toPath();
+    createJar(inputTwo, ImmutableMap.of("example", "Purple!"));
+
+    Path outputJar = temp.newFile("out.jar").toPath();
+
+    MergeJars.main(new String[]{
+            "--output", outputJar.toAbsolutePath().toString(),
+            "--sources", inputOne.toAbsolutePath().toString(),
+            "--sources", inputTwo.toAbsolutePath().toString()});
+
+    Map<String, String> contents = readJar(outputJar);
+
+    // One entry for the manifest, one for the file "example", and one for "example/file.txt"
+    assertEquals("Yellow!", contents.get("example/file.txt"));
+    assertEquals("Purple!", contents.get("example"));
+  }
+
   private void createJar(Path outputTo, Map<String, String> pathToContents) throws IOException {
     try (OutputStream os = Files.newOutputStream(outputTo);
          ZipOutputStream zos = new ZipOutputStream(os)) {