groovy
  1. groovy
  2. GROOVY-3413

Grab doesn't dependencies available for compilation when class loaded via GroovyScriptEngine

    Details

    • Type: Bug Bug
    • Status: Closed Closed
    • Priority: Major Major
    • Resolution: Won't Fix
    • Affects Version/s: 1.6
    • Fix Version/s: None
    • Component/s: Grape
    • Labels:
      None
    • Environment:
      MacOS 10.5, junit tests run in maven build
    • Testcase included:
      yes
    • Number of attachments :
      0

      Description

      I have a test script which uses Grab ...

      @Grab(group='commons-primitives', module='commons-primitives', version='1.0')
      public class GrapeClass {
      	
       def createEmptyInts() { new ArrayIntList() }
      
       def verify() {
         def ints = createEmptyInts()
         ints.add(0, 42)
         assert ints.size() == 1
         assert ints.get(0) == 42		
         return "ok"
       }
      }
      

      which is successfully loaded via a GroovyClassLoader as follows

      public class GrapeTestCase extends GroovyTestCase {
      	void testGrape() {	
      		def loader = new GroovyClassLoader(this.class.classLoader)
      		assert loader.parseClass(
      			new File("target/test-classes/sites/thirdparty/plugins/GrapeClass.groovy"))
      			.newInstance().verify()=="ok"
      	}
      }
      

      but not via a GroovyScriptEngine ...

      public class GroovySriptEngineGrapeTestCase extends GroovyTestCase {
      	void testGrape() {	
      		GroovyScriptEngine gse = new GroovyScriptEngine("target/test-classes/sites/thirdparty/plugins/");
      		Class<?> clazz = gse.loadScriptByName("GrapeClass");
      		assert clazz.newInstance().verify() == "ok";
      	}
      }
      

      This final test cases throws the error

      Test set: com.bemoko.live.platform.test.thirdparty.groovy.GroovySriptEngineGrapeTestCase
      -------------------------------------------------------------------------------
      Tests run: 1, Failures: 0, Errors: 1, Skipped: 0, Time elapsed: 1.358 sec <<< FAILURE!
      testGrape(com.bemoko.live.platform.test.thirdparty.groovy.GroovySriptEngineGrapeTestCase)  Time elapsed: 1.335 sec  <<< ERROR!
      groovy.util.ScriptException: Could not parse scriptName: GrapeClass.groovy
             at groovy.util.GroovyScriptEngine.updateCacheEntry(GroovyScriptEngine.java:335)
             at groovy.util.GroovyScriptEngine.loadScriptByName(GroovyScriptEngine.java:282)
             at sun.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
             at sun.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:39)
             at sun.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:25)
             at java.lang.reflect.Method.invoke(Method.java:597)
             at org.codehaus.groovy.runtime.callsite.PojoMetaMethodSite$PojoCachedMethodSiteNoUnwrapNoCoerce.invoke(PojoMetaMethodSite.java:229)
             at org.codehaus.groovy.runtime.callsite.PojoMetaMethodSite.call(PojoMetaMethodSite.java:52)
             at org.codehaus.groovy.runtime.callsite.CallSiteArray.defaultCall(CallSiteArray.java:43)
             at org.codehaus.groovy.runtime.callsite.AbstractCallSite.call(AbstractCallSite.java:116)
             at org.codehaus.groovy.runtime.callsite.AbstractCallSite.call(AbstractCallSite.java:124)
             at com.bemoko.live.platform.test.thirdparty.groovy.GroovySriptEngineGrapeTestCase.testGrape(GroovySriptEngineGrapeTestCase.groovy:9)
             at sun.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
             at sun.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:39)
             at sun.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:25)
             at java.lang.reflect.Method.invoke(Method.java:597)
             at junit.framework.TestCase.runTest(TestCase.java:168)
             at junit.framework.TestCase.runBare(TestCase.java:134)
             at junit.framework.TestResult$1.protect(TestResult.java:110)
             at junit.framework.TestResult.runProtected(TestResult.java:128)
             at junit.framework.TestResult.run(TestResult.java:113)
             at junit.framework.TestCase.run(TestCase.java:124)
             at junit.framework.TestSuite.runTest(TestSuite.java:232)
             at junit.framework.TestSuite.run(TestSuite.java:227)
             at org.junit.internal.runners.JUnit38ClassRunner.run(JUnit38ClassRunner.java:79)
             at org.apache.maven.surefire.junit4.JUnit4TestSet.execute(JUnit4TestSet.java:62)
             at org.apache.maven.surefire.suite.AbstractDirectoryTestSuite.executeTestSet(AbstractDirectoryTestSuite.java:140)
             at org.apache.maven.surefire.suite.AbstractDirectoryTestSuite.execute(AbstractDirectoryTestSuite.java:127)
             at org.apache.maven.surefire.Surefire.run(Surefire.java:177)
             at sun.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
             at sun.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:39)
             at sun.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:25)
             at java.lang.reflect.Method.invoke(Method.java:597)
             at org.apache.maven.surefire.booter.SurefireBooter.runSuitesInProcess(SurefireBooter.java:345)
             at org.apache.maven.surefire.booter.SurefireBooter.main(SurefireBooter.java:1009)
      Caused by: org.codehaus.groovy.control.MultipleCompilationErrorsException: startup failed, GrapeClass.groovy: 7: unable to resolve class org.apache.commons.collecti
      ons.primitives.ArrayIntList
      @ line 7, column 1.
      1 error
      
             at org.codehaus.groovy.control.ErrorCollector.failIfErrors(ErrorCollector.java:296)
             at org.codehaus.groovy.control.CompilationUnit.applyToSourceUnits(CompilationUnit.java:816)
             at org.codehaus.groovy.control.CompilationUnit.compile(CompilationUnit.java:466)
             at groovy.lang.GroovyClassLoader.parseClass(GroovyClassLoader.java:281)
             at groovy.lang.GroovyClassLoader.parseClass(GroovyClassLoader.java:252)
             at groovy.lang.GroovyClassLoader.parseClass(GroovyClassLoader.java:247)
             at groovy.util.GroovyScriptEngine.updateCacheEntry(GroovyScriptEngine.java:333)
             ... 34 more
      

      Which seems to indicate that the class can't be found via the classloader. If I dump the classpath of the classloader then I see that the jar is on the classpath ...

      log.debug(this.class.classLoader.classPath.join(":"))

      outputs

      /Users/ianhomer/.groovy/grapes/commons-primitives/commons-primitives/jars/commons-primitives-1.0.jar

        Activity

        Hide
        Ian Homer added a comment -

        I think the problem is that ScriptClassLoader overrides findClass without ever calling the URLClassloader (which is where the Grab dependency is found)

        Not sure on the appropriate fix for this as the findClass method get's called mutliple times trying to find the appropriate script / class name, e.g.

        DEBUG : findClass : java.lang.org$apache$commons$collections$primitives$ArrayIntList
        DEBUG : findClass : java.io.org$apache$commons$collections$primitives$ArrayIntList
        DEBUG : findClass : java.net.org$apache$commons$collections$primitives$ArrayIntList
        DEBUG : findClass : java.util.org$apache$commons$collections$primitives$ArrayIntList
        DEBUG : findClass : groovy.lang.org$apache$commons$collections$primitives$ArrayIntList
        DEBUG : findClass : groovy.util.org$apache$commons$collections$primitives$ArrayIntList
        DEBUG : findClass : org.apache$commons$collections$primitives$ArrayIntList
        DEBUG : findClass : org.apache.commons$collections$primitives$ArrayIntList
        DEBUG : findClass : org.apache.commons.collections$primitives$ArrayIntList
        DEBUG : findClass : org.apache.commons.collections.primitives$ArrayIntList
        DEBUG : findClass : org.apache.commons.collections.primitives.ArrayIntList

        however a naive fix where we return the super.findClass value if we get a ResourceException in the ScriptClassLoader.findClass execution.

        } catch (ResourceException e) {
        try

        { return super.findClass(className); }

        catch (ClassNotFoundException cnfe)

        { throw new ClassNotFoundException("Could not read " + scriptName + ": " + e;); }

        This seems to works in our system stack, however it bypasses the search for alternative script names listed above since it'll find it on the first pass through. I think this'd be a problem if a grabbed jar included a class that overrode a groovy script in our GroovyScriptEngine directories.

        Show
        Ian Homer added a comment - I think the problem is that ScriptClassLoader overrides findClass without ever calling the URLClassloader (which is where the Grab dependency is found) Not sure on the appropriate fix for this as the findClass method get's called mutliple times trying to find the appropriate script / class name, e.g. DEBUG : findClass : java.lang.org$apache$commons$collections$primitives$ArrayIntList DEBUG : findClass : java.io.org$apache$commons$collections$primitives$ArrayIntList DEBUG : findClass : java.net.org$apache$commons$collections$primitives$ArrayIntList DEBUG : findClass : java.util.org$apache$commons$collections$primitives$ArrayIntList DEBUG : findClass : groovy.lang.org$apache$commons$collections$primitives$ArrayIntList DEBUG : findClass : groovy.util.org$apache$commons$collections$primitives$ArrayIntList DEBUG : findClass : org.apache$commons$collections$primitives$ArrayIntList DEBUG : findClass : org.apache.commons$collections$primitives$ArrayIntList DEBUG : findClass : org.apache.commons.collections$primitives$ArrayIntList DEBUG : findClass : org.apache.commons.collections.primitives$ArrayIntList DEBUG : findClass : org.apache.commons.collections.primitives.ArrayIntList however a naive fix where we return the super.findClass value if we get a ResourceException in the ScriptClassLoader.findClass execution. } catch (ResourceException e) { try { return super.findClass(className); } catch (ClassNotFoundException cnfe) { throw new ClassNotFoundException("Could not read " + scriptName + ": " + e;); } This seems to works in our system stack, however it bypasses the search for alternative script names listed above since it'll find it on the first pass through. I think this'd be a problem if a grabbed jar included a class that overrode a groovy script in our GroovyScriptEngine directories.
        Hide
        Roshan Dawrani added a comment -

        Versions 1.7.x and 1.8.x do not have this issue because GSE rewrite effected rewrite of ScriptClassLoader as well and the new ScriptClassLoader version does not have the reported problem.

        Do we want to fix this issue on 1.6.x as well (because the fix suggested also has some downsides as identified by Ian in his comment)?

        Or, is it enough to have this feature working on 1.7.x and 1.8.x?

        Show
        Roshan Dawrani added a comment - Versions 1.7.x and 1.8.x do not have this issue because GSE rewrite effected rewrite of ScriptClassLoader as well and the new ScriptClassLoader version does not have the reported problem. Do we want to fix this issue on 1.6.x as well (because the fix suggested also has some downsides as identified by Ian in his comment)? Or, is it enough to have this feature working on 1.7.x and 1.8.x?
        Hide
        Guillaume Laforge added a comment -

        I guess it depends on how complicated this issue would be for 1.6.x. If it's trivial, we can fix it, if not, then I'm not sure it's really worth the effort considering Groovy 1.7 is the current official stable branch, and that everybody should migrate to this version. So from my windows, this is good enough it's fixed in 1.7 – unless potentially if a trivial fix can be found.

        Show
        Guillaume Laforge added a comment - I guess it depends on how complicated this issue would be for 1.6.x. If it's trivial, we can fix it, if not, then I'm not sure it's really worth the effort considering Groovy 1.7 is the current official stable branch, and that everybody should migrate to this version. So from my windows, this is good enough it's fixed in 1.7 – unless potentially if a trivial fix can be found.
        Hide
        Roshan Dawrani added a comment -

        More than the size of the fix, what worries me is that any fix made to GSE will also affect the Groovlets, etc and since the fix is related to classloader used in GSE (and that it should delegate to parents) and the fact that app/web servers have all kinds of classloader hierarchies, difficult to be certain of the impact unless we test widely.

        Show
        Roshan Dawrani added a comment - More than the size of the fix, what worries me is that any fix made to GSE will also affect the Groovlets, etc and since the fix is related to classloader used in GSE (and that it should delegate to parents) and the fact that app/web servers have all kinds of classloader hierarchies, difficult to be certain of the impact unless we test widely.
        Hide
        Guillaume Laforge added a comment -

        I definitely can't agree more with your concerns.
        It really sounds just safer to "not fix" it for 1.6 and just ask people to upgrade to the current official stable version of Groovy.

        Show
        Guillaume Laforge added a comment - I definitely can't agree more with your concerns. It really sounds just safer to "not fix" it for 1.6 and just ask people to upgrade to the current official stable version of Groovy.
        Hide
        Roshan Dawrani added a comment -

        Let's see what Jochen/Paul think. If they also agree, we can close this one.

        It's anyway not on critical path - usage of @Grab in scripts loaded via GSE.

        Show
        Roshan Dawrani added a comment - Let's see what Jochen/Paul think. If they also agree, we can close this one. It's anyway not on critical path - usage of @Grab in scripts loaded via GSE.
        Hide
        Paul King added a comment -

        I am not keen on the partial fix. I think we close as won't fix. If someone wants to provide a more complete fix, then we can relook at the problem but I think otherwise we have higher priority issues to address in 1.7 and 1.8 that need attention.

        Show
        Paul King added a comment - I am not keen on the partial fix. I think we close as won't fix. If someone wants to provide a more complete fix, then we can relook at the problem but I think otherwise we have higher priority issues to address in 1.7 and 1.8 that need attention.
        Hide
        Guillaume Laforge added a comment -

        Closing the issue as "won't fix" as the problem is not present anymore in 1.7 and beyond, and it'd be good to upgrade to the official stable release of Groovy instead of staying on 1.6 anyway.

        Show
        Guillaume Laforge added a comment - Closing the issue as "won't fix" as the problem is not present anymore in 1.7 and beyond, and it'd be good to upgrade to the official stable release of Groovy instead of staying on 1.6 anyway.

          People

          • Assignee:
            Unassigned
            Reporter:
            Ian Homer
          • Votes:
            1 Vote for this issue
            Watchers:
            2 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved: