Shaded dependencies should be removed or have provided scope

[sslavic]

Kryo shaded artifact (with all dependencies, like minlog) replaces main Kryo Maven artifact (jar without dependencies) and becomes the one that gets published to repositories and used. Problem is that the Kryo pom artifact published on repositories does not reflect that dependencies are shaded, it still lists them as compile scoped. So shading a project which depends on Kryo produces merge conflicts, same classes provided from Kryo, and from its dependencies.

Please adjust Kryo build script to have published pom list shaded dependencies as with provided scope, or removed from dependencies list. See maven-shade-plugin configuration

[magro]

Have you checked 2.23.0? This only contains the shaded reflectasm jar (which contains asm), while minlog and objenesis are pulled in as normal transitive dependencies. reflectasm is also not defined as dependency, see http://central.maven.org/maven2/com/esotericsoftware/kryo/kryo/2.23.0/kryo-2.23.0.pom

[sslavic]

I have. I did see that pom before reporting issue, and was confused to see maven-shade-plugin configuration (inclusion of only one dependency), but also Log.class and Log$Logger.class in kryo jar under "kryo-2.23.0.jar\com\esotericsoftware\minlog". Shading such kryo jar and minlog reports conflict.

[magro]

D'oh, you're right. This seems to be caused by the felix bundle plugin and the bundle packaging (for OSGI support). The bundle plugin needs more tuning to prevent this issue.

What's the issue that you experience, how can I reproduce this? In kryo-serializers I use kryo 2.23.0 and minlog and don't have any issues.

[sslavic]

Here is simplest sbt project I could think of, which reproduces the issue: https://dl.dropboxusercontent.com/u/11530426/dummy.zip

Project declares a single dependency, on kryo-2.23.0. It makes use of sbt-assembly plugin to make a fat jar. With sbt 0.13.1 installed, after unpacking, just run "sbt assembly" from project root. For me that fails because minlog (transitive dependency through kryo) and kryo have classes on same path with same name but with different content, and default assembly strategy only can deal with duplicates, and fails the build for any other collision case.

[magro]

Thanx for the sample project! I just adjusted the felix bundle plugin config that caused this issue, so that now the minlog classes are no longer contained.
You can get the latest kryo version by adding a resolver for the sonatype snaphots repo (https://oss.sonatype.org/content/repositories/snapshots) and using 2.23.1-SNAPSHOT as version.

Thanx for reporting!

[sslavic]

Checked, it works! Thanks for fixing this so fast!

[lichtin]

With 2.24.0 now getting

Unresolved constraint in bundle com.esotericsoftware.kryo [120]: Unable to resolve 120.0: missing requirement [120.0] osgi.wiring.package; (osgi.wiring.package=com.esotericsoftware.minlog)

I guess minlog is a required dependency? Unfortunately minlog-1.2 is not an OSGi bundle.

[magro]

What would you suggest as a solution? Should minlog be OSGI'ed? Do you want to submit appropriate pull requests?

[lichtin]

I found embedding minlog a good solution, perhaps we can keep it? sslavic would be happy with this as well, as mentioned above, we just want to scope minlog as 'provided'.

<Embed-Dependency>*;inline=true;artifactId=minlog</Embed-Dependency>

[magro]

@lichtin What is the advantage of Embed-Dependency+inline over Export-Package? Export-Package inlines classes as well and additionally exports them so that clients could access minlog classes AFAICS. So I'd say we should also be able to use Export-Package as it's done for reflectasm as well.

@romix @NathanSweet To resolve the "missing requirement" issue I'd go back to inline minlog classes (and define the minlog dependency as provided). Any objections?

[NathanSweet]

I don't know what "inline minlog classes" means.

On Fri, Jul 18, 2014 at 11:47 PM, Martin Grotzke [email protected]
wrote:

@lichtin https://github.com/lichtin What is the advantage of
Embed-Dependency+inline over Export-Package? Export-Package inlines classes
as well and additionally exports them so that clients could access minlog
classes AFAICS. So I'd say we should also be able to use Export-Package as
it's done for reflectasm as well.

@romix https://github.com/romix @NathanSweet
https://github.com/NathanSweet To resolve the "missing requirement"
issue I'd go back to inline minlog classes (and define the minlog
dependency as provided). Any objections?

—
Reply to this email directly or view it on GitHub
#189 (comment)
.

[magro]

@NathanSweet It's the same as the shaded jar stuff - minlog classes will be put into the final jar, so that users don't need to have the minlog jar in the classpath. AFAICS the alternative solution for OSGI users would be to OSGI'ify minlog.

[NathanSweet]

Ah. I prefer not shading dependencies, but I am fine with it if you guys
are.

On Sat, Jul 19, 2014 at 12:37 AM, Martin Grotzke [email protected]
wrote:

@NathanSweet https://github.com/NathanSweet It's the same as the shaded
jar stuff - minlog classes will be put into the final jar, so that users
don't need to have the minlog jar in the classpath. AFAICS the alternative
solution for OSGI users would be to OSGI'ify minlog.

—
Reply to this email directly or view it on GitHub
#189 (comment)
.

[lichtin]

@magro Comparing Embed-Dependency+inline vs Export-Package is a bit like apples vs oranges. The former only embeds dependencies, whereas the latter only defines what to export, it does not inline anything. You use a specific Maven plugin to embed code from a dependent artifact (reflectasm). Could of course apply the same pattern for minlog as you suggest.

[magro]

@NathanSweet Ok. Alternatively we could turn minlog into an OSGi bundle. What do you prefer? Touch minlog, or inline/include it in the kryo jar?

@lichtin Yes, I just added com.esotericsoftware.minlog* to the Export-Package declaration and by this

• minlog is added to the Export-Package: list in the manifest
• minlog classes are added to the kryo jar (inlined)
  So I think this seems to do everything we're trying to achieve, and we'd only have to change minlog to provided scope.

[NathanSweet]

I'd prefer touching minlog if it's all the same to you. I gave you access
just now. If it's a pain for you, do what is easier.

On Mon, Jul 21, 2014 at 12:01 AM, Martin Grotzke [email protected]
wrote:

@NathanSweet https://github.com/NathanSweet Ok. Alternatively we could
turn minlog into an OSGi bundle. What do you prefer? Touch minlog, or
inline/include it in the kryo jar?

@lichtin https://github.com/lichtin Yes, I just added
com.esotericsoftware.minlog* to the Export-Package declaration and by this

• minlog is added to the Export-Package: list in the manifest
• minlog classes are added to the kryo jar (inlined) So I think this
  seems to do everything we're trying to achieve, and we'd only have to
  change minlog to provided scope.

—
Reply to this email directly or view it on GitHub
#189 (comment)
.

[magro]

Ok, I can OSGi'ify minlog. What about the groupId? It got changed to com.esotericsoftware - shall we leave this?
What about reflectasm? Should we OSGi'ify that as well?

[NathanSweet]

I guess we use the newer groupid, the one we changed to.

Sure, you can go nuts on reflectasm. I have added you.

On Mon, Jul 21, 2014 at 12:15 AM, Martin Grotzke [email protected]
wrote:

Ok, I can OSGi'ify minlog. What about the groupId? It got changed to
com.esotericsoftware - shall we leave this?
What about reflectasm? Should we OSGi'ify that as well?

—
Reply to this email directly or view it on GitHub
#189 (comment)
.

[magro]

Should we change the groupId of reflectasm as well to com.esotericsoftware?

[NathanSweet]

I think so, would be nice for them to all be the same.

On Mon, Jul 21, 2014 at 12:24 AM, Martin Grotzke [email protected]
wrote:

Should we change the groupId of reflectasm as well to com.esotericsoftware
?

—
Reply to this email directly or view it on GitHub
#189 (comment)
.

[magro]

Hmm, then switch to semantic versioning as well? E.g. release minlog as 1.3.0?

[NathanSweet]

If you'd like, I don't mind. MinLog changes almost never, on account of
being so min.

On Mon, Jul 21, 2014 at 12:36 AM, Martin Grotzke [email protected]
wrote:

Hmm, then switch to semantic versioning as well? E.g. release minlog as
1.3.0?

—
Reply to this email directly or view it on GitHub
#189 (comment)
.

[magro]

Ok, the I'd release minlog as 1.3.0 and reflectasm as 1.10.0.
Regarding reflectasm: it is built as both "normal" jar and the "shaded" jar with asm included (and relocated, to prevent conflicts with other asm versions). But there's only one pom exported that still lists the dependency on asm. I'm just thinking that it probably would be better to only create/publish the shaded jar and the reduced pom without the dependency on asm. What do you think?

[magro]

@NathanSweet ASM is available in version 5.0.3 in the meantime. reflectasm tests are all green with this version - should we upgrade?

[magro]

@NathanSweet I'm rephrasing my thoughts on kryo, reflectasm and shading:

kryo should continue to depend on the shaded reflectasm (with asm included and relocated), because otherwise users (using maven, not OSGi) will be running into compatibility issues with asm.
The reflectasm pom (that kryo depends on) should not list asm as a dependency, because the reflectasm classes just do not use normal asm classes (they use the included, relocated asm classes).
Because there's only 1 pom file for an artifact, for reflectasm only the shaded version should be published together with the reduced pom (asm is removed from the list of dependencies).

If we change reflectasm to export only the shaded artifact as described above, we no longer need to include/shade reflectasm in kryo, but kryo can just depend on reflectasm (the shaded version, without asm dependency).

Do you agree?

[magro]

@lichtin Can you please test with kryo 2.24.1-SNAPSHOT (from https://oss.sonatype.org/content/repositories/snapshots/)? This now depends on an OSGi'ified minlog version.

[NathanSweet]

My feelings on shading are that it is a pretty nasty practice. I think the
right way to do it is to release a completely non-shaded lib and,
separately, a shaded lib. Only someone who has lib conflicts would use the
shaded lib (which is not everyone).

On Mon, Jul 21, 2014 at 2:16 AM, Martin Grotzke [email protected]
wrote:

@NathanSweet https://github.com/NathanSweet I'm rephrasing my thoughts
on kryo, reflectasm and shading:

kryo should continue to depend on the shaded reflectasm (with asm included
and relocated), because otherwise users (using maven, not OSGi) will be
running into compatibility issues with asm.
The reflectasm pom (that kryo depends on) should not list asm as a
dependency, because the reflectasm classes just do not use normal asm
classes (they use the included, relocated asm classes).
Because there's only 1 pom file for an artifact, for reflectasm only the
shaded version should be published together with the reduced pom (asm is
removed from the list of dependencies).

If we change reflectasm to export only the shaded artifact as described
above, we no longer need to include/shade reflectasm in kryo, but kryo can
just depend on reflectasm (the shaded version, without asm dependency).

Do you agree?

—
Reply to this email directly or view it on GitHub
#189 (comment)
.

[magro]

@NathanSweet Yes, I also don't like shading. But when we let kryo depend on the non-shaded reflectasm, we'd introduce a regression on #116. After reading through related issues and mailing list threads (there's been "some" discussion regarding this topic in the past) I'd prefer not to take this route as we'd get issues reported for this again and again.
Pushing shading down one level to reflectasm (only release the OSGi'ed, shaded jar and the dependency reduced pom) would IMO improve the situation, because then from a kryo perspective reflectasm is the same as objenesis or minlog (kryo would simply depend on the reflectasm jar with the dependency reduced pom) - no shading in kryo itself. This would still introduce a change but I don't see how this should break things (of course the risk is there).
What do you think?

[NathanSweet]

A user needs to manage his dependencies for his own project. If his servlet
container or OSGi thing has conflicts with a lib he pulls in, then he
should fix it. In Kryo's case, that user would need to get the shaded Kryo
that we would provide instead of the unshaded one.

I don't think we should preemptively shade any libraries. Doing that is
non-standard and means if a user needs Kryo and ASM then they have ASM
twice. I would prefer to do it the normal, expected way for the main
artifact (no shading) and then also provide a shaded artifact for users who
encounter conflicting libs. This is simple and straightforward for all
users, IMO.

On Mon, Jul 21, 2014 at 1:41 PM, Martin Grotzke [email protected]
wrote:

@NathanSweet https://github.com/NathanSweet Yes, I also don't like
shading. But when we let kryo depend on the non-shaded reflectasm, we'd
introduce a regression on #116
#116. After reading
through related issues and mailing list threads (there's been "some"
discussion regarding this topic in the past) I'd prefer not to take this
route as we'd get issues reported for this again and again.
Pushing shading down one level to reflectasm (only release the OSGi'ed,
shaded jar and the dependency reduced pom) would IMO improve the situation,
because then from a kryo perspective reflectasm is the same as objenesis or
minlog (kryo would simply depend on the reflectasm jar with the dependency
reduced pom) - no shading in kryo itself. This would still introduce a
change but I don't see how this should break things (of course the risk is
there).
What do you think?

—
Reply to this email directly or view it on GitHub
#189 (comment)
.

[magro]

I'd be willing to follow your direction because you're the owner of kryo. If we change it as you suggest we should at least change the readme to ask users to check if they already have asm in their classpath before they pull in kryo+reflectasm+asm so that they should not run into issues if they're following our advice. I'll also address this on the mailing list and ask for objections to give users the chance to raise their voice.

[magro]

Here's the post on the mailing list.

[romix]

@magro BTW, Martin, I have a comment, which could be related to this issue. As far as I can see, one of the technical problems is that it is somewhat difficult to produce different artifacts with different settings/customizations using a single pom.xml file. I recently started using Gradle at work (after many years of using Maven) and can report that it makes life much easier compared to Maven, while still inheriting most of Maven's advantages. Producing multiple artifacts with Gradle is pretty easy...

[sslavic]

@romix I don't find it difficult - there are lots of options to produce multiple artifacts from a single pom.xml, like maven assembly plugin, and there are various plugins for various kinds of including jars in another jar (of which shading is just one), which typically produce additional artifact along with main one, but with different configurable classifier for distinction.

[magro]

@romix Sounds interesting, but moving to gradle is also a rather big change. I'd be happy if we could avoid that.

@sslavic Can you share more information which plugin would be useful? Differentiating artificats by classifier is not that helpful because essentially they would be the same artifact sharing the same pom.xml. So instead of having

• kryo with artifactId=kryo (and no classifier) and kryo with artifactId=kryo and classifier=shaded (both sharing the same pom.xml that contains a dependency on reflectasm) we should have
• kryo with artifactId=kryo (dep on reflectasm) and a kryo with artifactId=kryo-shaded (no dependency on reflectasm) - 2 completely different artifacts.

Do you know any plugin that provides that and that integrates well with the maven-release-plugin? (We could achieve this with profiles, but then we'd need to perform every release twice with going back to a common git tag, so releasing gets much harder)

[magro]

@romix Can you point me to some documentation that shows how to release/publish several artifacts for a single project? I couldn't find a simple solution with a 1-minute research ;-)

[romix]

@magro Some links about publishing multiple artifacts with Gradle:

http://www.gradle.org/docs/current/userguide/maven_plugin.html#sub%3amultiple_artifacts_per_project

http://www.gradle.org/docs/current/userguide/publishing_maven.html

http://forums.gradle.org/gradle/topics/how_to_publish_multiple_artifacts_w_sources_per_project

[magro]

@romix Thanks!

[magro]

@lichtin @sslavic The kryo artifact now no longer shades reflectasm+asm but has a regular dependency on reflectasm (which is now OSGi'ified). For users with asm on the classpath already there's now kryo-shaded which shades/includes reflectasm+asm.
It would be great if you could test kryo 2.24.1-SNAPSHOT (from https://oss.sonatype.org/content/repositories/snapshots/) in your OSGi environment and report if it's working as expected. Thanks in advance!

[romix]

@lichtin @sslavic Could you test kryo 2.25.0-SNAPSHOT (from https://oss.sonatype.org/content/repositories/snapshots/) in your OSGi environment and report if it's working as expected?

Once it is confirmed, we can close this issue.

[magro]

Was not reported any more.