ceylon / ceylon-module-resolver

DEPRECATED
Apache License 2.0
23 stars 9 forks source link

option `--auto-export-maven-dependencies` doesn't #117

Open jvasileff opened 9 years ago

jvasileff commented 9 years ago

At least some transitive maven dependencies are not automatically shared with this option.

Using csmpetstore commit e9ce80f33a4f00df for testing, Java 7, and the original overrides.xml file referenced in .ceylon/config, I get the following errors:

$ ceylon compile --auto-export-maven-dependencies
...
source/com/vasileff/csmpetstore/module.ceylon:35: error: Error while loading the org.springframework:spring-web/4.1.4.RELEASE module:
    import "org.springframework:spring-web" "4.1.4.RELEASE";
   ^
   Declaration 'javax.servlet.Filter' could not be found in module 'org.springframework:spring-web' or its imported modules
source/com/vasileff/csmpetstore/web/config/WebAppConfig.ceylon:8: error: package not found in imported modules: 'javax.servlet' (add module import to module descriptor of 'com.vasileff.csmpetstore')
import javax.servlet {
      ^
...
ceylon compile: There were 45 errors

Adding shared="true" to the override below (which shouldn't be necessary) resolves the compile errors (original shown):

    <artifact groupId="org.springframework" artifactId="spring-web" version="4.1.4.RELEASE">
        <add groupId="javax.servlet" artifactId="javax.servlet-api" version="3.1.0"/> <!-- replacing 3.0.1 (PROVIDED) -->
    </artifact>

The flag does clearly work for ceylon run, since after compiled, the program runs correctly with the flag, but produces the following when the flag is missing (as expected due to unwanted classloader isolation):

$ ceylon run com.vasileff.csmpetstore
...
java.lang.IllegalStateException: Failed to introspect annotations: class com.vasileff.csmpetstore.config.AppConfig
...
Caused by: java.lang.annotation.AnnotationFormatError: Invalid default: public abstract java.lang.Class org.mybatis.spring.annotation.MapperScan.nameGenerator()

An additional test is to remove necessary version overrides. With the following removed:

    <artifact groupId="org.mybatis" artifactId="mybatis-spring" version="1.2.2">
        <add groupId="org.mybatis" artifactId="mybatis" version="3.2.8"/> <!-- replacing 3.2.3 -->
        <add groupId="org.springframework" artifactId="spring-jdbc" version="4.1.4.RELEASE"/> <!-- replacing 3.1.4.RELEASE -->
    </artifact>

ceylon compile --auto-export-maven-dependencies still works, rather than producing the expected version conflict errors.

Running with ceylon run --auto-export-maven-dependencies com.vasileff.csmpetstore then fails (as expected). In this case, the exception is:

Note: JSR-330 'javax.inject.Named' annotation found and supported for component scanning 
Note: Refreshing org.springframework.context.annotation.AnnotationConfigApplicationContext@45937244: startup date [Wed Mar 18 15:31:25 EDT 2015]; root of context hierarchy 
Note: Destroying singletons in org.springframework.beans.factory.support.DefaultListableBeanFactory@6ea24dc6: defining beans [org.springframework.context.annotation.internalConfigurationAnnotationProcessor,org.springframework.context.annotation.internalAutowiredAnnotationProcessor,org.springframework.context.annotation.internalRequiredAnnotationProcessor,org.springframework.context.annotation.internalCommonAnnotationProcessor,appConfig,org.springframework.context.annotation.ConfigurationClassPostProcessor.importAwareProcessor]; root of factory hierarchy 
ceylon run: Failed to load bean class: com.vasileff.csmpetstore.config.
AppConfig; nested exception is java.io.FileNotFoundException: class path
resource [application.properties] cannot be opened because it does not exist
org.springframework.beans.factory.BeanDefinitionStoreException: Failed to load bean class: com.vasileff.csmpetstore.config.AppConfig; nested exception is java.io.FileNotFoundException: class path resource [application.properties] cannot be opened because it does not exist
    at org.springframework.context.annotation.ConfigurationClassPostProcessor.processConfigBeanDefinitions(ConfigurationClassPostProcessor.java:293)
    at org.springframework.context.annotation.ConfigurationClassPostProcessor.postProcessBeanDefinitionRegistry(ConfigurationClassPostProcessor.java:227)
...
Caused by: java.io.FileNotFoundException: class path resource [application.properties] cannot be opened because it does not exist
    at org.springframework.core.io.ClassPathResource.getInputStream(ClassPathResource.java:157)

which I can't really explain, but I'm sure would make sense given enough effort trying to understand whatever classloader arrangement this is running under.

Another test is to comment out the following:

    <artifact groupId="org.springframework.retry" artifactId="spring-retry" version="1.0.2.RELEASE">
        <!-- transitive dependency path: "mybatis-spring" to "spring-batch-infrastructure" to "spring-retry" !!! -->
        <add groupId="org.springframework" artifactId="spring-context" version="4.1.4.RELEASE"/> <!-- replacing 3.0.5.RELEASE -->
    </artifact>

resulting in the runtime error:

$ ceylon run --auto-export-maven-dependencies com.vasileff.csmpetstore
Note: JSR-330 'javax.inject.Named' annotation found and supported for component scanning 
Note: Refreshing org.springframework.context.annotation.AnnotationConfigApplicationContext@183b677d: startup date [Wed Mar 18 15:34:39 EDT 2015]; root of context hierarchy 
Note: Destroying singletons in org.springframework.beans.factory.support.DefaultListableBeanFactory@79906abc: defining beans [org.springframework.context.annotation.internalConfigurationAnnotationProcessor,org.springframework.context.annotation.internalAutowiredAnnotationProcessor,org.springframework.context.annotation.internalRequiredAnnotationProcessor,org.springframework.context.annotation.internalCommonAnnotationProcessor,appConfig]; root of factory hierarchy 
ceylon run: Configuration problem: org.mybatis.spring.annotation.
MapperScannerRegistrar was imported as a Configuration class but is not
annotated with @Configuration nor does it declare any @Bean methods. Update the
class to meet either of these requirements or do not attempt to import it.
Offending resource: class path resource
[org/mybatis/spring/annotation/MapperScannerRegistrar.class]
org.springframework.beans.factory.parsing.BeanDefinitionParsingException: Configuration problem: org.mybatis.spring.annotation.MapperScannerRegistrar was imported as a Configuration class but is not annotated with @Configuration nor does it declare any @Bean methods. Update the class to meet either of these requirements or do not attempt to import it.
Offending resource: class path resource [org/mybatis/spring/annotation/MapperScannerRegistrar.class]
    at org.springframework.beans.factory.parsing.FailFastProblemReporter.error(FailFastProblemReporter.java:68)
...

which is probably more easily explained as being a classpath or classloader problem.


Now, as a potentially related issue, testing with Java 8 produces different results. (Still using Ceylon compiled with Java 7, but now using Java 8 to compile and run csmpetstore.)

As in the original test, shared="true" must be explicitly added. But unlike the original test, ceylon run --auto-export-maven-dependencies com.vasileff.csmpetstore fails with an exception resembling the exception in the second case above:

$ ceylon run --auto-export-maven-dependencies com.vasileff.csmpetstore
Note: Refreshing org.springframework.context.annotation.AnnotationConfigApplicationContext@771d1ffb: startup date [Wed Mar 18 15:42:29 EDT 2015]; root of context hierarchy 
Note: Destroying singletons in org.springframework.beans.factory.support.DefaultListableBeanFactory@19f7222e: defining beans [org.springframework.context.annotation.internalConfigurationAnnotationProcessor,org.springframework.context.annotation.internalAutowiredAnnotationProcessor,org.springframework.context.annotation.internalRequiredAnnotationProcessor,org.springframework.context.annotation.internalCommonAnnotationProcessor,appConfig,org.springframework.context.annotation.ConfigurationClassPostProcessor.importAwareProcessor]; root of factory hierarchy 
ceylon run: Failed to load bean class: com.vasileff.csmpetstore.config.
AppConfig; nested exception is java.io.FileNotFoundException: class path
resource [application.properties] cannot be opened because it does not exist
org.springframework.beans.factory.BeanDefinitionStoreException: Failed to load bean class: com.vasileff.csmpetstore.config.AppConfig; nested exception is java.io.FileNotFoundException: class path resource [application.properties] cannot be opened because it does not exist
    at org.springframework.context.annotation.ConfigurationClassPostProcessor.processConfigBeanDefinitions(ConfigurationClassPostProcessor.java:291)
...
Caused by: java.io.FileNotFoundException: class path resource [application.properties] cannot be opened because it does not exist
    at org.springframework.core.io.ClassPathResource.getInputStream(ClassPathResource.java:157)
    at org.springframework.core.io.support.EncodedResource.getInputStream(EncodedResource.java:143)

The proof of concept patch (https://github.com/jvasileff/ceylon-module-resolver/commits/maven-share-deps) does not exhibit these problems (at least on Java 7), and supports shared="false".

FroMage commented 9 years ago

So, in the case of your first errors:

source/com/vasileff/csmpetstore/module.ceylon:35: error: Error while loading the org.springframework:spring-web/4.1.4.RELEASE module:
    import "org.springframework:spring-web" "4.1.4.RELEASE";
   ^
   Declaration 'javax.servlet.Filter' could not be found in module 'org.springframework:spring-web' or its imported modules
source/com/vasileff/csmpetstore/web/config/WebAppConfig.ceylon:8: error: package not found in imported modules: 'javax.servlet' (add module import to module descriptor of 'com.vasileff.csmpetstore')
import javax.servlet {
      ^

They are both due to the fact that I only auto-export maven dependencies from a Maven module to an importing Maven module. I don't want Ceylon modules to have every dependency of a Maven module auto-imported. Ceylon modules should explicitly list every module they import. I don't want bad POMs to leak into Ceylon modules. So your second error is normal: you should import the servlet module in your module descriptor, rather than rely on spring to export it for you.

The first error is related, but slightly different: because we only add (to the classpath) transitive dependencies visible to the modules we compile. So spring gets added, but because its servlet dependency is not shared, it does not get added to the classpath. And if that dependency did not need to be shared (did not appear in the spring public API) then you would never get any error about it. But it is in its public API and so it must be shared. And again we don't want them to be shared by default for the reason listed above. Yes I know your patch did that, but I still think it's wrong, for Ceylon importers. I also disagree with having shared be the default, since it will be the default for Maven modules if you have --auto-export-maven-dependencies and it should not be the default for Ceylon modules.

Now, as to the rest of your errors, honestly I have no idea.

jvasileff commented 9 years ago

Well, that's completely broken and useless even if it does do what you now describe. This isn't about polluted namespaces. It's about class loading. You have to solve the class loading problem first, and then worry about narrowing visibility, and in a way that doesn't just re-break class loading.

I suggest the --auto-export-maven-dependencies option be removed.

Now, as to the rest of your errors, honestly I have no idea.

Too bad. I wish I was better able to describe the problem and the solution from the start, but I don't know what more I could have done. I put a lot of effort into making Ceylon + JBoss Modules work with maven dependencies, and haven't seemed to achieve or contribute anything aside from an outdated patch and a misunderstood description of an approach that actually does work. I wish I could contribute more, but we can't even get past the first step of a minimally working system. I'll set this aside for now. I hope you do figure all of this out, and best of luck.