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

Pom dependencies that should be "compile" are "runtime" #151

Closed
eli-darkly opened this issue Dec 20, 2018 · 7 comments
Closed

Pom dependencies that should be "compile" are "runtime" #151

eli-darkly opened this issue Dec 20, 2018 · 7 comments

Comments

@eli-darkly
Copy link
Contributor

In the process of fixing #122, it looks like we made an unintended change.

Previously: all of the SDK's dependencies were appearing in pom.xml with "compile" scope— even the ones that are bundled in the jar and shaded, and therefore should not be loaded separately. This was due to a problem in how we were configuring the Gradle Shadow plugin.

Now: the bundled dependencies are no longer in the pom, but Gson and SLF4J— which are not bundled, and must be provided by the host app— are listed with "runtime" scope. Therefore, Java build tools will not automatically include them in the classpath at compile time. You'll get a compile error if your application 1. does not explicitly pull in Gson and SLF4J in its own build configuration, and 2. does reference them in your code.

The workaround is simply to add those as explicit compile-time dependencies if they're not already there.

(This is for the default jar. The jar with the "all" classifier provides Gson and SLF4J itself, and therefore doesn't need them in the pom.)

@rkennedy-mode
Copy link

rkennedy-mode commented Jan 3, 2020

This is causing us active pain at the moment as we're having to manually manage which version of gson being used. dependabot then happily tells us it's behind v.latest and unhelpfully offers to upgrade us to gson v.latest, which potentially introduces a backwards incompatible change (see v2.8.5).

Compile fails even if you're not referencing any gson classes. Our code calls LDUser.Builder.custom(String, String) but the compiler complains because there's another version of that method that takes a gson JsonElement: LDUser.Builder.custom(String, JsonElement).

@eli-darkly
Copy link
Contributor Author

Could you say more about what you mean by "potentially introduces a backwards incompatible change"? That changelog item says that they changed utils.VersionUtils, which is not something that is referenced by the Java SDK, so I see no reason why you shouldn't be able to use a newer version. Since the SDK uses a fairly limited subset of Gson functionality and always uses it in the same ways, you should be able to tell right away whether there is actually a problem.

I know we still need to fix the dependency scopes, I'm just trying to understand why the current workaround is such a problem.

@eli-darkly
Copy link
Contributor Author

Also, is there a reason you can't use the "all" distribution that bundles Gson and SLF4j?

@rkennedy-mode
Copy link

That changelog item says that they changed utils.VersionUtils, which is not something that is referenced by the Java SDK, so I see no reason why you shouldn't be able to use a newer version.

LaunchDarkly is not the only library in my application that's using gson. I'm using other things that also use gson. protobuf (an indirect dependency of ours through some Google Cloud libraries we're using for BigQuery), for example, also uses gson. I can test that my own code doesn't call the affected methods, but it's going to be very difficult to tell if (at runtime) some part of some other library is going to want to call one of the affected methods. So it's best if I can allow my dependencies to specify their desired version of their dependencies.

Also, is there a reason you can't use the "all" distribution that bundles Gson and SLF4j?

The "all" distribution bundles gson and slf4j without shading them. The bundled version of slf4j is incompatible with our application, resulting in ClassCastExceptions.

@eli-darkly
Copy link
Contributor Author

I believe we now know how to fix this. Apologies for the delay— we wasted a lot of time on the assumption that we were using the Shadow plugin incorrectly, but it seems like this is actually a known issue in Shadow and just has to be worked around. And then it fell off of our radar at some point last year because no one else had mentioned it. We should have a release with the fix shortly.

A related question: would it be better for the dependency to specify a range like [2.7,3.0) rather than the current specific version? The SDK should not care if another library prefers a later version of Gson as long as it's in the same major version. The only reason I'm hesitating to make that change is that then we would probably need to have a separate build for the "all" jar (the one that bundles Gson), retaining a specific version dependency there, because otherwise we would not have a reproducible build of that distribution.

@eli-darkly
Copy link
Contributor Author

@rkennedy-mode We've released version 4.10.1 which should fix the scope problem; the question about dependency ranges is something that we can revisit separately.

@rkennedy-mode
Copy link

Thanks for the fast turnaround, Eli!

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

2 participants