Skip to content

Commit

Permalink
feature: Automatically add -release option when needed
Browse files Browse the repository at this point in the history
Previously, if we compiled with a Bloop running an newer JDK we would not see any errors even when using methods belonging to a newer JDK. Now, we automatically add -release flag, which makes sure that we will fail any compilation that should not pass with an older JDK (if specified in configuration)
  • Loading branch information
tgodzik committed Oct 10, 2023
1 parent 3558e4c commit ccd23c8
Show file tree
Hide file tree
Showing 4 changed files with 143 additions and 33 deletions.
39 changes: 38 additions & 1 deletion backend/src/main/scala/bloop/Compiler.scala
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ import java.util.concurrent.Executor

import scala.collection.mutable
import scala.concurrent.Promise
import scala.util.control.NonFatal

import bloop.io.AbsolutePath
import bloop.io.ParallelOps
Expand All @@ -23,6 +24,7 @@ import bloop.task.Task
import bloop.tracing.BraveTracer
import bloop.util.AnalysisUtils
import bloop.util.CacheHashCode
import bloop.util.JavaRuntime
import bloop.util.UUIDUtil

import monix.execution.Scheduler
Expand Down Expand Up @@ -289,7 +291,42 @@ object Compiler {
// Sources are all files
val sources = inputs.sources.map(path => converter.toVirtualFile(path.underlying))
val classpath = inputs.classpath.map(path => converter.toVirtualFile(path.underlying))
val optionsWithoutFatalWarnings = inputs.scalacOptions.flatMap { option =>
def existsReleaseSetting = inputs.scalacOptions.exists(opt =>
opt.startsWith("-release") ||
opt.startsWith("--release") ||
opt.startsWith("-java-output-version")
)
def sameHome = inputs.javacBin match {
case Some(bin) => bin.getParent.getParent == JavaRuntime.home
case None => false
}

val scalacOptions = inputs.javacBin.flatMap(binary =>
// <JAVA_HOME>/bin/java
JavaRuntime.getJavaVersionFromJavaHome(binary.getParent.getParent)
) match {
case None => inputs.scalacOptions
case Some(_) if existsReleaseSetting || sameHome => inputs.scalacOptions
case Some(version) =>
try {
val numVer = if (version.startsWith("1.8")) 8 else version.takeWhile(_.isDigit).toInt
val bloopNumVer = JavaRuntime.version.takeWhile(_.isDigit).toInt
if (bloopNumVer > numVer) {
inputs.scalacOptions ++ List("-release", numVer.toString())
} else {
logger.warn(
s"Bloop is run with ${JavaRuntime.version} but your code requires $version to compile, " +
"this might cause some compilation issues when using JDK API unsupported by the Bloop's current JVM version"
)
inputs.scalacOptions
}
} catch {
case NonFatal(_) =>
inputs.scalacOptions
}
}

val optionsWithoutFatalWarnings = scalacOptions.flatMap { option =>
if (option != "-Xfatal-warnings") List(option)
else {
if (!isFatalWarningsEnabled) isFatalWarningsEnabled = true
Expand Down
31 changes: 30 additions & 1 deletion backend/src/main/scala/bloop/util/JavaRuntime.scala
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,12 @@ import scala.util.Try

import bloop.io.AbsolutePath

import com.typesafe.config.ConfigException
import com.typesafe.config.ConfigFactory
import com.typesafe.config.ConfigParseOptions
import com.typesafe.config.ConfigSyntax
import scala.collection.concurrent.TrieMap

sealed trait JavaRuntime
object JavaRuntime {
case object JDK extends JavaRuntime
Expand All @@ -17,10 +23,33 @@ object JavaRuntime {
val home: AbsolutePath = AbsolutePath(sys.props("java.home"))
val version: String = sys.props("java.version")
val javac: Option[AbsolutePath] = javacBinaryFromJavaHome(home)

private val versions: TrieMap[AbsolutePath, Option[String]] =
TrieMap.empty[AbsolutePath, Option[String]]
// Has to be a def, not a val, otherwise we get "loader constraint violation"
def javaCompiler: Option[JavaCompiler] = Option(ToolProvider.getSystemJavaCompiler)

def getJavaVersionFromJavaHome(javaHome: AbsolutePath): Option[String] = versions.getOrElseUpdate(
javaHome, {
val releaseFile = javaHome.resolve("release")
def rtJar = javaHome.resolve("lib").resolve("rt.jar")
if (releaseFile.exists) {
val properties = ConfigFactory.parseFile(
releaseFile.toFile,
ConfigParseOptions.defaults().setSyntax(ConfigSyntax.PROPERTIES)
)
try Some(properties.getString("JAVA_VERSION").stripPrefix("\"").stripSuffix("\""))
catch {
case _: ConfigException =>
None
}
} else if (rtJar.exists) {
Some("1.8")
} else {
None
}
}
)

/**
* Detects the runtime of the running JDK instance.
*/
Expand Down
40 changes: 9 additions & 31 deletions frontend/src/main/scala/bloop/data/Project.scala
Original file line number Diff line number Diff line change
Expand Up @@ -23,11 +23,8 @@ import bloop.io.ByteHasher
import bloop.logging.DebugFilter
import bloop.logging.Logger
import bloop.task.Task
import bloop.util.JavaRuntime

import com.typesafe.config.ConfigException
import com.typesafe.config.ConfigFactory
import com.typesafe.config.ConfigParseOptions
import com.typesafe.config.ConfigSyntax
import scalaz.Cord
import xsbti.compile.ClasspathOptions
import xsbti.compile.CompileOrder
Expand Down Expand Up @@ -225,39 +222,20 @@ final case class Project(
case i => i
}
}
def getJavaVersionFromJavaHome(javaHome: AbsolutePath): String = {
val releaseFile = javaHome.resolve("release")
def rtJar = javaHome.resolve("lib").resolve("rt.jar")
if (releaseFile.exists) {
val properties = ConfigFactory.parseFile(
releaseFile.toFile,
ConfigParseOptions.defaults().setSyntax(ConfigSyntax.PROPERTIES)
)
try properties.getString("JAVA_VERSION").stripPrefix("\"").stripSuffix("\"")
catch {
case _: ConfigException =>
logger.error(
s"$javaHome release file missing JAVA_VERSION property - using Bloop's JVM version ${Properties.javaVersion}"
)
Properties.javaVersion
}
} else if (rtJar.exists) {
// jdk 8 doesn't have `release` file
"1.8.0"
} else {
logger.error(
s"No `release` file found in $javaHome - using Bloop's JVM version ${Properties.javaVersion}"
)
Properties.javaVersion
}
}

val compileVersion = compileJdkConfig
.map(f =>
if (f.javaHome == AbsolutePath(Properties.javaHome))
Properties.javaVersion
else
getJavaVersionFromJavaHome(f.javaHome)
JavaRuntime.getJavaVersionFromJavaHome(f.javaHome).getOrElse {

logger.error(
s"${f.javaHome} release file missing JAVA_VERSION property - using Bloop's JVM version ${Properties.javaVersion}"
)

Properties.javaVersion
}
)
.getOrElse(Properties.javaVersion)
.split("-")
Expand Down
66 changes: 66 additions & 0 deletions frontend/src/test/scala/bloop/JavaVersionSpec.scala
Original file line number Diff line number Diff line change
@@ -0,0 +1,66 @@
package bloop

import bloop.cli.ExitStatus
import bloop.config.Config
import bloop.logging.RecordingLogger
import bloop.util.TestProject
import bloop.util.TestUtil

object JavaVersionSpec extends bloop.testing.BaseSuite {

private val jvmManager = coursierapi.JvmManager.create()

def checkFlag(scalacOpts: List[String], jdkVersion: String = "8") = {
val javaHome = jvmManager.get(jdkVersion).toPath()
val jvmConfig = Some(Config.JvmConfig(Some(javaHome), Nil))
TestUtil.withinWorkspace { workspace =>
val sources = List(
"""/main/scala/Foo.scala
|class Foo{
| val output = "La ".repeat(2) + "Land";
|}
""".stripMargin
)

val logger = new RecordingLogger(ansiCodesSupported = false)
val `A` =
TestProject(workspace, "a", sources, jvmConfig = jvmConfig, scalacOptions = scalacOpts)
val projects = List(`A`)
val state = loadState(workspace, projects, logger)
val compiledState = state.compile(`A`)
if (jdkVersion == "8") {
assertExitStatus(compiledState, ExitStatus.CompilationError)
val targetFoo = TestUtil.universalPath("a/src/main/scala/Foo.scala")
assertNoDiff(
logger.renderErrors(),
s"""|[E1] $targetFoo:2:22
| value repeat is not a member of String
| L2: val output = "La ".repeat(2) + "Land";
| ^
|a/src/main/scala/Foo.scala: L2 [E1]
|Failed to compile 'a'
|""".stripMargin
)
} else {
assertExitStatus(compiledState, ExitStatus.Ok)
}
}
}

test("flag-is-added-correctly") {
checkFlag(Nil)
}

test("flag-is-not-added-correctly") {
checkFlag(List("-release", "8"))
checkFlag(List("-release:8"))
}

test("compiles-with-11") {
checkFlag(Nil, jdkVersion = "11")
}

test("compiles-with-17") {
checkFlag(Nil, jdkVersion = "17")
}
}

0 comments on commit ccd23c8

Please sign in to comment.