JakeWharton / ThreeTenABP

An adaptation of the JSR-310 backport for Android.
Apache License 2.0
3.55k stars 133 forks source link

R8 issue with Ser #122

Closed eygraber closed 4 years ago

eygraber commented 4 years ago

It looks like the public constructor is getting stripped.

Our solution is:

-keepclassmembers class org.threeten.bp.Ser {
    <init>();
}

but not sure if more would need to be done.

wbervoets commented 4 years ago

I can verfiy that the above solution is working. I'm using the R8 version bundled with Android Studio 3.6.0

JakeWharton commented 4 years ago

We should see if threetenbp would ship rules inside their jar before adding to this library. Does anyone want to file an issue upstream?

On Wed, Mar 11, 2020, at 5:42 AM, wbervoets wrote:

I can verfiy that the above solution is working. I'm using the R8 version bundled with Android Studio 3.6.0

— You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub https://github.com/JakeWharton/ThreeTenABP/issues/122?email_source=notifications&email_token=AAAQIELNWTOS5LNZQG3QGW3RG5MKFA5CNFSM4KMFPGS2YY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOEOO2UYY#issuecomment-597535331, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAAQIEPB4VIIOS6A5PJ5O3DRG5MKFANCNFSM4KMFPGSQ.

JakeWharton commented 4 years ago

Which type were you serializing when you ran into this? I'm trying to write a test for it.

We actually already have a serialization test for ZonedDateTime passing through Java serialization in a build with minification enabled because there was an issue once where it lost information.

eygraber commented 4 years ago

I believe it was LocalTime

JakeWharton commented 4 years ago

I cannot reproduce the crash. What version of AGP are you on?

eygraber commented 4 years ago

We've had the proguard rule in for a while now. I can dig and see what version we were on when the bug was filed.

JakeWharton commented 4 years ago

Thanks. I might just ship the update which bumps the threetenbp without a regression test. I expect this project to enter maintenance mode with AGP 4.0 shipping desugaring of java.time anyway. This project is already low-overhead so it's not like things are changing rapidly.

eygraber commented 4 years ago

Yeah we're hoping to start relying on the desugaring once AGP 4 is stable.

JakeWharton commented 4 years ago

I would strongly recommend someone trying it ASAP while problems can still be addressed. It's already getting late in the 4.0 cycle. You should be able to mass replace imports and smoke test is fairly quickly.

JakeWharton commented 4 years ago

Are you running R8 in full mode?

JakeWharton commented 4 years ago

Reproduced!

JakeWharton commented 4 years ago

AGP 3.6 and R8 full mode causes the existing serialization test to fail because of Ser's missing ctor (on 1.4.1). Bumping to 1.4.2 fixes it.

wbervoets commented 4 years ago

Sorry I'm too late commenting... :) I indeed had the issue on AGP 3.6.x and R8 full mode enabled on 1.4.1

eygraber commented 4 years ago

We also use full mode.

And the java.time desugaring seems to be working well.