Skip to content

Commit a7dd77e

Browse files
fryanpanclaude
andcommitted
ADFA-4128: qb 10 review fixes — explicit-intent rewrite, loud AAR injection, min-AGP guard
Review finding 1 (renamed services/receivers silently break explicit intents) → services and receivers now keep their real manifest names, per the design doc's own no-proxy path: the appComponentFactory instantiates the manifest name through the payload loader (like the Application), Android has no service/receiver alias to compensate a rename with, and neither kind uses the activity-only getClassLoader injection. They stay recorded in setup.json (null proxyClass) so the restart rule still sees them; resolver-skipped library components stay out, as before. Covered by QuickBuildManifestTransformerTest ("services keep their real manifest names so explicit start-service intents still resolve", "receivers keep their real name...", "project-owned services stay recorded...", plus the rewritten skip/numbering tests). Docs updated in step (component-proxying-design.md, quickbuild/README.md, ComponentInfo.kt, live-reload-alternatives.md). Review finding 3 (fail-quiet runtime-AAR injection) → the injection path now goes through QuickBuildPlugin.requireRuntimeConfiguration, which throws a GradleException naming the variant and the unrecognized AGP variant type instead of silently producing a proxy APK that crashes at launch; the .flat-overlay caller keeps its documented graceful degrade. Covered by QuickBuildPluginTest ("requireRuntimeConfiguration fails the build loudly on an unrecognized variant type", "runtimeConfigurationOrNull degrades to null for the resources overlay path"). Review finding 2 (deleted min-AGP guard) → restored as a minAgpCheck source set wired into `check`: it recompiles every non-Quick-Build plugin source against AGP_VERSION_MINIMUM, so an AGP-8-only API in LogSenderPlugin/AndroidIDEGradlePlugin goes red again. Quick Build sources are excluded (they genuinely need the newer AGP and load only when enabled); to keep that exclusion compilable, AndroidIDEGradlePlugin applies QuickBuildPlugin by name, pinned to the real class by QuickBuildPluginTest ("the reflective quick build plugin name resolves to the real class"). The guard task itself is the red light for build-file regressions. This restore is the conservative option; dropping the guard again can be re-proposed separately with rationale if the team prefers compile-against-latest only. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Kj9YeCDHGp9DU8LPtfWJ7W
1 parent 918e7ac commit a7dd77e

10 files changed

Lines changed: 330 additions & 93 deletions

File tree

‎gradle-plugin/build.gradle.kts‎

Lines changed: 34 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -17,8 +17,10 @@
1717

1818
@file:Suppress("UnstableApiUsage")
1919

20+
import com.itsaky.androidide.build.config.AGP_VERSION_MINIMUM
2021
import com.itsaky.androidide.build.config.BuildConfig
2122
import com.itsaky.androidide.build.config.ProjectConfig
23+
import org.gradle.api.file.SourceDirectorySet
2224

2325
plugins {
2426
id("org.jetbrains.kotlin.jvm")
@@ -81,7 +83,8 @@ dependencies {
8183
// shipped inside AGP's builder artifact, so this module compiles against the repo's
8284
// AGP instead of AGP_VERSION_MINIMUM. Projects on older AGPs are unaffected at
8385
// runtime: QuickBuildPlugin's classes load only when quick build is enabled, and the
84-
// other plugins stick to APIs that exist since the minimum supported version.
86+
// other plugins stick to APIs that exist since the minimum supported version - a
87+
// claim the minAgpCheck guard below keeps honest by recompiling them against it.
8588
add("androidBuildTool", libs.android.gradle.plugin)
8689

8790
testImplementation(gradleTestKit())
@@ -92,6 +95,36 @@ dependencies {
9295
testRuntimeOnly(libs.tests.junit.platformLauncher)
9396
}
9497

98+
// Min-AGP compatibility guard (restored after review). The main compile moved to the repo's
99+
// AGP for Quick Build (above), which deleted the old red light: an innocent AGP-8-only API
100+
// in LogSenderPlugin or AndroidIDEGradlePlugin would compile green and then fail every user
101+
// project on an older AGP at configuration time, with Quick Build off. This source set
102+
// recompiles the non-Quick-Build sources against AGP_VERSION_MINIMUM so that mistake goes
103+
// red in `check`. The Quick Build sources are excluded on purpose: they genuinely need the
104+
// newer AGP and only load when quick build is enabled (AndroidIDEGradlePlugin applies
105+
// QuickBuildPlugin by name, not by class literal, to keep this compile honest).
106+
val minAgpCheck: SourceSet =
107+
sourceSets.create("minAgpCheck") {
108+
java.setSrcDirs(emptyList<String>())
109+
resources.setSrcDirs(emptyList<String>())
110+
}
111+
(minAgpCheck.extensions.getByName("kotlin") as SourceDirectorySet).apply {
112+
setSrcDirs(listOf("src/main/java"))
113+
exclude("**/QuickBuildPlugin.kt", "**/quickbuild/**")
114+
}
115+
116+
dependencies {
117+
"minAgpCheckCompileOnly"(gradleApi())
118+
"minAgpCheckCompileOnly"("com.android.tools.build:gradle:$AGP_VERSION_MINIMUM")
119+
"minAgpCheckImplementation"(libs.composite.constants)
120+
"minAgpCheckImplementation"(projects.gradlePluginConfig)
121+
"minAgpCheckImplementation"(projects.buildInfo)
122+
}
123+
124+
tasks.named("check") {
125+
dependsOn(tasks.named("minAgpCheckClasses"))
126+
}
127+
95128
gradlePlugin {
96129
website.set(ProjectConfig.REPO_URL)
97130
vcsUrl.set(ProjectConfig.REPO_URL)

‎gradle-plugin/src/main/java/com/itsaky/androidide/gradle/AndroidIDEGradlePlugin.kt‎

Lines changed: 12 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -32,6 +32,15 @@ import org.gradle.api.logging.Logging
3232
class AndroidIDEGradlePlugin : Plugin<Project> {
3333
companion object {
3434
private val logger = Logging.getLogger(AndroidIDEGradlePlugin::class.java)
35+
36+
/**
37+
* QuickBuildPlugin's FQN, applied reflectively below so this file carries no
38+
* compile-time reference to it: the minAgpCheck guard (see build.gradle.kts)
39+
* recompiles every non-Quick-Build source against AGP_VERSION_MINIMUM, and only
40+
* the Quick Build sources are allowed newer AGP APIs. Pinned to the real class by
41+
* `QuickBuildPluginTest`.
42+
*/
43+
internal const val QUICK_BUILD_PLUGIN_CLASS = "com.itsaky.androidide.gradle.QuickBuildPlugin"
3544
}
3645

3746
override fun apply(target: Project) {
@@ -57,7 +66,9 @@ class AndroidIDEGradlePlugin : Plugin<Project> {
5766

5867
val isQuickBuildEnabled = findProperty(PROPERTY_QUICK_BUILD_ENABLED) == "true"
5968
if (isQuickBuildEnabled) {
60-
pluginManager.apply(QuickBuildPlugin::class.java)
69+
// By name, not ::class: Quick Build classes load (and touch newer AGP APIs)
70+
// only when the property enables them - see QUICK_BUILD_PLUGIN_CLASS.
71+
pluginManager.apply(Class.forName(QUICK_BUILD_PLUGIN_CLASS))
6172
}
6273
}
6374
}

‎gradle-plugin/src/main/java/com/itsaky/androidide/gradle/QuickBuildPlugin.kt‎

Lines changed: 46 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -71,6 +71,42 @@ class QuickBuildPlugin : Plugin<Project> {
7171
* jar dependency. AGP-internal like the constant above, so the raw string is used directly.
7272
*/
7373
internal const val CLASSES_JAR_ARTIFACT_TYPE = "android-classes-jar"
74+
75+
/**
76+
* The variant's runtime-classpath [Configuration], reached through AGP-internal variant
77+
* types, or null when the variant is neither known impl - i.e. an AGP this plugin has not
78+
* been taught about.
79+
*
80+
* @param variant the variant AGP handed to `onVariants`, possibly analytics-wrapped.
81+
* @return the runtime classpath configuration, or null for an unrecognized variant type.
82+
*/
83+
internal fun runtimeConfigurationOrNull(variant: ApplicationVariant): Configuration? =
84+
when (variant) {
85+
is ApplicationVariantImpl -> variant.variantDependencies.runtimeClasspath
86+
is AnalyticsEnabledApplicationVariant -> runtimeConfigurationOrNull(variant.delegate)
87+
else -> null
88+
}
89+
90+
/**
91+
* [runtimeConfigurationOrNull] for the runtime-AAR injection, which must never fail quiet:
92+
* without the injected AAR the proxy APK still names [APP_COMPONENT_FACTORY] in its
93+
* manifest, so the app dies at launch with ClassNotFoundException, on device, far from the
94+
* cause. An AGP whose variant impl this lookup does not recognize therefore fails the
95+
* build here, where the message can say what actually broke.
96+
*
97+
* @param variant the variant AGP handed to `onVariants`, possibly analytics-wrapped.
98+
* @return the runtime classpath configuration, never null.
99+
* @throws GradleException when the variant type is unrecognized.
100+
*/
101+
internal fun requireRuntimeConfiguration(variant: ApplicationVariant): Configuration =
102+
runtimeConfigurationOrNull(variant)
103+
?: throw GradleException(
104+
"Quick Build cannot inject its runtime into variant '${variant.name}': " +
105+
"unrecognized AGP variant type '${variant.javaClass.name}'. Without the " +
106+
"runtime AAR the proxy app would crash at launch, so the build stops " +
107+
"here; this AGP version needs QuickBuildPlugin.runtimeConfigurationOrNull " +
108+
"taught about its variant type. Use a Standard Run meanwhile.",
109+
)
74110
}
75111

76112
override fun apply(target: Project) {
@@ -143,9 +179,9 @@ class QuickBuildPlugin : Plugin<Project> {
143179
project.path,
144180
)
145181

146-
variant.withRuntimeConfiguration {
147-
dependencies.add(project.dependencies.create(project.fileTree(runtimeAar)))
148-
}
182+
requireRuntimeConfiguration(variant)
183+
.dependencies
184+
.add(project.dependencies.create(project.fileTree(runtimeAar)))
149185

150186
val buildDirectory = project.layout.buildDirectory
151187
val variantDir = "quickbuild/${variant.name}"
@@ -200,9 +236,9 @@ class QuickBuildPlugin : Plugin<Project> {
200236
task.proxySources.set(generate.flatMap { it.proxySources })
201237
task.manifestInfoFile.set(generate.flatMap { it.manifestInfoFile })
202238
task.compileClasspath.from(variant.compileClasspath)
203-
// Components are proxied uniformly, including ones whose class arrives on
204-
// the RUNTIME-only classpath (CoGo's injected LogSender service): javac
205-
// needs the superclass, so the injected AAR joins the proxy classpath.
239+
// A proxied component's class can arrive on the RUNTIME-only classpath
240+
// (CoGo's injected LogSender AAR carries the LogSenderInstaller provider):
241+
// javac needs the superclass, so the injected AAR joins the proxy classpath.
206242
task.runtimeAar.addRuntimeAars(project, runtimeAar)
207243
task.bootClasspath.from(bootClasspath)
208244
task.minApiLevel.set(payloadMinApi)
@@ -324,14 +360,6 @@ class QuickBuildPlugin : Plugin<Project> {
324360
}
325361
}
326362

327-
private fun ApplicationVariant.withRuntimeConfiguration(action: Configuration.() -> Unit) {
328-
if (this is ApplicationVariantImpl) {
329-
variantDependencies.runtimeClasspath.action()
330-
} else if (this is AnalyticsEnabledApplicationVariant) {
331-
delegate.withRuntimeConfiguration(action)
332-
}
333-
}
334-
335363
/**
336364
* Every dependency's classes as jars: a lenient `ArtifactView` over the variant's COMPILE
337365
* configuration filtered to [CLASSES_JAR_ARTIFACT_TYPE].
@@ -365,9 +393,10 @@ class QuickBuildPlugin : Plugin<Project> {
365393
variant: ApplicationVariant,
366394
project: Project,
367395
): FileCollection {
368-
var configuration: Configuration? = null
369-
variant.withRuntimeConfiguration { configuration = this }
370-
val resolvedConfiguration = configuration ?: return project.files()
396+
// Graceful here, unlike the runtime-AAR injection: missing .flat overlays only
397+
// degrade resource relinks, and the injection above already failed the build for
398+
// any variant type this cannot resolve.
399+
val resolvedConfiguration = runtimeConfigurationOrNull(variant) ?: return project.files()
371400
return resolvedConfiguration.incoming
372401
.artifactView { view ->
373402
view.attributes {

‎gradle-plugin/src/main/java/com/itsaky/androidide/gradle/quickbuild/QuickBuildManifestTransformer.kt‎

Lines changed: 60 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -30,15 +30,16 @@ enum class ComponentType(
3030
/**
3131
* One component of the user's merged manifest, paired with the proxy generated for it.
3232
*
33-
* The custom Application appears here with a null [proxyClass]: nothing addresses it by manifest
34-
* name, so it keeps the user FQN and the runtime's instantiateApplication routes it through the
35-
* payload loader.
33+
* The custom Application, services and receivers appear here with a null [proxyClass]: they keep
34+
* the user FQN in the manifest and the runtime's instantiate hooks route them through the payload
35+
* loader by that real name. For the Application nothing addresses it by manifest name anyway; for
36+
* services and receivers the real name IS the addressing - see
37+
* [QuickBuildManifestTransformer.transformComponents].
3638
*
37-
* @property type which manifest element this came from; the Application is the only type that
38-
* gets no proxy.
39+
* @property type which manifest element this came from.
3940
* @property userClass fully-qualified user class.
4041
* @property proxyClass fully-qualified generated proxy class that replaces it in the
41-
* manifest, or null for the Application entry.
42+
* manifest, or null for the Application, service and receiver entries.
4243
* @property isLauncher whether an activity declares the MAIN/LAUNCHER intent filter.
4344
*/
4445
data class ProxiedComponent(
@@ -64,8 +65,8 @@ data class UnproxiedComponent(
6465
* The rewritten manifest plus what the rewrite did to each component.
6566
*
6667
* @property document the transformed manifest, mutated in place from the parsed input.
67-
* @property components every proxied component, in manifest order per type, plus the proxy-less
68-
* Application entry when the manifest declares one.
68+
* @property components every recorded component, in manifest order per type - proxied or (for
69+
* services, receivers and the Application) kept under its real name with a null proxyClass.
6970
* @property unproxied components left under their real name, for the caller to log.
7071
*/
7172
class ManifestTransformResult(
@@ -83,8 +84,9 @@ class ManifestTransformResult(
8384
}
8485

8586
/**
86-
* Rewrites a merged Android manifest into the proxy-app manifest: each component's android:name
87-
* becomes a generated proxy FQN and the `<application>` gains the quick-build runtime's
87+
* Rewrites a merged Android manifest into the proxy-app manifest: each activity's and provider's
88+
* android:name becomes a generated proxy FQN, services and receivers keep their real name (see
89+
* [transformComponents] for why), and the `<application>` gains the quick-build runtime's
8890
* android:appComponentFactory, everything else verbatim. Components [proxiability] rejects keep
8991
* their real name and land in [ManifestTransformResult.unproxied]; an attribute the proxy app
9092
* cannot host yet (android:process, isolated services, multiprocess providers) fails the build.
@@ -148,8 +150,23 @@ class QuickBuildManifestTransformer(
148150
val components = mutableListOf<ProxiedComponent>()
149151
val unproxied = mutableListOf<UnproxiedComponent>()
150152
components += transformActivities(application, manifestPackage, unproxied)
151-
components += transformComponents(application, ComponentType.SERVICE, manifestPackage, unproxied, "isolatedProcess")
152-
components += transformComponents(application, ComponentType.RECEIVER, manifestPackage, unproxied)
153+
components +=
154+
transformComponents(
155+
application,
156+
ComponentType.SERVICE,
157+
manifestPackage,
158+
unproxied,
159+
unsupportedAttribute = "isolatedProcess",
160+
proxied = false,
161+
)
162+
components +=
163+
transformComponents(
164+
application,
165+
ComponentType.RECEIVER,
166+
manifestPackage,
167+
unproxied,
168+
proxied = false,
169+
)
153170
components += transformComponents(application, ComponentType.PROVIDER, manifestPackage, unproxied, "multiprocess")
154171
applicationComponent(application, manifestPackage)?.let { components += it }
155172

@@ -262,18 +279,34 @@ class QuickBuildManifestTransformer(
262279
}
263280

264281
/**
265-
* Renames every proxiable component of one non-activity kind to its proxy.
282+
* Renames every proxiable component of one non-activity kind to its proxy - or, when
283+
* [proxied] is false, records it under its real name without renaming.
266284
*
267285
* Activities keep [transformActivities] to themselves: only they carry alias handling.
268286
*
287+
* Services and receivers pass [proxied] = false, because renaming them silently breaks
288+
* explicit-component addressing and Android has no `<service>`/`<receiver>` alias to
289+
* compensate with (activities get exactly that alias in [transformActivities]):
290+
* `startService(Intent(ctx, SyncService::class.java))` resolves the real class name against
291+
* the manifest, finds nothing, and no-ops with only a logcat warning; an AlarmManager
292+
* `PendingIntent.getBroadcast` aimed at a renamed receiver is simply never delivered.
293+
* Keeping the real name costs nothing: the runtime's appComponentFactory instantiates the
294+
* manifest name through the payload loader, exactly as the custom Application already does,
295+
* and neither kind uses the one thing only a proxy can inject (the activity proxies'
296+
* getClassLoader override).
297+
*
269298
* @param application the `<application>` element, mutated in place.
270299
* @param type the kind to rewrite; its [ComponentType.jsonName] is also the manifest tag.
271300
* @param manifestPackage the manifest's package, for expanding android:name shorthand.
272-
* @param unproxied accumulator for components [skipProxy] rejects.
301+
* @param unproxied accumulator for components [skipProxy] rejects; a rejected component is
302+
* also left out of the returned list, keeping library-owned components (whose classes
303+
* never travel in the payload) invisible to the deploy policy's restart rule.
273304
* @param unsupportedAttribute an android attribute the proxy app cannot host when it is
274305
* `"true"` (a service's isolatedProcess, a provider's multiprocess), or null for a kind
275306
* with none.
276-
* @return the proxied components, in manifest order; skipped ones are absent.
307+
* @param proxied whether this kind is renamed to a generated proxy; false keeps the real
308+
* name (written fully qualified) and records a null proxyClass.
309+
* @return the recorded components, in manifest order; skipped ones are absent.
277310
* @throws IllegalArgumentException if a component declares [unsupportedAttribute].
278311
*/
279312
private fun transformComponents(
@@ -282,6 +315,7 @@ class QuickBuildManifestTransformer(
282315
manifestPackage: String,
283316
unproxied: MutableList<UnproxiedComponent>,
284317
unsupportedAttribute: String? = null,
318+
proxied: Boolean = true,
285319
): List<ProxiedComponent> {
286320
val tag = type.jsonName
287321
var proxyIndex = 0
@@ -299,6 +333,17 @@ class QuickBuildManifestTransformer(
299333
if (skipProxy(userClass, unproxied)) {
300334
return@mapIndexedNotNull null
301335
}
336+
if (!proxied) {
337+
// Keep the user class but write it fully qualified, for the same reason as
338+
// applicationComponent: the runtime resolves this name against the payload
339+
// dex, so shorthand left verbatim is fragile.
340+
element.setAttributeNS(ANDROID_NS, "android:name", userClass)
341+
return@mapIndexedNotNull ProxiedComponent(
342+
type = type,
343+
userClass = userClass,
344+
proxyClass = null,
345+
)
346+
}
302347
val proxyClass = "$proxyPackage.${proxySimpleName(proxyIndex, type)}"
303348
proxyIndex++
304349
element.setAttributeNS(ANDROID_NS, "android:name", proxyClass)
Lines changed: 60 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,60 @@
1+
package com.itsaky.androidide.gradle
2+
3+
import com.android.build.api.variant.ApplicationVariant
4+
import com.google.common.truth.Truth.assertThat
5+
import org.gradle.api.GradleException
6+
import org.junit.jupiter.api.Test
7+
import org.junit.jupiter.api.assertThrows
8+
import java.lang.reflect.Proxy
9+
10+
/**
11+
* JVM-pure coverage for [QuickBuildPlugin]'s AGP-internal seams; the full plugin runs only
12+
* inside a real Gradle build ([QuickBuildProxyAppBuildTest]).
13+
*/
14+
class QuickBuildPluginTest {
15+
/**
16+
* An [ApplicationVariant] that is neither of the AGP variant impls the plugin knows -
17+
* the shape a future AGP hands over when its internals change. A JDK dynamic proxy
18+
* rather than a stub class, so no AGP-internal type is subclassed here either.
19+
*/
20+
private fun unknownVariantType(): ApplicationVariant =
21+
Proxy.newProxyInstance(
22+
ApplicationVariant::class.java.classLoader,
23+
arrayOf(ApplicationVariant::class.java),
24+
) { _, method, _ ->
25+
when (method.name) {
26+
"getName" -> "demoDebug"
27+
else -> throw UnsupportedOperationException(method.name)
28+
}
29+
} as ApplicationVariant
30+
31+
@Test
32+
fun `requireRuntimeConfiguration fails the build loudly on an unrecognized variant type`() {
33+
// The injection path must never fail quiet: a proxy APK built without the runtime
34+
// AAR still names QuickBuildAppComponentFactory in its manifest, so it dies at
35+
// launch with ClassNotFoundException on device, far from the cause.
36+
val error =
37+
assertThrows<GradleException> {
38+
QuickBuildPlugin.requireRuntimeConfiguration(unknownVariantType())
39+
}
40+
41+
assertThat(error).hasMessageThat().contains("demoDebug")
42+
assertThat(error).hasMessageThat().contains("runtime")
43+
assertThat(error).hasMessageThat().contains("Standard Run")
44+
}
45+
46+
@Test
47+
fun `runtimeConfigurationOrNull degrades to null for the resources overlay path`() {
48+
// The .flat-overlay caller may stay graceful - missing overlays only degrade
49+
// resource relinks - which is exactly why the injection path above must not.
50+
assertThat(QuickBuildPlugin.runtimeConfigurationOrNull(unknownVariantType())).isNull()
51+
}
52+
53+
@Test
54+
fun `the reflective quick build plugin name resolves to the real class`() {
55+
// AndroidIDEGradlePlugin applies QuickBuildPlugin by name so the minAgpCheck guard
56+
// can compile it without the Quick Build sources; this pins the string to the class.
57+
assertThat(Class.forName(AndroidIDEGradlePlugin.QUICK_BUILD_PLUGIN_CLASS))
58+
.isEqualTo(QuickBuildPlugin::class.java)
59+
}
60+
}

0 commit comments

Comments
 (0)