Hi. Do you still feel strongly about this?

Hi @cpopov, I think this is still a good idea. If we continue to expose internal dependencies (like commons-io), it's only a matter of time before somebody using this in their project has to deal with a version conflict.

I've seen my share of issues with either approach. With shade (assuming you repackage) it then results in possibly having several different versions of a single class on the classpath (the official one, and one or more shaded versions in different packages), which can result in confusion for users of the library. If you don't repackage, it's even worse; multiple versions of the class in the same package, with the use used depending on classpath ordering. Personally, I think it's best for libraries not to shade, and let the users deal with potential version conflicts in the standard mechanism, which provides visibility and techniques to specify the desired resolution.

The good thing is that I'm not proposing we only distribute the shaded artifact.

@spmiller
I don't know the shade plugin and just read Stackoverflow [1] to somewhat get it.
As far as I understand it (please correct me if I'm wrong) this patch would allow a user to check out the code base and run Maven to create a -dep jar file. This -dep jar would not contain the excluded dependencies and would somehow rename org.apache.commons.io to internaldeps.org.apache.commons.io. This means the o.a.commons.io classes are in the class path under two packages for this user.

If we would not shade, the user could also check out the code base, exclude the dependencies (Rhino, commons-logging, commons-io) in the pom and install the jar.

What advantage would the user have with the configured shade plugin?

For users that do nothing and don't care nothing would change and everyone could user lesscss as usual, right?

Note: The pull request conflicts with the current code base but it's clear how to resolve them. The fixes of the integration test config and one test case are easy as they are already contained in the pull request for #42.

[1] - http://stackoverflow.com/questions/13620281/what-is-the-maven-shade-plugin-used-for-and-why-would-you-want-to-relocate-java

Hi @sandroboehme,

You're broadly correct. The existing jar has dependencies (like Commons IO) that a user must include into their project for our compiler to work. The -dep jar includes all these dependencies internally into one jar file. This patch does not change how the existing jar behaves.

The advantage is this: if we depend on version X of a library, and a user depends on version Y, the -dep jar allows us both to use our preferred versions of the library. Obviously we will have only tested our code with version X, so the alternative of using version Y risks subtle bugs or the dreaded NoSuchMethodError.

Other implementations of the same concept are Jar Jar Links and OneJar.

Hi @spmiller,
thanks for explaining. It's always interesting to learn something new.
As a user I would expect to exclude the version I don't need and include the version I need by myself. Wouldn't that already solve the problem without needing to enhance our pom?

Best,

Sandro

The issues I have with distributing both a shaded and non-shaded version of a single library are:

1. It introduces additional complexity for end-users: they need to know about the difference, and choose which one to use
2. If both the shaded and non-shaded versions of the jars are included in an application (whether directly or indirectly by transitive dependencies), it causes runtime errors that can be tricky to diagnose, and that the dependency management capabilities in build tools tend not to be able to help with. I've run into problems several times with this using Groovy and/or JUnit, which use this strategy.

I am supportive of the following options:

• Continue using commons-io and commons-lang as normal dependencies (do nothing)
• Adopt use of shading, and only distribute the shaded jar, with no classifier
• Add a org.lesscss.util package and provide utility classes as needed there to eliminate the commons-io and commons-lang dependencies, copying code from the appropriate Apache projects and/or Guava (maintaining appropriate license headers, of course).

If we want to go with a util package, I'm willing to provide an initial implementation.

Hi @sandroboehme,

Assume for a moment we use SomeClass.someMethod(), which is provided in SomeLibrary version 1.8. In version 2.0 the developers realise that this is a silly name, and rename it do MeaningfulClass.meaningfulName().

If I am a user of our library and I want to use version 2.0 of SomeLibrary, I can't just exclude v1.8 from the pom file because that would break the Less compiler. This means I'm stuck on v1.8 until the Less compiler is upgraded (and then I have to go to v2.0 whether I like it or not).

Hi @spmiller,

thanks for the simple, realistic and logical explaination. I appreciate that!

Hi @davidmc24,
you wrote "I think it's best for libraries not to shade, and let the users deal with potential version conflicts in the standard mechanism, which provides visibility and techniques to specify the desired resolution.".
What standard mechanism do you mean?

Best,

Sandro

@sandroboehme It depends on the build tool in place. For Gradle, there are several dependency management options available. They include forcing a particular versions of a dependency, dependency resolve rules, excluding a particular transitive dependency from a dependency, and disabling transitive dependencies for a dependency. For Maven, I believe it would normally be to use a dependencyManagement section.

@spmiller is right that these techniques can't address a situation this library and another library depend on incompatible versions of one of our transitive dependencies.

Hi @davidmc24 and @spmiller,

as I understand, even with the dependencyManagement section the user cannot do much about the incompatible versions.
This leaves us with the util package or with shading. I understand the util package solution as a repackage of the dependencies (e.g. Apache commons) into a util package that is distributed within the main less compiler artifact.
This is similar to shading as we would repackage the dependencies as well but not within the same artifact. Instead the repackaged dependencies would be moved into a new generated jar artifact (generated by the shade plugin) that we include in the lesscss-java pom as a dependency.
In both cases, if the user includes lesscss-java into his project he could still add other versions of Apache commons while lesscss-java uses its own repackaged version. He would just need to make sure that he uses the official packages of the classes and not our packages as the class could behave different than expected.
Is this the right understanding or is there something about it that needs to be corrected?
If it's right - what is the advantage of having a separate jar artifact?

Cheers,

Sandro

My understanding is that the proposed shaded jar would be an alternative to the standard lesscss-java jar. It would include both the lesscss-java classes as well as the repackaged dependencies. A user of this library would need to include either lesscss-java or lesscss-java-dep in their classpath, but not both.

Thanks for the explaination @davidmc24.
You wrote "...only distribute the shaded jar, with no classifier" as your second option. What I would like to understand is: We should distribute only the shaded jar in contrast to which other scenario?
So with shading we would distribute two jars with two different artifact names. They should make it clear as good as possible for an uninformed user that one artifact contains some repackaged dependencies. Even if the user only finds the artifact by a search on search.maven.org. Without further investigation.
I suggest lesscss-java-with-inline-deps as artifact name to reflect that.
What do you think?

The scenario that you described, where there is both a lesscss-java artifact and an lesscss-java-with-inline-deps artifact, is exactly the scenario that I think we should avoid. While it might be perfectly obvious if a user is manually searching through search.maven.org and looking through that those might be two forms of the same thing, cases where lesscss-java is a transitive dependency become ugly.

For example, let's say that the user is writing a Grails app. They have been using the lesscss-resources plugin, which depends on the lesscss-java artifact. Then, they start using another plugin, awesome-docs (hypothetical), which depends on lesscss-java-with-inline-deps. While this is a dependency conflict, it's one that the Grails dependency report (or, for that matter, any other dependency management tool I'm aware of) can detect as a conflict. Since the user didn't explicitly request either version of the lesscss-java library, they aren't aware of the two different artifacts, which one they should/shouldn't use, or how to resolve the conflict.

Hi,

I have followed your discussion about shading. I would second the point
that a shaded and a non-shaded library creates confusion. For me, shading
and renaming the packages is merely an option if you already have trouble
(prominent example was Hibernate/Groovy with ASM and cglib, they ended up
repacking the jars). As lesscss-java is a library, it should play nice in
most environments rather than cause trouble, which means it should have the
least possible dependencies.

I've had a look, commons-lang3 is only used in "test" scope at the minute
and commons-io is only used in one Method (LessSource.loadResource()) so in
this case I would strongly suggest resolving this by having less
dependencies and rewriting the code.

As the used commons-io code is not very likely to change that much, using a
version range for the dependency could also solve the problem:
http://books.sonatype.com/mvnref-book/reference/pom-relationships-sect-project-dependencies.html

Best,
martin

On Thu, Mar 27, 2014 at 2:06 AM, David M. Carr notifications@github.comwrote:

The scenario that you described, where there is both a lesscss-javaartifact and an
lesscss-java-with-inline-deps artifact, is exactly the scenario that I
think we should avoid. While it might be perfectly obvious if a user is
manually searching through search.maven.org and looking through that
those might be two forms of the same thing, cases where lesscss-java is a
transitive dependency become ugly.

For example, let's say that the user is writing a Grails app. They have
been using the lesscss-resources plugin, which depends on the lesscss-javaartifact. Then, they start using another plugin,
awesome-docs (hypothetical), which depends on
lesscss-java-with-inline-deps. While this is a dependency conflict, it's
one that the Grails dependency report (or, for that matter, any other
dependency management tool I'm aware of) can detect as a conflict. Since
the user didn't explicitly request either version of the lesscss-java
library, they aren't aware of the two different artifacts, which one they
should/shouldn't use, or how to resolve the conflict.

Reply to this email directly or view it on GitHubhttps://github.com//pull/15#issuecomment-38759164
.

@damaddin Thanks for commenting. I agree with your reasoning.

Guys, I'm not sure where the confusion could come from (especially if you use an explicit name like @sandroboehme is proposing). Having two artifacts seems to work for JUnit, which I hope is pretty widely used. However, it's difficult to debate this without falling back on your own opinion -- either you think it's confusing or you don't, and I don't imagine anybody will change their opinion from their gut instinct.

@damaddin is right when he says a library should have as few dependencies as possible. My solution to this problem was to shade the dependencies as it created the least amount of code to maintain. The alternative of manually inlining these specific methods works until the next time somebody adds another dependency and nobody remembers to tell them otherwise.

Ok, I understand that build tools might detect different versions of the same artifact but not different versions of the same class or package. Why aren't classifiers like ´with-inline-deps`a solution here?
@davidmc24 in this light I don't understand your comment
#15 (comment) as it reads like you are suggesting two artifacts.

After removing the two dependencies and check the compile errors I think it is non trivial to replace them. E.g. to replace the BOMInputStream I guess one would have to specify the charset as a user.

The version range is a good way to lessen the problem.

@sandroboehme My understanding is that classifiers are mostly intended to attach additional artifacts to a primary artifact. In particular, I don't think that they are able to have their own POM, which is especially important if your intention is that the "main" artifact has a dependency on commons-io but the with-inline-deps artifact does not. Of the main libraries I'm aware of that do separate shaded artifacts (junit, groovy, cglib), all publish with separate artifact IDs, not with classifiers.

To actually try out shading I merged this pull request to https://github.com/sandroboehme/lesscss-java, resolved the merge conflicts, removed commons logging from this pull request, made a trivial change to the code to be able to move commons.lang to the test classpath, configured the shading to replace to current jar and added slf4j to the shading config.
It almost works but the pom is not replaced with the dependency-reduced-pom.xml in the resulting jar. Does somebody have an idea what do to about it? I already tried a few config options. But without success.

@sandroboehme Checked out that branch, and tried it out. I don't much care about the POM embedded within the resulting JAR. As far as I know, no build tools use it as part of their dependency management support. If we're going this direction (which is fine by me), I'd like to propose two tweaks:

• Change the shadedPattern from internalDeps.* to de.sandroboehme.lesscss.deps.*. This avoids conflicts with other libraries that are using shading, and better reflects that these are our dependencies, not general dependencies to be used by anyone.
• Don't shade slf4j-api. Currently, I believe that the approach taken by lesscss-java is to use SLF4J if it's available, and use JULI logging if it isn't. Shading SLF4J breaks this.

Thanks for the two tweaks @davidmc24!
I'm just wondering, how the build tools can know the dependencies if not by using the pom.

It uses the POM, just not the POM embedded in the JAR. Rather, it uses the standalone POM file that's published in the Maven repository. For example, when I run mvn install on this branch, I get:

[INFO] Installing /Users/carrd/repos/tmp/lesscss-java/target/lesscss-1.6.1.1.1-SNAPSHOT.jar to /Users/carrd/.m2/repository/de/sandroboehme/lesscss/lesscss/1.6.1.1.1-SNAPSHOT/lesscss-1.6.1.1.1-SNAPSHOT.jar
[INFO] Installing /Users/carrd/repos/tmp/lesscss-java/dependency-reduced-pom.xml to /Users/carrd/.m2/repository/de/sandroboehme/lesscss/lesscss/1.6.1.1.1-SNAPSHOT/lesscss-1.6.1.1.1-SNAPSHOT.pom

Note that the .pom file that's installed is from the dependency-reduced-pom.xml

Thanks for the information David! I tried it by myself and you are right, for some reason dependency-reduced-pom.xml is installed as lesscss-1.6.1.[version].pom.
It still feels wired, that a different source will be released than the one that is checked in because the shading does this postprocessing. But at the end I agree that it's valueable for the user that we are going in this direction. I have no open questions anymore.

Is everybody else also ok with doing it that way?