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

Old ProGuard version is used #724

Closed
WonderCsabo opened this issue Apr 22, 2016 · 15 comments
Closed

Old ProGuard version is used #724

WonderCsabo opened this issue Apr 22, 2016 · 15 comments

Comments

@WonderCsabo
Copy link
Contributor

In the proguard task, the proguard jar is used which can be found in Android/sdk/tools/proguard. However this is a really old version. The Android Gradle plugin uses the latest version, i guess it just resoles the dependency from Maven Central. This plugin should also use the latest ProGuard jar.

@william-ferguson-au
Copy link
Contributor

I thought you could override the Proguard library by adding a dependency to the plugin.
Ie adding this as a dependency to the android-maven-plugin.

               <dependencies>
                        <dependency>
                            <groupId>net.sf.proguard</groupId>
                            <artifactId>proguard-base</artifactId>
                            <version>5.2.1</version>
                        </dependency>
                    </dependencies>

It's what I've been doing.

@mosabua
Copy link
Member

mosabua commented Apr 22, 2016

We had it as a dependency in the past and changed to pulling it from the SDK since that was better supported (or so we thought anyway). I am more than happy to change it back to an explicit dependency on proguard and upgrade to the latest? Wanna send a PR anyone?

@william-ferguson-au
Copy link
Contributor

Ahh, this is probably why I haven't been able to move past AMP-4.3

Anyone know how easy to would be o pull it from the dependencies if it is listed, but if not, fall back to whatever is provided by Android SDK?

The nice thing about deferring to Android SDK is that means they need to manage the compatibility between Proguard and Android Dex. But by allowing that to be overridden by a dependency declaration it means uses aren't limited by that.

@WonderCsabo
Copy link
Contributor Author

We should replicate the behavior of the gradle plugin. But I am not sure how the gradle plugin chooses the ProGuard version. I will investigate later.

@WonderCsabo
Copy link
Contributor Author

Adding as a dependency definitely allows more config for the user @william-ferguson-au.

@william-ferguson-au
Copy link
Contributor

william-ferguson-au commented Apr 23, 2016

I'm in two minds on this.

Option 1: Replicating what the Gradle plugin does
Pros:

  1. Users moving from a Gradle build to a Maven build will get the same experience.
  2. Simplified config for users wanting the latest (whatever that means) version of Proguard.

Cons:

  1. Gradle is not necessarily doing the correct or expected thing.
  2. AMP assumes responsibility for ensuring compatibility between whatever version of Proguard we bind to and Dex.

Option 2: Use Android SDK version of Proguard unless overridden by user.

Pros:

  1. Continues default AMP behaviour unless overridden.
  2. Allows user to choose the version of Proguard they want to use.
  3. Responsibility for compatibility between Proguard and Dex is user's or Android SDK's, not ours.

Cons:

  1. More verbose config if user wants different version of Proguard than Android SDK.

Don't get me wrong. Personally I'm all for using more recent versions of Proguard.
But forcing a version on user's may come back to bite us.

@mosabua
Copy link
Member

mosabua commented Apr 23, 2016

I prefer Option 2 to be honest. Maybe all we have to do is make it clearer in the docs on how to override the proguard version. I remember writing a blog post about this years ago ..

What if we just add some docs here

http://simpligility.github.io/android-maven-plugin/proguard.html

@william-ferguson-au
Copy link
Contributor

Hmm, this http://simpligility.github.io/android-maven-plugin/proguard-mojo.html#proguardProguardJarPath suggests that the Proguard task already does (2).

@mosabua
Copy link
Member

mosabua commented Apr 25, 2016

Thats what I thought from memory but did not get around to look it up. I think we really only should update the docs at this stage.

@malachid
Copy link
Contributor

Just as a side note, this last week I was debugging two service providers
who were showing "(Unknown Source)" in the retraced stack trace. Turns out
that, since the app was being built with gradle (and thus using 5.2.1) the
Android SDK version (4.7) failed to retrace correctly.

IMHO there should probably be a bug against the Android SDK for it being 5
years out of date, if there isn't already. Having Gradle and Maven
generating mapping.txt files in different formats is likely to cause
confusion.

I think the correct solution is to probably default to the current 2015
version, not the 2011 version. Allowing it to be overridden is fine - but
nothing should default to a 5 year old version.
On Apr 24, 2016 9:39 PM, "Manfred Moser" [email protected] wrote:

Thats what I thought from memory but did not get around to look it up. I
think we really only should update the docs at this stage.


You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub
#724 (comment)

@mosabua
Copy link
Member

mosabua commented Apr 25, 2016

Maybe we should invert the logic then.. use the one we add as dependency from the classpath and have a flag that replaces it with the one from the SDK. .. although I am not sure that works. So maybe forget about the one from the SDK .. seems like its not maintained if its that old.. wdyt @simpligility/android-maven-plugins-core-committers

@WonderCsabo
Copy link
Contributor Author

WonderCsabo commented Apr 25, 2016 via email

@william-ferguson-au
Copy link
Contributor

OK, I'm leaning towards using the latest overridden by whatever it defined as a plugin dependency. But let's serve as default whatever the Gradle plugin is using.

@mosabua
Copy link
Member

mosabua commented Apr 26, 2016

I am more leaning towards latest than towards following the Gradle plugin. And I am fine to abandon the one from the SDK as well..

@william-ferguson-au
Copy link
Contributor

OK. I'm out voted :)
Go for it Csaba

On Tue, 26 Apr 2016, 2:59 PM Manfred Moser [email protected] wrote:

I am more leaning towards latest than towards following the Gradle plugin.


You are receiving this because you were mentioned.

Reply to this email directly or view it on GitHub
#724 (comment)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants