Groovy extension modules do not work with static compilation using the Gradle compiler


(Cédric Champeau) #1

If you annotate a groovy class with @TypeChecked or @CompileStatic and that the code uses an extension module (see http://docs.codehaus.org/display/GROOVY/Creating+an+extension+module), then compilation fails as if the type checker doesn’t find the extension classes.

Using

compileTestGroovy.groovyOptions.useAnt = true

fixes the problem.

To reproduce, first step will build and install an extension module into local maven repo.

$ git clone
https://github.com/swilliams-vmw/gemfire-groovy-extensions.git
$ cd gemfire-groovy-extensions
$ ./gradlew install

Second, download this test project: http://dl.dropbox.com/u/20288797/divers/test.tgz

$ cd test
$ ./gradlew clean test

passes. Now open build.gradle, remove the useAnt=true option and the project compilation fails.


(Peter Niederwieser) #2

Does the type checker use a class loader to load the extension classes? If so, which one?


(Cédric Champeau) #3

Yes, it’s not specifically the type checker which loads those classes, but the groovy runtime. In particular, this is done here:

https://github.com/groovy/groovy-core/blob/master/src/main/org/codehaus/groovy/runtime/metaclass/MetaClassRegistryImpl.java#L111


(Peter Niederwieser) #4

The problem is that ‘MetaClassRegistryImpl’ makes the assumption that its class loader can load any module class. At compile time this assumption doesn’t hold because Gradle, for good reasons, uses different class loaders for the compiler itself and the compile class path. (Note that other integrators like GMaven will likely do the same.)

It looks like this will need a fix on Groovy’s side. Two potential solutions that I can think of:

  1. Change ‘MetaClassRegistryImpl’ to also try with the thread context class loader (which integrators like Gradle could set appropriately). 2. Change ‘StaticTypeCheckingSupport’ to load ‘MetaClassRegistryImpl’ with the class loader for the compile class path.

Just to let you know, the groovyc Ant task isn’t a good reference point for class loader related issues because it has some peculiarities that aren’t possible/feasible for other compiler integrators:

  • It resides in the same (groovy-all) Jar as the compiler. * From what I remember, it loads compiler and compile class path with one and the same class loader when run in forked mode. (I think the Groovy command-line compiler does the same thing.) Non-forked mode has its own set of class loader related problems, but all I remember is that it has major limitations when used with Gradle. (For a long time, Gradle didn’t even support non-forked mode.)

(Cédric Champeau) #5

Thanks for the insight, I’ll see what I can do. The meta class registry is a bit painful wrt to class loading, true :frowning:


(Peter Niederwieser) #6

Yet another solution would be to add a ‘MetaClassRegistryImpl’ constructor that accepts a class loader. Compiler classes like ‘StaticTypeCheckingSupport’ could then share an instance of ‘MetaClassRegistryImpl’ initialized with the class loader for the compile class path. This is probably better than my second suggestion above.


(Cédric Champeau) #7

That’s what I was discussing with the Groovy team. However, there’s a major issue with this which is compilation time. I would need to rebuild an index of meta methods for modules/DGM-like classes for each module, which is not cheap…


(Peter Niederwieser) #8

You mean because there would be two MetaClassRegistryImpl instances hanging around? Wouldn’t it be possible to avoid double initialization, say with some lazy initialization (and all compiler code using the same instance)?


(Cédric Champeau) #9

I mean I need to generate MetaMethods for each module. If we don’t use the classloader from the registry, then I need to choose between several class loaders (the most obvious being the source unit class loader, but would it work with Gradle (I’m thinking of the recently fixed issue with transform loader)?).

Currently the StaticTypeCheckingSupport class uses a cache of those methods, which is only created once (from the MCRI). Here, each time we create a type checking visitor (which may happen several times if we have more than one annotation in a file), then I would have to create a list of modules from classpath.

My current thinking is to make the type checking support class change from a singleton cache to a cache which uses a class loader as the key. Each time the class loader changes, then discard the cache.


(Peter Niederwieser) #10

From an integrator’s perspective, there are three class loaders involved:

  1. The class loader for loading the Groovy compiler itself. 2. The class loader for loading the Groovy compile class path. 3. The class loader for loading transforms.

For the purpose of static compilation, user-defined modules should probably be loaded with 2 (if you can get away with it). 1. is out of the question, and 3. doesn’t seem right unless you need to execute the module’s code. I guess 2. is also what the source unit class loader points to, but I’d have to double-check.

I don’t see why you’d need to decide between multiple class loaders. I think the goal should be to have a single instance of MCRI per Groovy “system”, or at least a single initialized one. For the Groovy system used by the compiler, this MCRI instance would use class loader 2. from above.

By the way, I still hope that the (standalone) Groovy compiler will stop using class loaders for the compile class path at some point (as discussed at the last GroovyCon). This would then also have to hold for modules. (In a nutshell, the compiler should only load user-defined classes that it needs to execute, e.g. transforms.) I know it sounds a bit like an academic problem, but it is a very real one in practice as well, especially for larger Groovy projects. Add a compiler facade that handles the transform class loading on its own, and all integrators will be (equally!) happy. :slight_smile:


(Cédric Champeau) #11

Yes, I plan to use the source unit loader only. But you know as we had strange CL issues and that we had to use the transform loader at some point instead of the source unit loader, I was wondering. Anyway, I have code changes ready, all I need is to test :slight_smile:


(Peter Niederwieser) #12

From what I know, this was about code executed by the compiler (compile scripts), which means that it should use 3.


(Cédric Champeau) #13

Fix seems to work :slight_smile:

Regarding the fact of avoiding using classloaders in the compilation process, this is still in queue :slight_smile:


(Peter Niederwieser) #14

Good to hear. Let us know in case there’s anything that needs to be done on Gradle’s side.


(Cédric Champeau) #15

Fixed in master and 2.1.x. If you want to try it out by yourself. See https://github.com/groovy/groovy-core/commit/99b68038f2b082f177558bdc6c10f5ff52d536f1


(Paolo Di Tommaso) #16

By using Gradle 1.12-rc-1 that example reports a compilation error, either with or without

compileTestGroovy.groovyOptions.useAnt = true

Moreover Gradle reports the following warning

The GroovyCompileOptions.useAnt property has been deprecated and is scheduled to be removed in Gradle 2.0. There is no replacement for this property.

This should be addressed somehow.


(Peter Niederwieser) #17

Can you provide a self-contained reproducible example? This will allow us to investigate what the cause of the problem is, and whether it can be fixed on our side.


(Paolo Di Tommaso) #18

With pleasure. You can find it here

https://github.com/pditommaso/gradle-test

best, p


(Cédric Champeau) #19

Just to be sure, does it work with Gradle 1.11?


(Peter Niederwieser) #20

I’ve tried most combinations of Groovy 2.1.3/2.2.2 and Gradle 1.8/1.10/1.11, and it works with neither of them.