[sbt 0.13.15] calling compile twice causes full compilation on JDK9

[eed3si9n]

Originally reported by @smarter

steps

1. compile
2. compile

problem

It causes full compilation.

expectation

No compilation the second times.

notes

https://gist.github.com/eed3si9n/8ee47e2e5dafaa5ef9f00833679a0a0b

[smarter]

Some more information:

1. As far as I can tell, this does not happen with sbt 1.0.0-M5, something must have changed in zinc but I don't know what.
2. I think this happen because incremental compilation somehow cares about the Scala bootclasspath containing the JVM bootclasspath, otherwise it gets confused and say something like [debug] [naha] Invalidating /root/.sbt/0.13/java9-rt-ext-oracle_corporation_9_debian/rt.jar: could not find class java.lang.Object on the classpath. in the log above. Adding the JVM bootclasspath to the Scala bootclasspath is normally done by

   sbt/compile/src/main/scala/sbt/compiler/CompilerArguments.scala

   Line 48 in 4ab11d7

   val originalBoot = System.getProperty("sun.boot.class.path", "")

   but in Java 9 sun.boot.class.path is always empty, you're not supposed to use it. I have no idea why sbt messes with the Scala bootclasspath in the first place, it shouldn't be necessary.
3. You don't need Java 9 to reproduce this issue, you just need to remove the JVM bootclasspath from the Scala bootclasspath, this can be done by setting classpathOptions := classpathOptions.value.copy(autoBoot = false, filterLibrary = false) (autoBoot = false leaves the Scala bootclasspath empty, filterLibrary = false adds the scala-library to the regular classpath since it's normally part of the bootclasspath which is now empty). Here's an example project: https://github.com/smarter/sbt-hello/tree/bootclasspath

[dwijnand]

/cc @retronym.

[retronym]

For this recompilation bug, on the first compile, a dependency on rt.jar:!/java.lang.Object.class is recorded. On the subsequent compile command, the incremental compiler is unable to find rt.jar in what it believes is the definitive classpath for the second compile, and hence triggers are recompile (expecting a compile error about the missing dependency).

SBT does have logic to add sun.boot.classpath to its list of dependencies, but as you've pointed out, this no longer exists in Java 9. The Scala compiler has always used a more general lookup of the first system property that matches *.boot.classpath and our updated script now pass scala.boot.classpath through to compensate for the JDK 9's inabiltiy to introspect on the JARs in the boot classpath.

At least in the context of supporting Java 9, SBT adds the exported rt.jar to the bootclasspath of any Scalac by using the system property scala.ext.dirs. This allowed basic support for Java 9, even before we had made any modifications to the Scala compiler itself. However, the incremental compiler never understood this system property, and was not updated when SBT started using it.

I've submitted a PR that seems to fix this.

[retronym]

The upcoming build of Scala 2.10.7 will have native support to deal with the removal of rt.jar in Java 9. We plan to release SBT 0.13.next based on this version. That will simplify the launcher script, the rt.jar export and -Dscala.ext.dirs=... property will no longer be required.

However, we might also need to make the incremental compilers aware of the new platform classloader, to avoid regressing into this overcompilation bug. I'll look at that in the coming days.

[retronym]

Indeed, SBT overcompiles when using the native support for rt.jar.

With the project Scala version set to 2.10.7-SNAPSHOT:

> eval System.setProperty("scala.ext.dirs", "")
[info] ans: String =
> set scalaHome := Some(file("/Users/jz/code/scala/build/pack"))
[info] Defining *:scalaHome
[info] The new value will be used by *:allDependencies, *:evicted and 5 others.
[info]      Run `last` for details.
[info] Reapplying settings...
[info] Set current project to scratch2 (in build file:/Users/jz/code/scratch2/)
> compile
[info] Compiling 1 Scala source to /Users/jz/code/scratch2/target/scala-2.10/classes...
[success] Total time: 0 s, completed 1 Nov. 2017, 10:22:59 am
> compile
[info] Compiling 1 Scala source to /Users/jz/code/scratch2/target/scala-2.10/classes...
[success] Total time: 0 s, completed 1 Nov. 2017, 10:23:01 am

2.12.4 does not appear to have the problem:

> set List(scalaHome := None, scalaVersion := "2.12.4")
[info] Defining *:scalaHome, *:scalaVersion
[info] The new values will be used by *:allDependencies, *:crossScalaVersions and 17 others.
[info]      Run `last` for details.
[info] Reapplying settings...
[info] Set current project to scratch2 (in build file:/Users/jz/code/scratch2/)
> eval System.getProperty("scala.ext.dirs")
[info] ans: String =
> eval System.getProperty("java.specification.version")
[info] ans: String = 9

> compile
[info] Updating {file:/Users/jz/code/scratch2/}scratch2...
[info] Resolving jline#jline;2.14.5 ...
[info] Done updating.
[info] Compiling 1 Scala source and 1 Java source to /Users/jz/code/scratch2/target/scala-2.12/classes...
[success] Total time: 0 s, completed 1 Nov. 2017, 10:25:43 am
> compile
[success] Total time: 0 s, completed 1 Nov. 2017, 10:25:45 am

[retronym]

Right, that's because on the 2.10.x backport, I modelled the virtual rt.jar entries as ZipEntry, whereas on 2.12.x we use a new sort of AbstractFile, PlainNioFile.

SBT 0.13 ignores 2.12's PlainNioFile, so no binary dependency is registered for things from the platform classpath:

                f match {
                  case ze: ZipArchive#Entry => for (zip <- ze.underlyingSource; zipFile <- Option(zip.file)) binaryDependency(zipFile, className)
                  case pf: PlainFile        => binaryDependency(pf.file, className)
                  case _                    => ()
                }

⚡ (java_use 9; scala)
Welcome to Scala 2.12.4 (Java HotSpot(TM) 64-Bit Server VM, Java 9).
Type in expressions for evaluation. Or try :help.
scala> val f = global.classPath.findClass("java.lang.Object").get.binary.get; (f.getClass.toString, f.toString)
f: scala.tools.nsc.io.AbstractFile = PlainNioFile()
res8: (String, String) = (class scala.reflect.io.PlainNioFile,/modules/java.base/java/lang/Object.class)

⚡ (java_use 9; ./build/quick/bin/scala -nobootcp)
Welcome to Scala version 2.10.6-20171030-113547-4b848761c3 (Java HotSpot(TM) 64-Bit Server VM, Java 9).

scala> {val f = global.classPath.findClass("java.lang.Object").get.binary.get; println(f.getClass); println(f.toString)}
class scala.reflect.io.JavaToolsPlatformArchive$$anonfun$iterator$1$FileEntry$3
/Library/Java/JavaVirtualMachines/jdk-9.jdk/Contents/Home(java/lang/Object.class)

So for 2.12, SBT undercompiles if the JDK happened to be overwritten, and in 2.11 is overcompiles (probably because the underlying source of the ZipEntry is reported as $JAVA_HOME and it gets confused seeing a directory in code paths where only JAR files have previously ventured.

[retronym]

I've added a commit to my PR that makes SBT 0.13 ignore directories as the underlying source of ZipFile. That makes 2.10/2.11 consistent with 2.12, which is the lesser of two evils.

[larsrh]

I believe this issue can be closed now, because #3701 has been merged.

[dwijnand]

Thanks, Lars.