From 7147ff36f45b4a4eef07515abc5fe6fd17f95a51 Mon Sep 17 00:00:00 2001 From: David Schachter Date: Sat, 8 Aug 2026 14:53:06 -0700 Subject: [PATCH] ADFA-5068: Fix javac-services dependency scope without breaking AGP's consistency check javac-services declared kotlin-stdlib, guava, :common, and :logger as implementation even though all are resident, leaking androidx.core's 2.87MB of dead resources and duplicate guava/kotlin-stdlib bytecode into java-compiler-carrier.apk. Switching those to compileOnly broke AGP's compileClasspath.shouldResolveConsistentlyWith(runtimeClasspath) check one level up in java-compiler-impl: removing :common's implicit implementation edge meant nothing but AGP's own low-pinned viewbinding was left providing androidx.annotation/kotlin-stdlib/org.jetbrains: annotations on the runtime classpath, while compileOnly deps (sora- editor, appcompat, material, lsp:indexing) still demanded much higher versions on the compile side. Relocated ReflectUtils (the one :common symbol javac-services actually uses; VMUtils and ILogger already lived in :shared/:logger) there, and added a constraints{} block in java-compiler-impl pinning the three conflicting artifacts to the versions already used everywhere else in the project -- harmonizing both classpaths without re-adding a real dependency edge. guava was dropped outright: javac-services' own code never references it directly. Verified via a from-scratch java-compiler-carrier release build: 32.4MB -> 10.2MB, resources.arsc 2.27MB -> 40 bytes (empty), dex 9.1+8.1+10.5+2.6MB across 5 files -> 8.4+2.1MB across 2. --- .../0012-lazy-load-javac-via-dexclassloader.md | 1 + gradle/libs.versions.toml | 2 ++ lsp/java-compiler-impl/build.gradle.kts | 17 +++++++++++++++++ .../com/itsaky/androidide/utils/ReflectUtils.kt | 0 .../itsaky/androidide/utils/ReflectUtilsTest.kt | 0 subprojects/javac-services/build.gradle.kts | 13 +++++++++---- 6 files changed, 29 insertions(+), 4 deletions(-) rename {common => shared}/src/main/java/com/itsaky/androidide/utils/ReflectUtils.kt (100%) rename {common => shared}/src/test/java/com/itsaky/androidide/utils/ReflectUtilsTest.kt (100%) diff --git a/docs/adr/0012-lazy-load-javac-via-dexclassloader.md b/docs/adr/0012-lazy-load-javac-via-dexclassloader.md index d34bda2096..52319972a6 100644 --- a/docs/adr/0012-lazy-load-javac-via-dexclassloader.md +++ b/docs/adr/0012-lazy-load-javac-via-dexclassloader.md @@ -52,6 +52,7 @@ Investigation found this coupling narrower than it first looked: none of `CacheF - First `.java`-file interaction in a session now pays a one-time synchronous latency spike (asset extraction on first run + `DexClassLoader` construction + `JavaCompilerService`/`SourceFileManager` bootstrap) on top of ADFA-5052's own deferred-reset cost. - A third resident/isolated classloader boundary to reason about (after Kotlin's and the plugin system's). The same rule as ADR 0011 applies and now has two worked examples of getting it wrong: an `api` dependency anywhere in a vendored composite build's *own* `build.gradle.kts` propagates to every consumer's runtime classpath regardless of how the consumer declares its dependency — `compileOnly` has to be applied at the source of the leak, not just where it's consumed. A second, distinct rule this ADR adds: every resident member the isolated fork calls across the boundary must be `public` — `protected`/package-private access throws `IllegalAccessError` at runtime even when both classes share a package name, since ART checks classloader identity, not just the package string, and this has no build-time or unit-test signal at all. - The debugger's breakpoint/stack-frame source-path resolution (`JavaDebugAdapter`/`ModelUtils.asLspLocation`) took on a narrow, real dependency on `JavaCompilerProvider`/`SourceFileObject` that the isolation boundary can't ignore; it now resolves through `IJavaCompilerSession.findSourceFilePath` (returning a plain path, not the isolated `SourceFileObject` type) instead, and degrades to filename-only when no session exists yet. +- A third rule (ADFA-5068): converting a resident dependency from `implementation` to `compileOnly` can trip AGP's `compileClasspath.shouldResolveConsistentlyWith(runtimeClasspath)` check for any `com.android.library` module, if that dependency was incidentally anchoring a high-enough transitive version (of e.g. `androidx.annotation`, `kotlin-stdlib`, `org.jetbrains:annotations`) against AGP's own unconditionally-injected `androidx.databinding:viewbinding`, which pins those same artifacts much lower on the runtime side once nothing else pulls them in. The fix isn't to re-add the dependency (that reintroduces the duplication this rule exists to avoid) but a `constraints {}` block on `implementation` bumping just the conflicting artifact(s) to the version already used everywhere else — a constraint, unlike a dependency, only rescopes an edge that's already reachable (here, via viewbinding), so it doesn't add anything new to the carrier beyond a version bump on a few KB of annotation classes. See `lsp/java-compiler-impl/build.gradle.kts`'s `constraints` block and `subprojects/javac-services/build.gradle.kts`. ## Alternatives considered diff --git a/gradle/libs.versions.toml b/gradle/libs.versions.toml index 897fb631c7..0c69068adb 100644 --- a/gradle/libs.versions.toml +++ b/gradle/libs.versions.toml @@ -173,6 +173,8 @@ common-javaparser = { module = "com.github.javaparser:javaparser-symbol-solver-c common-lang3 = { module = "org.apache.commons:commons-lang3", version = "3.14.0" } common-io = { module = "commons-io:commons-io", version = "2.15.1" } common-kotlin = { module = "org.jetbrains.kotlin:kotlin-stdlib-jdk8", version.ref = "kotlin" } +common-kotlin-stdlib = { module = "org.jetbrains.kotlin:kotlin-stdlib", version.ref = "kotlin" } +common-jetbrains-annotations = { module = "org.jetbrains:annotations", version = "24.1.0" } common-kotlin-coroutines-core = { module = "org.jetbrains.kotlinx:kotlinx-coroutines-core", version.ref = "kotlin-coroutines" } common-kotlin-coroutines-android = { module = "org.jetbrains.kotlinx:kotlinx-coroutines-android", version.ref = "kotlin-coroutines" } common-jkotlin = { module = "org.jetbrains.kotlin:kotlin-stdlib-jdk8", version.ref = "kotlin" } diff --git a/lsp/java-compiler-impl/build.gradle.kts b/lsp/java-compiler-impl/build.gradle.kts index 77f7c52f06..edc359a22f 100644 --- a/lsp/java-compiler-impl/build.gradle.kts +++ b/lsp/java-compiler-impl/build.gradle.kts @@ -40,6 +40,23 @@ kapt { } dependencies { + // javac-services' (and this module's own) compileOnly androidx-heavy deps (sora-editor, + // appcompat, material, lsp:indexing, etc.) pull androidx.annotation/kotlin-stdlib/ + // org.jetbrains:annotations at high versions on the compile classpath, but being compileOnly + // they're absent from the runtime classpath -- which then only sees AGP's unconditionally + // injected viewbinding's much lower transitive pins for the same artifacts. AGP's + // compile/runtime consistency check fails to reconcile the two. Constraining these three (not + // adding a dependency: each is already reachable via viewbinding, just at the wrong version) + // harmonizes both classpaths at the version already used everywhere else in the project, + // without adding a new edge. All three are a few KB of interfaces/annotations with no + // resources, unlike the androidx.core duplication this whole compileOnly effort exists to + // avoid -- see ADFA-5068. + constraints { + implementation(libs.androidx.annotation) + implementation(libs.common.kotlin.stdlib) + implementation(libs.common.jetbrains.annotations) + } + kapt(projects.annotationProcessors) // Resident (bundled in the main app dex via editor/editor-api/lsp:java/etc.) -- like the diff --git a/common/src/main/java/com/itsaky/androidide/utils/ReflectUtils.kt b/shared/src/main/java/com/itsaky/androidide/utils/ReflectUtils.kt similarity index 100% rename from common/src/main/java/com/itsaky/androidide/utils/ReflectUtils.kt rename to shared/src/main/java/com/itsaky/androidide/utils/ReflectUtils.kt diff --git a/common/src/test/java/com/itsaky/androidide/utils/ReflectUtilsTest.kt b/shared/src/test/java/com/itsaky/androidide/utils/ReflectUtilsTest.kt similarity index 100% rename from common/src/test/java/com/itsaky/androidide/utils/ReflectUtilsTest.kt rename to shared/src/test/java/com/itsaky/androidide/utils/ReflectUtilsTest.kt diff --git a/subprojects/javac-services/build.gradle.kts b/subprojects/javac-services/build.gradle.kts index ab698259d4..df965cbc15 100644 --- a/subprojects/javac-services/build.gradle.kts +++ b/subprojects/javac-services/build.gradle.kts @@ -16,10 +16,15 @@ android { } dependencies { - implementation(libs.common.kotlin) - implementation(libs.google.guava) - implementation(projects.common) - implementation(projects.logger) + // Resident (see docs/adr/0012): kotlin-stdlib, :shared (ReflectUtils, VMUtils) and :logger + // (ILogger) are all already loaded by the parent classloader -- implementation here would + // duplicate their bytecode (and :common's androidx/guava graph, previously reached via + // projects.common) into the isolated carrier dex. compileOnly for the same reason as the + // block below. libs.google.guava was dropped entirely: this module's own code never + // references it directly -- it only ever arrived transitively through :common's api(guava). + compileOnly(libs.common.kotlin) + compileOnly(projects.shared) + compileOnly(projects.logger) // The actual javac fork this module wraps -- bundled with this module wherever it ends up // (isolated carrier, per ADFA-5053).