IssueTracker

Currently, it is controlled by android.r8.failOnMissingClasses gradle property and the default value is false. Do we want to make the default value to true or we want to remove this gradle property and never inject the -ignorewarnings?

Based on what we said in the release note and step 3 of the plan in the design doc, then we should just remove the injection of -ignorewarnings together with the android.r8.failOnMissingClasses property.

Thanks for the clarification, working on it!

When I stop passing the -ignorewarnings(ag/19775214), we got some "missing class" failures from our integration tests. And those tests are set up normally instead of not adding reference classes as input on purpose. I assume our users would probably see these errors too. Is there anything they can do to make the build succeed? Or we just want them to add -dontwarn classA to their proguard file?

Some errors from our tests:

Missing class java.lang.management.ManagementFactory (referenced from: void org.jacoco.agent.rt.internal_b6258fc.JmxRegistration.<init>(org.jacoco.agent.rt.IAgent))
...
Missing class javax.annotation.concurrent.ThreadSafe (referenced from: com.google.common.util.concurrent.CycleDetectingLockFactory$Policy and 2 other contexts)
...
Missing class javax.annotation.Nullable (referenced from: java.lang.Object com.google.common.base.Equivalence$EquivalentToPredicate.target and 969 other contexts)
...

Without that change, do you get a warning in the build output saying that keep rules should be added? Also, in 8.0 we should still generate a helpful error message instead of just outputting the R8 exception.

The javax annotations should be in https://mvnrepository.com/artifact/com.google.code.findbugs/jsr305.

For java.lang.management you will likely need a -dontwarn rule. (Ideally this would not be part of the live program, since this code is clearly not meant to be run on Android. This could require changes to jacoco, however.)

re #6, we do get warnings about missing classes which is the same as the one that we would get as errors if we don't add -ignorewarnings.

WARNING:R8: Missing class com.google.j2objc.annotations.Weak (referenced from: java.util.concurrent.ConcurrentMap com.google.common.cache.LocalCache$AbstractCacheSet.map and 21 other contexts)
Missing class java.lang.ClassValue (referenced from: java.lang.ClassValue com.google.common.util.concurrent.FuturesGetChecked$GetCheckedTypeValidatorHolder$ClassValueValidator.isValidClass and 3 other contexts)
......

And I will make sure we wrap r8 error instead of throwing them directly.

re #7, I fixed most of the missing class errors by adding corresponding dependencies. But I am not sure about java.lang.ClassValue which is added from jdk to android recently(https://b.corp.google.com/issues/32299208). I guess if users build with a higher version of sdk, then the error should disappear. To fix it in our test now, I can probably add -dontwarn rule for it.

Ivan, just to clarify your comments about a helpful error message, you mean we want to have some error message in addition to showing the exception thrown from r8? The error message should contains some instructions on how to solve this problem? like "please add missing classes or add -dontwarn rule"

Regarding java.lang.ClassValue, adding a -dontwarn it the right thing to do. If a developer use a dependency which has live references to java.lang.ClassValue, then they have to explicitly consider that issue and add the warning. The concrete Guava use that you saw seems to be from the JRE version of Guava, as the Android version does not have it. Missing java. types are common in libraries which are not explicitly designed for Android.

Re #10: Yes, we should suggest a clear user action similar to the one we had in 7.x. If we can collect all missing dontwarn(s) and write them out to a file (as in 7.x) that'd be even better. Alternatively, if we get them one by one, we can output a better error message in stderr.

Looks like we already have a clear user action thanks to our implementation here https://source.corp.google.com/studio-main/tools/base/build-system/builder/src/main/java/com/android/builder/dexing/r8/R8DiagnosticsHandler.kt;l=52?q=r8diagnosticshandler, e.g.

=================== Stderr ===================
ERROR:Missing classes detected while running R8. Please add the missing classes or apply additional keep rules that are generated in xxxxx/debug/missing_rules.txt.
This will become an error in AGP 8.0.
ERROR:R8: Missing class test.B (referenced from: test.B test.A.foo())
Missing class test.C (referenced from: test.C test.A.bar())

So we only need to change the error message a little bit.

fixed with change Iee631c0faa8ac1b32579732dfd82f96921082672

I just updated to 8.0.0 alpha and started getting these errors:

ERROR:R8: Missing class android.support.v4.app.FragmentActivity (referenced from: void leakcanary.internal.AndroidSupportFragmentDestroyWatcher.invoke(android.app.Activity))
Missing class android.support.v4.app.FragmentManager$FragmentLifecycleCallbacks (referenced from: void leakcanary.internal.AndroidSupportFragmentDestroyWatcher$fragmentLifecycleCallbacks$1.<init>(leakcanary.internal.AndroidSupportFragmentDestroyWatcher) and 2 other contexts)
Missing class android.support.v4.app.FragmentManager (referenced from: void leakcanary.internal.AndroidSupportFragmentDestroyWatcher.invoke(android.app.Activity))
Missing class androidx.asynclayoutinflater.view.AsyncLayoutInflater$OnInflateFinishedListener (referenced from: void com.mapbox.maps.ViewAnnotationManagerImpl.addViewAnnotation(int, com.mapbox.maps.ViewAnnotationOptions, androidx.asynclayoutinflater.view.AsyncLayoutInflater, kotlin.jvm.functions.Function1))
Missing class androidx.asynclayoutinflater.view.AsyncLayoutInflater (referenced from: void com.mapbox.maps.ViewAnnotationManagerImpl.addViewAnnotation(int, com.mapbox.maps.ViewAnnotationOptions, androidx.asynclayoutinflater.view.AsyncLayoutInflater, kotlin.jvm.functions.Function1) and 1 other context)
Missing class androidx.work.multiprocess.RemoteListenableWorker (referenced from: leakcanary.internal.RemoteHeapAnalyzerWorker)

Do I just need to add the dontwarn commands that are generated to my config, or should I report these somewhere (here, or the library that is mentioned)?

The missing classes are from leakcanary (Square) and com.mapbox.maps (mapbox). If you don't have any control over these libraries adding the -dontwarn is one option. Another option will be to add more dependencies to your project with the missing classes. The missing classes are in code which R8 did not remove, but of course that can still be code which will never be hit at runtime.

I suggest that you also report this to the library authors, so they can update their libraries to avoid consumers getting these errors. On the library side they can either 1) add more dependencies or 2) add the -dontwarn to the library consumer keep rules. There can be good reasons for a library having missing classes if it has code which checks for different supported dependencies. E.g. leakcanary might work with both old support library and new Jetpack (androidx) libraries, so you chose the dependency and leakcanary will work with both.

Re #16: The error message should also point to AGP-generated keep rules file that you can add to your build configuration. Something like this.