From 085c6a974eea0eb8fcbc8b98e5366b655b61c9a4 Mon Sep 17 00:00:00 2001 From: Jeremiah Zucker Date: Fri, 5 Jan 2024 12:54:52 -0500 Subject: [PATCH 1/8] aar packaging support --- private/rules/has_maven_deps.bzl | 4 +- private/rules/maven_project_jar.bzl | 9 ++- .../rules_jvm_external/jar/MergeJars.java | 73 +++++++++++++++++++ 3 files changed, 83 insertions(+), 3 deletions(-) diff --git a/private/rules/has_maven_deps.bzl b/private/rules/has_maven_deps.bzl index ccdd5add2..07f42e4a1 100644 --- a/private/rules/has_maven_deps.bzl +++ b/private/rules/has_maven_deps.bzl @@ -95,6 +95,7 @@ _gathered = provider( "artifact_infos", "transitive_exports", "dep_infos", + "artifact_coordinates", ], ) @@ -104,7 +105,7 @@ def _extract_from(gathered, maven_info, dep, include_transitive_exports): gathered.all_infos.append(maven_info) gathered.label_to_javainfo.update(maven_info.label_to_javainfo) if java_info: - if maven_info.coordinates: + if maven_info.coordinates and maven_info.coordinates != gathered.artifact_coordinates: gathered.dep_infos.append(dep[JavaInfo]) else: gathered.artifact_infos.append(dep[JavaInfo]) @@ -129,6 +130,7 @@ def _has_maven_deps_impl(target, ctx): transitive_exports = [], dep_infos = [], label_to_javainfo = {target.label: target[JavaInfo]}, + artifact_coordinates = coordinates, ) for attr in _ASPECT_ATTRS: diff --git a/private/rules/maven_project_jar.bzl b/private/rules/maven_project_jar.bzl index 5467dd7b3..8fe1d65e2 100644 --- a/private/rules/maven_project_jar.bzl +++ b/private/rules/maven_project_jar.bzl @@ -1,5 +1,5 @@ load(":has_maven_deps.bzl", "MavenInfo", "calculate_artifact_jars", "calculate_artifact_source_jars", "has_maven_deps") -load(":maven_utils.bzl", "determine_additional_dependencies") +load(":maven_utils.bzl", "determine_additional_dependencies", "unpack_coordinates") DEFAULT_EXCLUDED_WORKSPACES = [ # Note: we choose to drop the dependency entirely because @@ -64,7 +64,12 @@ def _maven_project_jar_impl(ctx): ) # Merge together all the binary jars - bin_jar = ctx.actions.declare_file("%s.jar" % ctx.label.name) + packaging = unpack_coordinates(info.coordinates).type or "jar" + bin_jar = ctx.actions.declare_file("%s.%s" % (ctx.label.name, packaging)) + + if packaging == "aar" and AndroidLibraryAarInfo in target and target[AndroidLibraryAarInfo].aar: + artifact_jars = artifact_jars + [target[AndroidLibraryAarInfo].aar] + _combine_jars( ctx, ctx.executable._merge_jars, 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 index 8fa7d9a1c..18559de83 100644 --- 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 @@ -28,12 +28,15 @@ import java.io.IOException; import java.io.InputStream; import java.io.OutputStream; +import java.nio.file.FileSystems; import java.nio.file.Files; import java.nio.file.Path; +import java.nio.file.PathMatcher; import java.nio.file.Paths; import java.security.MessageDigest; import java.security.NoSuchAlgorithmException; import java.util.Arrays; +import java.util.Enumeration; import java.util.HashMap; import java.util.HashSet; import java.util.LinkedHashSet; @@ -51,12 +54,20 @@ public class MergeJars { + private enum Packaging { + JAR, AAR; + } + public static void main(String[] args) throws IOException { Path out = null; // Insertion order may matter Set sources = new LinkedHashSet<>(); Set excludes = new HashSet<>(); DuplicateEntryStrategy onDuplicate = LAST_IN_WINS; + Packaging packaging = Packaging.JAR; + PathMatcher aarMatcher = FileSystems.getDefault().getPathMatcher("glob:*.aar"); + // AAR to build from + Path aarSource = null; for (int i = 0; i < args.length; i++) { switch (args[i]) { @@ -75,6 +86,9 @@ public static void main(String[] args) throws IOException { case "--output": out = Paths.get(args[++i]); + if (aarMatcher.matches(out.getFileName())) { + packaging = Packaging.AAR; + } break; case "--sources": @@ -87,6 +101,15 @@ public static void main(String[] args) throws IOException { } } + if (packaging == Packaging.AAR) { + aarSource = sources.stream() + .filter(source -> aarMatcher.matches(source.getFileName())) + .findFirst() // AAR is explicitly only added for top level distribution target, so we _should_ only ever have 1 + .orElseThrow(() -> new IllegalArgumentException("For AAR packaging, we require a prebuilt AAR that already contains the Android resources that we'll add the transitive source closure to.")); + + sources.remove(aarSource); + } + Objects.requireNonNull(out, "Output path must be set."); if (sources.isEmpty()) { // Just write an empty jar and leave @@ -171,6 +194,56 @@ public static void main(String[] args) throws IOException { // jar and not useful for consumers. manifest.getMainAttributes().remove(new Attributes.Name("Target-Label")); + switch (packaging) { + case JAR: + writeClassesJar(out, manifest, allServices, sources, fileToSourceJar); + break; + case AAR: + Path classesJar = out.getParent().resolve("classes.jar"); + writeClassesJar(classesJar, manifest, allServices, sources, fileToSourceJar); + writeAar(out, aarSource, classesJar); + } + } + + private static void writeAar(Path out, Path aarSource, Path classesJar) throws IOException { + try (OutputStream os = Files.newOutputStream(out); + JarOutputStream jos = new JarOutputStream(os)) { + jos.setMethod(DEFLATED); + jos.setLevel(BEST_COMPRESSION); + + ZipEntry je = new StableZipEntry(classesJar.toFile().getName()); + jos.putNextEntry(je); + + try (InputStream is = Files.newInputStream(classesJar)) { + ByteStreams.copy(is, jos); + } + jos.closeEntry(); + + try (ZipFile aar = new ZipFile(aarSource.toFile())) { + Enumeration entries = aar.entries(); + while (entries.hasMoreElements()) { + ZipEntry entry = entries.nextElement(); + + // transitive class closure is captured in our classes.jar + if ("classes.jar".equals(entry.getName())) { + continue; + } + + jos.putNextEntry(entry); + try (InputStream is = aar.getInputStream(entry)) { + ByteStreams.copy(is, jos); + } + jos.closeEntry(); + } + } + } + } + + private static void writeClassesJar(Path out, + Manifest manifest, + Map> allServices, + Set sources, + Map fileToSourceJar) throws IOException { // Now create the output jar Files.createDirectories(out.getParent()); From 246e0c093e11e2bc3031690edd1393e70cd7fe6d Mon Sep 17 00:00:00 2001 From: Jeremiah Zucker Date: Fri, 5 Jan 2024 13:08:42 -0500 Subject: [PATCH 2/8] remove problematic compression --- .../github/bazelbuild/rules_jvm_external/jar/MergeJars.java | 3 --- 1 file changed, 3 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 index 18559de83..5293b75cb 100644 --- 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 @@ -208,9 +208,6 @@ public static void main(String[] args) throws IOException { private static void writeAar(Path out, Path aarSource, Path classesJar) throws IOException { try (OutputStream os = Files.newOutputStream(out); JarOutputStream jos = new JarOutputStream(os)) { - jos.setMethod(DEFLATED); - jos.setLevel(BEST_COMPRESSION); - ZipEntry je = new StableZipEntry(classesJar.toFile().getName()); jos.putNextEntry(je); From 64431d0caa7ddf1b3061f4e4552741e5b5eff447 Mon Sep 17 00:00:00 2001 From: Jeremiah Zucker Date: Fri, 5 Jan 2024 16:14:15 -0500 Subject: [PATCH 3/8] add packaging to default pom --- private/templates/pom.tpl | 1 + 1 file changed, 1 insertion(+) diff --git a/private/templates/pom.tpl b/private/templates/pom.tpl index 1028295d1..c9acd719c 100644 --- a/private/templates/pom.tpl +++ b/private/templates/pom.tpl @@ -6,6 +6,7 @@ {groupId} {artifactId} {version} + {type} {dependencies} From 0b6de45e5f801cc2c3fb373c2bd5b2822517817e Mon Sep 17 00:00:00 2001 From: Jeremiah Zucker Date: Fri, 5 Jan 2024 16:42:50 -0500 Subject: [PATCH 4/8] upload jar first --- .../bazelbuild/rules_jvm_external/maven/MavenPublisher.java | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/private/tools/java/com/github/bazelbuild/rules_jvm_external/maven/MavenPublisher.java b/private/tools/java/com/github/bazelbuild/rules_jvm_external/maven/MavenPublisher.java index a9af8b5fe..9ef8e184b 100644 --- a/private/tools/java/com/github/bazelbuild/rules_jvm_external/maven/MavenPublisher.java +++ b/private/tools/java/com/github/bazelbuild/rules_jvm_external/maven/MavenPublisher.java @@ -93,7 +93,6 @@ public static void main(String[] args) try { List> futures = new ArrayList<>(); - futures.add(upload(repo, credentials, coords, ".pom", pom, gpgSign)); if (mainArtifact != null) { String ext = @@ -101,6 +100,8 @@ public static void main(String[] args) futures.add(upload(repo, credentials, coords, "." + ext, mainArtifact, gpgSign)); } + futures.add(upload(repo, credentials, coords, ".pom", pom, gpgSign)); + if (args.length > 3 && !args[3].isEmpty()) { List extraArtifactTuples = Splitter.onPattern(",").splitToList(args[3]); for (String artifactTuple : extraArtifactTuples) { From ec9f9044528868fb0472b9cac80199d9974b09f7 Mon Sep 17 00:00:00 2001 From: Jeremiah Zucker Date: Fri, 5 Jan 2024 19:47:34 -0500 Subject: [PATCH 5/8] strip r classes --- .../bazelbuild/rules_jvm_external/jar/MergeJars.java | 7 +++++++ 1 file changed, 7 insertions(+) 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 index 5293b75cb..2deb33b21 100644 --- 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 @@ -48,6 +48,7 @@ import java.util.jar.Attributes; import java.util.jar.JarOutputStream; import java.util.jar.Manifest; +import java.util.regex.Pattern; import java.util.zip.ZipEntry; import java.util.zip.ZipFile; import java.util.zip.ZipInputStream; @@ -167,6 +168,12 @@ public static void main(String[] args) throws IOException { continue; } + // TODO: Why do we need to do this?? Is there a better way? + Pattern rClassMatcher = Pattern.compile("^.*\\/R(\\$.*)?\\.(class|java)"); + if (rClassMatcher.asMatchPredicate().test(entry.getName())) { + continue; + } + if (!entry.isDirectory()) { // Duplicate files, however may not be. We need the hash to determine // whether we should do anything. From cfe7c4c6049d740d7d695336c3edaa7420c02da0 Mon Sep 17 00:00:00 2001 From: Jeremiah Zucker Date: Wed, 24 Jan 2024 23:46:00 -0500 Subject: [PATCH 6/8] use nested classes.jar as source for mergejars --- .bazelrc | 2 ++ .../rules_jvm_external/jar/MergeJars.java | 15 ++++++++++++--- 2 files changed, 14 insertions(+), 3 deletions(-) diff --git a/.bazelrc b/.bazelrc index a9e05a75a..186c2d358 100644 --- a/.bazelrc +++ b/.bazelrc @@ -1,3 +1,5 @@ +build --experimental_google_legacy_api + build --java_language_version=11 build --java_runtime_version=remotejdk_11 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 index 2deb33b21..83d95621e 100644 --- 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 @@ -102,6 +102,7 @@ public static void main(String[] args) throws IOException { } } + Objects.requireNonNull(out, "Output path must be set."); if (packaging == Packaging.AAR) { aarSource = sources.stream() .filter(source -> aarMatcher.matches(source.getFileName())) @@ -109,10 +110,18 @@ public static void main(String[] args) throws IOException { .orElseThrow(() -> new IllegalArgumentException("For AAR packaging, we require a prebuilt AAR that already contains the Android resources that we'll add the transitive source closure to.")); sources.remove(aarSource); - } - Objects.requireNonNull(out, "Output path must be set."); - if (sources.isEmpty()) { + // Pull out classes jar and add to source set + Path aarClassesJar = out.getParent().resolve("aar-classes.jar"); + try (ZipFile aar = new ZipFile(aarSource.toFile())) { + ZipEntry classes = aar.getEntry("classes.jar"); + try (InputStream is = aar.getInputStream(classes); + OutputStream fos = Files.newOutputStream(aarClassesJar)) { + ByteStreams.copy(is, fos); + } + } + sources.add(aarClassesJar); + } else if (sources.isEmpty()) { // Just write an empty jar and leave try (OutputStream fos = Files.newOutputStream(out); JarOutputStream jos = new JarOutputStream(fos)) { From 44f4355b2dbe0d6fd73d690ad66bf5744d482a29 Mon Sep 17 00:00:00 2001 From: Jeremiah Zucker Date: Thu, 25 Jan 2024 15:17:32 -0500 Subject: [PATCH 7/8] revert change to existing empty sources check --- .../github/bazelbuild/rules_jvm_external/jar/MergeJars.java | 6 ++++-- 1 file changed, 4 insertions(+), 2 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 index 83d95621e..e0cd22308 100644 --- 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 @@ -102,7 +102,6 @@ public static void main(String[] args) throws IOException { } } - Objects.requireNonNull(out, "Output path must be set."); if (packaging == Packaging.AAR) { aarSource = sources.stream() .filter(source -> aarMatcher.matches(source.getFileName())) @@ -121,7 +120,10 @@ public static void main(String[] args) throws IOException { } } sources.add(aarClassesJar); - } else if (sources.isEmpty()) { + } + + Objects.requireNonNull(out, "Output path must be set."); + if (sources.isEmpty()) { // Just write an empty jar and leave try (OutputStream fos = Files.newOutputStream(out); JarOutputStream jos = new JarOutputStream(fos)) { From 73b63ba801f14d1bde7807994cc8c15db226ceec Mon Sep 17 00:00:00 2001 From: Jeremiah Zucker Date: Fri, 5 Jul 2024 12:41:40 -0400 Subject: [PATCH 8/8] fix issue with copying binaries by creating new entries --- .../com/github/bazelbuild/rules_jvm_external/jar/MergeJars.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) 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 index e0cd22308..56377ea92 100644 --- 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 @@ -244,7 +244,7 @@ private static void writeAar(Path out, Path aarSource, Path classesJar) throws I continue; } - jos.putNextEntry(entry); + jos.putNextEntry(new ZipEntry(entry.getName())); try (InputStream is = aar.getInputStream(entry)) { ByteStreams.copy(is, jos); }