Skip to content

Commit 8639c81

Browse files
fryanpanclaude
andcommitted
ADFA-4128: qb-10 review fixes - alias attributes, skipped launchers, universal APK
Applies the fix-now items from the 2026-08-31 review triage (items 1, 5, 6, 7, 8, 9, 10, 11, 12; 2 retracted, 3 is PR-body-only, 4 deferred to its own ticket). - The synthesized activity-alias copies its target's android:permission and android:enabled beside android:exported, with the same raw copy-through (any of them can be a resource reference). Both are alias-DECLARED attributes, not inherited, so dropping them left an exported, permission-guarded activity reachable unguarded under its real name. - A skipped activity is recorded as ProxiedComponent(proxyClass = null) instead of dropped, so a skipped LAUNCHER no longer misreports entryActivity == null - it keeps its real manifest name and is launchable as-is. The alias loop skips null-proxy entries; the skip path also normalises the element's android:name to the FQN like the other kept-under-real-name sites. - The proxy-app report selects the built artifact with empty filters (the universal APK) instead of firstOrNull(); an all-splits output fails with a GradleException naming the enabled splits, since guessing a split fails later, on device. - QuickBuildPayloadTransformTask filters both copy loops (and the retained-jar pass) on existence, matching the tolerance its own walkTopDown path already had. - ClassOpener passes the reader to ClassWriter, making its copy-through KDoc true - in lockstep with qb-09's identical FinalStripper change. - Three UNPROXIABLE_BY_NAME reasons rewritten to their real current effect (kept out of the component list / deploy policy); the rename they argued against no longer happens for services and receivers. - Doc corrections: the jacoco block's comment no longer claims a coverage gate nothing enforces, and the dex min-API KDoc states the payload floor (d8 skips desugaring at 30) rather than a nonexistent device gate. Tests verified RED first against the pre-fix code: the extended alias test, both skipped-activity tests, the divert missing-input test, and both universal-APK selection tests (the selection seam initially carried the old first-element behavior). One pre-existing test pinning the old drop-the-skipped-activity contract was updated. Full :gradle-plugin:test green. Triage item 13 (utils.kt indentation) is NOT here: the prescribed remedy, spotlessApply, is a no-op on the file under this ktlint config. Also: plain-language pass over the comments added by these fixes Also: stop the build when a variant exposes no assets source set. A null variant.sources.assets used to skip wiring the payload dex and the baseline stamp, producing a proxy APK whose manifest names classes with no dex behind them; requireAssets fails the build with the variant name instead, matching requireRuntimeConfiguration, and a JVM test pins it (Akash's 08-31 MINOR on QuickBuildPlugin.kt:248, #1722). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01STCsdMzx9daNBcqMN424Ci
1 parent 0f10b49 commit 8639c81

11 files changed

Lines changed: 439 additions & 45 deletions

‎gradle-plugin/build.gradle.kts‎

Lines changed: 7 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -163,11 +163,13 @@ tasks.named<Jar>("jar") {
163163
archiveVersion.set("")
164164
}
165165

166-
// DoD coverage gate: >=90% line+branch. This JVM module keeps the default
167-
// build/jacoco/test.exec location; the report only needs xml enabled (for
168-
// tooling to read the percentages) and the explicit test dependency so
169-
// `:gradle-plugin:jacocoTestReport` is runnable on its own. Test failures do
170-
// not block it: the root build sets ignoreFailures on every Test task.
166+
// Coverage REPORTING only - no verification gate is wired here. This JVM
167+
// module keeps the default build/jacoco/test.exec location; the report just
168+
// needs xml enabled (for tooling to read the percentages) and the explicit
169+
// test dependency so `:gradle-plugin:jacocoTestReport` is runnable on its own.
170+
// The percentages under-count: the TestKit-driven tests exercise the plugin in
171+
// a separate Gradle JVM this JaCoCo run does not instrument. Test failures do
172+
// not block the report: the root build sets ignoreFailures on every Test task.
171173
tasks.named<JacocoReport>("jacocoTestReport") {
172174
dependsOn(tasks.named("test"))
173175
reports {

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

Lines changed: 23 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@ import com.android.build.api.component.analytics.AnalyticsEnabledApplicationVari
66
import com.android.build.api.variant.ApplicationAndroidComponentsExtension
77
import com.android.build.api.variant.ApplicationVariant
88
import com.android.build.api.variant.ScopedArtifacts
9+
import com.android.build.api.variant.SourceDirectories
910
import com.android.build.api.variant.impl.ApplicationVariantImpl
1011
import com.itsaky.androidide.gradle.quickbuild.BaselineGenerationAsset
1112
import com.itsaky.androidide.gradle.quickbuild.QuickBuildBaselineGenerationTask
@@ -107,6 +108,24 @@ class QuickBuildPlugin : Plugin<Project> {
107108
"here; this AGP version needs QuickBuildPlugin.runtimeConfigurationOrNull " +
108109
"taught about its variant type. Use a Standard Run meanwhile.",
109110
)
111+
112+
/**
113+
* The variant's assets source set, which carries the payload dex and the baseline
114+
* stamp into the proxy APK.
115+
*
116+
* AGP declares it nullable, but every application variant this plugin configures has
117+
* one. Should a future AGP ever return null, wiring nothing would build a proxy APK
118+
* whose manifest names classes with no dex behind them, so the build stops here.
119+
*
120+
* @throws GradleException when the variant exposes no assets source set.
121+
*/
122+
internal fun requireAssets(variant: ApplicationVariant): SourceDirectories.Layered =
123+
variant.sources.assets
124+
?: throw GradleException(
125+
"Quick Build cannot add its payload dex to variant '${variant.name}': the " +
126+
"variant exposes no assets source set. Without the dex the proxy app would " +
127+
"crash at launch, so the build stops here. Use a Standard Run meanwhile.",
128+
)
110129
}
111130

112131
override fun apply(target: Project) {
@@ -244,8 +263,8 @@ class QuickBuildPlugin : Plugin<Project> {
244263
task.minApiLevel.set(payloadMinApi)
245264
task.proxyClasses.set(buildDirectory.dir("$variantDir/proxy-classes"))
246265
}
247-
variant.sources.assets
248-
?.addGeneratedSourceDirectory(dex, QuickBuildPayloadDexTask::generatedAssets)
266+
requireAssets(variant)
267+
.addGeneratedSourceDirectory(dex, QuickBuildPayloadDexTask::generatedAssets)
249268

250269
val stamp =
251270
project.tasks.register(
@@ -262,8 +281,8 @@ class QuickBuildPlugin : Plugin<Project> {
262281
)
263282
task.generatedAssets.set(buildDirectory.dir("$variantDir/baseline-generation-assets"))
264283
}
265-
variant.sources.assets
266-
?.addGeneratedSourceDirectory(stamp, QuickBuildBaselineGenerationTask::generatedAssets)
284+
requireAssets(variant)
285+
.addGeneratedSourceDirectory(stamp, QuickBuildBaselineGenerationTask::generatedAssets)
267286

268287
val report =
269288
project.tasks.register(

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

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -33,7 +33,10 @@ object ClassOpener {
3333
*/
3434
fun stripFinalModifier(classBytes: ByteArray): ByteArray {
3535
val reader = ClassReader(classBytes)
36-
val writer = ClassWriter(0)
36+
// The reader enables ASM's copy-through of the constant pool and untouched methods
37+
// (roughly halving the rewrite); the access flags still change because the visitors
38+
// below rewrite them explicitly. Mirrors the daemon's FinalStripper.
39+
val writer = ClassWriter(reader, 0)
3740
reader.accept(
3841
object : ClassVisitor(Opcodes.ASM9, writer) {
3942
override fun visit(

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

Lines changed: 6 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -103,11 +103,14 @@ class ComponentProxiabilityResolver(
103103
"androidx.startup.InitializationProvider" to
104104
"resolves its own component by name at runtime; a renamed proxy breaks androidx App Startup",
105105
"androidx.profileinstaller.ProfileInstallReceiver" to
106-
"not on every proxy compile classpath, so the generated subclass would not compile",
106+
"library plumbing whose class never travels in the payload; skipping keeps it out " +
107+
"of the component list and the deploy policy's restart rule",
107108
"com.itsaky.androidide.quickbuild.runtime.QuickBuildKeepAliveService" to
108-
"CoGo binds this keep-alive by component name; a renamed proxy would leave the app freezer-eligible",
109+
"the quick-build runtime's own keep-alive, not user code; skipping keeps it out " +
110+
"of the component list and the deploy policy's restart rule",
109111
"com.google.firebase.components.ComponentDiscoveryService" to
110-
"Firebase reads this service's own metadata by component name; a renamed proxy makes it discover zero ComponentRegistrars",
112+
"Firebase plumbing whose class never travels in the payload; skipping keeps it out " +
113+
"of the component list and the deploy policy's restart rule",
111114
)
112115

113116
/**

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

Lines changed: 55 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -34,12 +34,14 @@ enum class ComponentType(
3434
* the user FQN in the manifest and the runtime's instantiate hooks route them through the payload
3535
* loader by that real name. For the Application nothing addresses it by manifest name anyway; for
3636
* services and receivers the real name IS the addressing - see
37-
* [QuickBuildManifestTransformer.transformComponents].
37+
* [QuickBuildManifestTransformer.transformComponents]. An activity the proxiability resolver
38+
* skipped also carries a null [proxyClass]: it keeps its real `<activity>` entry, so it stays
39+
* launchable and its [isLauncher] fact must survive.
3840
*
3941
* @property type which manifest element this came from.
4042
* @property userClass fully-qualified user class.
4143
* @property proxyClass fully-qualified generated proxy class that replaces it in the
42-
* manifest, or null for the Application, service and receiver entries.
44+
* manifest, or null for the Application, service, receiver and skipped-activity entries.
4345
* @property isLauncher whether an activity declares the MAIN/LAUNCHER intent filter.
4446
*/
4547
data class ProxiedComponent(
@@ -66,19 +68,23 @@ data class UnproxiedComponent(
6668
*
6769
* @property document the transformed manifest, mutated in place from the parsed input.
6870
* @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.
71+
* services, receivers, the Application and skipped activities) kept under its real name with
72+
* a null proxyClass.
7073
* @property unproxied components left under their real name, for the caller to log.
7174
*/
7275
class ManifestTransformResult(
7376
val document: Document,
7477
val components: List<ProxiedComponent>,
7578
val unproxied: List<UnproxiedComponent> = emptyList(),
7679
) {
77-
/** The proxied activities, in manifest order. */
80+
/** The recorded activities, in manifest order; a skipped one carries a null proxyClass. */
7881
val activities: List<ProxiedComponent>
7982
get() = components.filter { it.type == ComponentType.ACTIVITY }
8083

81-
/** User class of the LAUNCHER activity, or null when the manifest declares none. */
84+
/**
85+
* User class of the LAUNCHER activity, or null when the manifest declares none. A skipped
86+
* launcher still counts: it keeps its real manifest name, so it is launchable as-is.
87+
*/
8288
val entryActivity: String?
8389
get() = activities.firstOrNull { it.isLauncher }?.userClass
8490
}
@@ -201,32 +207,51 @@ class QuickBuildManifestTransformer(
201207
* @param application the `<application>` element, mutated in place.
202208
* @param manifestPackage the manifest's package, for expanding android:name shorthand.
203209
* @param unproxied accumulator for components [skipProxy] rejects.
204-
* @return the proxied activities, in manifest order; skipped ones are absent.
210+
* @return the recorded activities, in manifest order; a skipped one appears with a null
211+
* [ProxiedComponent.proxyClass] and consumes no proxy index.
205212
*/
206213
private fun transformActivities(
207214
application: Element,
208215
manifestPackage: String,
209216
unproxied: MutableList<UnproxiedComponent>,
210217
): List<ProxiedComponent> {
211218
val activities = mutableListOf<ProxiedComponent>()
212-
// android:exported per renamed activity, so the back-reference alias below can carry the
213-
// target's own value. The raw attribute, not a parsed boolean: the value may be a
214-
// resource reference (@bool/...), which parses as neither true nor false and must be
215-
// copied through rather than collapsed. Kept local rather than added to
219+
// The alias-declared attributes per renamed activity, so the back-reference alias below
220+
// can carry the target's own values. The raw attributes, not parsed booleans: any of
221+
// them may be a resource reference (@bool/...), which parses as neither true nor false
222+
// and must be copied through rather than collapsed. Kept local rather than added to
216223
// ProxiedComponent, which is shared with the setup.json writer.
217-
val exportedByUserClass = mutableMapOf<String, String>()
224+
val aliasAttrsByUserClass = mutableMapOf<String, Map<String, String>>()
218225
var proxyIndex = 0
219226
application.childElements("activity").forEachIndexed { index, activity ->
220227
val userClass = requireComponentName(activity, "activity", index, manifestPackage)
221228
rejectUnsupported(activity, "activity", userClass)
222-
// An alias targeting a skipped activity (below) then finds no proxy mapping
229+
// An alias targeting a skipped activity (below) then finds a null proxy mapping
223230
// and correctly leaves its targetActivity pointed at the real class.
224231
if (skipProxy(userClass, unproxied)) {
232+
// Written back fully qualified, for the same reason as the non-proxied kinds
233+
// in transformComponents: consumers resolve this name against the payload
234+
// dex, so shorthand left verbatim is fragile.
235+
activity.setAttributeNS(ANDROID_NS, "android:name", userClass)
236+
// Recorded with a null proxyClass rather than dropped: the activity keeps its
237+
// real manifest name and stays launchable, so dropping it would discard the
238+
// isLauncher fact and misreport a skipped launcher as entryActivity == null.
239+
activities.add(
240+
ProxiedComponent(
241+
type = ComponentType.ACTIVITY,
242+
userClass = userClass,
243+
proxyClass = null,
244+
isLauncher = isLauncher(activity),
245+
),
246+
)
225247
return@forEachIndexed
226248
}
227249
val proxyClass = "$proxyPackage.${proxySimpleName(proxyIndex, ComponentType.ACTIVITY)}"
228250
proxyIndex++
229-
exportedByUserClass[userClass] = activity.getAttributeNS(ANDROID_NS, "exported")
251+
aliasAttrsByUserClass[userClass] =
252+
listOf("exported", "permission", "enabled").associateWith {
253+
activity.getAttributeNS(ANDROID_NS, it)
254+
}
230255
activity.setAttributeNS(ANDROID_NS, "android:name", proxyClass)
231256
activities.add(
232257
ProxiedComponent(
@@ -260,19 +285,33 @@ class QuickBuildManifestTransformer(
260285
// activity was reachable under its real name before the rename, and a pinned shortcut or
261286
// share target the app itself published records that name, so forcing false rejects a
262287
// launch that works under a standard run. Never widen it - an absent attribute reads as
263-
// false, which is also what a merged manifest states explicitly from API 31.
288+
// false, which is also what a merged manifest states explicitly from API 31. The
289+
// target's permission and enabled travel with it: both are alias-DECLARED attributes,
290+
// not inherited, so dropping them would leave a permission-guarded activity reachable
291+
// unguarded under its real name, or resurrect a disabled entry point.
264292
val document = application.ownerDocument
265293
activities.forEach { component ->
294+
// A skipped activity kept its real <activity> entry, so it needs no alias - and one
295+
// with the same name would collide with it at install time.
296+
val proxyClass = component.proxyClass ?: return@forEach
297+
val attrs = aliasAttrsByUserClass[component.userClass].orEmpty()
266298
val alias = document.createElement("activity-alias")
267299
alias.setAttributeNS(ANDROID_NS, "android:name", component.userClass)
268-
alias.setAttributeNS(ANDROID_NS, "android:targetActivity", component.proxyClass!!)
300+
alias.setAttributeNS(ANDROID_NS, "android:targetActivity", proxyClass)
269301
alias.setAttributeNS(
270302
ANDROID_NS,
271303
"android:exported",
272304
// An absent attribute is only implicitly false before API 31, where it is a hard
273305
// manifest error, so the alias states it.
274-
exportedByUserClass[component.userClass]?.takeIf { it.isNotBlank() } ?: "false",
306+
attrs["exported"]?.takeIf { it.isNotBlank() } ?: "false",
275307
)
308+
// Unlike exported, these stay absent when the target declares neither: an empty
309+
// android:permission is not "no permission".
310+
listOf("permission", "enabled").forEach { name ->
311+
attrs[name]?.takeIf { it.isNotBlank() }?.let {
312+
alias.setAttributeNS(ANDROID_NS, "android:$name", it)
313+
}
314+
}
276315
application.appendChild(alias)
277316
}
278317
return activities

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

Lines changed: 56 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
11
package com.itsaky.androidide.gradle.quickbuild
22

3+
import com.android.build.api.variant.BuiltArtifact
34
import com.android.build.api.variant.BuiltArtifactsLoader
45
import com.android.tools.r8.CompilationFailedException
56
import com.android.tools.r8.CompilationMode
@@ -177,14 +178,19 @@ abstract class QuickBuildPayloadTransformTask : DefaultTask() {
177178
@TaskAction
178179
fun divert() {
179180
val root = payloadClasses.get().asFile.cleanDirectory()
180-
allJars.get().forEachIndexed { index, jar ->
181+
// AGP hands over declared artifact locations, not guaranteed files. The retained-jar
182+
// walk below already tolerates a missing directory, so the copies here skip missing
183+
// inputs the same way instead of throwing on them.
184+
val jars = allJars.get().filter { it.asFile.exists() }
185+
val directories = allDirectories.get().filter { it.asFile.exists() }
186+
jars.forEachIndexed { index, jar ->
181187
jar.asFile.copyTo(File(root, "jars/$index.jar"))
182188
}
183-
allDirectories.get().forEachIndexed { index, dir ->
189+
directories.forEachIndexed { index, dir ->
184190
dir.asFile.copyRecursively(File(root, "dirs/$index"))
185191
}
186192

187-
writeRetainedApkJar()
193+
writeRetainedApkJar(jars, directories)
188194
}
189195

190196
/**
@@ -199,15 +205,18 @@ abstract class QuickBuildPayloadTransformTask : DefaultTask() {
199205
return name == "R.class" || (name.startsWith("R$") && name.endsWith(".class"))
200206
}
201207

202-
/** Collects every R class from the inputs into [outputJar] so the APK keeps them. */
203-
private fun writeRetainedApkJar() {
208+
/** Collects every R class from the (existing) inputs into [outputJar] so the APK keeps them. */
209+
private fun writeRetainedApkJar(
210+
jars: List<RegularFile>,
211+
directories: List<Directory>,
212+
) {
204213
val seen = HashSet<String>()
205214
JarOutputStream(outputJar.get().asFile.outputStream()).use { out ->
206215
// A zip must contain at least one entry even when no R classes exist.
207216
out.putNextEntry(JarEntry("META-INF/com.itsaky.androidide.quickbuild.diverted"))
208217
out.closeEntry()
209218

210-
allDirectories.get().forEach { dir ->
219+
directories.forEach { dir ->
211220
dir.asFile.walkTopDown().filter { it.isFile && isResourceClass(it.name) }.forEach { file ->
212221
val entry = file.relativeTo(dir.asFile).invariantSeparatorsPath
213222
if (seen.add(entry)) {
@@ -217,7 +226,7 @@ abstract class QuickBuildPayloadTransformTask : DefaultTask() {
217226
}
218227
}
219228
}
220-
allJars.get().forEach { jar ->
229+
jars.forEach { jar ->
221230
JarFile(jar.asFile).use { jf ->
222231
jf.entries().asSequence().filter { !it.isDirectory && isResourceClass(it.name) }.forEach { entry ->
223232
if (seen.add(entry.name)) {
@@ -273,7 +282,11 @@ abstract class QuickBuildPayloadDexTask : DefaultTask() {
273282
@get:Classpath
274283
abstract val runtimeAar: ConfigurableFileCollection
275284

276-
/** Effective dex min API; at least 30 because Quick Build is gated to API 30+ devices. */
285+
/**
286+
* Effective dex min API - the payload floor, NOT the device floor: at least 30 so d8 skips
287+
* desugaring, while the emitted dex format still loads on the API 28+ devices Quick Build
288+
* supports. See QuickBuildPlugin.MIN_PAYLOAD_API.
289+
*/
277290
@get:Input
278291
abstract val minApiLevel: Property<Int>
279292

@@ -643,8 +656,8 @@ abstract class QuickBuildProxyAppReportTask : DefaultTask() {
643656
.get()
644657
.load(apkDirectory.get())
645658
?.elements
646-
?.firstOrNull()
647-
?.outputFile
659+
?.takeIf { it.isNotEmpty() }
660+
?.let { selectUniversalApk(it, apkDirectory.get().asFile) }
648661
?: apkDirectory
649662
.get()
650663
.asFile
@@ -734,6 +747,39 @@ abstract class QuickBuildProxyAppReportTask : DefaultTask() {
734747
* @return absolute paths of every `.flat` unit found, sorted; empty when neither source
735748
* exists, which the caller logs rather than treating as an error.
736749
*/
750+
companion object {
751+
/**
752+
* Picks the one APK every device can install out of AGP's built-artifact metadata.
753+
*
754+
* With APK splits enabled the metadata lists each split beside the universal APK in no
755+
* contractual order, so "the first element" is an arbitrary split that installs - or
756+
* fails to - depending on the device it meets. Only the unfiltered (universal) element
757+
* is safe to hand to CoGo.
758+
*
759+
* @param elements the loaded metadata's artifacts; must be non-empty.
760+
* @param apkDirectory the APK output directory, for the error message only.
761+
* @return the universal APK's path, as recorded in the metadata.
762+
* @throws GradleException when every element is a split: Quick Build does not support
763+
* splits, and picking one here would only fail later, on the device.
764+
*/
765+
internal fun selectUniversalApk(
766+
elements: Collection<BuiltArtifact>,
767+
apkDirectory: File,
768+
): String =
769+
elements.firstOrNull { it.filters.isEmpty() }?.outputFile
770+
?: throw GradleException(
771+
"Quick Build: every APK under '$apkDirectory' is a split (" +
772+
elements
773+
.flatMap { it.filters }
774+
.map { "${it.filterType}=${it.identifier}" }
775+
.distinct()
776+
.sorted()
777+
.joinToString(", ") +
778+
"); Quick Build does not support APK splits yet - disable splits for " +
779+
"this variant or use a Standard Run",
780+
)
781+
}
782+
737783
private fun collectLibraryResourcePaths(): List<String> {
738784
val mergedRes =
739785
mergedResSearchDir.orNull

0 commit comments

Comments
 (0)