Implements and tests build cache for ScalaCompile and ScalaDoc

[dbrewster]

Issue: #1956

Provides an implementation for ScalaCompile and ScalaDoc that makes
them cacheable.

Also added tests from ScalaCompile and DID NOT add tests for ScalaDoc
as I didn't see tests for the Groovy or Java versions of these

Context

https://groups.google.com/d/msg/gradle-dev/-2hVXXDiQEU/mIbgR0FRAgAJ

Contributor Checklist

• Review Contribution Guidelines
• Sign Gradle CLA
• Link to Design Spec for changes that affect more than 1 public API (that is, not in an internal package) or updates to > 20 files
• Provide integration tests (under <subproject>/src/integTest) to verify changes from a user perspective
• Provide unit tests (under <subproject>/src/test) to verify logic
• Update User Guide, DSL Reference, and Javadoc for public-facing changes
• Ensure that tests pass locally: ./gradlew quickCheck <impacted-subproject>:check

Gradle Core Team Checklist

• Verify design and implementation
• Verify test coverage and CI build status
• Verify documentation including proper use of @since and @Incubating annotations for all public APIs
• Recognize contributor in release notes

[dbrewster]

There is one test breaking that I believe I can't do anything about 'org.gradle.AbstractCachedCompileIntegrationTest#compilation is cached if the build executed from a different directory'

I think the scala compiler is detecting the different directory and rebuilding anyway. I don't know if I can change that in the current implementation. Please advise.

[eriwen]

@lptr assigned to you to prioritize with your team.

[big-guy]

Hey @dbrewster,

Thanks for the PR. We're not going to get to this for the next release because we're trying to wrap up other things, but it's on our radar to review.

WRT the test failure, if the Scala compiler is running, it's Gradle that decides to run it. So it's probably an issue with an absolute path being used as a @Input or @InputFile somewhere in the Scala compiler options (ScalaCompileOptions or a super class).

We would try tracking this down by reviewing the existing inputs/annotations on the task and enabling --debug in the tests and looking at the output from DefaultTaskOutputCachingBuildCacheKeyBuilder. For properties that change value between runs, that points to either 1) absolute paths or 2) non-deterministic behavior.

Thanks again! We'll let you know when we're ready to merge this.

[dbrewster]

Sounds good. I will track down the test failure and have this ready

Is there a doc on release timelines somewhere?

[big-guy]

It's not a fixed schedule, but we try to release every 6 weeks (this is week 5). We're a bit behind right now, so I suspect the final release won't be out for 2-3 more weeks. If all goes well, I would expect the next-next release to be early July. We have our conference at the end of June, so it's likely we'll have another long release period because of it.

[eriwen]

@dbrewster release schedule is loosely defined here: https://github.com/gradle/gradle/milestones

[big-guy]

@dbrewster I've taken a quick look at the failures. I think the fix is straightforward.

By default, the SourceTask considers absolute paths to any source files (this is just the historical default). For GroovyCompile and JavaCompile, we change that path sensitivity to NAME_ONLY: https://github.com/gradle/gradle/blob/master/subprojects/language-java/src/main/java/org/gradle/api/tasks/compile/JavaCompile.java#L84-L91

We need to do the same for AbstractScalaCompile, but we should use RELATIVE instead of NAME_ONLY. We may be able to use NAME_ONLY with the Scala compiler too, but this will be good enough until we can look at what the Scala compiler requires/guarantees WRT source file name handling.

Can you also add a Scala equivalent of GroovyCompileRelocationIntegrationTest?

We should have coverage for Groovydoc/Scaladoc, but you don't need to add that for now.

This still won't make the coming release, but I know there's a lot of interest in moving this forward, so hopefully we get can this into a nightly soon.

Thanks again!

[big-guy]

Can you remove this duplicate header?

[dbrewster]

@big-guy I'm working on adding the relocation test for scala but am stuck on the zincClasspath setting for the task:

        task compile(type: ScalaCompile) {
            sourceCompatibility = JavaVersion.current()
            targetCompatibility = JavaVersion.current()
            destinationDir = file("build/classes")
            source "src/main/scala"
            classpath = files()
            scalaClasspath = files('$classpath')
            zincClasspath = ?????
        }

What do I put there? I'm assuming that you intended to define a new task specific to this so you can control where and how things are getting compiled but I can't figure out how to get a legal setting for that class path.

[dbrewster]

@big-guy I pushed my code with a broken test due to the question above.

I need help figuring out how to either set the scalaClasspath/zincClasspath or redo the build file in the test to accomplish the controls on caching. Sorry for being a noob at gradle...

[fkorotkov]

@dbrewster

./gradlew :plugins:integTest --tests=org.gradle.scala.compile.CachedScalaCompileIntegrationTest

fails for me on your branch with Plugin [id: 'scala'] was not found in any of the following sources. How do you run it? Other tests are passing for me.

[dbrewster]

@big-guy I don't know where we left this PR; however, I'm on vacation the next two weeks. What do we still need to do to get this merged into the next release (after 4)

[wolfs]

There are still a few issues open:

• What does IncrementalCompileOptions.analysisFile contain? If we pull from the cache this is probably not accurate anymore. We could add this as an output file but I do not know if the contents are relocateable. I am also quite unsure what should happen to IncrementalCompileOptions.publishedCode. An alternative would be to disable caching if incremental compilation is enabled as a first step.
• How is Scala compilation dependent on the Java version used to 1) run Gradle 2) used in compileOptions to compile Java code in src/<name>/scala?
  We ran into some problems with Groovy here that the bytecode was different depending on what Java version Gradle was running with. Do you have any insights here? We would also require more test coverage in this area.
• Test coverage for relocateability and caching for Scaladoc. It would probably be easier to remove Scaladoc from this PR and only focus on ScalaCompile. As soon as this PR is merged there can be a follow up regarding caching of Scaladoc.

Do you know how to address these problems? It would be very valuable to us if you can provide some further insights into the questions above.

[wolfs]

@dbrewster I started working on making Scala compile Cacheable. I based my work on your branch. See #2399. I will close this PR in favor of my PR. If you have any more insights into Scala compilation or ScalaDoc, please comment on the new PR.
Thanks for the time you invested in this PR and for your contribution.