Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Minify + Shrink resources removes String resources #331

Closed
AllanWang opened this issue Jul 8, 2017 · 20 comments

Comments

@AllanWang
Copy link
Contributor

commented Jul 8, 2017

Obviously this is the case, and it can be fixed by keeping the R class, but I was wondering if there is hopefully a better alternative somehow. -keepresources looked promising but it seems to be a dexguard feature rather than a proguard feature.

In comparison, keeping all the R classes adds around 60kb to a < 3mb app, which is somewhat significant.

@rubengees

This comment has been minimized.

Copy link
Contributor

commented Jul 8, 2017

I had success with this:

-keepclasseswithmembers class **.R$* {
    public static final int define_*;
}

This only keeps the definitions used by AboutLibraries for autodetection but no other Strings.

@mikepenz

This comment has been minimized.

Copy link
Owner

commented Jul 8, 2017

@rubengees but wouldn't this also remove the strings of the libs themself? So do you add them manually to your project in addition?

@mikepenz mikepenz self-assigned this Jul 8, 2017

@mikepenz mikepenz added the question label Jul 8, 2017

@rubengees

This comment has been minimized.

Copy link
Contributor

commented Jul 8, 2017

Hmm, no it works out of the box in my case. Maybe proguard intelligently recognizes the usage by the library? I do not use shrinkResources, but only proguard itself though.

@AllanWang

This comment has been minimized.

Copy link
Contributor Author

commented Jul 8, 2017

Shrink resources is the issue, not proguard.
I'm going to try what you have plus
public static final int library_*; and see if that works

@mikepenz

This comment has been minimized.

Copy link
Owner

commented Jul 8, 2017

It can't as those are loaded during the runtime. So this means your definition still keeps the resources.

@AllanWang I do not think there is anything I can do about this, or the lib can do about it.

@rubengees

This comment has been minimized.

Copy link
Contributor

commented Jul 8, 2017

Just tried it with shrinkResources enabled on my project, that works.

@AllanWang

This comment has been minimized.

Copy link
Contributor Author

commented Jul 8, 2017

@rubengees is your project open sourced?

@AllanWang

This comment has been minimized.

Copy link
Contributor Author

commented Jul 8, 2017

Also since this keeps the whole R class, I'm not sure if this will be better than the original rules

@rubengees

This comment has been minimized.

Copy link
Contributor

commented Jul 8, 2017

Yes, here here are the proguard rules: https://github.com/proxer/ProxerAndroid/blob/master/app/proguard-rules.pro

Also since this keeps the whole R class, I'm not sure if this will be better than the original rules

I am not a proguard expert so this may be true. It did help me avoid the 65k field reference limit though, which popped up due to another lib (I only had the rule to keep entire R from the README before).

@mikepenz

It can't as those are loaded during the runtime.

As far as I know, proguard is able to recognize patterns of Strings when using reflection and keeps those in the resources. If that is the case, I do not know, why the define_ statements does not get keeped...

@rubengees

This comment has been minimized.

Copy link
Contributor

commented Jul 8, 2017

@AllanWang

From the proguard documentation:

-keepclasseswithmembers [,modifier,...] class_specification
Specifies classes and class members to be preserved, on the condition that all of the
specified class members are present. For example, you may want to keep all
applications that have a main method, without having to list them explicitly.

I think that means that entire R does not get keeped, but only the fields we use or specify with this rule.

@AllanWang

This comment has been minimized.

Copy link
Contributor Author

commented Jul 8, 2017

@rubengees I think it means that it does, since the only constraint is that a class contains a define_ string and it's preserving the entire class rather than just some members. I'm still waiting for travis to build so I can decompile it later. Plus I think the reason it works with define_ is because the rest of the class is kept, otherwise all your library_ strings will be gone too.

(Not a proguard expert either, so what you're saying is ideally what I want to happen)

@mikepenz

This comment has been minimized.

Copy link
Owner

commented Jul 8, 2017

The only thing which can be maybe done in the future is to split up the core, and move all libraries to a different module. So you can either use the module with all the resources, or you manually add the resources you want

@rubengees

This comment has been minimized.

Copy link
Contributor

commented Jul 8, 2017

I just decompiled my app and the R file is really small. I think it works the way we want to (I could be mistaken of course).

@mikepenz

This comment has been minimized.

Copy link
Owner

commented Jul 8, 2017

@rubengees just drag&drop the apk onto Android Studio 3.0, it has a really cool visualisation, and also shows the contained resources

@AllanWang

This comment has been minimized.

Copy link
Contributor Author

commented Jul 8, 2017

@mikepenz The issue still stands with all the external libraries with their strings,

Using the rules defined above definitely cleans it a fair bit though, since it seems to remove all the appcompat resources.

My R class with from 6k variables to 3k, and About library went from 3709 to ~300.

For the app, the size

  • with the proguard above is 3.92
  • without proguard is 3.98
  • with proguard and no R restrictions is 3.91
@mikepenz

This comment has been minimized.

Copy link
Owner

commented Jul 8, 2017

well I can't do anything about external libraries.

The only reason the R class has to be kept is for the auto-detection. You can enable proguard, don't add the rule, and just manually set which libraries needs to show up

@AllanWang

This comment has been minimized.

Copy link
Contributor Author

commented Jul 8, 2017

Our cases are for proguard + shrink + no auto detection. Because without this rule those strings will be gone anyways. I don't think any changes need to be done on your end since it's fine the way it is, but perhaps the proguard lines can be added to the readme for those who want to shrink their resources as much as possible and are willing to input libraries manually.

Thanks rubengees!

-keepclasseswithmembers class **.R$* {
    public static final int define_*;
}
@rubengees

This comment has been minimized.

Copy link
Contributor

commented Jul 8, 2017

@AllanWang My pleasure :)
@mikepenz I could send a PR if you like.

@mikepenz

This comment has been minimized.

Copy link
Owner

commented Jul 8, 2017

@rubengees already writing the README update :)

@mikepenz

This comment has been minimized.

@mikepenz mikepenz closed this Jul 8, 2017

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
3 participants
You can’t perform that action at this time.