On 23 July 2014 at 8:07:00 am, Adam Murdoch (adam.murd...@gradleware.com) wrote:
On 19 Jul 2014, at 8:57 am, Luke Daley <luke.da...@gradleware.com> wrote: That means a breaking change as GroovyRuntime is public. Granted we did already break it for 2.0. We didn't mention it AFAICT. GroovyRuntime is incubating, and due for an overhaul when we add the new groovy plugin, so I’m not too concerned about changing it. If we wanted, we would replace (via deprecation) the existing method with two other methods - one to calculate the groovy compiler classpath and one to calculate the ant based groovydoc tool classpath. I missed that it was incubating. Given that we got grief other than this issue it might be ok to break compatibility. On Sat, Jul 19, 2014 at 8:11 AM, Adam Murdoch <adam.murd...@gradleware.com> wrote: On 10 Jul 2014, at 11:17 am, Luke Daley <luke.da...@gradleware.com> wrote: On 10 July 2014 at 11:13:22 am, Luke Daley (luke.da...@gradleware.com) wrote: On 9 July 2014 at 4:49:27 pm, Perryn Fowler (perryn.fow...@gradleware.com) wrote: I did some investigation and I *think* I know whats going on with this regression. If I'm right, the way it used to work is probably not desireable anyway, so it would be good to discuss what to do about it... What I think is going on: GROOVY CLASSPATH INFERENCE The GroovyPlugin sets the classpath of the GroovyDoc task to the compile classpath of the main sourceset The GroovyBasePlugin sets the groovyClasspath by using GroovyRuntime to infer it from the (compile) classpath The inference mechanism looks for a groovy jar on the classpath If it finds groovy-all.jar it just uses that jar. If it finds a different groovy jar it creates a new configuration and adds the jar as a dependency (I am not sure what this achieves over just using the jar) It would make more sense to me if it added a 'tool' version of the jar instead (in fact the comments say it *does* do this) This behaviour seems to have been changed by Adam in commit 818e0b9 I suspect this commit introduced the regression Right, here’s the culprit: https://github.com/gradle/gradle/commit/818e0b9#diff-95e522b5be7746a11b2f9d31a2f29d9dL101 HOWEVER... The exception reported in the forums does not seem to occur when executing the groovydoc tool itself, but when instantiating a gradle BasicAntBuilder (in order to define and execute an ant groovydoc task) This is because BasicAntBuilder inherits from groovy.util.AntBuilder This is loaded by a classloader that by default has a classpath containing classPathRegistry.getClassPath("GROOVY"), but this appears to be replaced by AntGroovyDoc with the groovyClasspath from the GroovyDoc task This all leads me to suspect that this used to work (prior to Adam's commit) by picking up groovy.util.AntBuilder from the users compile configuration rather than from the gradle installation That doesn't seem desireable... We need to change GroovyRuntime to have another method that returns a classpath with the ant classes and wire that to the groovydoc task. Just noticed that GroovyRuntime is public API (why?). This means we should actually just revert the change to this method. Only when generating Javadoc. We don’t want to make groovy-ant visible at compilation time if it’s not declared as a dependency. -- Adam Murdoch Gradle Co-founder http://www.gradle.org CTO Gradleware Inc. - Gradle Training, Support, Consulting http://www.gradleware.com -- Adam Murdoch Gradle Co-founder http://www.gradle.org CTO Gradleware Inc. - Gradle Training, Support, Consulting http://www.gradleware.com
