Android library publishing support #344
Conversation
| @@ -88,12 +97,14 @@ def _generate_class_jar(ctx, pom_file): | |||
| maven_coordinates = _parse_maven_coordinates(target[JarInfo].name) | |||
|
|
|||
| jar = None | |||
| if hasattr(target, "files") and target.files.to_list() and target.files.to_list()[0].extension == "jar": | |||
| if (_is_android_library(target)): | |||
| jar = target[AndroidLibraryAarInfo].aar | |||
There was a problem hiding this comment.
The information we need here is only the path to the JAR/AAR file.
Is there any way we could retrieve this in a way that is generic to both Java JARs and Android AARs, without requiring a check for _is_android_library? Since ideally we would like to keep this rule as generalised as possible.
| @@ -301,6 +327,7 @@ assemble_maven = rule( | |||
|
|
|||
| MavenDeploymentInfo = provider( | |||
| fields = { | |||
| 'packaging': 'The type of target to publish (jar, war, aar, etc.)', | |||
There was a problem hiding this comment.
Given that we are deploying either a JAR or an AAR file, I think it would be more intuitive if we simply read the extension of the file being passed into the deploy_maven rule (instead of relying on a new field in MavenDeploymentInfo) to determine the packaging used in deploy.py.
| null | ||
| } | ||
| keys.contains(entry.name) -> { | ||
| // TODO: Investigate why I'm getting duplicates |
There was a problem hiding this comment.
How serious is this? Can we remove it?
| @@ -28,17 +28,20 @@ import java.io.BufferedInputStream | |||
| import java.io.BufferedOutputStream | |||
There was a problem hiding this comment.
It's cool that this AAR support is added with minimal code change! However, I think the domains of Java and Android are conceptually different (even though Android uses Java code extensively.) As such, I would propose:
- Rename this file (and class) to
Assembler. Given that we are in themavenpackage, that's unambiguous.Assemblershould contain all the logic common to both JAR and AAR. - Introduce two inner classes of
Assembler:JarandAar, which encapsulate the JAR and AAR-specific code. (They will, presumably, need a reference to theAssemblerclass in their constructors.)
|
Apologies for the delay. I'll be able to revisit this PR in a little while. Just want to reassure that it is my goal to get this merged to help provide broader publishing support to the community 😄 |
What is the goal of this PR?
Add publishing support for
android_librarytargets (#233)What are the changes implemented in this PR?
First, the rules, assemblers, and scripts need to take into account other packaging types. Determining the packaging type and passing that info along via the providers was the necessary first step.
Second, the
JarAssembler.ktfile only supported packaging aJAR. Rather than duplicate the script forAARsupport, I added support directly in that file to reuse a lot of the same logic used to assembleJARs. There is one TODO left surrounding duplicate entries, but I believe that might be something related to my specific project.Third, handling the Android SDK and neverlink dependencies. In order to be able to perform the
aggregate_dependency_infoaspect, the Android SDK target needed to be handled. It is a specific rule type baked into the Bazel codebase. I wasn't really sure how to determine the rule type within the aspect, so I just hardcoded the label as something to ignore (gross, I know). Additionally, a third party ruleset I use for databinding support had an additionaljava_librarythat captured the Android SDK. Luckily, this was marked asneverlink, so I was able to key off of that attribute to avoid capturing the Android SDK from that target in the transitive closure. That should be a decent strategy to follow since the artifacts we're assembling probably shouldn't includeneverlinkdependencies that were just used to compile.