diff --git a/examples/simple-android/BUILD.bazel b/examples/simple-android/BUILD.bazel index 40dd1c4..b7794f7 100644 --- a/examples/simple-android/BUILD.bazel +++ b/examples/simple-android/BUILD.bazel @@ -6,6 +6,9 @@ android_library( name = "lib", srcs = ["Foo.java"], custom_package = "com.rules.android.lint.examples", + deps = [ + "@maven//:androidx_activity_activity", + ], ) android_lint( @@ -13,6 +16,9 @@ android_lint( srcs = ["Foo.java"], android_lint_config = "lint.xml", lib = ":lib", + deps = [ + "@maven//:androidx_activity_activity", + ], ) android_lint_test( diff --git a/examples/simple-android/MODULE.bazel b/examples/simple-android/MODULE.bazel index e5be0d2..5fba450 100644 --- a/examples/simple-android/MODULE.bazel +++ b/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") diff --git a/rules/collect_aar_outputs_aspect.bzl b/rules/collect_aar_outputs_aspect.bzl index 823e834..1663945 100644 --- a/rules/collect_aar_outputs_aspect.bzl +++ b/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, ), ), diff --git a/rules/impl.bzl b/rules/impl.bzl index 80dbdbf..b2d9289 100644 --- a/rules/impl.bzl +++ b/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, diff --git a/src/cli/AndroidLintActionArgs.kt b/src/cli/AndroidLintActionArgs.kt index 84d6669..f71e9e2 100644 --- a/src/cli/AndroidLintActionArgs.kt +++ b/src/cli/AndroidLintActionArgs.kt @@ -13,6 +13,10 @@ internal class AndroidLintActionArgs( private val argsParserPathTransformer: String.() -> Path = { Paths.get(this) } + private val argsParserAarPairPathTransformer: String.() -> Pair = { + 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 by parser .adding( - names = arrayOf("--classpath"), + names = arrayOf("--classpath-jar"), help = "", transform = argsParserPathTransformer, ).default { emptyList() } + val classpathAarPairs: List> by parser + .adding( + names = arrayOf("--classpath-aar"), + help = "", + transform = argsParserAarPairPathTransformer, + ).default { emptyList() } + val autofix: Boolean by parser .flagging( names = arrayOf("--autofix"), diff --git a/src/cli/AndroidLintRunner.kt b/src/cli/AndroidLintRunner.kt index dcdac10..e0d992f 100644 --- a/src/cli/AndroidLintRunner.kt +++ b/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, - dstDirectory: Path, - executorService: ExecutorService = Executors.newFixedThreadPool(6), - ): List> { - 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 } - } } diff --git a/src/cli/AndroidLintWrapperUtils.kt b/src/cli/AndroidLintWrapperUtils.kt deleted file mode 100644 index 9aa1faf..0000000 --- a/src/cli/AndroidLintWrapperUtils.kt +++ /dev/null @@ -1,30 +0,0 @@ -package com.rules.android.lint.cli - -import java.io.File -import java.nio.file.Path -import java.util.zip.ZipInputStream -import kotlin.io.path.inputStream - -internal fun unzip( - src: Path, - dst: Path, -) { - val dstFile = dst.toFile() - val bufferedZipInputStream = src.inputStream().buffered() - ZipInputStream(bufferedZipInputStream).use { zipStream -> - var zipEntry = zipStream.nextEntry - while (zipEntry != null) { - if (zipEntry.isDirectory) { - File(dstFile, zipEntry.name).mkdirs() - } else { - File(dstFile, zipEntry.name) - .also { it.parentFile.mkdirs() } - .outputStream() - .use { fileOutputStream -> zipStream.copyTo(fileOutputStream) } - } - - zipStream.closeEntry() - zipEntry = zipStream.nextEntry - } - } -} diff --git a/tests/src/cli/AndroidLintActionArgsTest.kt b/tests/src/cli/AndroidLintActionArgsTest.kt index a82b789..234d799 100644 --- a/tests/src/cli/AndroidLintActionArgsTest.kt +++ b/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