code review feedback

defs.bzl

@@ -19,7 +19,7 @@ load("//private/rules:java_export.bzl", _java_export = "java_export")
 load("//private/rules:javadoc.bzl", _javadoc = "javadoc")
 load("//private/rules:maven_bom.bzl", _maven_bom = "maven_bom")
 load("//private/rules:maven_install.bzl", _maven_install = "maven_install")
-load("//private/rules:maven_publish.bzl", _MavenPublishInfo = "MavenPublishInfo")
+load("//private/rules:maven_publish.bzl", _MavenPublishInfo = "MavenPublishInfo", _maven_publish="maven_publish")
 load("//private/rules:pom_file.bzl", _pom_file = "pom_file")
 
 DEFAULT_REPOSITORY_NAME = _DEFAULT_REPOSITORY_NAME
@@ -33,3 +33,4 @@ maven_install = _maven_install
 pom_file = _pom_file
 read_coordinates = _read_coordinates
 MavenPublishInfo = _MavenPublishInfo
+maven_publish = _maven_publish

examples/java-export/BUILD

@@ -1,4 +1,4 @@
-load("@rules_jvm_external//:defs.bzl", "java_export", "maven_bom")
+load("@rules_jvm_external//:defs.bzl", "java_export", "maven_bom", "maven_publish")
 # To export the file, run:
 #
 # `bazel run //:example-export.publish --define "maven_repo=file://$(pwd)/repository"`
@@ -9,14 +9,21 @@ load("@rules_jvm_external//:defs.bzl", "java_export", "maven_bom")
 java_export(
     name = "example-export",
     extra_artifacts = {
-        "release": "//src/main/java/com/github/bazelbuild/rulesjvmexternal/example/export:tar",
+        "//src/main/java/com/github/bazelbuild/rulesjvmexternal/example/export:tar":"release",
     },
     maven_coordinates = "com.example:bazel-example:0.0.1",
     runtime_deps = [
         "//src/main/java/com/github/bazelbuild/rulesjvmexternal/example/export",
     ],
 )
 
+maven_publish(
+    name = "artifact-publish",
+    artifact = "//src/main/java/com/github/bazelbuild/rulesjvmexternal/example/export:tar",
+    coordinates = "com.example:artifact-example:0.0.1",
+)
+
+
 maven_bom(
     name = "bom",
     dependencies_maven_coordinates = "com.example:bazel-example-dependencies:0.0.1",

examples/java-export/src/main/java/com/github/bazelbuild/rulesjvmexternal/example/export/BUILD

@@ -16,7 +16,6 @@ java_library(
 pkg_tar(
     name = "tar",
     srcs = glob(["*"]),
-    extension = "tar.gz",
     visibility = [
         "//:__pkg__",
     ],

private/rules/java_export.bzl

@@ -165,9 +165,6 @@ def maven_export(
             testonly = testonly,
         )
 
-    extra_artifacts_targets = extra_artifacts.values()
-    extra_classifiers = extra_artifacts.keys()
-
     pom_file(
         name = "%s-pom" % name,
         target = ":%s" % lib_name,
@@ -178,13 +175,12 @@ def maven_export(
     )
 
     maven_publish(
-        extra_artifacts = extra_artifacts_targets,
-        extra_classifiers = extra_classifiers,
+        extra_artifacts = extra_artifacts,
         name = "%s.publish" % name,
         coordinates = maven_coordinates,
         pom = "%s-pom" % name,
         javadocs = docs_jar,
-        artifact_jar = ":%s-maven-artifact" % name,
+        artifact = ":%s-maven-artifact" % name,
         source_jar = ":%s-maven-source" % name,
         visibility = visibility,
         tags = tags,

private/rules/maven_publish.bzl

@@ -3,17 +3,16 @@ MavenPublishInfo = provider(
         "coordinates": "Maven coordinates for the project, which may be None",
         "pom": "Pom.xml file for metdata",
         "javadocs": "Javadoc jar file for documentation files",
-        "artifact_jar": "Jar with the code and metadata for execution",
+        "artifact": "Jar with the code and metadata or war/tar artifacts",
         "source_jar": "Jar with the source code for review",
-        "extra_classifiers": "Extra classifiers for Artifacts",
-        "extra_artifacts": "Extra artifacts such as tars, wars",
+        "extra_artifacts": "Extra artifacts such as tars, wars to classifiers mapping",
     },
 )
 
 _TEMPLATE = """#!/usr/bin/env bash
 
 echo "Uploading {coordinates} to {maven_repo}"
-{uploader} {maven_repo} {gpg_sign} {user} {password} {coordinates} {pom} {artifact_jar} {source_jar} {javadoc} {extra_classifiers} {extra_artifacts}
+{uploader} {maven_repo} {gpg_sign} {user} {password} {coordinates} {pom} {artifact} {source_jar} {javadoc}  {extra_artifacts}
 """
 
 def _maven_publish_impl(ctx):
@@ -26,10 +25,13 @@ def _maven_publish_impl(ctx):
 
     # Expand maven coordinates for any variables to be replaced.
     coordinates = ctx.expand_make_variables("coordinates", ctx.attr.coordinates, {})
-    artifacts_short_path = ctx.file.artifact_jar.short_path if ctx.file.artifact_jar else "''"
+    artifact_short_path = ctx.file.artifact.short_path if ctx.file.artifact else "''"
     source_short_path = ctx.file.source_jar.short_path if ctx.file.source_jar else "''"
+    pom_short_path = ctx.file.pom.short_path if ctx.file.pom else "''"
     javadocs_short_path = ctx.file.javadocs.short_path if ctx.file.javadocs else "''"
-    extra_artifacts = [file.short_path for file in ctx.files.extra_artifacts]
+    print(ctx.attr.extra_artifacts.items())
+    extra_files = [target.files.to_list()[0] for target,_ in ctx.attr.extra_artifacts.items()]
+    extra_artifacts = ["{}={}".format(classifier,target.files.to_list()[0].short_path) for target, classifier in ctx.attr.extra_artifacts.items()]
 
     ctx.actions.write(
         output = executable,
@@ -41,18 +43,19 @@ def _maven_publish_impl(ctx):
             maven_repo = maven_repo,
             password = password,
             user = user,
-            pom = ctx.file.pom.short_path,
-            artifact_jar = artifacts_short_path,
+            pom = pom_short_path,
+            artifact = artifact_short_path,
             source_jar = source_short_path,
             javadoc = javadocs_short_path,
-            extra_classifiers = ",".join(ctx.attr.extra_classifiers),
             extra_artifacts = ",".join(extra_artifacts),
         ),
     )
 
-    files = ctx.files.extra_artifacts + [ctx.file.pom]
-    if ctx.file.artifact_jar:
-        files.append(ctx.file.artifact_jar)
+    files = extra_files
+    if ctx.file.pom:
+        files.append(ctx.file.pom)
+    if ctx.file.artifact:
+        files.append(ctx.file.artifact)
     if ctx.file.source_jar:
         files.append(ctx.file.source_jar)
     if ctx.file.javadocs:
@@ -69,7 +72,7 @@ def _maven_publish_impl(ctx):
         ),
         MavenPublishInfo(
             coordinates = coordinates,
-            artifact_jar = ctx.file.artifact_jar,
+            artifact = ctx.file.artifact,
             javadocs = ctx.file.javadocs,
             source_jar = ctx.file.source_jar,
             pom = ctx.file.pom,
@@ -97,20 +100,18 @@ When signing with GPG, the current default key is used.
             mandatory = True,
         ),
         "pom": attr.label(
-            mandatory = True,
             allow_single_file = True,
         ),
         "javadocs": attr.label(
             allow_single_file = True,
         ),
-        "artifact_jar": attr.label(
+        "artifact": attr.label(
             allow_single_file = True,
         ),
         "source_jar": attr.label(
             allow_single_file = True,
         ),
-        "extra_classifiers": attr.string_list(),
-        "extra_artifacts": attr.label_list(),
+        "extra_artifacts": attr.label_keyed_string_dict(),
         "_uploader": attr.label(
             executable = True,
             cfg = "exec",

private/tools/java/rules/jvm/external/maven/BUILD

@@ -7,6 +7,10 @@ java_binary(
     visibility = ["//visibility:public"],
     deps = [
         "//private/tools/java/rules/jvm/external:byte-streams",
+        artifact(
+            "com.google.guava:guava",
+            repository_name = "rules_jvm_external_deps",
+        ),
         artifact(
             "com.google.auth:google-auth-library-oauth2-http",
             repository_name = "rules_jvm_external_deps",

private/tools/java/rules/jvm/external/maven/MavenPublisher.java

@@ -53,6 +53,9 @@
 import java.util.concurrent.Executors;
 import java.util.concurrent.TimeoutException;
 import java.util.logging.Logger;
+
+import com.google.common.base.Splitter;
+import com.google.common.base.Strings;
 import rules.jvm.external.ByteStreams;
 import software.amazon.awssdk.services.s3.S3Client;
 import software.amazon.awssdk.services.s3.model.PutObjectRequest;
@@ -85,19 +88,19 @@ public static void main(String[] args)
     Coordinates coords = new Coordinates(parts.get(0), parts.get(1), parts.get(2));
 
     // Calculate md5 and sha1 for each of the inputs
-    Path pom = Paths.get(args[5]);
+    Path pom = getPathIfSet(args[5]);
     Path binJar = getPathIfSet(args[6]);
     Path srcJar = getPathIfSet(args[7]);
     Path docJar = getPathIfSet(args[8]);
-    String extraClassifiers = args.length > 9 ? args[9] : null;
-    String extraArtifacts = args.length > 10 ? args[10] : null;
 
     try {
       List<CompletableFuture<Void>> futures = new ArrayList<>();
-      futures.add(upload(repo, credentials, coords, ".pom", pom, gpgSign));
-
+      if(pom != null) {
+        futures.add(upload(repo, credentials, coords, ".pom", pom, gpgSign));
+      }
       if (binJar != null) {
-        futures.add(upload(repo, credentials, coords, ".jar", binJar, gpgSign));
+        String ext = com.google.common.io.Files.getFileExtension(binJar.getFileName().toString());
+        futures.add(upload(repo, credentials, coords, "." + ext, binJar, gpgSign));
       }
 
       if (srcJar != null) {
@@ -107,12 +110,17 @@ public static void main(String[] args)
       if (docJar != null) {
         futures.add(upload(repo, credentials, coords, "-javadoc.jar", docJar, gpgSign));
       }
-
-      if(extraClassifiers != null && extraArtifacts != null) {
-        LOG.info(String.format("extraClassifiers: %s, extraArtifacts: %s", extraClassifiers, extraArtifacts));
-        futures.add(uploadExtraArtifacts(repo, credentials, coords, extraClassifiers, extraArtifacts));
+      if(args.length > 9) {
+        List<String> extraArtifactsList = Splitter.onPattern(",").splitToList(args[9]);
+        for (String extraArtifactTuple : extraArtifactsList) {
+          // Get the extra artifacts in tuple format. (classifier = artifactPath)
+          String[] splits = extraArtifactTuple.split("=");
+          String classifier = splits[0];
+          Path artifactPath = getPathIfSet(splits[1]);
+          String ext = com.google.common.io.Files.getFileExtension(artifactPath.getFileName().toString());
+          futures.add(upload(repo, credentials, coords, String.format("-%s.%s", classifier, ext), artifactPath, gpgSign));
+        }
       }
-
       CompletableFuture<Void> all = CompletableFuture.allOf(futures.toArray(new CompletableFuture[0]));
       all.get(30, MINUTES);
     } finally {
@@ -176,34 +184,6 @@ private static CompletableFuture<Void> upload(
     return CompletableFuture.allOf(uploads.toArray(new CompletableFuture<?>[0]));
   }
 
-  private static CompletableFuture<Void> uploadExtraArtifacts(
-      String repo,
-      Credentials credentials,
-      Coordinates coords,
-      String extraClassifiers,
-      String extraArtifacts
-    ){
-    List<CompletableFuture<?>> uploads = new ArrayList<>();
-    String[] extraClassifiersArr = extraClassifiers.split(",");
-    String[] extraArtifactsArr = extraArtifacts.split(",");
-    for(int index=0;  index<extraClassifiersArr.length; ++index) {
-      Path artifact = getPathIfSet(extraArtifactsArr[index]);
-      String fileName = artifact.toString();
-      String ext = fileName.substring(fileName.lastIndexOf("."));
-      String base = String.format(
-          "%s/%s/%s/%s/%s-%s",
-          repo.replaceAll("/$", ""),
-          coords.groupId.replace('.', '/'),
-          coords.artifactId,
-          coords.version,
-          coords.artifactId,
-          coords.version);
-
-      uploads.add(upload(String.format("%s-%s%s", base, extraClassifiersArr[index], ext), credentials, artifact));
-    }
-    return CompletableFuture.allOf(uploads.toArray(new CompletableFuture<?>[0]));
-  }
-
   private static String toSha1(byte[] toHash) {
     return toHexS("%040x", "SHA-1", toHash);
   }

repositories.bzl

@@ -31,6 +31,7 @@ def rules_jvm_external_deps(repositories = _DEFAULT_REPOSITORIES):
             "com.google.googlejavaformat:google-java-format:jar:1.15.0",
             "org.apache.maven:maven-artifact:3.8.6",
             "software.amazon.awssdk:s3:2.17.183",
+            "com.google.guava:guava:31.1-jre"
         ],
         maven_install_json = "@rules_jvm_external//:rules_jvm_external_deps_install.json",
         fail_if_repin_required = True,