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

Fix shading and upgrade dependencies #132

Merged
merged 6 commits into from Oct 16, 2018
Merged

Fix shading and upgrade dependencies #132

merged 6 commits into from Oct 16, 2018

Conversation

jillesvangurp
Copy link
Contributor

@jillesvangurp jillesvangurp commented Aug 28, 2018

The shading pr merged earlier still included all dependencies in the pom.xml, which defeats the purpose.

  • I fixed this with some gradle config to customize the pom file to not do that.
  • I also updated all the dependencies to their latest version
  • I removed ivy-publish in favor of the maven plugin (supported by the shadow plugin) and added two plugins to make examining the dependencies easier.
  • I moved mockito to the testCompile classpath. I assume this was there by mistake.

To be sure this now works correctly, I've released my fork on jitpack:

    implementation 'com.github.Inbot:java-stellar-sdk:v0.3.2.1-inbot-shaded-7'

@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

./gradlew clean -xtest install

Then check the contents of ~/.m2/repository/stellar/java-stellar-sdk/0.3.2

It should list something like:

-rw-r--r--  1 jillesvangurp  staff   6.5M Aug 28 11:52 java-stellar-sdk-0.3.2-all.jar
-rw-r--r--  1 jillesvangurp  staff   6.5M Aug 28 11:52 java-stellar-sdk-0.3.2.jar
-rw-r--r--  1 jillesvangurp  staff   396B Aug 28 11:52 java-stellar-sdk-0.3.2.pom

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.

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
@jillesvangurp
Copy link
Contributor Author

@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
Copy link
Contributor

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!

@jillesvangurp
Copy link
Contributor Author

jillesvangurp commented Sep 21, 2018 via email

@bartekn
Copy link
Contributor

bartekn commented Sep 21, 2018

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.

@jillesvangurp
Copy link
Contributor Author

jillesvangurp commented Sep 22, 2018 via email

@jillesvangurp
Copy link
Contributor Author

@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

@jillesvangurp
Copy link
Contributor Author

Using homebrew installed gradle and using gradle wrapper

@bartekn bartekn merged commit 89429eb into lightsail-network:master Oct 16, 2018
@jillesvangurp jillesvangurp deleted the fix-shading-and-upgrade-dependencies branch October 16, 2018 09:50
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

Successfully merging this pull request may close these issues.

2 participants