Merge branch 'bazelbuild-master' into vinnybod/sha256-sha512

docs/api.md

@@ -34,7 +34,7 @@ Generate a javadoc from all the `deps`
 | <a id="javadoc-doc_deps"></a>doc_deps |  `javadoc` targets referenced by the current target.<br><br>Use this to automatically add appropriate `-linkoffline` javadoc options to resolve references to packages documented by the given javadoc targets that have `url` specified.   | <a href="https://bazel.build/concepts/labels">List of labels</a> | optional |  `[]`  |
 | <a id="javadoc-doc_url"></a>doc_url |  The URL at which this documentation will be hosted.<br><br>This information is only used by javadoc targets depending on this target.   | String | optional |  `""`  |
 | <a id="javadoc-excluded_workspaces"></a>excluded_workspaces |  A list of bazel workspace names to exclude from the generated jar   | List of strings | optional |  `["com_google_protobuf", "protobuf"]`  |
-| <a id="javadoc-javadocopts"></a>javadocopts |  javadoc options. Note sources and classpath are derived from the deps. Any additional options can be passed here.   | List of strings | optional |  `[]`  |
+| <a id="javadoc-javadocopts"></a>javadocopts |  javadoc options. Note sources and classpath are derived from the deps. Any additional options can be passed here. If nothing is passed, a default list of options is used: ["-notimestamp", "-use", "-quiet", "-Xdoclint:-missing", "-encoding", "UTF8"]   | List of strings | optional |  `["-notimestamp", "-use", "-quiet", "-Xdoclint:-missing", "-encoding", "UTF8"]`  |
 
 
 <a id="java_export"></a>

private/rules/java_export.bzl

@@ -199,7 +199,6 @@ def maven_export(
     manifest_entries = manifest_entries if manifest_entries else {}
     deploy_env = deploy_env if deploy_env else []
     excluded_workspaces = excluded_workspaces if excluded_workspaces else {}
-    javadocopts = javadocopts if javadocopts else []
     doc_url = doc_url if doc_url else ""
     doc_deps = doc_deps if doc_deps else []
     tags = tags if tags else []

private/rules/javadoc.bzl

@@ -8,6 +8,15 @@ _JavadocInfo = provider(
     },
 )
 
+_DEFAULT_JAVADOCOPTS = [
+    "-notimestamp",
+    "-use",
+    "-quiet",
+    "-Xdoclint:-missing",
+    "-encoding",
+    "UTF8",
+]
+
 def generate_javadoc(
         ctx,
         javadoc,
@@ -64,7 +73,6 @@ def _javadoc_impl(ctx):
     # from dep[JavaInfo].compilation_info (which, FWIW, always returns
     # `None` https://github.com/bazelbuild/bazel/issues/10170). For this
     # reason we allow people to set javadocopts via the rule attrs.
-
     generate_javadoc(
         ctx,
         ctx.executable._javadoc,
@@ -107,8 +115,10 @@ javadoc = rule(
         "javadocopts": attr.string_list(
             doc = """javadoc options.
             Note sources and classpath are derived from the deps. Any additional
-            options can be passed here.
-            """,
+            options can be passed here. If nothing is passed, a default list of options is used:
+            %s
+            """ % _DEFAULT_JAVADOCOPTS,
+            default = _DEFAULT_JAVADOCOPTS,
         ),
         "doc_deps": attr.label_list(
             doc = """`javadoc` targets referenced by the current target.

private/rules/kt_jvm_export.bzl

@@ -68,7 +68,7 @@ def kt_jvm_export(
     maven_coordinates_tags = ["maven_coordinates=%s" % maven_coordinates]
     lib_name = "%s-lib" % name
 
-    javadocopts = kwargs.pop("javadocopts", [])
+    javadocopts = kwargs.pop("javadocopts", None)
 
     # ensure that the kotlin-stdlib is included in deploy_env
     if KOTLIN_STDLIB not in deploy_env:

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());

private/tools/java/com/github/bazelbuild/rules_jvm_external/javadoc/JavadocJarMaker.java

@@ -155,10 +155,6 @@ public static void main(String[] args) throws IOException {
       }
       Version version = Runtime.version();
 
-      options.addAll(
-          Arrays.asList(
-              "-notimestamp", "-use", "-quiet", "-Xdoclint:-missing", "-encoding", "UTF8"));
-
       // Generate frames if we can. Java prior to v9 generates frames automatically.
       // In Java 13, the flag was removed.
       if (version.compareTo(JAVA_9) > 0 && version.compareTo(JAVA_13) < 0) {

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)) {