facebook / flipper

A desktop debugging platform for mobile developers.
https://fbflipper.com/
MIT License
13.36k stars 955 forks source link

[Android] Proguard rules for Flipper #314

Closed simaofelgueirasJM closed 3 years ago

simaofelgueirasJM commented 6 years ago

I am migrating from Stetho to Flipper. For Stetho in proguard I had something like this:

-keep class com.facebook.stetho.** { *; } -dontwarn com.facebook.stetho.**

Use case: What I am trying to do is to be able of keep debugging with my nearest build for release.

Any suggestions for Flipper proguard rules? I am searching and did not found it. Currently having problems when the app builds with proguard.

I managed to put it work on my proguard-debug.txt with this rules

-keep class com.facebook.** { *; } -dontwarn com.** -dontwarn org.** -dontwarn java.**

The -dontwarn let me force the build, and it seems to work fine. Can you guide me for a better solution?

ColtonIdle commented 5 years ago

Retrofits README includes this statement:

R8 / ProGuard

If you are using R8 the shrinking and obfuscation rules are included automatically.

ProGuard users must manually add the options from this file. (Note: You might also need rules for OkHttp and Okio which are dependencies of this library)

It would be nice if Flipper could do the same.

SaqibJDev commented 5 years ago

I was able to make it work with -keep class com.facebook.** { *; } only.

dral3x commented 5 years ago

I was able to make it work with just 2 lines:

-keep class com.facebook.jni.** { *; }
-keep class com.facebook.flipper.** { *; }

If you use -keep class com.facebook.** { *; }, you're also keeping the whole FB SDK, not just flipper.

simaofelgueirasJM commented 5 years ago

That's true, I still need the -dontwarn lines, did not research a lot about it. Since it is not a release build I did not care much.

The rules that I figure it out to be the most specific ones for flipper are this ones:

-keep class com.facebook.jni.**  {  *;  }
-keep class com.facebook.flipper.** {  *;  }

-dontwarn com.facebook.litho.**
-dontwarn com.facebook.flipper.**
-dontwarn com.facebook.yoga.**
-dontwarn org.mozilla.**
-dontwarn  com.facebook.fbui.**
passy commented 5 years ago

@simaofelgueirasJM You may want to use ``` to surround your code block with, otherwise GitHub will interpret the asterisks as Markdown syntax.

simaofelgueirasJM commented 5 years ago

Ya, really thank you @passy , the answer is updated. I completely forgot that, because did not consider the proguard rules to be code and for some reason the rest was working as expected.

passy commented 5 years ago

I'm cleaning up some old issues and this one hasn't seen updates in a long time. Please let me know if this is still a problem and we can reopen it. Thanks!

ColtonIdle commented 5 years ago

@passy I always thought of this ticket as providing documentation for how to support R8/Proguard in your app. In that case I would keep it open, or I can just open a new ticket requesting documentation for it. Most Android libraries at this point have explicit directions on how to use with R8/Proguard because Google has been really pushing R8 in my opinion. It'd be nice to have a canonical reference of how to use R8/Proguard effectively with Flipper and Flipper no-op artifacts.

passy commented 5 years ago

@ColinCampbell I suppose my concern about adding documentation for using Flipper with proguard is that Flipper should generally not make it into a release build to which Proguard is applied.

I would like to understand the scenarios in which this is necesassary better before adding this to our docs.

ColtonIdle commented 5 years ago

In that case, if it doesn't need any proguard rules for release builds, then I guess I'm okay with that.

I don't really know how no-op dependencies work, and didn't know if doing Flipper initialization with a no-op dependency + Proguard (stripping some methods out) would potentially lead to a crash during the init of Flipper. I'm so used to going line by line in my dependencies and reading each dependency README for how to use with proguard and I thought it'd be nice if Flipper did the same for those with less experience with proguard but still want to take googles advice of enabling it on release builds.

dral3x commented 5 years ago

I would like to understand the scenarios in which this is necesassary better before adding this to our docs.

@passy I have Proguard enabled also in debug builds so when I build the release version, there are no surprises in production.

simaofelgueirasJM commented 5 years ago

@passy Flipper also does not make part of my release build and I do not run Proguard in my debug build. However, for my use case I have a candidate build where I have interest to run Proguard and still be able to use flipper. Candidate is a pre-release build that Quality Assurance Engineers (QAE) use to test and analyze all the intended scenarios. On our side flipper is used mostly by QAEs.

passy commented 5 years ago

Thanks a lot for sharing your scenarios, this helps a lot! I'll try to enable minifyEnabled on the debug sample app and see what it is still required to make this work. I definitely see the need for some more accessible documentation for this now.

stale[bot] commented 4 years ago

Hey there, it looks like there has been no activity on this issue recently. Has the issue been fixed, or does it still require the community's attention? This issue may be closed if no further activity occurs. You may also label this issue as a "Discussion" or add it to the "Backlog" and I will leave it open. Thank you for your contributions.

ColtonIdle commented 4 years ago

I would still say that this is valid?

carlonzo commented 4 years ago

yeah, this is still an issue with latest version. if I have minifyEnabled and run the app, it crashes

stale[bot] commented 3 years ago

Hey there, it looks like there has been no activity on this issue recently. Has the issue been fixed, or does it still require the community's attention? This issue may be closed if no further activity occurs. You may also label this issue as a "Discussion" or add it to the "Backlog" and I will leave it open. Thank you for your contributions.

stale[bot] commented 3 years ago

Closing this issue after a prolonged period of inactivity. If this issue is still present in the latest release, please feel free to create a new issue with up-to-date information.

Whathecode commented 3 years ago

Seems this was closed due to oversight. 👁️ @passy's decision seemed to have been to include it in documentation.

Here is a related thread on React native community support: ClassNotFoundException: Didn't find class "com.facebook.jni.NativeRunnable"

But, applying the rules mentioned there gives me: Failed to register native method com.facebook.flipper.android.FlipperClientImpl.stop()

So, I'm still using the following, which seems overly inclusive.

-keep class com.facebook.jni.** { *; }
-keep class com.facebook.flipper.** { *; }