[4.8] compileGroovy.destinationDir does not respect sourceSets.main.groovy.outputDir

[bjhargrave]

Expected Behavior

compileGroovy.destinationDir should default to the value set for sourceSets.main.groovy.outputDir. This worked as expected until 4.8.

Current Behavior

With Gradle 4.8, compileGroovy.destinationDir is ignoring the value of sourceSets.main.groovy.outputDir when sourceSets.main.groovy.outputDir is set to a value.

Context

My build fails since the compileGroovy output is placed in the wrong folder.

Steps to Reproduce (for bugs)

This simple build.gradle file shows the problem. When run on Gradle 4.7, both directories are the same. When run on Gradle 4.8, the compileGroovy.destinationDir value is not the same as the set value for sourceSets.main.groovy.outputDir.

apply plugin: 'groovy'
sourceSets.main.groovy.outputDir = new File(buildDir,'bin')
task display {
  doLast {
    println "sourceSets.main.groovy.outputDir: ${sourceSets.main.groovy.outputDir}"
    println "compileGroovy.destinationDir:     ${compileGroovy.destinationDir}"
  }
}
defaultTasks = ['display']

Your Environment

$ gradle --version

------------------------------------------------------------
Gradle 4.8
------------------------------------------------------------

Build time:   2018-06-04 10:39:58 UTC
Revision:     9e1261240e412cbf61a5e3a5ab734f232b2f887d

Groovy:       2.4.12
Ant:          Apache Ant(TM) version 1.9.11 compiled on March 23 2018
JVM:          1.8.0_162 (Oracle Corporation 25.162-b12)
OS:           Mac OS X 10.13.4 x86_64

[oehme]

Let me first say that adding SourceDirectorySet.outputDir was a mistake and this method should be removed. It's the wrong model. SourceDirectorySet is just what the name says - a set of directories. Having an "outputDir" on that class doesn't make much sense. For instance, multiple such sets can be combined into a composite one, where the outputDir concept completely breaks down.

Instead please set the output directory directly on the corresponding compile task. Also, may I ask why you are changing it at all? What's the use case?

That being said, this shouldn't just silently break. @big-guy I'm assigning you since you were the last one to change the Groovy plugin as part of the lazy task work. A potential fix might be removing this method and advising users to configure the compile task (the method is incubating and hopefully not too widespread). Alternatively we could restore the old behavior and remove it in another release, maybe even deprecate first.

[bjhargrave]

Let me first say that adding SourceDirectorySet.outputDir was a mistake and this method should be removed. It's the wrong model. SourceDirectorySet is just what the name says - a set of directories. Having an "outputDir" on that class doesn't make much sense. For instance, multiple such sets can be combined into a composite one, where the outputDir concept completely breaks down.

In Gradle 4.0, when each language was given its own output folder, the release notes told us to use SourceDirectorySet.outputDir which is why I use it :-)

See https://docs.gradle.org/4.0/release-notes.html#deprecated-apis

Instead please set the output directory directly on the corresponding compile task. Also, may I ask why you are changing it at all? What's the use case?

The use case is that Bnd allows the user to set things like source and output folders in a bnd file, and the Bnd Gradle plugin then needs to configure gradle to use those folders. So this includes java compilation and also groovy (since the Bnd Gradle plugin is built in the Bnd build and is written in groovy). I found this issue when testing the Bnd build with Gradle 4.8 and the Bnd Gradle plugin portion of the build failed since it could not locate the compileGroovy output in the configured output folder.

That being said, this shouldn't just silently break. @big-guy I'm assigning you since you were the last one to change the Groovy plugin as part of the lazy task work. A potential fix might be removing this method and advising users to configure the compile task (the method is incubating and hopefully not too widespread). Alternatively we could restore the old behavior and remove it in another release, maybe even deprecate first.

If you plan to remove the incubating SourceDirectorySet.outputDir property, then the docs needs to be updated in many places since the compiler tasks all indicate the default value for their destinationDir property is the SourceDirectorySet.outputDir property. For example, see https://docs.gradle.org/current/dsl/org.gradle.api.tasks.compile.GroovyCompile.html#org.gradle.api.tasks.compile.GroovyCompile:destinationDir and https://docs.gradle.org/current/dsl/org.gradle.api.tasks.compile.JavaCompile.html#org.gradle.api.tasks.compile.JavaCompile:destinationDir.

[bjhargrave]

One more question. Currently, setting SourceDirectorySet.outputDir means the directory will appear in the SourceSetOutput.classesDirs property. Setting the compile task's destinationDir property does not result in the directory appearing in the SourceSetOutput.classesDirs property. So, when you set the compile task's destinationDir property, should you also add that folder to the SourceSetOutput? For example:

compileTask.destinationDir = outputDir
sourceSets.main.output.dir(outputDir, builtBy: compileTask)

[oehme]

Yes, if we remove this method, the classesDirs would have to react to setting the destination on the compile task directly.

Given this has leaked so in so many places I guess we'll have to deprecate first before removing.

[donat]

@lacasseio #5726 fixed this issue too, right?

[lacasseio]

My bad, I forgot to close this issue.