-
Notifications
You must be signed in to change notification settings - Fork 159
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
Fix shading and upgrade dependencies #132
Conversation
upgrade all dependencies to their current version relocate a few more packages remove ivy-publish
commit 051f3e2aca47754356b926814ef8fadee70ff50c Author: Jilles van Gurp <jillesvangurp@users.noreply.github.com> Date: Tue Aug 28 11:43:22 2018 +0200 fix gradle to generate a pom file without the listed jars get rid of redundant config for ivy-publish commit f558a13 Author: Jilles van Gurp <jillesvangurp@users.noreply.github.com> Date: Mon Aug 27 17:51:50 2018 +0200 use compileOnly to not have shaded jars in pom commit 0dc71ef Merge: 02f01ba 6a6a8c4 Author: Jilles van Gurp <jillesvangurp@users.noreply.github.com> Date: Mon Aug 27 17:05:47 2018 +0200 Merge remote-tracking branch 'upstream/master' into update-dependencies commit 02f01ba Author: Jilles van Gurp <jillesvangurp@users.noreply.github.com> Date: Mon Aug 27 16:57:25 2018 +0200 fix jitpack build issues with missing install task commit 4b6fc30 Author: Jilles van Gurp <jillesvangurp@users.noreply.github.com> Date: Mon Aug 20 14:10:07 2018 +0200 Revert "use guava-26.1-android instead of 26.0-android" This reverts commit b31bfe6. wrong version commit b31bfe6 Author: Jilles van Gurp <jillesvangurp@users.noreply.github.com> Date: Mon Aug 20 14:07:50 2018 +0200 use guava-26.1-android instead of 26.0-android commit 45a4536 Author: Jilles van Gurp <jillesvangurp@users.noreply.github.com> Date: Mon Aug 20 14:05:30 2018 +0200 use android version of guava probably good idea to preserve compatibility with android commit 94023e2 Author: Jilles van Gurp <jillesvangurp@users.noreply.github.com> Date: Mon Aug 20 13:59:01 2018 +0200 fix mockito dependency and move it to testCompile commit 35d9f07 Author: Jilles van Gurp <jillesvangurp@users.noreply.github.com> Date: Mon Aug 20 13:53:26 2018 +0200 run gradle wrapper to update to 2.9 commit a83d0ea Author: Jilles van Gurp <jillesvangurp@users.noreply.github.com> Date: Mon Aug 20 13:53:00 2018 +0200 shade two more packages commit c3b0b14 Author: Jilles van Gurp <jillesvangurp@users.noreply.github.com> Date: Mon Aug 20 13:45:47 2018 +0200 update dependencies to current versions add two plugins for dealing with dependencies Merge remote-tracking branch 'upstream/master' into fix-shading-and-upgrade-dependencies
@bartekn I just merged latest changes from master. Would be nice to get this in now. |
distributionBase=GRADLE_USER_HOME | ||
distributionPath=wrapper/dists | ||
distributionUrl=https\://services.gradle.org/distributions/gradle-4.9-bin.zip |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I downloaded this file and noticed that gradle/wrapper/gradle-wrapper.jar
files differ. Why? Other than that LGTM!
I upgraded gradle. 4.10 should be out by now so we could upgrade it again.
Jilles
…On Thu, Sep 20, 2018, 17:38 Bartek Nowotarski ***@***.***> wrote:
***@***.**** commented on this pull request.
------------------------------
In gradle/wrapper/gradle-wrapper.properties
<#132 (comment)>
:
> distributionBase=GRADLE_USER_HOME
distributionPath=wrapper/dists
+distributionUrl=https\://services.gradle.org/distributions/gradle-4.9-bin.zip
I downloaded this file and noticed that gradle/wrapper/gradle-wrapper.jar
files differ. Why? Other than that LGTM!
—
You are receiving this because you authored the thread.
Reply to this email directly, view it on GitHub
<#132 (review)>,
or mute the thread
<https://github.com/notifications/unsubscribe-auth/AAx_8yBgNqpy1H0hlSKUS2X1PhrYtpWjks5uc7Z5gaJpZM4WPTV0>
.
|
I understand that it was because of the Gradle upgrade but the file I downloaded (the same version) is binary different that the one in this PR. |
Interesting. You may have pulled 4.10. i updated to 4.9 with a 'gradle
wrapper'.
…On Fri, Sep 21, 2018, 16:22 Bartek Nowotarski ***@***.***> wrote:
I understand that it was because of the Gradle upgrade but the file I
downloaded (the same version) is binary different that the one in this PR.
—
You are receiving this because you authored the thread.
Reply to this email directly, view it on GitHub
<#132 (comment)>,
or mute the thread
<https://github.com/notifications/unsubscribe-auth/AAx_823jCXWRYsGMdGPm7A3HrBZzGdsIks5udPYEgaJpZM4WPTV0>
.
|
@bartekn I've merged all the latest changes on master; minor conflict on build.gradle. I have no explanation for the binary differences on the jar. But to be sure, just updated once more to gradle 4.10.2 |
Using homebrew installed gradle and using |
The shading pr merged earlier still included all dependencies in the pom.xml, which defeats the purpose.
To be sure this now works correctly, I've released my fork on jitpack:
@bartekn I'm depending on this in my kotlin wrapper for the SDK; as soon as you merge and cut a new release I can switch to that.
To test this locally in the future, run
Then check the contents of
~/.m2/repository/stellar/java-stellar-sdk/0.3.2
It should list something like:
The important bit here is that we have a fat jar that has relocated dependencies and that the pom file does not have any dependencies (included in the jar). I also examine the contents of the jar to determine which packages need shading. So, when adding dependencies in the future, be sure to check they are relocated.