Shading: Relocate com.github.jnr artifacts to avoid classpath issues

[mkjensen]

Trying to fix a conflict with cassandra-driver-core (which has a couple of dependencies to com.github.jnr artifacts) by relocating package jnr to com.spotify.docker.client.shaded.jnr.

As discussed at #272 (comment).

I did try to run tests locally but I had failures even before my changes, probably because my Docker setup is a bit complicated with some custom network details. I'm guessing that by creating this PR the tests will be run automatically.

[mkjensen]

Hmm, I thought that creating this PR would trigger a build running tests using Travis but that doesn't seem to happen?

[mkjensen]

I actually ended up using the non-shaded version of docker-client (and I seem to be getting away with it as long as I ignore some missinglink stuff, uh oh) because the shaded one isn't a separate artifact and doesn't have a dependency reduced POM as already discussed in #272.

I get that it's probably a larger effort to fix the shading stuff so that won't just happen. Anyway, you are very welcome to use the change in this PR if it makes sense to you or decline it otherwise.

[mattnworb]

Travis should run - let me take a look at that.

Do you have more info on what the dependency conflict/problems were? Could they have been resolved by making sure your Maven project used the highest jnr version that both dependencies (cassandra-driver-core, docker-client) need? Or do they depend on different major versions that are API-incompatible with each other?

the reason why I ask is that it feels to me like relocating dependencies in docker-client should be a last resort when there is no other simple way to resolve conflicts - otherwise I could see the list of things we relocate continue to grow until it we relocate every dependency used in docker-client.

[mattnworb]

for some reason Travis thought this repo was "inactive", but that seems to be fixed now.

[mkjensen]

Do you have more info on what the dependency conflict/problems were? Could they have been resolved by making sure your Maven project used the highest jnr version that both dependencies (cassandra-driver-core, docker-client) need? Or do they depend on different major versions that are API-incompatible with each other?

I got different results with missinglink depending on whether cassandra-driver-core or docker-client ended up first on the classpath. All OK when cassandra-driver-core was first, a bunch of errors when docker-client was first.

You are right that I could depend on the highest JNR versions for both dependencies (which means that cassandra-driver-core wins) and I tried that but that still does not change that if docker-client gets on the classpath first, then - because it doesn't relocate JNR - the older version from docker-client is used. Does that make sense?

Maybe the solution is not to relocate JNR but to stop shading it? I guess shading without relocation is the real problem here?

[mattnworb]

One way that you can avoid needing to care about which order you list the dependencies in your pom.xml is to add jnr to the section with the version you wish to be pulled in. That will override whatever version Maven would otherwise pull in for a transitive dependency.

[mkjensen]

One way that you can avoid needing to care about which order you list the dependencies in your pom.xml is to add jnr to the section with the version you wish to be pulled in. That will override whatever version Maven would otherwise pull in for a transitive dependency.

Hmm, yeah, with the non-shaded version. I got that working. So I guess we could just leave it at that and close this PR. That is fine with me :)

Just a note on the shaded artifact (and please do correct me if I got this wrong): When docker-client:shaded is built then the versions of the com.github.jnr artifacts are shaded using the same Java package name (jnr) as used by the JNR artifacts themselves. Then, even if you dependency manage the JNR versions to your liking, you cannot change what was shaded into docker-client:shaded at build time. But maybe you're saying that the JNR artifacts depended on by docker-client:shaded (since it's not a dependency reduced POM) are put on the classpath before docker-client and that saves the day. (But then shading the JNR artifacts doesn't make sense at all?)

Hmm, this shading stuff can become a little confusing. It still seems to me like shading without relocation is problematic so you should either not shade at all or shade with relocation? And then you probably need a new artifact (e.g. docker-client-shaded) for which a dependency reduced POM is generated because otherwise your artifact (even with a shaded classifier) would depend on the stuff that you just shaded.

[ghost]

Hello!
Recently we ran into the same problem you had @mkjensen (It was just another package). I fully agree with you that shading without relocation is a bad idea.
For example, it disables the effect of "forced-version" and makes it even worse, because when changing a version the build does not break but docker-client fails during runtime with some missing method exception or so, because it doesn't get the version from inside the shaded jar but the changed one that appears first on classpath (It's really cumbersome to find out the rootcause for that). Therefore it makes no sense for me to shade packages without relocating them.
It would be really cool (and helpful for us) if docker-client:shaded would be a REAL shaded jar..
So i would totally vote for not closing that PR but expand it to relocate all third-party dependencies inside the shaded jar.