dagger-reflect icon indicating copy to clipboard operation
dagger-reflect copied to clipboard

Add R8/ProGuard rules to the codegen artifact

Open JakeWharton opened this issue 6 years ago • 11 comments

Also process the codegen test code with both R8 and ProGuard to ensure they work correctly in practice.

JakeWharton avatar Jul 14 '19 21:07 JakeWharton

The ruleset crashed on my branch using the activity in integration-tests:android. https://github.com/Laimiux/dagger-reflect/tree/laimonas/r8_rule_test.

To replicate the issue:

  1. I've replaced proguard-rules.pro with the new rules in this PR
  2. Ran ./gradlew :integration-tests:android:installCodegenDebug

The current broader ruleset works.

Laimiux avatar Jul 15 '19 18:07 Laimiux

Thanks. I'll look soon. Although I'd be happy to land this as-is and fix that separately as a bug.

JakeWharton avatar Jul 18 '19 14:07 JakeWharton

Using this ruleset even basic use of AppComponent crashes. I suspect that the R8 doesn't run correctly during the test you have written because that exact use case crashes for me.

Laimiux avatar Jul 18 '19 17:07 Laimiux

I might be misunderstanding how it works, do we need to mark components with @Keep?

Laimiux avatar Jul 18 '19 17:07 Laimiux

When I wrote them I was inspecting the classfile output of both tools and both looked correct.

On Thu, Jul 18, 2019 at 1:22 PM Laimonas Turauskas [email protected] wrote:

Using this ruleset even basic use of AppComponent crashes. I suspect that the R8 doesn't run correctly during the test you have written because that exact use case crashes for me.

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub https://github.com/JakeWharton/dagger-reflect/pull/143?email_source=notifications&email_token=AAAQIEIPJPSV5GZJJVRWF4LQACREDA5CNFSM4IDSBLTKYY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOD2JGBRA#issuecomment-512909508, or mute the thread https://github.com/notifications/unsubscribe-auth/AAAQIEK2NTOEMFMZISL4CALQACREDANCNFSM4IDSBLTA .

JakeWharton avatar Jul 18 '19 17:07 JakeWharton

Any ideas of what could be the cause for the crash?

Laimiux avatar Jul 18 '19 18:07 Laimiux

I haven't had time to look at it yet so I don't even know what's crashing.

On Thu, Jul 18, 2019 at 2:26 PM Laimonas Turauskas [email protected] wrote:

Any ideas of what could be the cause for the crash?

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub https://github.com/JakeWharton/dagger-reflect/pull/143?email_source=notifications&email_token=AAAQIEKL4XJOIKKPJR5XFGTQACYUFA5CNFSM4IDSBLTKYY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOD2JLUDQ#issuecomment-512932366, or mute the thread https://github.com/notifications/unsubscribe-auth/AAAQIEMCOAVEK5PUBP2STP3QACYUFANCNFSM4IDSBLTA .

JakeWharton avatar Jul 18 '19 18:07 JakeWharton

@JakeWharton Played around a bit, but couldn't make it work.

  1. In the current state applying the proguard rules, DaggerAppComponent is not kept:
// integration-tests/android/build.gradle
  buildTypes {
    debug {
      minifyEnabled true
      shrinkResources minifyEnabled
    }
  }
     at android.app.ActivityThread.handleBindApplication(ActivityThread.java:5876)
        at android.app.ActivityThread.access$1100(ActivityThread.java:199)
        at android.app.ActivityThread$H.handleMessage(ActivityThread.java:1650)
        at android.os.Handler.dispatchMessage(Handler.java:106)
        at android.os.Looper.loop(Looper.java:193)
        at android.app.ActivityThread.main(ActivityThread.java:6669)
        at java.lang.reflect.Method.invoke(Native Method)
        at com.android.internal.os.RuntimeInit$MethodAndArgsCaller.run(RuntimeInit.java:493)
        at com.android.internal.os.ZygoteInit.main(ZygoteInit.java:858)
     Caused by: java.lang.IllegalStateException: Unable to find generated component implementation com.example.DaggerAppComponent for component com.example.AppComponent
        at a.b.a.c(:61)
        at a.b.a.a(:25)
        at a.a.a(:22)
        at com.example.ExampleApp.onCreate(:14)
        at android.app.Instrumentation.callApplicationOnCreate(Instrumentation.java:1154)
        at android.app.ActivityThread.handleBindApplication(ActivityThread.java:5871)
        at android.app.ActivityThread.access$1100(ActivityThread.java:199) 
        at android.app.ActivityThread$H.handleMessage(ActivityThread.java:1650) 
        at android.os.Handler.dispatchMessage(Handler.java:106) 
        at android.os.Looper.loop(Looper.java:193) 
        at android.app.ActivityThread.main(ActivityThread.java:6669) 
        at java.lang.reflect.Method.invoke(Native Method) 
        at com.android.internal.os.RuntimeInit$MethodAndArgsCaller.run(RuntimeInit.java:493) 
        at com.android.internal.os.ZygoteInit.main(ZygoteInit.java:858) 
     Caused by: java.lang.ClassNotFoundException: Didn't find class "com.example.DaggerAppComponent" on path: DexPathList[[zip file "/data/app/com.example-cfNVrdQea-4kuqzGxdZGhg==/base.apk"],nativeLibraryDirectories=[/data/app/com.example-cfNVrdQea-4kuqzGxdZGhg==/lib/x86_64, /system/lib64]]
        at dalvik.system.BaseDexClassLoader.findClass(BaseDexClassLoader.java:134)
        at java.lang.ClassLoader.loadClass(ClassLoader.java:379)
        at java.lang.ClassLoader.loadClass(ClassLoader.java:312)
        at a.b.a.c(:54)
        at a.b.a.a(:25) 
        at a.a.a(:22) 
        at com.example.ExampleApp.onCreate(:14) 
        at android.app.Instrumentation.callApplicationOnCreate(Instrumentation.java:1154) 
        at android.app.ActivityThread.handleBindApplication(ActivityThread.java:5871) 
        at android.app.ActivityThread.access$1100(ActivityThread.java:199) 
        at android.app.ActivityThread$H.handleMessage(ActivityThread.java:1650) 
        at android.os.Handler.dispatchMessage(Handler.java:106) 
        at android.os.Looper.loop(Looper.java:193) 
        at android.app.ActivityThread.main(ActivityThread.java:6669) 
        at java.lang.reflect.Method.invoke(Native Method) 
        at com.android.internal.os.RuntimeInit$MethodAndArgsCaller.run(RuntimeInit.java:493) 
        at com.android.internal.os.ZygoteInit.main(ZygoteInit.java:858) 
  1. The rule looks correct, but doesn't keep:
-whyareyoukeeping class com.example.AppComponent
-whyareyoukeeping class com.example.DaggerAppComponent
android.enableR8=true

com.example.AppComponent
|- is referenced in keep rule:
|  <PROGUARD_COMPATIBILITY_RULE>:Unknown
Nothing is keeping com.example.DaggerAppComponent
android.enableR8=true
android.enableR8.fullMode=true

Nothing is keeping com.example.AppComponent
Nothing is keeping com.example.DaggerAppComponent
android.enableR8=false

com.example.AppComponent
  is invoked by    com.example.DaggerAppComponent.create (32:32)
  is kept by a directive in the configuration.

com.example.DaggerAppComponent
  is kept by a directive in the configuration.

# but still crashes with java.lang.ClassNotFoundException: Didn't find class "com.example.Daggera"
  1. Manually adding the rules, everything works with all variants of shrinking:
-keepnames class com.example.AppComponent
-keep class com.example.DaggerAppComponent {
  static ** create();
}
  1. Whenever I looked in the APK classes.dex, it matched the whyareyoukeeping output.

TWiStErRob avatar Aug 03 '19 02:08 TWiStErRob

  1. Manually adding the rules, everything works with all variants of shrinking:
-keepnames class com.example.AppComponent
-keep class com.example.DaggerAppComponent {
  static ** create();
}
  1. Whenever I looked in the APK classes.dex, it matched the whyareyoukeeping output.

You stated works and matched @TWiStErRob, did the point 3 & 4 solved the crashing issue?

mochadwi avatar Apr 08 '20 20:04 mochadwi

Uuuuh, @mochadwi I have no memory of doing this investigation :D, but here are my thoughts: 3. works: I said "everything works", from me that usually means app works as expected, no warnings, no errors, no crashes, just works. 4. matched: I validated whether ProGuard/r8's -whyareyoukeeping log output matches what's actually bundled in the classes.dex inside the minified APK.

The whole point of the comment is validating @Laimiux's claim that in the current state it crashes, and trying to find out why to help @JakeWharton, which is: the keeps to achieve what's in 3. is missing (or handled differently in all 3 shrinking modes as seen in 2.) from the generic rules in this PR.

So of course, manually keeping your @Components and their reflectively accessed generated implementations works as expected, but the goal of this PR is to make it work in a general case (only 1.) without having to add our entry points manually (3.). to our ProGuard rules.

TWiStErRob avatar Apr 08 '20 23:04 TWiStErRob

Uuuuh, @mochadwi I have no memory of doing this investigation :D, but here are my thoughts: 3. works: I said "everything works", from me that usually means app works as expected, no warnings, no errors, no crashes, just works. 4. matched: I validated whether ProGuard/r8's -whyareyoukeeping log output matches what's actually bundled in the classes.dex inside the minified APK.

The whole point of the comment is validating @Laimiux's claim that in the current state it crashes, and trying to find out why to help @JakeWharton, which is: the keeps to achieve what's in 3. is missing (or handled differently in all 3 shrinking modes as seen in 2.) from the generic rules in this PR.

So of course, manually keeping your @Components and their reflectively accessed generated implementations work as expected, but the goal of this PR is to make it work in a general case (only 1.) without having to add our entry points manually (3.). to our ProGuard rules.

Nice, I really appreciate it for your answer. That's explained everything about this issue still open. @TWiStErRob

mochadwi avatar Apr 13 '20 18:04 mochadwi