Fix aar support and move the unpacking out of the wrapper

examples/simple-android/BUILD.bazel

@@ -6,13 +6,19 @@ android_library(
     name = "lib",
     srcs = ["Foo.java"],
     custom_package = "com.rules.android.lint.examples",
+    deps = [
+        "@maven//:androidx_activity_activity",
+    ],
 )
 
 android_lint(
     name = "lib_lint",
     srcs = ["Foo.java"],
     android_lint_config = "lint.xml",
     lib = ":lib",
+    deps = [
+        "@maven//:androidx_activity_activity",
+    ],
 )
 
 android_lint_test(

examples/simple-android/MODULE.bazel

@@ -15,7 +15,27 @@ bazel_dep(name = "platforms", version = "0.0.11")
 bazel_dep(name = "rules_android", version = "0.6.0")
 bazel_dep(name = "rules_jvm_external", version = "6.6")
 
+remote_android_extensions = use_extension(
+    "@rules_android//bzlmod_extensions:android_extensions.bzl",
+    "remote_android_tools_extensions",
+)
+use_repo(remote_android_extensions, "android_gmaven_r8", "android_tools")
+
 android_sdk_repository_extension = use_extension("@rules_android//rules/android_sdk_repository:rule.bzl", "android_sdk_repository_extension")
 use_repo(android_sdk_repository_extension, "androidsdk")
 
 register_toolchains("@androidsdk//:sdk-toolchain", "@androidsdk//:all")
+
+maven = use_extension("@rules_jvm_external//:extensions.bzl", "maven")
+maven.install(
+    aar_import_bzl_label = "@rules_android//rules:rules.bzl",
+    artifacts = [
+        "androidx.activity:activity:1.7.0",
+    ],
+    repositories = [
+        "https://maven.google.com",
+        "https://repo1.maven.org/maven2",
+    ],
+    use_starlark_android_rules = True,
+)
+use_repo(maven, "maven")

rules/collect_aar_outputs_aspect.bzl

@@ -1,31 +1,71 @@
 """Aspect to collect the aar outputs from aar_import
 """
 
+load("@rules_android//providers:providers.bzl", "AndroidLibraryAarInfo")
+
 AndroidLintAARInfo = provider(
     "A provider to collect all aars from transitive dependencies",
     fields = {
-        "aars": "depset of aars",
+        "aar": "direct aar",
+        "transitive_aar_artifacts": "depset of the transitive aars",
+    },
+)
+
+AndroidLintAARNodeInfo = provider(
+    "A provider to collect the individual aar to aar_directory pairs",
+    fields = {
+        "aar": "direct aar",
+        "aar_dir": "path to the aar directory",
     },
 )
 
-def _collect_aar_outputs_aspect(_, ctx):
+def _collect_aar_outputs_aspect(tgt, ctx):
     deps = getattr(ctx.rule.attr, "deps", [])
     exports = getattr(ctx.rule.attr, "exports", [])
     associates = getattr(ctx.rule.attr, "associates", [])
+
+    # Collect the transitive aar artifacts for the given dependencies
     transitive_aar_depsets = []
     for dep in deps + exports + associates:
         if AndroidLintAARInfo in dep:
-            transitive_aar_depsets.append(dep[AndroidLintAARInfo].aars)
+            transitive_aar_depsets.append(dep[AndroidLintAARInfo].transitive_aar_artifacts)
 
-    direct_aars = None
-    if hasattr(ctx.rule.attr, "aar"):
+    # Collect the direct aar artifact for the given target
+    aar = None
+    if ctx.rule.kind == "aar_import":
+        if not hasattr(ctx.rule.attr, "aar"):
+            fail("Found aar import without 'aar' rule attribute!")
         aar = ctx.rule.attr.aar.files.to_list()[0]
-        direct_aars = [aar]
+        # TODO(bencodes) I don't think we need this case since we should be able to extract the information we need
+        # directly when constructing the lint action itself. Keeping for now though as it preserves the existing behavior.
+
+    elif AndroidLibraryAarInfo in tgt:
+        aar = tgt[AndroidLibraryAarInfo].aar
+
+    current_info = AndroidLintAARNodeInfo(
+        aar = None,
+        aar_dir = None,
+    )
+
+    if aar:
+        aar_extract = ctx.actions.declare_directory("aars/" + ctx.label.name + "-aar-contents")
+        ctx.actions.run_shell(
+            inputs = [aar],
+            outputs = [aar_extract],
+            mnemonic = "ExtractLintAar",
+            progress_message = "Extracting AAR %s's " % (ctx.label.name),
+            command = ("unzip -q -o %s -d %s/ " % (aar.path, aar_extract.path)),
+        )
+        current_info = AndroidLintAARNodeInfo(
+            aar = aar,
+            aar_dir = aar_extract,
+        )
 
     return [
         AndroidLintAARInfo(
-            aars = depset(
-                direct = direct_aars,
+            aar = current_info,
+            transitive_aar_artifacts = depset(
+                direct = [current_info],
                 transitive = transitive_aar_depsets,
             ),
         ),

rules/impl.bzl

@@ -3,7 +3,6 @@
 
 load(
     "@rules_android//providers:providers.bzl",
-    "AndroidLibraryAarInfo",
     "AndroidLibraryResourceClassJarProvider",
 )
 load(
@@ -31,6 +30,7 @@ def _run_android_lint(
         output,
         srcs,
         deps,
+        aars,
         resource_files,
         manifest,
         compile_sdk_version,
@@ -55,6 +55,7 @@ def _run_android_lint(
         output: The output file
         srcs: The source files
         deps: Depset of aars and jars to include on the classpath
+        aars: Depset of the aar nodes
         resource_files: The Android resource files
         manifest: The Android manifest file
         compile_sdk_version: The Android compile SDK version
@@ -116,10 +117,17 @@ def _run_android_lint(
     for check in enable_checks:
         args.add("--enable-check", check)
     for dep in _utils.list_or_depset_to_list(deps):
-        if not dep.path.endswith(".aar") and not dep.path.endswith(".jar"):
-            continue
-        args.add("--classpath", dep)
+        if not dep.path.endswith(".jar"):
+            fail("Error: Found artifact that is not an aar: %s" % dep.path)
+        args.add("--classpath-jar", dep)
         inputs.append(dep)
+    for aar_node_info in _utils.list_or_depset_to_list(aars):
+        aar = aar_node_info.aar
+        aar_dir = aar_node_info.aar_dir
+        if aar and aar_dir:
+            args.add("--classpath-aar", "%s:%s" % (aar.path, aar_dir.path))
+            inputs.append(aar)
+            inputs.append(aar_dir)
     if android_lint_enable_check_dependencies:
         args.add("--enable-check-dependencies")
 
@@ -187,15 +195,22 @@ def process_android_lint_issues(ctx, regenerate):
 
     # Collect the transitive classpath jars to run lint against.
     deps = []
+    aars = []
     for dep in ctx.attr.deps:
         if JavaInfo in dep:
             deps.append(dep[JavaInfo].compile_jars)
         if AndroidLibraryResourceClassJarProvider in dep:
             deps.append(dep[AndroidLibraryResourceClassJarProvider].jars)
-        if AndroidLibraryAarInfo in dep:
-            deps.append(dep[AndroidLibraryAarInfo].transitive_aar_artifacts)
         if _AndroidLintAARInfo in dep:
-            deps.append(dep[_AndroidLintAARInfo].aars)
+            direct = []
+            if dep[_AndroidLintAARInfo].aar:
+                direct = [dep[_AndroidLintAARInfo].aar]
+            aars.append(depset(
+                direct = direct,
+                transitive = [
+                    dep[_AndroidLintAARInfo].transitive_aar_artifacts,
+                ],
+            ))
 
     # Append the compiled R files for our self
     if ctx.attr.lib and AndroidLibraryResourceClassJarProvider in ctx.attr.lib:
@@ -217,6 +232,7 @@ def process_android_lint_issues(ctx, regenerate):
         output = output,
         srcs = ctx.files.srcs,
         deps = depset(transitive = deps),
+        aars = depset(transitive = aars),
         resource_files = ctx.files.resource_files,
         manifest = manifest,
         compile_sdk_version = _utils.get_android_lint_toolchain(ctx).compile_sdk_version,

src/cli/AndroidLintActionArgs.kt

@@ -13,6 +13,10 @@ internal class AndroidLintActionArgs(
   private val argsParserPathTransformer: String.() -> Path = {
     Paths.get(this)
   }
+  private val argsParserAarPairPathTransformer: String.() -> Pair<Path, Path> = {
+    val (aar, aarDir) = this.split(":")
+    Pair(Paths.get(aar), Paths.get(aarDir))
+  }
 
   val androidLintCliTool: Path by parser.storing(
     names = arrayOf("--android-lint-cli-tool"),
@@ -80,11 +84,18 @@ internal class AndroidLintActionArgs(
 
   val classpath: List<Path> by parser
     .adding(
-      names = arrayOf("--classpath"),
+      names = arrayOf("--classpath-jar"),
       help = "",
       transform = argsParserPathTransformer,
     ).default { emptyList() }
 
+  val classpathAarPairs: List<Pair<Path, Path>> by parser
+    .adding(
+      names = arrayOf("--classpath-aar"),
+      help = "",
+      transform = argsParserAarPairPathTransformer,
+    ).default { emptyList() }
+
   val autofix: Boolean by parser
     .flagging(
       names = arrayOf("--autofix"),

src/cli/AndroidLintRunner.kt

@@ -3,14 +3,9 @@ package com.rules.android.lint.cli
 import java.nio.file.Files
 import java.nio.file.Path
 import java.nio.file.Paths
-import java.util.concurrent.ExecutorService
-import java.util.concurrent.Executors
-import java.util.concurrent.TimeUnit
 import kotlin.io.path.absolutePathString
 import kotlin.io.path.exists
-import kotlin.io.path.extension
 import kotlin.io.path.isRegularFile
-import kotlin.io.path.name
 import kotlin.io.path.pathString
 import kotlin.io.path.writeText
 
@@ -26,18 +21,9 @@ internal class AndroidLintRunner {
       Files.copy(args.baselineFile!!, baselineFile)
     }
 
-    // Split the aars and jars
-    val aars = args.classpath.filter { it.extension == "aar" }
-    val jars = args.classpath.filter { it.extension == "jar" }
-    require(aars.size + jars.size == args.classpath.size) { "Error: Classpath size mismatch" }
-
-    // Unarchive the AARs to avoid lint having to do this work. This also prevents some
-    // concurrency issues inside of Lint when multiplex workers are enabled
-    val unpackedAars = unpackAars(aars, workingDirectory.resolve("aars"))
-
     // Collect the custom lint rules from the unpacked aars
     val aarLintRuleJars =
-      unpackedAars
+      args.classpathAarPairs
         .asSequence()
         .map { it.first.resolve("lint.jar") }
         .filter { it.exists() && it.isRegularFile() }
@@ -54,9 +40,9 @@ internal class AndroidLintRunner {
         srcs = args.srcs.sortedDescending(),
         resources = args.resources.sortedDescending(),
         androidManifest = args.androidManifest,
-        classpathJars = jars.sortedDescending(),
+        classpathJars = args.classpath.sortedDescending(),
         classpathAars = emptyList(),
-        classpathExtractedAarDirectories = unpackedAars,
+        classpathExtractedAarDirectories = args.classpathAarPairs,
         customLintChecks = (args.customChecks + aarLintRuleJars).sortedDescending(),
       ),
     )
@@ -146,23 +132,4 @@ internal class AndroidLintRunner {
     invoker.setCheckDependencies(actionArgs.enableCheckDependencies)
     return invoker.invoke(args.toTypedArray())
   }
-
-  /**
-   * Takes a list of AARs and unarchives them into the provided directory
-   * with this structure: ${tmpDirectory}/${aarFileName}--aar-unzipped/
-   *
-   * This is a necessary workaround for Lint wanting to unpack these aars into a global
-   * shared directory, which causes lots of obscure concurrency issues inside of lint
-   * when operating in persistent worker mode.
-   */
-  private fun unpackAars(
-    aars: List<Path>,
-    dstDirectory: Path,
-    executorService: ExecutorService = Executors.newFixedThreadPool(6),
-  ): List<Pair<Path, Path>> {
-    val aarsToUnpack = aars.map { it to dstDirectory.resolve("${it.name}-aar-contents") }
-    aarsToUnpack.forEach { (src, dst) -> unzip(src, dst) }
-    executorService.awaitTermination(15, TimeUnit.SECONDS)
-    return aarsToUnpack.sortedBy { it.first }
-  }
 }

tests/src/cli/AndroidLintActionArgsTest.kt

@@ -32,10 +32,10 @@ class AndroidLintActionArgsTest {
             "lint_config.xml",
             "--custom-rule",
             "custom_rule.jar",
-            "--classpath",
+            "--classpath-jar",
             "classpath.jar",
-            "--classpath",
-            "classpath.aar",
+            "--classpath-aar",
+            "classpath.aar:aar_directory",
             "--autofix",
             "--regenerate-baseline-files",
             "--warnings-as-errors",
@@ -61,7 +61,9 @@ class AndroidLintActionArgsTest {
     assertThat(parseArgs.config).isEqualTo(Paths.get("lint_config.xml"))
     assertThat(parseArgs.customChecks).containsExactly(Paths.get("custom_rule.jar"))
     assertThat(parseArgs.classpath)
-      .containsExactly(Paths.get("classpath.jar"), Paths.get("classpath.aar"))
+      .containsExactly(Paths.get("classpath.jar"))
+    assertThat(parseArgs.classpathAarPairs)
+      .containsExactly(Paths.get("classpath.aar") to Paths.get("aar_directory"))
     assertThat(parseArgs.autofix).isTrue
     assertThat(parseArgs.regenerateBaselineFile).isTrue
     assertThat(parseArgs.warningsAsErrors).isTrue