Retain comments / licenses in service provider files when merging jars (

#1123) Do so by concatenating service files instead of prepending license with extra property
bazel-contrib · Sep 7, 2024 · b452dd5 · b452dd5
1 parent 0f751e0
commit b452dd5
Show file tree

Hide file tree

Showing 2 changed files with 43 additions and 8 deletions.
diff --git a/private/tools/java/com/github/bazelbuild/rules_jvm_external/jar/MergeJars.java b/private/tools/java/com/github/bazelbuild/rules_jvm_external/jar/MergeJars.java
@@ -33,15 +33,16 @@
 import java.nio.file.Paths;
 import java.security.MessageDigest;
 import java.security.NoSuchAlgorithmException;
+import java.util.ArrayList;
 import java.util.Arrays;
 import java.util.HashMap;
 import java.util.HashSet;
 import java.util.LinkedHashSet;
+import java.util.List;
 import java.util.Map;
 import java.util.Objects;
 import java.util.Set;
 import java.util.TreeMap;
-import java.util.TreeSet;
 import java.util.jar.Attributes;
 import java.util.jar.JarOutputStream;
 import java.util.jar.Manifest;
@@ -109,7 +110,7 @@ public static void main(String[] args) throws IOException {
     Manifest manifest = new Manifest();
     manifest.getMainAttributes().put(Attributes.Name.MANIFEST_VERSION, "1.0");
 
-    Map<String, Set<String>> allServices = new TreeMap<>();
+    Map<String, List<String>> allServices = new TreeMap<>();
     Set<String> excludedPaths = readExcludedFileNames(excludes);
     Set<String> duplicateExceptions = Set.of("COPYRIGHT", "LICENSE", "NOTICE");
 
@@ -139,10 +140,9 @@ public static void main(String[] args) throws IOException {
 
           if (entry.getName().startsWith("META-INF/services/") && !entry.isDirectory()) {
             String servicesName = entry.getName().substring("META-INF/services/".length());
-            Set<String> services =
-                allServices.computeIfAbsent(servicesName, key -> new TreeSet<>());
-            String content = new String(ByteStreams.toByteArray(zis));
-            services.addAll(Arrays.asList(content.split("\n")));
+            List<String> services =
+                allServices.computeIfAbsent(servicesName, key -> new ArrayList<>());
+            services.add(new String(ByteStreams.toByteArray(zis)));
             continue;
           }
 
@@ -204,10 +204,12 @@ public static void main(String[] args) throws IOException {
           jos.closeEntry();
           createdDirectories.add(entry.getName());
         }
-        for (Map.Entry<String, Set<String>> kv : allServices.entrySet()) {
+        for (Map.Entry<String, List<String>> kv : allServices.entrySet()) {
           entry = new StableZipEntry("META-INF/services/" + kv.getKey());
           bos = new ByteArrayOutputStream();
-          bos.write(String.join("\n", kv.getValue()).getBytes());
+
+          bos.write(String.join("\n\n", kv.getValue()).getBytes());
+          bos.write("\n".getBytes());
           entry.setSize(bos.size());
           jos.putNextEntry(entry);
           jos.write(bos.toByteArray());

diff --git a/tests/com/github/bazelbuild/rules_jvm_external/jar/MergeJarsTest.java b/tests/com/github/bazelbuild/rules_jvm_external/jar/MergeJarsTest.java
@@ -553,6 +553,39 @@ public void mergedJarManifestSpecialAttributesAreHandled() throws IOException {
     }
   }
 
+  @Test
+  public void mergedJarServiceProviderFilePreservesComments() throws IOException {
+    Path inputOne = temp.newFile("one.jar").toPath();
+    String inputOneContents = "# This is a comment\n# This is another comment\ncom.example.Foo";
+    createJar(
+        inputOne,
+        ImmutableMap.of("META-INF/services/com.example.ServiceProvider", inputOneContents)
+    );
+
+    Path inputTwo = temp.newFile("two.jar").toPath();
+    String inputTwoContents = "# My License\ncom.example.Bar";
+    createJar(
+        inputTwo,
+        ImmutableMap.of("META-INF/services/com.example.ServiceProvider", inputTwoContents)
+    );
+
+    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);
+
+    String expected = String.join("\n\n", inputOneContents, inputTwoContents) + "\n";
+
+    assertEquals(expected, contents.get("META-INF/services/com.example.ServiceProvider"));
+  }
+
   private void createJar(Path outputTo, Map<String, String> pathToContents) throws IOException {
     try (OutputStream os = Files.newOutputStream(outputTo);
         ZipOutputStream zos = new ZipOutputStream(os)) {