Status Update
No update yet.
Comments
za...@capitalone.com
As I understand it, we need to migrate namespace definitions both from AndroidManifest.xml and from implicit definitions from applicationId / testApplicationId? Is that right? So an 8.0 project would have a
namespace <n>
testNamespace <tn>
where <n> is taken from AndroidManifest.xml package definitions if present, and applicationId if not, and <tn> is testApplicationId if present and <n>.test if not?
za...@capitalone.com
Sorry for the confusion. AGP would change how it computes the namespace for the androidTest variant. All the Upgrade assistant would need to do is update the source code for the import of the R class.
sg...@google.com
OK! And, I think, this should happen at the same time as the namespace declaration is migrated from AndroidManifest.xml?
sg...@google.com
It's unrelated, but it is possible that we move 8.0 to also only declare the namespace into the DSL. I would make it a separate refactoring because they are not tied at all.
my...@gmail.com
I would consider first adding the right new value to models (as a new property) and then, to avoid Studio interpreting manifests and build configurations, just update to whatever the synced value of this new property is.
my...@gmail.com
Updated version of this in #16 below
Currently, AUA removes the package attribute from the main manifest and instead specifies that value via the namespace DSL.
In addition to that, AUA should:
(1) Check for apackageattribute in the (possibly non-existent) androidTestAndroidManifest.xml. If there's a value there, it should be removed from that manifest, and if it's not equal tonamespace + ".test", it should be specified via thetestNamespaceDSL.Ifnamespace==testNamespace, this is problematic because the androidTest and regularRclasses will have the same namespace. Not sure what the best behavior is here? Maybe an error explaining that the user should first reset the androidTest AndroidManifest'spackageattribute and change the androidTest source code package accordingly?
- (2)
Compare the newtestNamespacewith the oldAndroidProject.androidTestNamespacefrom the model. If they are different, search the androidTest source code for references to anRclass namespaced withAndroidProejct.androidTestNamespace, and change the namespace totestNamespace.
sg...@google.com
I have a draft implementation for the second part of (1) from namespace and testNamespace are equal. Is there any documentation we can point to explaining how the user should proceed? (My in-window space budget for explanatory text is pretty limited).
mk...@google.com
+Amy
Amy, would it make sense to add documentation about setting the namespace for testing? Similar to what we have
It could mention the following:
- the default test namespace is
namespace+".test" - to customize the test namespace, use the
testNamespace - If setting a value for
testNamespace, users should not set the same value asnamespace, otherwise the test and testedRandBuildConfigclasses will have the same namespace.
ap...@google.com
Yeah definitely, I can draft it and send you a CL. Thanks! Looks like we might need to update this section too: package attribute is only used to set the applicationId when you first create your project, right (assuming people are setting the namespace using the DSL)?
Would it also be helpful to add a release note about this? I guess it's technically not new with Chipmunk, but looks like we didn't mention it in the last couple stable releases.
ap...@google.com
Looks like we might need to update this section too:
https://developer.android.com/guide/topics/manifest/manifest-element#package
Yes, setting the package attribute in the manifest is deprecated, will cause build warnings in AGP 7.3.0, and will cause build errors in AGP 8.0.
Currently the package attribute is only used to set the applicationId when you first create your project, right (assuming people are setting the namespace using the DSL)?
The namespace DSL will always trump the package attribute, even when determining the applicationId. E.g., if a dev sets package to "com.example.package", namespace to "com.example.namespace", and doesn't set the applicationId explicitly in the DSL, the applicationId will be "com.example.namespace".
Would it also be helpful to add a release note about this? I guess it's technically not new with Chipmunk, but looks like we didn't mention it in the last couple stable releases.
I think a 7.3 release note and an 8.0 release note since we're warning starting in 7.3 and will break the build in 8.0 when the manifest has a package attribute.
ap...@google.com
Gotcha, thanks--sent you cl/449846661, to start. Is the warning/removal happening in certain Dolphin/EE preview releases, or not until the stable releases?
ap...@google.com
Thanks!
Yes, the warning started in AGP 7.3.0-alpha04.
I expect we'll switch to an error in an early AGP 8.0 alpha.
ap...@google.com
Okay sounds good, I added the Dolphin preview release note to cl/450515477 (messed up the previous CL) as well. Please let me know if you need help whenever the EE canary with the update is released!
ap...@google.com
Updated version of #7:
Currently, AUA removes the package attribute from the main manifest and instead specifies that value via the namespace DSL.
In addition to that, AUA should:
- (1) Check for a
packageattribute in the (possibly non-existent) androidTestAndroidManifest.xml. If there's a value there, it should be removed from that manifest, and if it's not equal to.namespace + ".test", it should be specified via thetestNamespaceDSL - (2) If
AndroidProject.androidTestNamespace != null&&AndroidProject.androidTestNamespace != namespace, set thetestNamespaceDSL withAndroidProject.androidTestNamespace. - (3) If instead
AndroidProject.androidTestNamespace == namespace, set thetestNamespaceDSL withAndroidProject.androidTestNamespace + ".test"and warn the user that their androidTest code might no longer compile because the namespace of their androidTestRclass has changed fromAndroidProject.androidTestNamespacetoAndroidProject.androidTestNamespace + ".test", so they might need to modify their androidTest source code accordingly.
ap...@google.com
LGTM. We should make sure to have a dac page ready (I guess for both the removal of the package attribute, and the potential conflict between namespace and testNamespace and point the users to it whenever AUA does anything related to this change.
ap...@google.com
I filed
my...@gmail.com
I've attached a project which demonstrates when a build will break in case (3) from #16.
In this project, the app gets its namespace ("com.example.namespace") from the package attribute of the AndroidManifest.xml.
The AndroidTest component gets its namespace from the testApplicationId, and it's the same namespace as the app's.
The AndroidTest component calls com.example.namespace.R.string.app_android_test_string in ExampleInstrumentedTest, which compiles as-is (./gradlew :app:assembleAndroidTest), but if we add namespace "com.example.namespace" to the DSL, then it doesn't compile anymore because the AndroidTest component's namespace becomes "com.example.namespace.test".
ap...@google.com
Thanks for the test project.
What are the downsides of preserving the workingness of the project by adding testNamespace "com.example.namespace" in the case that there is a collision, like in this test project? It builds successfully for me, but maybe there are subsequent problems?
my...@gmail.com
Thanks. (But not in the test project from -alpha09).
mk...@google.com
Ah, yes, looks like that change was merged in -alpha10.
dm...@izettle.com
mk...@google.com
Thanks! Closing this bug as I still have
dm...@izettle.com
After upgrading to Android Gradle Plugin 8.0.1 and moving all namespaces from Manifests to build scripts, my android tests started failing, giving me the following error:
Caused by: java.lang.ClassNotFoundException: Didn't find class "com.myapp.android.test.TestApplication"
The problem seems to be related to the namespace: the namespace for android tests is %namespace% + ".test". When the OS tries to load a component that is declared in Manifest with a relative path like android:name=".TestApplication" it uses the test namespace and eventually cannot find it on the classpath.
Is there a way to fix this?
mk...@google.com
dm...@izettle.com
Is there a way to fix this?
Can you use the fully qualified name instead of ".TestApplication"?
If that doesn't work for you, can you give more details so I can reproduce the issue? IIUC, the actual fully qualified class name is com.myapp.android.TestApplication, and android:name=".TestApplication is located in the androidTest AndroidManifest.xml... is that correct?
mk...@google.com
Using fully qualified name fixes the problem, but it is not an elegant solution for a huge project with hundred of modules with android tests where it all needs to be fixed manually.
It worked well with "package" in Manifest.
Just wonder, is this the only solution?
dm...@izettle.com
Using fully qualified name fixes the problem, but it is not an elegant solution for a huge project with hundred of modules with android tests where it all needs to be fixed manually.
Unfortunately, I can't think of another way to fix the problem.
It worked well with "package" in Manifest.
Yes, but we've opted to (1) disallow setting a value for package that is different than the (test) namespace and (2) disallow having namespace match testNamespace. (1) - because it is confusing, and (2) - because there can be unintended collisions of production vs test android resources.
Very sorry for the breaking change.
mk...@google.com
dm...@izettle.com
Isn't this issue more related to the change we made to the test applicationID as part of this change? It used to be that the test appID was computed badly based on the test package, but we change the computation (I don't remember exactly the change).
The relative path in the manifest is expanded based on testApplicationId, not the testNamespace (computed or explicit).
You should configure this to be what you expect it to be.
mk...@google.com
The relative path in the manifest is expanded based on testApplicationId, not the testNamespace (computed or explicit).
That was the old behavior, but AGP uses the namespace now, not the application ID. See
dm...@izettle.com
I did ./gradlew assembleRelease -Dorg.gradle.jvmargs='-ea:com.android.tools.r8' --no-daemon --stacktrace and got the same stacktrace as initially, the one with NPE.
mk...@google.com
Did you try:
./gradlew assembleRelease -Dorg.gradle.jvmargs='-ea:com.android.tools.r8...' --no-daemon --stacktrace
(notice the dots in the end of r8.)
dm...@izettle.com
I've tried with dots and got new callstack :)
Caused by: java.lang.NullPointerException
at com.android.tools.r8.cf.code.frame.UninitializedNew.<init>(UninitializedNew.java:22)
at com.android.tools.r8.cf.code.frame.FrameType.uninitializedNew(FrameType.java:135)
at com.android.tools.r8.graph.LazyCfCode$MethodCodeVisitor.getFrameType(LazyCfCode.java:542)
at com.android.tools.r8.graph.LazyCfCode$MethodCodeVisitor.parseStack(LazyCfCode.java:521)
at com.android.tools.r8.graph.LazyCfCode$MethodCodeVisitor.visitFrame(LazyCfCode.java:501)
at com.android.tools.r8.org.objectweb.asm.ClassReader.readCode(ClassReader.java:2068)
at com.android.tools.r8.org.objectweb.asm.ClassReader.readMethod(ClassReader.java:1514)
at com.android.tools.r8.org.objectweb.asm.ClassReader.accept(ClassReader.java:744)
at com.android.tools.r8.org.objectweb.asm.ClassReader.accept(ClassReader.java:424)
at com.android.tools.r8.graph.LazyCfCode.parseCode(LazyCfCode.java:205)
at com.android.tools.r8.graph.LazyCfCode.internalParseCode(LazyCfCode.java:161)
at com.android.tools.r8.utils.ExceptionUtils.lambda$withOriginAndPositionAttachmentHandler$5(ExceptionUtils.java:238)
at com.android.tools.r8.utils.ExceptionUtils.withOriginAndPositionAttachmentHandler(ExceptionUtils.java:246)
at com.android.tools.r8.utils.ExceptionUtils.withOriginAndPositionAttachmentHandler(ExceptionUtils.java:234)
at com.android.tools.r8.utils.ExceptionUtils.withOriginAttachmentHandler(ExceptionUtils.java:225)
at com.android.tools.r8.graph.LazyCfCode.asCfCode(LazyCfCode.java:143)
at com.android.tools.r8.ir.desugar.NonEmptyCfInstructionDesugaringCollection.needsDesugaring(NonEmptyCfInstructionDesugaringCollection.java:349)
at com.android.tools.r8.shaking.Enqueuer.addToPendingDesugaring(Enqueuer.java:4004)
at com.android.tools.r8.shaking.Enqueuer.traceNonDesugaredCode(Enqueuer.java:4798)
at com.android.tools.r8.shaking.Enqueuer.markMethodAsLive(Enqueuer.java:4742)
at com.android.tools.r8.shaking.EnqueuerWorklist$MarkMethodLiveAction.run(EnqueuerWorklist.java:166)
at com.android.tools.r8.shaking.Enqueuer.trace(Enqueuer.java:4395)
at com.android.tools.r8.shaking.Enqueuer.traceApplication(Enqueuer.java:3622)
at com.android.tools.r8.R8.runEnqueuer(R8.java:1012)
at com.android.tools.r8.R8.run(R8.java:369)
at com.android.tools.r8.R8.run(R8.java:252)
at com.android.tools.r8.R8.lambda$runForTesting$1(R8.java:243)
at com.android.tools.r8.utils.ExceptionUtils.withCompilationHandler(ExceptionUtils.java:80)
... 35 more
Suppressed: com.android.tools.r8.utils.ExceptionUtils$OriginAttachmentException: java.lang.NullPointerException
at com.android.tools.r8.utils.ExceptionUtils$OriginAttachmentException.wrap(ExceptionUtils.java:258)
at com.android.tools.r8.utils.ExceptionUtils.withOriginAndPositionAttachmentHandler(ExceptionUtils.java:248)
at com.android.tools.r8.utils.ExceptionUtils.withOriginAndPositionAttachmentHandler(ExceptionUtils.java:234)
at com.android.tools.r8.utils.ExceptionUtils.withOriginAttachmentHandler(ExceptionUtils.java:225)
at com.android.tools.r8.graph.LazyCfCode.asCfCode(LazyCfCode.java:143)
at com.android.tools.r8.ir.desugar.NonEmptyCfInstructionDesugaringCollection.needsDesugaring(NonEmptyCfInstructionDesugaringCollection.java:349)
at com.android.tools.r8.shaking.Enqueuer.addToPendingDesugaring(Enqueuer.java:4004)
at com.android.tools.r8.shaking.Enqueuer.traceNonDesugaredCode(Enqueuer.java:4798)
at com.android.tools.r8.shaking.Enqueuer.markMethodAsLive(Enqueuer.java:4742)
at com.android.tools.r8.shaking.EnqueuerWorklist$MarkMethodLiveAction.run(EnqueuerWorklist.java:166)
at com.android.tools.r8.shaking.Enqueuer.trace(Enqueuer.java:4395)
at com.android.tools.r8.shaking.Enqueuer.traceApplication(Enqueuer.java:3622)
at com.android.tools.r8.R8.runEnqueuer(R8.java:1012)
at com.android.tools.r8.R8.run(R8.java:369)
at com.android.tools.r8.R8.run(R8.java:252)
at com.android.tools.r8.R8.lambda$runForTesting$1(R8.java:243)
at com.android.tools.r8.utils.ExceptionUtils.withCompilationHandler(ExceptionUtils.java:80)
at com.android.tools.r8.utils.ExceptionUtils.withR8CompilationHandler(ExceptionUtils.java:69)
at com.android.tools.r8.R8.runForTesting(R8.java:239)
at com.android.tools.r8.R8.run(R8.java:187)
at com.android.builder.dexing.R8Tool.runR8(r8Tool.kt:308)
at com.android.build.gradle.internal.tasks.R8Task$Companion.shrink(R8Task.kt:642)
at com.android.build.gradle.internal.tasks.R8Task$R8Runnable.execute(R8Task.kt:712)
at org.gradle.workers.internal.DefaultWorkerServer.execute(DefaultWorkerServer.java:63)
at org.gradle.workers.internal.NoIsolationWorkerFactory$1$1.create(NoIsolationWorkerFactory.java:66)
at org.gradle.workers.internal.NoIsolationWorkerFactory$1$1.create(NoIsolationWorkerFactory.java:62)
at org.gradle.internal.classloader.ClassLoaderUtils.executeInClassloader(ClassLoaderUtils.java:100)
at org.gradle.workers.internal.NoIsolationWorkerFactory$1.lambda$execute$0(NoIsolationWorkerFactory.java:62)
at org.gradle.workers.internal.AbstractWorker$1.call(AbstractWorker.java:44)
at org.gradle.workers.internal.AbstractWorker$1.call(AbstractWorker.java:41)
at org.gradle.internal.operations.DefaultBuildOperationRunner$CallableBuildOperationWorker.execute(DefaultBuildOperationRunner.java:204)
at org.gradle.internal.operations.DefaultBuildOperationRunner$CallableBuildOperationWorker.execute(DefaultBuildOperationRunner.java:199)
at org.gradle.internal.operations.DefaultBuildOperationRunner$2.execute(DefaultBuildOperationRunner.java:66)
at org.gradle.internal.operations.DefaultBuildOperationRunner$2.execute(DefaultBuildOperationRunner.java:59)
at org.gradle.internal.operations.DefaultBuildOperationRunner.execute(DefaultBuildOperationRunner.java:157)
at org.gradle.internal.operations.DefaultBuildOperationRunner.execute(DefaultBuildOperationRunner.java:59)
at org.gradle.internal.operations.DefaultBuildOperationRunner.call(DefaultBuildOperationRunner.java:53)
at org.gradle.internal.operations.DefaultBuildOperationExecutor.call(DefaultBuildOperationExecutor.java:73)
at org.gradle.workers.internal.AbstractWorker.executeWrappedInBuildOperation(AbstractWorker.java:41)
at org.gradle.workers.internal.NoIsolationWorkerFactory$1.execute(NoIsolationWorkerFactory.java:59)
at org.gradle.workers.internal.DefaultWorkerExecutor.lambda$submitWork$2(DefaultWorkerExecutor.java:212)
at java.base/java.util.concurrent.FutureTask.run(FutureTask.java:264)
at org.gradle.internal.work.DefaultConditionalExecutionQueue$ExecutionRunner.runExecution(DefaultConditionalExecutionQueue.java:187)
at org.gradle.internal.work.DefaultConditionalExecutionQueue$ExecutionRunner.access$700(DefaultConditionalExecutionQueue.java:120)
at org.gradle.internal.work.DefaultConditionalExecutionQueue$ExecutionRunner$1.run(DefaultConditionalExecutionQueue.java:162)
at org.gradle.internal.Factories$1.create(Factories.java:31)
at org.gradle.internal.work.DefaultWorkerLeaseService.withLocks(DefaultWorkerLeaseService.java:249)
at org.gradle.internal.work.DefaultWorkerLeaseService.runAsWorkerThread(DefaultWorkerLeaseService.java:109)
at org.gradle.internal.work.DefaultWorkerLeaseService.runAsWorkerThread(DefaultWorkerLeaseService.java:114)
at org.gradle.internal.work.DefaultConditionalExecutionQueue$ExecutionRunner.runBatch(DefaultConditionalExecutionQueue.java:157)
at org.gradle.internal.work.DefaultConditionalExecutionQueue$ExecutionRunner.run(DefaultConditionalExecutionQueue.java:126)
at java.base/java.util.concurrent.Executors$RunnableAdapter.call(Executors.java:515)
at java.base/java.util.concurrent.FutureTask.run(FutureTask.java:264)
at org.gradle.internal.concurrent.ExecutorPolicy$CatchAndRecordFailures.onExecute(ExecutorPolicy.java:64)
at org.gradle.internal.concurrent.ManagedExecutorImpl$1.run(ManagedExecutorImpl.java:49)
at java.base/java.util.concurrent.ThreadPoolExecutor.runWorker(ThreadPoolExecutor.java:1128)
at java.base/java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:628)
at java.base/java.lang.Thread.run(Thread.java:829)
Caused by: [CIRCULAR REFERENCE: java.lang.NullPointerException]
ap...@google.com
Branch: main
commit d3be3e93b26b4797fcfa4b071da30e5184ac27b5
Author: Morten Krogh-Jespersen <mkroghj@google.com>
Date: Thu Feb 02 10:38:36 2023
Throw compilation error with origin when failing to parse code
Bug:
Change-Id: I358dc5d95aacfe448ec9d1e2d5e5d5deb4921631
M src/main/java/com/android/tools/r8/graph/LazyCfCode.java
mk...@google.com
I've created a new CL ToT that will show the origin of the class-file we cannot parse. Hopefully you can share that library/file with us. To use the new version, you have amend your build.gradle file:
pluginManagement {
buildscript {
repositories {
mavenCentral()
maven {
url = uri("https://storage.googleapis.com/r8-releases/raw/main")
}
}
dependencies {
classpath("com.android.tools:r8:d3be3e93b26b4797fcfa4b071da30e5184ac27b5")
classpath('com.google.guava:guava:30.1.1-jre') // <-- THIS IS REQUIRED UNTIL R8 3.2.4-dev
}
}
}
(Note that the maven repository is different)
Then run the .gradlew command again.
mk...@google.com
I will close this bug waiting for answer since it marked as P1 and cannot change it since we backported an issue. Please open a new bug when you have the source files that we cannot parse.
Description
Getting the following exception when executing
:app:minifyVariantWithR8when upgrading to7.4.0-beta02. This worked without issues with AGP7.3.0.