Skip to content
This repository has been archived by the owner on May 30, 2024. It is now read-only.

gson should be excluded from shading #103

Closed
pkwarren opened this issue Oct 13, 2017 · 10 comments
Closed

gson should be excluded from shading #103

pkwarren opened this issue Oct 13, 2017 · 10 comments

Comments

@pkwarren
Copy link

pkwarren commented Oct 13, 2017

In the launchdarkly client, all dependencies are shaded and relocated except for slf4j and gson. slf4j is not shaded (so will be required at runtime), however gson is shaded and not relocated. This will break dependency resolution and potentially lead to multiple versions of GSON on the classpath (for example, if an application uses launchdarkly-client and gson 2.8.0), both GSON 2.7 and 2.8.0 will be on the classpath at the same location.

If gson is not relocated when shading, it would be better to exclude it from shading and require it to be pulled in at runtime (i.e. add it here: https://github.com/launchdarkly/java-client/blob/8c381c8820d5e77d049eb6b57974182de611fc5e/build.gradle#L102). This would allow dependency resolution to work to pull in the appropriate version (and not break tools like https://maven.apache.org/enforcer/enforcer-rules/requireUpperBoundDeps.html).

@pkaeding
Copy link
Contributor

Hi @pkwarren

We include the dependencies (mostly shaded) in the default jar, to avoid conflicts where possible, but we can't shade the GSON dependency because it is part of the SDK's public API.

That said, you are right that it doesn't make sense to include the dependency if we are not shading it. I will work on fixing this, so the default jar does not include GSON.

We do provide a 'thin' jar, which has nothing included, and it will levae all dependency resolution up to maven. To use that, just add the thin classifier.

@pkaeding
Copy link
Contributor

Hi @pkwarren

I just published version 2.3.3 which should fix this problem for you. Please let me know if you have any further issues.

Thanks!

@bryanrcampbell
Copy link

bryanrcampbell commented Oct 19, 2017

Hi @pkaeding - I pulled in 2.3.3 to see if that resolved the problem but i'm seeing the same behavior as 2.3.2. I diffed the two versions and it looks like they look identical. Maybe the change didn't make it into master before you cut? Could you take a look? Thanks.

@pkaeding
Copy link
Contributor

Hi @bryanrcampbell hmm, you're right. It looks like something went wrong between when I ran the build locally and when our CI system did so, and published. I will look into this further.

@pkaeding pkaeding reopened this Oct 19, 2017
@bryanrcampbell
Copy link

Hi @pkaeding - were you able to track down what went wrong with 2.3.3? Thanks.

@pkaeding
Copy link
Contributor

Hi @bryanrcampbell

My apologies-- I screwed up when I performed the 2.3.3 release. I just pushed 2.3.4, and confirmed that the artifact on maven central does not include the GSON class files.

Please give 2.3.4 a shot, and let me know if that works for you.

@bryanrcampbell
Copy link

bryanrcampbell commented Oct 26, 2017

looks good to me - thank you!

@pkaeding
Copy link
Contributor

Great, I will close this issue. Of course, please let us know if you run into any other problems.

@apottere
Copy link

I just chased down this issue because our enforcer was complaining about duplicate classes - it would be good to update the version of the client in the quickstart so others get this fix: https://docs.launchdarkly.com/docs/getting-started

As of the time of writing it says to install 2.3.2.

@sarahlessner
Copy link

Thank you for bringing this to our attention. The docs have been updated to reflect the current version, 2.5.0.

eli-darkly added a commit that referenced this issue Oct 26, 2018
test that flags loaded from a file actually work
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

5 participants