Gradle should pass -sourcepath to javac by default to avoid false positives

(Thomas Broyer) #1

Here’s a self-contained repro:

Note that setting ‘-sourcepath’ to the task’s ‘source.asPath’ won’t work, as soon as the source set has includes or excludes (i.e. does not include everything in the srcDirs). Because Gradle lists all source files as arguments, the ‘-sourcepath’ can be empty as javac shouldn’t need to resolve any dependency that’s not already available as a compiled class in the classpath.

(Thomas Broyer) #2

Sigh, an empty sourcepath will cause issues with annotation processors using ‘processingEnv.getFiler().getResource(StandardLocation.SOURCE_PATH, …)’, but I’d assuming this is the exception rather than the norm and people can easily configure a custom sourcepath. Gradle already has that issue when the resource the annotation processors looks for is in src/main/resources; so I think an empty sourcepath by default is OK.

(Thomas Broyer) #3

Here’s an in-depth explanation of the problem:

(Luke Daley) #4

I’m not sure it’s worth deviating from javac’s default behaviour of defaulting sourcepath to classpath. It is problematic that we don’t null it out in some cases (there’s a handful of threads on this forum about this), but aligning with how ‘javac’'s interface works has some appeal.

It’s a step too far to implicitly set a sourcepath IMO. If a user is having problems because JARs on their classpath contain source, they can always null out the source path themselves.

(Thomas Broyer) #5

Thanks for the reply.

How about defaulting to an empty sourcepath with the ability to ‘null’ it out and revert to javac’s default behavior?

Something like ‘CompileOptions.sourcepath’ defaulting to either the empty string or ‘File.pathSeparator’ or an empty ‘FileCollection’, and allowing people to set it to ‘null’ or ‘task.classpath’ to revert to the “no sourcepath” or “sourcepath == classpath” behavior.

Alternatively, defining a ‘CompileOptions.sourcepath’ property, even defaulting to null, to make it easier to change the sourcepath (rather than adding to ‘compilerArgs’), would be a welcome addition.

But honestly, with such a “high-level” tool like Gradle, I can’t imagine any reason why you’d want the “sourcepath == classpath” behavior: with dependency management that also resolves transitive dependencies, people don’t really check what’s in their classpath (bad practice but we can’t blame them, we all just throw a bunch of “dependencies.compile ‘somelib:somelib’” in our build scripts without running ‘./gradlew dependencies’ and checking the content of the JARs, and/or setting up a blessed repo manager where only verified dependencies are ever deployed); the “principle of least surprise” would dictate that the build fails if compiled sources reference anything not in ‘.class’ form in the classpath.

Following that principle, you’d at least use ‘-implicit:none’ to avoid getting ‘.class’ files for sources implicitly recompiled from your classpath.

(Luke Daley) #6

Apparently, the current intention of Gradle is to use an empty source path. The person who is ultimately responsible for such decisions gave the ok to make this change, as a conscious breaking change.

Are you interested in contributing this change? That would entail:

1. Adding a sourcepath property to ‘CompileOptions’ that took a file collection, defaulting to 'null’
2. Changing the compiler infrastructure to effectively use an empty sourcepath when compileOptions.sourcepath == null
3. We’d then need integration test coverage:
    1. something that proves we default to an empty source path (e.g. class referencing other class available as source on the classpath doesn’t compile)
    2. javac default behaviour can be simulated by manually setting ‘compileOptions.sourcepath = configurations.compile’

(Thomas Broyer) #7

Here it is:

This is my first contribution to Gradle so I hope I got everything right. I ran languageJava:check, languageScala:check and languageGroovy:check.

I wasn’t sure whether to condition sourcepath to the includeClasspath (vs. includeMainOptions). GIven the use of includeClasspath in languageScala (with the Zinc compiler), I thought it’d be better as a “main option”.

(Luke Daley) #8

Excellent. I put some comments on the PR about a little more test coverage.

I’ll push to get this into 2.4, but no promises. The window is closing on that release. It will be in 2.5 at the latest.