prepend_services argument for java_export (#9)

* initial 'prepend services' implementation

add tests, remove un-needed lines

add comments

* initial 'prepend services' implementation

add tests, remove un-needed lines

add comments

* remove System.out.println from MergeJars

* match indentation a bit better

* match indentation a bit better in MergeJars

* trigger ci

Author: vinnybod

private/rules/java_export.bzl

@@ -17,6 +17,7 @@ def java_export(
         tags = [],
         testonly = None,
         classifier_artifacts = {},
+        prepend_services = None,
         **kwargs):
     """Extends `java_library` to allow maven artifacts to be uploaded.
 
@@ -76,6 +77,7 @@ def java_export(
         Each label must correspond to a direct maven dependency of this target.
         Each exclusion is represented as a `group:artifact` string.
       classifier_artifacts: A dict of classifier -> artifact of additional artifacts to publish to Maven.
+      prepend_services: A file whose contents will be prepended to the generated `META-INF/services` files in the -project jar.
       doc_deps: Other `javadoc` targets that are referenced by the generated `javadoc` target
         (if not using `tags = ["no-javadoc"]`)
       doc_url: The URL at which the generated `javadoc` will be hosted (if not using
@@ -116,6 +118,7 @@ def java_export(
         testonly = testonly,
         javadocopts = javadocopts,
         classifier_artifacts = classifier_artifacts,
+        prepend_services = prepend_services,
         doc_deps = doc_deps,
         doc_url = doc_url,
         toolchains = toolchains,
@@ -135,6 +138,7 @@ def maven_export(
         testonly = False,
         javadocopts = [],
         classifier_artifacts = {},
+        prepend_services = None,
         *,
         doc_deps = [],
         doc_url = "",
@@ -195,6 +199,7 @@ def maven_export(
       exclusions: Mapping of target labels to a list of exclusions to be added to the POM file.
         Each label must correspond to a direct maven dependency of this target.
         Each exclusion is represented as a `group:artifact` string.
+      prepend_services: A file whose contents will be prepended to the generated `META-INF/services` files in the -project jar.
       doc_deps: Other `javadoc` targets that are referenced by the generated `javadoc` target
         (if not using `tags = ["no-javadoc"]`)
       doc_url: The URL at which the generated `javadoc` will be hosted (if not using
@@ -227,6 +232,7 @@ def maven_export(
         deploy_env = deploy_env,
         excluded_workspaces = excluded_workspaces.keys(),
         additional_dependencies = additional_dependencies,
+        prepend_services = prepend_services,
         visibility = visibility,
         tags = tags + maven_coordinates_tags,
         testonly = testonly,

private/rules/maven_project_jar.bzl

@@ -31,15 +31,21 @@ def _strip_excluded_workspace_jars(jar_files, excluded_workspaces):
 
     return to_return
 
-def _combine_jars(ctx, merge_jars, inputs, excludes, output):
+def _combine_jars(ctx, merge_jars, inputs, excludes, output, prepend_services = None):
     args = ctx.actions.args()
     args.add("--output", output)
     args.add_all(inputs, before_each = "--sources")
     args.add_all(excludes, before_each = "--exclude")
+    deps = [inputs, excludes]
+
+    if prepend_services:
+        prepend_services = prepend_services.files.to_list()[0]
+        args.add("--prepend_services", prepend_services)
+        deps.append(depset([prepend_services]))
 
     ctx.actions.run(
         mnemonic = "MergeJars",
-        inputs = depset(transitive = [inputs, excludes]),
+        inputs = depset(transitive = deps),
         outputs = [output],
         executable = merge_jars,
         arguments = [args],
@@ -76,6 +82,7 @@ def _maven_project_jar_impl(ctx):
                    [ji.transitive_runtime_jars for ji in info.dep_infos.to_list()] +
                    [jar[JavaInfo].transitive_runtime_jars for jar in ctx.attr.deploy_env]),
         intermediate_jar,
+        prepend_services = ctx.attr.prepend_services,
     )
 
     # Add manifest lines if necessary
@@ -123,6 +130,7 @@ def _maven_project_jar_impl(ctx):
                    [ji.transitive_source_jars for ji in info.dep_infos.to_list()] +
                    [jar[JavaInfo].transitive_source_jars for jar in ctx.attr.deploy_env]),
         src_jar,
+        prepend_services = ctx.attr.prepend_services,
     )
 
     java_toolchain = ctx.attr._java_toolchain[java_common.JavaToolchainInfo]
@@ -212,6 +220,11 @@ single artifact that other teams can download and use.
                 [JavaInfo],
             ],
         ),
+        "prepend_services": attr.label(
+            doc = "A file whose contents will be prepended to the generated `META-INF/services` files in the -project jar.",
+            default = None,
+            allow_single_file = True,
+        ),
         "_add_jar_manifest_entry": attr.label(
             executable = True,
             cfg = "exec",

private/tools/java/com/github/bazelbuild/rules_jvm_external/jar/MergeJars.java

@@ -33,10 +33,12 @@
 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;
@@ -45,6 +47,7 @@
 import java.util.jar.Attributes;
 import java.util.jar.JarOutputStream;
 import java.util.jar.Manifest;
+import java.util.stream.Collectors;
 import java.util.zip.ZipEntry;
 import java.util.zip.ZipFile;
 import java.util.zip.ZipInputStream;
@@ -53,6 +56,7 @@ public class MergeJars {
 
   public static void main(String[] args) throws IOException {
     Path out = null;
+    Path prependServices = null;
     // Insertion order may matter
     Set<Path> sources = new LinkedHashSet<>();
     Set<Path> excludes = new HashSet<>();
@@ -81,6 +85,11 @@ public static void main(String[] args) throws IOException {
           sources.add(isValid(Paths.get(args[++i])));
           break;
 
+        case "--prepend_services":
+          isValid(Paths.get(args[++i]));
+          prependServices = Paths.get(args[i]);
+          break;
+
         default:
           throw new IllegalArgumentException(
               "Unable to parse command line: " + Arrays.toString(args));
@@ -142,7 +151,12 @@ public static void main(String[] args) throws IOException {
             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> commentsRemoved = Arrays
+                .stream(content.split("\n"))
+                .filter(l -> !l.trim().startsWith("#"))
+                .filter(l -> !l.isEmpty())
+                .collect(Collectors.toList());
+            services.addAll(commentsRemoved);
             continue;
           }
 
@@ -207,7 +221,17 @@ public static void main(String[] args) throws IOException {
         for (Map.Entry<String, Set<String>> kv : allServices.entrySet()) {
           entry = new StableZipEntry("META-INF/services/" + kv.getKey());
           bos = new ByteArrayOutputStream();
-          bos.write(String.join("\n", kv.getValue()).getBytes());
+          List<String> toWrite = new ArrayList<>();
+
+          if (prependServices != null) {
+            String prepend = Files.readString(prependServices);
+            toWrite.addAll(Arrays.asList(prepend.split("\n")));
+            toWrite.add("");
+          }
+
+          toWrite.addAll(kv.getValue());
+          bos.write(String.join("\n", toWrite).getBytes());
+          bos.write("\n".getBytes());
           entry.setSize(bos.size());
           jos.putNextEntry(entry);
           jos.write(bos.toByteArray());

tests/com/github/bazelbuild/rules_jvm_external/jar/MergeJarsTest.java

@@ -553,6 +553,68 @@ public void mergedJarManifestSpecialAttributesAreHandled() throws IOException {
     }
   }
 
+  @Test
+  public void mergedJarServiceProviderFileRemovesCommentLines() throws IOException {
+    Path inputOne = temp.newFile("one.jar").toPath();
+    createJar(
+        inputOne,
+        ImmutableMap.of("META-INF/services/com.example.ServiceProvider", "# This is a comment")
+    );
+
+    Path inputTwo = temp.newFile("two.jar").toPath();
+    createJar(
+        inputTwo,
+        ImmutableMap.of("META-INF/services/com.example.ServiceProvider", "com.example.Foo")
+    );
+
+    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);
+
+    assertEquals("com.example.Foo\n", contents.get("META-INF/services/com.example.ServiceProvider"));
+  }
+
+  @Test
+  public void mergedJarServiceProviderFilePrependsLines() throws IOException {
+    Path inputOne = temp.newFile("one.jar").toPath();
+    createJar(
+        inputOne,
+        ImmutableMap.of("META-INF/services/com.example.ServiceProvider", "# This is a comment")
+    );
+
+    Path inputTwo = temp.newFile("two.jar").toPath();
+    createJar(
+        inputTwo,
+        ImmutableMap.of("META-INF/services/com.example.ServiceProvider", "com.example.Foo")
+    );
+
+    String prepend = "# This is a LICENSE\n # It should be kept\n";
+    Path inputThree = temp.newFile("LICENSE").toPath();
+    Files.write(inputThree, prepend.getBytes(UTF_8));
+
+    Path outputJar = temp.newFile("out.jar").toPath();
+
+    MergeJars.main(
+        new String[] {
+            "--output", outputJar.toAbsolutePath().toString(),
+            "--sources", inputOne.toAbsolutePath().toString(),
+            "--sources", inputTwo.toAbsolutePath().toString(),
+            "--prepend_services", inputThree.toAbsolutePath().toString(),
+        }
+    );
+
+    Map<String, String> contents = readJar(outputJar);
+
+    assertEquals(prepend + "\ncom.example.Foo\n", 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)) {