Merge pull request #217 from tpasternak/bloop-compiles-against-right-jvm

Bloop compile to correct JVM version

Author: romanowski

.github/workflows/ci.yml

@@ -42,7 +42,9 @@ jobs:
         if-no-files-found: error
         retention-days: 2
     - name: Unit tests
-      run: ./mill -i unitTests
+      run: |
+        ./mill -i unitTests
+        ./mill -i bloop-rifle._.test
     - name: JVM integration tests
       run: ./mill -i integration.jvm.test
 

build.sc

@@ -485,6 +485,7 @@ class TestRunner(val crossScalaVersion: String) extends CrossSbtModule with Scal
 }
 
 class BloopRifle(val crossScalaVersion: String) extends CrossSbtModule with ScalaCliPublishModule
+    with HasTests
     with ScalaCliScalafixModule {
   def scalacOptions = T {
     super.scalacOptions() ++ Seq("-Ywarn-unused", "-deprecation")
@@ -517,6 +518,8 @@ class BloopRifle(val crossScalaVersion: String) extends CrossSbtModule with Scal
     PathRef(dest)
   }
   def generatedSources = super.generatedSources() ++ Seq(constantsFile())
+
+  object test extends Tests with ScalaCliScalafixModule
 }
 
 class TastyLib(val crossScalaVersion: String) extends CrossSbtModule with ScalaCliPublishModule

modules/bloop-rifle/src/main/scala/scala/build/bloop/BloopServer.scala

@@ -3,31 +3,37 @@ package scala.build.bloop
 import ch.epfl.scala.bsp4j
 import org.eclipse.lsp4j.jsonrpc
 
-import java.io.IOException
+import java.io.{File, IOException}
 import java.net.{ConnectException, Socket}
-import java.nio.file.{Files, Path}
+import java.nio.file.{Files, Path, Paths}
 import java.util.concurrent.{Future => JFuture, ScheduledExecutorService, TimeoutException}
 
 import scala.annotation.tailrec
 import scala.build.bloop.bloop4j.BloopExtraBuildParams
+import scala.build.blooprifle.BloopRifleConfig.{AtLeast, Strict}
+import scala.build.blooprifle._
 import scala.build.blooprifle.internal.Constants
-import scala.build.blooprifle.{BloopRifle, BloopRifleConfig, BloopRifleLogger, BspConnection}
 import scala.concurrent.Await
-import scala.concurrent.duration.{Duration, FiniteDuration}
+import scala.concurrent.duration._
 import scala.jdk.CollectionConverters._
 
 trait BloopServer {
   def server: BuildServer
 
   def shutdown(): Unit
+
+  def jvmVersion: String
+
+  def bloopVersion: String
 }
 
 object BloopServer {
-
   private case class BloopServerImpl(
     server: BuildServer,
     listeningFuture: JFuture[Void],
-    socket: Socket
+    socket: Socket,
+    jvmVersion: String,
+    bloopVersion: String
   ) extends BloopServer {
     def shutdown(): Unit = {
       // Close the jsonrpc thread listening to input messages
@@ -37,31 +43,77 @@ object BloopServer {
     }
   }
 
+  private case class ResolvedBloopParameters(
+    bloopVersion: BloopVersion,
+    jvmRelease: Int,
+    javaPath: String
+  )
+
+  private def resolveBloopInfo(
+    bloopInfo: BloopServerRuntimeInfo,
+    config: BloopRifleConfig
+  ): ResolvedBloopParameters = {
+    val bloopV: BloopVersion = config.retainedBloopVersion match {
+      case AtLeast(version) =>
+        val ord = Ordering.fromLessThan[BloopVersion](_ isOlderThan _)
+        Seq(bloopInfo.bloopVersion, version).max(ord)
+      case Strict(version) => version
+    }
+    val jvmV          = List(bloopInfo.jvmVersion, config.minimumBloopJvm).max
+    val bloopInfoJava = Paths.get(bloopInfo.javaHome, "bin", "java").toString()
+    val expectedJava  = if (jvmV >= bloopInfo.jvmVersion) config.javaPath else bloopInfoJava
+    ResolvedBloopParameters(bloopV, jvmV, expectedJava)
+  }
+
   private def ensureBloopRunning(
     config: BloopRifleConfig,
     startServerChecksPool: ScheduledExecutorService,
     logger: BloopRifleLogger
-  ): Unit = {
-
-    val isBloopRunning = BloopRifle.check(config, logger, startServerChecksPool)
-
-    logger.debug(
-      if (isBloopRunning) s"Bloop is running on ${config.host}:${config.port}"
-      else s"No bloop daemon found on ${config.host}:${config.port}"
-    )
-
-    if (!isBloopRunning) {
-      logger.debug("Starting bloop server")
-      val serverStartedFuture = BloopRifle.startServer(
+  ): BloopServerRuntimeInfo = {
+    val workdir = new File(".").getCanonicalFile.toPath
+    def startBloop(bloopVersion: String, bloopJava: String) = {
+      logger.info(s"Starting bloop $bloopVersion on $bloopJava")
+      val fut = BloopRifle.startServer(
         config,
         startServerChecksPool,
-        logger
+        logger,
+        bloopVersion,
+        bloopJava
       )
-
-      Await.result(serverStartedFuture, Duration.Inf)
-      logger.debug("Bloop server started")
+      Await.result(fut, 30.seconds)
+    }
+    def exitBloop() = BloopRifle.exit(config, workdir, logger)
+
+    val bloopInfo =
+      BloopRifle.getCurrentBloopVersion(config, logger, workdir, startServerChecksPool)
+    val isRunning = BloopRifle.check(config, logger, startServerChecksPool)
+    val ResolvedBloopParameters(expectedBloopVersion, expectedBloopJvmRelease, javaPath) =
+      bloopInfo match {
+        case Left(error) =>
+          error match {
+            case BloopNotRunning =>
+            case ParsingFailed(bloopAboutOutput) =>
+              logger.info(s"Failed to parse output of 'bloop about':\n$bloopAboutOutput")
+          }
+          ResolvedBloopParameters(
+            config.retainedBloopVersion.version,
+            config.minimumBloopJvm,
+            config.javaPath
+          )
+        case Right(value) => resolveBloopInfo(value, config)
+      }
+    val bloopVersionIsOk = bloopInfo.exists(_.bloopVersion == expectedBloopVersion)
+    val bloopJvmIsOk     = bloopInfo.exists(_.jvmVersion == expectedBloopJvmRelease)
+    val isOk             = bloopVersionIsOk && bloopJvmIsOk
+
+    if (!isOk) {
+      logger.info(s"Bloop currently running: $bloopInfo")
+      if (isRunning) exitBloop()
+      startBloop(expectedBloopVersion.raw, javaPath)
     }
 
+    BloopRifle.getCurrentBloopVersion(config, logger, workdir, startServerChecksPool)
+      .getOrElse(throw new RuntimeException("Fatal error, could not spawn Bloop"))
   }
 
   private def connect(
@@ -99,9 +151,9 @@ object BloopServer {
     logger: BloopRifleLogger,
     period: FiniteDuration,
     timeout: FiniteDuration
-  ): (BspConnection, Socket) = {
+  ): (BspConnection, Socket, BloopServerRuntimeInfo) = {
 
-    ensureBloopRunning(config, threads.startServerChecks, logger)
+    val bloopInfo = ensureBloopRunning(config, threads.startServerChecks, logger)
 
     logger.debug("Opening BSP connection with bloop")
     Files.createDirectories(workspace.resolve(".scala/.bloop"))
@@ -116,7 +168,7 @@ object BloopServer {
 
     logger.debug(s"Connected to Bloop via BSP at ${conn.address}")
 
-    (conn, socket)
+    (conn, socket, bloopInfo)
   }
 
   def buildServer(
@@ -130,7 +182,8 @@ object BloopServer {
     logger: BloopRifleLogger
   ): BloopServer = {
 
-    val (conn, socket) = bsp(config, workspace, threads, logger, config.period, config.timeout)
+    val (conn, socket, bloopInfo) =
+      bsp(config, workspace, threads, logger, config.period, config.timeout)
 
     logger.debug(s"Connected to Bloop via BSP at ${conn.address}")
 
@@ -172,7 +225,7 @@ object BloopServer {
     }
 
     server.onBuildInitialized()
-    BloopServerImpl(server, f, socket)
+    BloopServerImpl(server, f, socket, bloopInfo.jvmVersion.toString(), bloopInfo.bloopVersion.raw)
   }
 
   def withBuildServer[T](
@@ -206,5 +259,4 @@ object BloopServer {
     }
     // format: on
   }
-
 }

modules/bloop-rifle/src/main/scala/scala/build/blooprifle/BloopRifle.scala

@@ -1,13 +1,6 @@
 package scala.build.blooprifle
 
-import java.io.{
-  ByteArrayOutputStream,
-  File,
-  FileInputStream,
-  FileOutputStream,
-  InputStream,
-  OutputStream
-}
+import java.io.{ByteArrayOutputStream, FileInputStream, FileOutputStream, InputStream, OutputStream}
 import java.nio.file.Path
 import java.util.concurrent.ScheduledExecutorService
 
@@ -37,14 +30,7 @@ object BloopRifle {
         config.port,
         logger
       )
-    check() && {
-      !BloopRifle.shutdownBloopIfVersionIncompatible(
-        config,
-        logger,
-        new File(".").getCanonicalFile.toPath,
-        scheduler
-      )
-    }
+    check()
   }
 
   /** Starts a new bloop server.
@@ -58,15 +44,17 @@ object BloopRifle {
   def startServer(
     config: BloopRifleConfig,
     scheduler: ScheduledExecutorService,
-    logger: BloopRifleLogger
+    logger: BloopRifleLogger,
+    version: String,
+    bloopJava: String
   ): Future[Unit] =
-    config.classPath() match {
+    config.classPath(version) match {
       case Left(ex) => Future.failed(new Exception("Error getting Bloop class path", ex))
       case Right(cp) =>
         Operations.startServer(
           config.host,
           config.port,
-          config.javaPath,
+          bloopJava,
           config.javaOpts,
           cp.map(_.toPath),
           scheduler,
@@ -188,53 +176,63 @@ object BloopRifle {
 
   def nullInputStream() = new FileInputStream(Util.devNull)
 
-  def extractVersionFromBloopAbout(stdoutFromBloopAbout: String): Option[String] =
-    stdoutFromBloopAbout.split("\n").find(_.startsWith("bloop v")).map(
-      _.split(" ")(1).trim().drop(1)
+  // Probably we should implement an endpoint in Bloop to get this
+  // information in better form. I'm not sure error here should be escalated or ignored.
+  private def extractVersionFromBloopAbout(stdoutFromBloopAbout: String)
+    : Option[BloopServerRuntimeInfo] = {
+
+    val bloopVersionRegex = "bloop v(.*)\\s".r
+    val bloopJvmRegex     = "Running on Java JDK v([0-9._A-Za-z]+) [(](.*)[)]".r
+
+    for {
+      bloopVersion    <- bloopVersionRegex.findFirstMatchIn(stdoutFromBloopAbout).map(_.group(1))
+      bloopJvmVersion <- bloopJvmRegex.findFirstMatchIn(stdoutFromBloopAbout).map(_.group(1))
+      javaHome        <- bloopJvmRegex.findFirstMatchIn(stdoutFromBloopAbout).map(_.group(2))
+      jvmRelease      <- VersionUtil.jvmRelease(bloopJvmVersion)
+    } yield BloopServerRuntimeInfo(
+      bloopVersion = BloopVersion(bloopVersion),
+      jvmVersion = jvmRelease,
+      javaHome = javaHome
     )
+  }
 
   def getCurrentBloopVersion(
     config: BloopRifleConfig,
     logger: BloopRifleLogger,
     workdir: Path,
     scheduler: ScheduledExecutorService
-  ) = {
-    val bufferedOStream = new ByteArrayOutputStream(100000)
-    Operations.about(
-      config.host,
-      config.port,
-      workdir,
-      nullInputStream(),
-      bufferedOStream,
-      nullOutputStream(),
-      logger,
-      scheduler
-    )
-    extractVersionFromBloopAbout(new String(bufferedOStream.toByteArray))
-  }
+  ): Either[BloopAboutFailure, BloopServerRuntimeInfo] = {
+    val isRunning = BloopRifle.check(config, logger, scheduler)
 
-  /** Sometimes we need some minimal requirements for Bloop version. This method kills Bloop if its
-    * version is unsupported.
-    * @returns
-    *   true if the 'exit' command has actually been sent to Bloop
-    */
-  def shutdownBloopIfVersionIncompatible(
-    config: BloopRifleConfig,
-    logger: BloopRifleLogger,
-    workDir: Path,
-    scheduler: ScheduledExecutorService
-  ): Boolean = {
-    val currentBloopVersion = getCurrentBloopVersion(config, logger, workDir, scheduler)
-    val isOk = config.acceptBloopVersion.forall { f =>
-      currentBloopVersion.forall(f(_))
-    }
-    if (isOk)
-      logger.debug("No need to restart Bloop")
-    else {
-      logger.debug(s"Shutting down unsupported Bloop $currentBloopVersion.")
-      val retCode = exit(config, workDir, logger)
-      logger.debug(s"Bloop exit code: $retCode")
+    if (isRunning) {
+      val bufferedOStream = new ByteArrayOutputStream(100000)
+      Operations.about(
+        config.host,
+        config.port,
+        workdir,
+        nullInputStream(),
+        bufferedOStream,
+        nullOutputStream(),
+        logger,
+        scheduler
+      )
+      val bloopAboutOutput = new String(bufferedOStream.toByteArray)
+      extractVersionFromBloopAbout(bloopAboutOutput) match {
+        case Some(value) => Right(value)
+        case None        => Left(ParsingFailed(bloopAboutOutput))
+      }
     }
-    !isOk
+    else
+      Left(BloopNotRunning)
   }
 }
+
+sealed trait BloopAboutFailure
+case object BloopNotRunning                        extends BloopAboutFailure
+case class ParsingFailed(bloopAboutOutput: String) extends BloopAboutFailure
+
+case class BloopServerRuntimeInfo(
+  bloopVersion: BloopVersion,
+  jvmVersion: Int,
+  javaHome: String
+)