eclipse-archived / ceylon

The Ceylon compiler, language module, and command line tools
http://ceylon-lang.org
Apache License 2.0
399 stars 62 forks source link

Support defining a list of module overrides for Maven/Java modules #4743

Closed CeylonMigrationBot closed 7 years ago

CeylonMigrationBot commented 11 years ago

[@FroMage] We should try to fix all the Maven issues by allowing the user to specify a file which lists a number of Maven overrides which allow us for each module name / version pair to:

[Migrated from ceylon/ceylon-module-resolver#81]

CeylonMigrationBot commented 11 years ago

[@akberc] +1 Thank you. In a very similar ceylon.build task, I am going to be using Ceylon declarative style to define Dependency { .... {Exclusion {

Please reference your approach so that we can align the IDE 'export Java module', this issue and the ceylon.build task.

I understand that they maybe totally unrelated, but they are tied to the user experience of being able to use Java modules without understanding too much of the deeper problems.

CeylonMigrationBot commented 11 years ago

[@FroMage] @alesj I merged your branch and added support for --maven-overrides parameters. I can confirm it lets me compile Maven-using modules more easily, but I have a much harder time at runtime.

Using the latest distribution (yes you have to build one), you can test with:

../ceylon-dist/dist/bin/ceylon --rep cmr-test --rep aether --maven-overrides cmr-test/overrides.xml com.redhat.ceylon.compiler.java.test.cmr.modules.bug1100/1

Using the cmr-test repo at http://stephane.epardaud.fr/cmr-test-81.zip

It will fail with:

Caused by: java.lang.LinkageError: loader constraint violation: when resolving method "org.eclipse.jetty.servlet.ServletHolder.setServlet(Ljavax/servlet/Servlet;)V" the class loader (instance of org/jboss/modules/ModuleClassLoader) of the current class, org/apache/camel/component/jetty/JettyHttpComponent, and the class loader (instance of org/jboss/modules/ModuleClassLoader) for resolved class, org/eclipse/jetty/servlet/ServletHolder, have different Class objects for the type t/Servlet;)V used in the signature

That's as far as I got. I added lots of overrides to make certain imports shared, thanks to the hard-to-decypher info I get with the --verbose flag, but then that one I just don't understand.

It seems that a lot of issues have to do with not only missing dependencies but also that they should be shared, even more so for the runtime than for the compiler.

Can you help me with these @alesj ?

The source code is:

module com.redhat.ceylon.compiler.java.test.cmr.modules.bug1100 "1" {
    import java.base "7";
    import "commons-codec:commons-codec" "1.4";
    import "org.apache.camel:camel-core" "2.9.4";
    import "org.apache.camel:camel-jetty" "2.9.4";
}
import org.apache.camel.impl { DefaultCamelContext }
import org.apache.camel.component.jetty { JettyHttpComponent }
import org.apache.camel.builder { RouteBuilder }
import java.lang { Thread { currentThread } }

"Run the module `simple`."
shared void run() {
  print("Start Camel");
  value context = DefaultCamelContext();
  context.addComponent("jetty", JettyHttpComponent());
  object routeBuilder extends RouteBuilder() {
    shared actual void configure() {
      from("jetty:http://localhost:8080").log("got request");
    }
  }
  context.addRoutes(routeBuilder);
  context.start();
  currentThread().sleep(10000);
  print("Stop Camel");
}
CeylonMigrationBot commented 11 years ago

[@FroMage] Also, I had lots of issues when importing maven modules with a . rather than a : separator, because the CMR imports those with a :, which then leads to duplicate modules foo:bar and foo.bar which causes lots of issues. So I vote we force Maven imports to use a : in the module name and:

We wanted that behaviour anyways but thought of adding an annotation to the import for that, but since not using the : separator leads to issues and ambiguity, I think this would solve all those issues at once.

CeylonMigrationBot commented 11 years ago

[@dmlloyd] Guys using version numbers in your run time imports is a very very bad idea, unless you want to see lots and lots of LinkageErrors like above and/or enter OSGi hell. I know it seems clever but it's going to break down on you really fast as the number of installed modules increases. It would make much more sense to keep a single (or perhaps two or at most three) parallel version streams which you keep up to date via whatever mechanism, and perhaps add in version constraints to make sure things are properly enforced. Depending on specific versions is going to give you nothing but trouble.

CeylonMigrationBot commented 11 years ago

[@dmlloyd] The LinkageError itself is due to inheriting from more than one version of the same base class or interface at the same time, which is a direct expression of this problem.

CeylonMigrationBot commented 11 years ago

[@FroMage] I don't know what you're telling us. I'm pretty sure I only import a single version of camel. I don't know which module is having multiple versions loaded at the same time and which versions these are, and who imported them and why they are even visible to each other.

Or how to solve this. This is using nothing but Maven modules, and they do specify versions. Should we attempt to merge them all to one?

Is there a way in JBoss Modules to dump a list of loaded modules and their dependencies?

CeylonMigrationBot commented 11 years ago

[@quintesse] @dmlloyd Not sure what you refer to when mentioning "version streams"? Or the problem with depending on specific versions. Isn't this what JBoss Modules does? Isn't this what slots are for?

CeylonMigrationBot commented 11 years ago

[@alesj]

Is there a way in JBoss Modules to dump a list of loaded modules and their dependencies?

I know there is some JMX support in it. But I doubt it knows to dump dependencies. @dmlloyd ?

CeylonMigrationBot commented 11 years ago

[@dmlloyd] No, slots are definitely not for supporting N versions of one JAR. The JVM simply is not capable of doing this. Even Maven resolves down to a single cohesive version set (or tries to anyway). And, I'm not the one telling you; the JVM is telling you specifically that you are linking to two versions of one class name from a subclass which is disallowed by the JVM linking rules. It even tells you where those classes came from.

Yes the latest versions of JBoss Modules have a dependency printer but iirc it only supports the built-in static module loader. It wouldn't be too hard to write one for your own module loader though.

CeylonMigrationBot commented 11 years ago

[@quintesse]

No, slots are definitely not for supporting N versions of one JAR

No, I meant when you say "using version numbers in your run time imports is a very very bad idea".

But isn't that what those versions numbers are for, to be able to decide what goes with what and maybe say "sorry, you're trying to use N different versions of this module, no can do"

CeylonMigrationBot commented 11 years ago

[@dmlloyd] Yes, if that's all you're using them for (a.k.a. "constraints"). It's when multiple versions of things get pulled in that it's a problem.

Note that this happens not only with versions but also with equivalent JARs with different GAV coordinates. This is why we do not directly use Maven information within the Wildfly appserver.

CeylonMigrationBot commented 11 years ago

[@gavinking] Caused by: java.lang.LinkageError: loader constraint violation: when resolving method "org.eclipse.jetty.servlet.ServletHolder.setServlet(Ljavax/servlet/Servlet;)V" the class loader (instance of org/jboss/modules/ModuleClassLoader) of the current class, org/apache/camel/component/jetty/JettyHttpComponent, and the class loader (instance of org/jboss/modules/ModuleClassLoader) for resolved class, org/eclipse/jetty/servlet/ServletHolder, have different Class objects for the type t/Servlet;)V used in the signature

the JVM is telling you specifically that you are linking to two versions of one class name from a subclass which is disallowed by the JVM linking rules. It even tells you where those classes came from.

I have an issue report in my web browser I've been meaning to submit for a couple of days, but let me say it here too: that error message is, in 2013, totally inadequate. JBoss Modules should be capable of a much better error message. This kind of thing was barely acceptable back in the Clinton administration, but not any more. ;-)

CeylonMigrationBot commented 11 years ago

[@dmlloyd] JBoss Modules can only provide what the JVM gives us access to, which is precious little.

CeylonMigrationBot commented 11 years ago

[@gavinking] @dmlloyd Well, let's debate that somewhere else.

CeylonMigrationBot commented 11 years ago

[@FroMage] So, @alesj WDYT? It seems to me that if we want to merge every imported Maven module so that only one can live at the same time with a single version, we have to stop doing all this lazy loading of modules because we need the full graph resolved at startup and all dependencies figured out for Maven.

So now we have overrides and I'm still not convinced this got us closer to supporting Maven modules.

I'm going to guess this isn't an issue for Ceylon modules because the shared imports are actually enforced so two a single module can't ever see two versions of the same module loaded at once, even if it happens privately.

CeylonMigrationBot commented 11 years ago

[@alesj] @FroMage I think overrides should be fine, you just need to do it properly aka more exact. Where I doubt flat / full graph will solve the problem either.

CeylonMigrationBot commented 11 years ago

[@FroMage] Well, did you find the proper override file to make this work? I doubt most humans will be able to. Especially when it "just works" using Maven and a flat classpath.

CeylonMigrationBot commented 11 years ago

[@alesj] @FroMage how did you create the overrides.xml in the first place? Afair, there was some missing class in the first place.

CeylonMigrationBot commented 11 years ago

[@FroMage] Tic-toc tic-toc…

CeylonMigrationBot commented 11 years ago

[@FroMage] I created it by trial and error looking at the compiler logs first and then the jboss modules logs for every time it failed and why.

CeylonMigrationBot commented 11 years ago

[@alesj] Well, atm it looks you only got the javax.servlet thing wrong -- from 10.000ft. :-) Dunno how to fix it exactly, but - since you have it all set - perhaps give it another try.

And yes, I do agree it's a non-human task, hence we'll probably need to add some tooling to it asap.

CeylonMigrationBot commented 11 years ago

[@FroMage] So why don't you try it and see if you can find the proper override?

CeylonMigrationBot commented 11 years ago

[@FroMage] This is going to slip to 1.1

CeylonMigrationBot commented 10 years ago

[@sgalles] @FroMage you asked for feedback about this feature, now I think that I can give my humble opinion about this.

Well, I'm a bit pessimistic actually. As is, I think that most developers will just give up before being able to just see their module run (or maybe compile).

For instance this is the Dropwizard dependency tree generated by gradle

compile
\--- com.yammer.dropwizard:dropwizard-core:0.6.2
     +--- com.sun.jersey:jersey-core:1.17.1
     +--- com.sun.jersey:jersey-server:1.17.1
     |    \--- asm:asm:3.1
     +--- com.sun.jersey:jersey-servlet:1.17.1
     +--- com.yammer.metrics:metrics-core:2.2.0
     |    \--- org.slf4j:slf4j-api:1.7.2 -> 1.7.4
     +--- com.yammer.metrics:metrics-servlet:2.2.0
     |    +--- com.yammer.metrics:metrics-core:2.2.0 (*)
     |    \--- com.fasterxml.jackson.core:jackson-databind:2.1.1 -> 2.1.4
     |         +--- com.fasterxml.jackson.core:jackson-annotations:2.1.4
     |         \--- com.fasterxml.jackson.core:jackson-core:2.1.4
     +--- com.yammer.metrics:metrics-jetty:2.2.0
     |    +--- com.yammer.metrics:metrics-core:2.2.0 (*)
     |    \--- org.eclipse.jetty:jetty-server:8.1.8.v20121106 -> 8.1.10.v20130312
     |         +--- org.eclipse.jetty.orbit:javax.servlet:3.0.0.v201112011016
     |         +--- org.eclipse.jetty:jetty-continuation:8.1.10.v20130312
     |         \--- org.eclipse.jetty:jetty-http:8.1.10.v20130312
     |              \--- org.eclipse.jetty:jetty-io:8.1.10.v20130312
     |                   \--- org.eclipse.jetty:jetty-util:8.1.10.v20130312
     +--- com.yammer.metrics:metrics-logback:2.2.0
     |    +--- com.yammer.metrics:metrics-core:2.2.0 (*)
     |    +--- ch.qos.logback:logback-core:1.0.7 -> 1.0.10
     |    \--- ch.qos.logback:logback-classic:1.0.7 -> 1.0.10
     |         +--- ch.qos.logback:logback-core:1.0.10
     |         \--- org.slf4j:slf4j-api:1.7.2 -> 1.7.4
     +--- com.yammer.metrics:metrics-jersey:2.2.0
     |    +--- com.yammer.metrics:metrics-core:2.2.0 (*)
     |    +--- com.yammer.metrics:metrics-annotation:2.2.0
     |    \--- com.sun.jersey:jersey-server:1.15 -> 1.17.1 (*)
     +--- com.fasterxml.jackson.core:jackson-databind:2.1.4 (*)
     +--- com.fasterxml.jackson.jaxrs:jackson-jaxrs-json-provider:2.1.4
     |    +--- com.fasterxml.jackson.core:jackson-core:2.1.4
     |    +--- com.fasterxml.jackson.core:jackson-databind:2.1.4 (*)
     |    \--- com.fasterxml.jackson.module:jackson-module-jaxb-annotations:2.1.4
     |         +--- com.fasterxml.jackson.core:jackson-core:2.1.4
     |         \--- com.fasterxml.jackson.core:jackson-databind:2.1.4 (*)
     +--- com.fasterxml.jackson.dataformat:jackson-dataformat-yaml:2.1.4
     |    \--- com.fasterxml.jackson.core:jackson-core:2.1.4
     +--- com.fasterxml.jackson.datatype:jackson-datatype-guava:2.1.2
     |    +--- com.fasterxml.jackson.core:jackson-databind:2.1.2 -> 2.1.4 (*)
     |    +--- com.fasterxml.jackson.core:jackson-core:2.1.2 -> 2.1.4
     |    \--- com.google.guava:guava:11.0.2 -> 14.0.1
     +--- net.sourceforge.argparse4j:argparse4j:0.4.0
     +--- org.slf4j:slf4j-api:1.7.4
     +--- org.slf4j:jul-to-slf4j:1.7.4
     |    \--- org.slf4j:slf4j-api:1.7.4
     +--- ch.qos.logback:logback-core:1.0.10
     +--- ch.qos.logback:logback-classic:1.0.10 (*)
     +--- org.slf4j:log4j-over-slf4j:1.7.4
     |    \--- org.slf4j:slf4j-api:1.7.4
     +--- org.eclipse.jetty:jetty-server:8.1.10.v20130312 (*)
     +--- org.eclipse.jetty:jetty-servlet:8.1.10.v20130312
     |    \--- org.eclipse.jetty:jetty-security:8.1.10.v20130312
     |         \--- org.eclipse.jetty:jetty-server:8.1.10.v20130312 (*)
     +--- org.eclipse.jetty:jetty-http:8.1.10.v20130312 (*)
     +--- com.google.guava:guava:14.0.1
     +--- com.google.code.findbugs:jsr305:2.0.1
     +--- org.hibernate:hibernate-validator:4.3.1.Final
     |    +--- javax.validation:validation-api:1.0.0.GA
     |    \--- org.jboss.logging:jboss-logging:3.1.0.CR2
     +--- joda-time:joda-time:2.2
     \--- com.fasterxml.jackson.datatype:jackson-datatype-joda:2.1.2
          +--- com.fasterxml.jackson.core:jackson-core:2.1.2 -> 2.1.4
          +--- com.fasterxml.jackson.core:jackson-databind:2.1.2 -> 2.1.4 (*)
          \--- joda-time:joda-time:2.1 -> 2.2

It does not matter if the dependencies here are conceptually correct or not. It just works in the java world.

First, you see that gradle (like Maven) use a strategy to remove the version conflicts (2.1.2 -> 2.1.4 means 2.1.2 requested, but 2.1.4 retained). Now, I think it is clear that this depency tree will generates java.lang.LinkageError all over the place the way the Maven interop works.

OK, so I guess that a tool could help to solve the java.lang.LinkageError problems. This tool could generate a overrides.xml that simulate the version conflict resolution used by Gradle or Maven.

Problem is, there are classes of problem that, I think, can only be resolved manually.

We have a very good example here.

At some point trying to make my Dropwizard work, I got : java.lang.ClassCastException: org.slf4j.helpers.NOPLogger cannot be cast to ch.qos.logback.classic.Logger Here no fancy classloading error. Simply, the slf4j-api could not find the ch.qos.logback.classic.Logger implementation of the logger.

Actually to make this work the tree must be transformed from

 +--- ch.qos.logback:logback-core
     +--- ch.qos.logback:logback-classic
     +--- org.slf4j:log4j-over-slf4j
     |    \--- org.slf4j:slf4j-api

to

 ++--- ch.qos.logback:logback-core
     +--- ch.qos.logback:logback-classic
     +--- org.slf4j:log4j-over-slf4j
     |    \--- org.slf4j:slf4j-api
                \--- ch.qos.logback:logback-classic <-- added here

This can't be automatized.

BTW, I have a better understanding now of the reason why I was able to make my exemple work by importing the Maven modules into Ceylon modules via gradles scripts (in my local Ceylon repos). By using the Gradle version conflict resolution, at least, the version conflict was solved before runtime (no java.lang.LinkageError ). Of course it was not enough, I've also done something plain hugly : my script created the modules with a dependency on all other modules in the dependency tree. I've vaguely emulated the 'flat java classpath'. I guess that's why it worked.

Bottom line, given that the existing Maven librairies were not created to be used in a non-flat classpath, I'm naively wodering if the classloading scheme of Ceylon should not aggregate the whole Maven dependency tree into a huge virtual 'Maven' Ceylon module (of course such module would share every single class :/ )

This is super ugly but this might be the price to pay to have a real easy interop with Maven. And anyway, it does not prevent people from creating clean Ceylon modules for these jars, with clean dependencies manually created.

My two cents.

CeylonMigrationBot commented 10 years ago

[@akberc] IMHO Maven overrides are too simplistic for real-life scenarios, and there may not be any further options under module resolver. We have had the conversation a few times and it appears that it would be better handled by 3rd-party tools and the Ceylon ecosystem and let the core Dev team tackle the bigger issues..

Please take a look here: https://github.com/dgwave/ceylon-maven-plugin https://github.com/dgwave/lahore-tasks

After 1.1, release ceylon maven plugin and ceylon.build tasks may provide more useful interaction with Maven: I don't have a solution yet, but I am trying to tackle the problem from both ends: Ceylon and Maven.

CeylonMigrationBot commented 10 years ago

[@sgalles]

IMHO Maven overrides are too simplistic for real-life scenarios

I agree.

there may not be any further options under module resolver

I don't know. The Gradle team has a nice programatic solution for this. For the Ceylon compiler/runtime it might work with a Java/Ceylon class plugin or Ceylon code in the module descriptor. http://www.gradle.org/docs/current/dsl/org.gradle.api.artifacts.ResolutionStrategy.html

it appears that it would be better handled by 3rd-party tools and the Ceylon ecosystem and let the core Dev team tackle the bigger issues..

Yes. I agree too. But it is a pity that the existing Maven interop infrastructure embedded in Ceylon can not be leveraged at least for 80% of the use cases (and the difficult 20% could indeed be handled by the 3rd-party tools)

Please take a look here: https://github.com/dgwave/ceylon-maven-plugin https://github.com/dgwave/lahore-tasks

Yes, I really like the approach you choose. And indeed it will definitly help to solve the 20% hard cases related to Maven interop per se. Problem is, now I'm really more worried about the altered behavior of libraries like org.slf4j:slf4j-api in the non-flat classpath of Ceylon. Even if the version conflicts are solved, at the end of the day, I will have to make my Maven libraries work in a non flat classpath, and there is a risk that I don't succed.

And this risk is worrying, because for every new Maven library this risk exists. And best case, I may succed, but it does not work out of the box and I can plan several hours (days ?) of trial and error to integrate this new Maven library.

That's why I was interested with the modules with multiple resource-root https://groups.google.com/forum/#!topic/ceylon-users/nHQajoGSfQ0 I wanted to check that at least I could fall back on this to create the 'huge maven module' I was talking about (statically in my local repos, or my private Herd repos for instance)

After 1.1, release ceylon maven plugin and ceylon.build tasks may provide more useful interaction with Maven: I don't have a solution yet, but I am trying to tackle the problem from both ends: Ceylon and Maven.

Great, I'm looking forward to it.

CeylonMigrationBot commented 10 years ago

[@gavinking] Isn't these problems something that assemblies would solve? Indeed, isn't this maven-overrides XML file just a rubbish hacked-in assembly descriptor?

CeylonMigrationBot commented 10 years ago

[@sgalles]

Isn't these problems something that assemblies would solve?

:+1: +1

CeylonMigrationBot commented 10 years ago

[@FroMage]

Isn't these problems something that assemblies would solve? Indeed, isn't this maven-overrides XML file just a rubbish hacked-in assembly descriptor?

Well, you can't say at the same time that it's an assembly descriptor and that it's rubbish. It is indeed a way to declare overrides for bogus Maven dependencies, which, I gather from guessing is part of your secret project for assemblies. It's XML because that was the easiest thing to try, and insofar as it's part of the assembly functionality, it doesn't seem to be working as expected, because figuring out how to fix Maven dependencies is not trivial, as we've seen.

It is, however a good way to test our ideas, and where assemblies will go and where they will face issues.

It has been suggested several times that we should put all of Maven dependencies inside a flat classpath, but I've rejected this approach so far because the notion of flat classpath is not well-defined when it comes to modularity. Consider two Ceylon modules which include two distinct Maven modules, which in turn import common Maven modules with different versions. Do we have one "flat classpath group" for each Ceylon module? Or a single one which unifies the two imports?

ATM we do dependency resolution on a per-import basis, so we can't unify the two groups after the fact. We could change this, but then when we support dynamic loading of modules, how are we going to unify that third Maven module when the two first are already loaded and resolved?

The problem gets worse if the Maven modules import Ceylon modules, because suddenly that flat classpath part can view classes from different versions of the same Ceylon module, which only works in a non-flat classpath.

I suppose that we can make the Maven imports much closer to how Maven does it by unifying all the imports and merging the constraints to generate the correct override list. Either with a new command that would produce an override.xml or do it automatically at runtime. But until now we've always made overrides explicit. Note that this would not solve the issue of missing module imports, which you noticed.

It's clear we have issues and we can improve. I suggest we improve one bit at a time to find the right solution. It's not clear at all that a flat classpath is a better solution. It may be, and yes it would solve the part about missing imports, but it would leave the issue about version override resolution, which we have to solve in both cases, and it would open questions about composability. If anyone has other ideas, we're taking.

CeylonMigrationBot commented 10 years ago

[@FroMage] Moving again.

gavinking commented 7 years ago

This was done a very long time ago.