From 9635e3d757938ebcf2e6816046248a792d15fb74 Mon Sep 17 00:00:00 2001 From: Alexandre Archambault Date: Wed, 6 Oct 2021 16:39:18 +0200 Subject: [PATCH] Better message for invalid binary Scala versions --- .../src/main/scala/scala/build/Build.scala | 30 ++++++--- .../main/scala/scala/build/CrossSources.scala | 4 +- .../main/scala/scala/build/bsp/BspImpl.scala | 12 ++-- .../InvalidBinaryScalaVersionError.scala | 5 ++ .../scala/build/options/BuildOptions.scala | 64 +++++++++++-------- .../scala/build/tests/SourcesTests.scala | 10 +-- .../scala/scala/cli/commands/Export.scala | 2 +- .../scala/scala/cli/commands/Metabrowse.scala | 4 +- .../scala/scala/cli/commands/SetupIde.scala | 2 +- 9 files changed, 79 insertions(+), 54 deletions(-) create mode 100644 modules/build/src/main/scala/scala/build/errors/InvalidBinaryScalaVersionError.scala diff --git a/modules/build/src/main/scala/scala/build/Build.scala b/modules/build/src/main/scala/scala/build/Build.scala index d7dfc5ce59..2fa37fbef4 100644 --- a/modules/build/src/main/scala/scala/build/Build.scala +++ b/modules/build/src/main/scala/scala/build/Build.scala @@ -3,6 +3,7 @@ package scala.build import ch.epfl.scala.bsp4j import com.swoval.files.FileTreeViews.Observer import com.swoval.files.{PathWatcher, PathWatchers} +import dependency.ScalaParameters import java.io.File import java.nio.file.{FileSystemException, Path} @@ -37,6 +38,7 @@ object Build { final case class Successful( inputs: Inputs, options: BuildOptions, + scalaParams: ScalaParameters, sources: Sources, artifacts: Artifacts, project: Project, @@ -119,16 +121,16 @@ object Build { ) } - val sources = crossSources.sources(options) + val sources = value(crossSources.sources(options)) val options0 = options.orElse(sources.buildOptions) val inputs0 = updateInputs(inputs, options) val generatedSources = sources.generateSources(inputs0.generatedSrcRoot) - def doBuild(buildOptions: BuildOptions) = { - buildClient.setProjectParams(buildOptions.projectParams) - build( + def doBuild(buildOptions: BuildOptions) = either { + buildClient.setProjectParams(value(buildOptions.projectParams)) + val res = build( inputs0, sources, inputs0.generatedSrcRoot, @@ -138,6 +140,7 @@ object Build { buildClient, bloopServer ) + value(res) } val mainBuild = value(doBuild(options0)) @@ -347,9 +350,9 @@ object Build { options: BuildOptions, logger: Logger, buildClient: BloopBuildClient - ): Either[BuildException, (os.Path, Artifacts, Project, Boolean)] = either { + ): Either[BuildException, (os.Path, ScalaParameters, Artifacts, Project, Boolean)] = either { - val params = options.scalaParams + val params = value(options.scalaParams) val allSources = sources.paths.map(_._1) ++ generatedSources.map(_.generated) val classesDir0 = classesDir(inputs.workspace, inputs.projectName) @@ -429,7 +432,7 @@ object Build { buildClient.clear() buildClient.setGeneratedSources(generatedSources) - (classesDir0, artifacts, project, updatedBloopConfig) + (classesDir0, params, artifacts, project, updatedBloopConfig) } def buildOnce( @@ -443,7 +446,7 @@ object Build { bloopServer: bloop.BloopServer ): Either[BuildException, Build] = either { - val (classesDir0, artifacts, project, updatedBloopConfig) = value { + val (classesDir0, scalaParams, artifacts, project, updatedBloopConfig) = value { prepareBuild( inputs, sources, @@ -486,7 +489,16 @@ object Build { updateTasty = project.scalaCompiler.scalaVersion.startsWith("3.") ) - Successful(inputs, options, sources, artifacts, project, classesDir0, buildClient.diagnostics) + Successful( + inputs, + options, + scalaParams, + sources, + artifacts, + project, + classesDir0, + buildClient.diagnostics + ) } else Failed(inputs, options, sources, artifacts, project, buildClient.diagnostics) diff --git a/modules/build/src/main/scala/scala/build/CrossSources.scala b/modules/build/src/main/scala/scala/build/CrossSources.scala index 99b460eb08..b481b0924d 100644 --- a/modules/build/src/main/scala/scala/build/CrossSources.scala +++ b/modules/build/src/main/scala/scala/build/CrossSources.scala @@ -14,14 +14,14 @@ final case class CrossSources( buildOptions: Seq[HasBuildRequirements[BuildOptions]] ) { - def sources(baseOptions: BuildOptions): Sources = { + def sources(baseOptions: BuildOptions): Either[BuildException, Sources] = either { val sharedOptions = buildOptions .filter(_.requirements.isEmpty) .map(_.value) .foldLeft(baseOptions)(_ orElse _) - val retainedScalaVersion = sharedOptions.scalaParams.scalaVersion + val retainedScalaVersion = value(sharedOptions.scalaParams).scalaVersion val buildOptionsWithScalaVersion = buildOptions .flatMap(_.withScalaVersion(retainedScalaVersion).toSeq) diff --git a/modules/build/src/main/scala/scala/build/bsp/BspImpl.scala b/modules/build/src/main/scala/scala/build/bsp/BspImpl.scala index 6be8ca7180..8367a18eb5 100644 --- a/modules/build/src/main/scala/scala/build/bsp/BspImpl.scala +++ b/modules/build/src/main/scala/scala/build/bsp/BspImpl.scala @@ -55,7 +55,7 @@ final class BspImpl( if (verbosity >= 3) pprint.better.log(crossSources) - val sources = crossSources.sources(buildOptions) + val sources = value(crossSources.sources(buildOptions)) if (verbosity >= 3) pprint.better.log(sources) @@ -67,7 +67,7 @@ final class BspImpl( actualLocalServer.setExtraDependencySources(buildOptions.classPathOptions.extraSourceJars) actualLocalServer.setGeneratedSources(generatedSources) - val (classesDir0, artifacts, project, buildChanged) = value { + val (classesDir0, scalaParams, artifacts, project, buildChanged) = value { Build.prepareBuild( inputs, sources, @@ -78,7 +78,7 @@ final class BspImpl( ) } - (sources, options0, classesDir0, artifacts, project, generatedSources, buildChanged) + (sources, options0, classesDir0, scalaParams, artifacts, project, generatedSources, buildChanged) } private def buildE( @@ -86,7 +86,7 @@ final class BspImpl( bloopServer: BloopServer, notifyChanges: Boolean ): Either[BuildException, Unit] = either { - val (sources, buildOptions, _, _, _, generatedSources, buildChanged) = + val (sources, buildOptions, _, _, _, _, generatedSources, buildChanged) = value(prepareBuild(actualLocalServer)) if (notifyChanges && buildChanged) notifyBuildChange(actualLocalServer) @@ -120,7 +120,7 @@ final class BspImpl( ): CompletableFuture[b.CompileResult] = { val preBuild = CompletableFuture.supplyAsync( () => { - val (_, _, classesDir0, artifacts, project, generatedSources, buildChanged) = + val (_, _, classesDir0, _, artifacts, project, generatedSources, buildChanged) = prepareBuild(actualLocalServer).orThrow if (buildChanged) notifyBuildChange(actualLocalServer) @@ -241,7 +241,7 @@ final class BspImpl( val remoteClient = launcher.getRemoteProxy actualLocalClient.forwardToOpt = Some(remoteClient) - prepareBuild(actualLocalServer) + prepareBuild(actualLocalServer) // FIXME We're discarding the error here logger.log { val hasConsole = System.console() != null diff --git a/modules/build/src/main/scala/scala/build/errors/InvalidBinaryScalaVersionError.scala b/modules/build/src/main/scala/scala/build/errors/InvalidBinaryScalaVersionError.scala new file mode 100644 index 0000000000..ea0bdc032c --- /dev/null +++ b/modules/build/src/main/scala/scala/build/errors/InvalidBinaryScalaVersionError.scala @@ -0,0 +1,5 @@ +package scala.build.errors + +final class InvalidBinaryScalaVersionError( + val binaryVersion: String +) extends BuildException(s"Cannot find matching Scala version for '$binaryVersion'") \ No newline at end of file diff --git a/modules/build/src/main/scala/scala/build/options/BuildOptions.scala b/modules/build/src/main/scala/scala/build/options/BuildOptions.scala index 4cd29d0c01..d4b15e40a9 100644 --- a/modules/build/src/main/scala/scala/build/options/BuildOptions.scala +++ b/modules/build/src/main/scala/scala/build/options/BuildOptions.scala @@ -9,7 +9,8 @@ import java.nio.charset.StandardCharsets import java.nio.file.Path import java.security.MessageDigest -import scala.build.errors.BuildException +import scala.build.EitherCps.{either, value} +import scala.build.errors.{BuildException, InvalidBinaryScalaVersionError} import scala.build.internal.Constants._ import scala.build.internal.{Constants, OsLibc, Util} import scala.build.{Artifacts, Logger, Os} @@ -31,38 +32,41 @@ final case class BuildOptions( replOptions: ReplOptions = ReplOptions() ) { - lazy val projectParams: Seq[String] = { + lazy val projectParams: Either[BuildException, Seq[String]] = either { val platform = if (scalaJsOptions.enable) "Scala.JS" else if (scalaNativeOptions.enable) "Scala Native" else "JVM" - Seq(s"Scala ${scalaParams.scalaVersion}", platform) + Seq(s"Scala ${value(scalaParams).scalaVersion}", platform) } def addRunnerDependency: Option[Boolean] = internalDependencies.addRunnerDependencyOpt .orElse(if (scalaJsOptions.enable || scalaNativeOptions.enable) Some(false) else None) - private def scalaLibraryDependencies: Seq[AnyDependency] = + private def scalaLibraryDependencies: Either[BuildException, Seq[AnyDependency]] = either { if (scalaOptions.addScalaLibrary.getOrElse(true)) { + val scalaParams0 = value(scalaParams) val lib = - if (scalaParams.scalaVersion.startsWith("3.")) - dep"org.scala-lang::scala3-library::${scalaParams.scalaVersion}" + if (scalaParams0.scalaVersion.startsWith("3.")) + dep"org.scala-lang::scala3-library::${scalaParams0.scalaVersion}" else - dep"org.scala-lang:scala-library:${scalaParams.scalaVersion}" + dep"org.scala-lang:scala-library:${scalaParams0.scalaVersion}" Seq(lib) } else Nil + } - private def dependencies: Seq[AnyDependency] = - scalaJsOptions.jsDependencies(scalaParams.scalaVersion) ++ + private def dependencies: Either[BuildException, Seq[AnyDependency]] = either { + scalaJsOptions.jsDependencies(value(scalaParams).scalaVersion) ++ scalaNativeOptions.nativeDependencies ++ - scalaLibraryDependencies ++ + value(scalaLibraryDependencies) ++ classPathOptions.extraDependencies + } - private def semanticDbPlugins: Seq[AnyDependency] = { + private def semanticDbPlugins: Either[BuildException, Seq[AnyDependency]] = either { val generateSemDbs = scalaOptions.generateSemanticDbs.getOrElse(false) && - scalaParams.scalaVersion.startsWith("2.") + value(scalaParams).scalaVersion.startsWith("2.") if (generateSemDbs) Seq( dep"$semanticDbPluginOrganization:::$semanticDbPluginModuleName:$semanticDbPluginVersion" @@ -71,11 +75,12 @@ final case class BuildOptions( Nil } - def compilerPlugins: Seq[AnyDependency] = - scalaJsOptions.compilerPlugins(scalaParams.scalaVersion) ++ + def compilerPlugins: Either[BuildException, Seq[AnyDependency]] = either { + scalaJsOptions.compilerPlugins(value(scalaParams).scalaVersion) ++ scalaNativeOptions.compilerPlugins ++ - semanticDbPlugins ++ + value(semanticDbPlugins) ++ scalaOptions.compilerPlugins + } def allExtraJars: Seq[Path] = classPathOptions.extraClassPath.map(_.toNIO) @@ -144,7 +149,7 @@ final case class BuildOptions( private def computeScalaVersions( scalaVersion: Option[String], scalaBinaryVersion: Option[String] - ): (String, String) = { + ): Either[BuildException, (String, String)] = either { import coursier.core.Version lazy val allVersions = { import coursier._ @@ -170,37 +175,38 @@ final case class BuildOptions( } modules.flatMap(moduleVersions).distinct } - val sv = scalaVersion match { - case None => Constants.defaultScalaVersion + val maybeSv = scalaVersion match { + case None => Right(Constants.defaultScalaVersion) case Some(sv0) => - if (Util.isFullScalaVersion(sv0)) sv0 + if (Util.isFullScalaVersion(sv0)) Right(sv0) else { val prefix = if (sv0.endsWith(".")) sv0 else sv0 + "." val matchingVersions = allVersions.filter(_.startsWith(prefix)) if (matchingVersions.isEmpty) - sys.error(s"Cannot find matching Scala version for '$sv0'") + Left(new InvalidBinaryScalaVersionError(sv0)) else - matchingVersions.map(Version(_)).max.repr + Right(matchingVersions.map(Version(_)).max.repr) } } + val sv = value(maybeSv) val sbv = scalaBinaryVersion.getOrElse(ScalaVersion.binary(sv)) (sv, sbv) } - lazy val scalaParams: ScalaParameters = { + lazy val scalaParams: Either[BuildException, ScalaParameters] = either { val (scalaVersion, scalaBinaryVersion) = - computeScalaVersions(scalaOptions.scalaVersion, scalaOptions.scalaBinaryVersion) + value(computeScalaVersions(scalaOptions.scalaVersion, scalaOptions.scalaBinaryVersion)) val maybePlatformSuffix = scalaJsOptions.platformSuffix .orElse(scalaNativeOptions.platformSuffix) ScalaParameters(scalaVersion, scalaBinaryVersion, maybePlatformSuffix) } - def artifacts(logger: Logger): Either[BuildException, Artifacts] = - Artifacts( - params = scalaParams, - compilerPlugins = compilerPlugins, - dependencies = dependencies, + def artifacts(logger: Logger): Either[BuildException, Artifacts] = either { + val maybeArtifacts = Artifacts( + params = value(scalaParams), + compilerPlugins = value(compilerPlugins), + dependencies = value(dependencies), extraClassPath = allExtraJars, extraCompileOnlyJars = allExtraCompileOnlyJars, extraSourceJars = allExtraSourceJars, @@ -213,6 +219,8 @@ final case class BuildOptions( extraRepositories = finalRepositories, logger = logger ) + value(maybeArtifacts) + } // FIXME We'll probably need more refined rules if we start to support extra Scala.JS or Scala Native specific types def packageTypeOpt: Option[PackageType] = diff --git a/modules/build/src/test/scala/scala/build/tests/SourcesTests.scala b/modules/build/src/test/scala/scala/build/tests/SourcesTests.scala index 2b5a34b08c..6bb26ac2c0 100644 --- a/modules/build/src/test/scala/scala/build/tests/SourcesTests.scala +++ b/modules/build/src/test/scala/scala/build/tests/SourcesTests.scala @@ -38,7 +38,7 @@ class SourcesTests extends munit.FunSuite { testInputs.withInputs { (_, inputs) => val crossSources = CrossSources.forInputs(inputs, Sources.defaultPreprocessors(CustomCodeWrapper)).orThrow - val sources = crossSources.sources(BuildOptions()) + val sources = crossSources.sources(BuildOptions()).orThrow expect(sources.buildOptions.classPathOptions.extraDependencies == expectedDeps) expect(sources.paths.isEmpty) @@ -68,7 +68,7 @@ class SourcesTests extends munit.FunSuite { testInputs.withInputs { (_, inputs) => val crossSources = CrossSources.forInputs(inputs, Sources.defaultPreprocessors(CustomCodeWrapper)).orThrow - val sources = crossSources.sources(BuildOptions()) + val sources = crossSources.sources(BuildOptions()).orThrow expect(sources.buildOptions.classPathOptions.extraDependencies == expectedDeps) expect(sources.paths.isEmpty) @@ -96,7 +96,7 @@ class SourcesTests extends munit.FunSuite { testInputs.withInputs { (_, inputs) => val crossSources = CrossSources.forInputs(inputs, Sources.defaultPreprocessors(CustomCodeWrapper)).orThrow - val sources = crossSources.sources(BuildOptions()) + val sources = crossSources.sources(BuildOptions()).orThrow expect(sources.buildOptions.classPathOptions.extraDependencies == expectedDeps) expect(sources.paths.isEmpty) @@ -154,7 +154,7 @@ class SourcesTests extends munit.FunSuite { testInputs.withInputs { (_, inputs) => val crossSources = CrossSources.forInputs(inputs, Sources.defaultPreprocessors(CustomCodeWrapper)).orThrow - val sources = crossSources.sources(BuildOptions()) + val sources = crossSources.sources(BuildOptions()).orThrow val parsedCodes: Seq[String] = sources.inMemory.map(_._3) @@ -183,7 +183,7 @@ class SourcesTests extends munit.FunSuite { testInputs.withInputs { (_, inputs) => val crossSources = CrossSources.forInputs(inputs, Sources.defaultPreprocessors(CustomCodeWrapper)).orThrow - val sources = crossSources.sources(BuildOptions()) + val sources = crossSources.sources(BuildOptions()).orThrow expect(sources.buildOptions.classPathOptions.extraDependencies == expectedDeps) expect(sources.paths.isEmpty) diff --git a/modules/cli/src/main/scala/scala/cli/commands/Export.scala b/modules/cli/src/main/scala/scala/cli/commands/Export.scala index 29abad5e2b..4a34a1b0f2 100644 --- a/modules/cli/src/main/scala/scala/cli/commands/Export.scala +++ b/modules/cli/src/main/scala/scala/cli/commands/Export.scala @@ -30,7 +30,7 @@ object Export extends ScalaCommand[ExportOptions] { ) ) } - val sources = crossSources.sources(buildOptions) + val sources = value(crossSources.sources(buildOptions)) if (verbosity >= 3) pprint.better.log(sources) diff --git a/modules/cli/src/main/scala/scala/cli/commands/Metabrowse.scala b/modules/cli/src/main/scala/scala/cli/commands/Metabrowse.scala index c91bd1ad76..7d09ab1948 100644 --- a/modules/cli/src/main/scala/scala/cli/commands/Metabrowse.scala +++ b/modules/cli/src/main/scala/scala/cli/commands/Metabrowse.scala @@ -57,7 +57,7 @@ object Metabrowse extends ScalaCommand[MetabrowseOptions] { .map(os.Path(_, os.pwd)) .getOrElse { val (url, changing) = - options.metabrowseBinaryUrl(successfulBuild.options.scalaParams.scalaVersion) + options.metabrowseBinaryUrl(successfulBuild.scalaParams.scalaVersion) FetchExternalBinary.fetch( url, changing, @@ -115,7 +115,7 @@ object Metabrowse extends ScalaCommand[MetabrowseOptions] { } def defaultDialect = { - val sv = successfulBuild.options.scalaParams.scalaVersion + val sv = successfulBuild.scalaParams.scalaVersion if (sv.startsWith("2.12.")) "Scala212" else if (sv.startsWith("2.13.")) "Scala213" else "Scala3" diff --git a/modules/cli/src/main/scala/scala/cli/commands/SetupIde.scala b/modules/cli/src/main/scala/scala/cli/commands/SetupIde.scala index 44b923ab28..fd8716405c 100644 --- a/modules/cli/src/main/scala/scala/cli/commands/SetupIde.scala +++ b/modules/cli/src/main/scala/scala/cli/commands/SetupIde.scala @@ -20,7 +20,7 @@ object SetupIde extends ScalaCommand[SetupIdeOptions] { Sources.defaultPreprocessors(CustomCodeWrapper) ) - val sourcesBuildOptions = crossSources.map(_.sources(options).buildOptions) + val sourcesBuildOptions = crossSources.flatMap(_.sources(options)).map(_.buildOptions) val joinedBuildOpts = sourcesBuildOptions.map(options orElse _).getOrElse(options) joinedBuildOpts.artifacts(logger) }