Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Performance tweaks #1939

Merged
merged 5 commits into from
Mar 28, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
4 changes: 2 additions & 2 deletions modules/build/src/main/scala/scala/build/Build.scala
Original file line number Diff line number Diff line change
Expand Up @@ -801,6 +801,7 @@ object Build {
compilerJvmVersionOpt: Option[Positioned[Int]],
scope: Scope,
logger: Logger,
artifacts: Artifacts,
maybeRecoverOnError: BuildException => Option[BuildException] = e => Some(e)
): Either[BuildException, Project] = either {

Expand All @@ -809,8 +810,6 @@ object Build {
val classesDir0 = classesDir(inputs.workspace, inputs.projectName, scope)
val scaladocDir = classesDir(inputs.workspace, inputs.projectName, scope, suffix = "-doc")

val artifacts = value(options.artifacts(logger, scope, maybeRecoverOnError))

val generateSemanticDbs = options.scalaOptions.generateSemanticDbs.getOrElse(false)

val releaseFlagVersion = releaseFlag(options, compilerJvmVersionOpt, logger).map(_.toString)
Expand Down Expand Up @@ -1007,6 +1006,7 @@ object Build {
compilerJvmVersionOpt,
scope,
logger,
artifacts,
maybeRecoverOnError
)
}
Expand Down
4 changes: 4 additions & 0 deletions modules/build/src/main/scala/scala/build/Directories.scala
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,9 @@ package scala.build

import coursier.cache.shaded.dirs.{GetWinDirs, ProjectDirectories}

import scala.build.errors.ConfigDbException
import scala.build.internal.JniGetWinDirs
import scala.cli.config.ConfigDb
import scala.util.Properties

trait Directories {
Expand All @@ -21,6 +23,8 @@ trait Directories {
.filter(_.trim.nonEmpty)
.map(os.Path(_, os.pwd))
.getOrElse(secretsDir / Directories.defaultDbFileName)

lazy val configDb = ConfigDb.open(dbPath.toNIO).left.map(ConfigDbException(_))
}

object Directories {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -79,13 +79,12 @@ case object MarkdownPreprocessor extends Preprocessor {
.map { wrappedMarkdown =>
val processingOutput: ProcessingOutput =
value {
ScalaPreprocessor.process(
ScalaPreprocessor.processSources(
content = wrappedMarkdown.code,
extractedDirectives = wrappedMarkdown.directives,
path = reportingPath,
scopeRoot = scopePath / os.up,
logger = logger,
maybeRecoverOnError = maybeRecoverOnError,
allowRestrictedFeatures = allowRestrictedFeatures,
suppressWarningOptions = suppressWarningOptions
)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -205,32 +205,30 @@ case object ScalaPreprocessor extends Preprocessor {
suppressWarningOptions: SuppressWarningOptions
): Either[BuildException, Option[ProcessingOutput]] = either {
val (contentWithNoShebang, _) = SheBang.ignoreSheBangLines(content)
val extractedDirectives = value(ExtractedDirectives.from(
val extractedDirectives: ExtractedDirectives = value(ExtractedDirectives.from(
contentWithNoShebang.toCharArray,
path,
logger,
scopeRoot,
maybeRecoverOnError
))
value(process(
value(processSources(
content,
extractedDirectives,
path,
scopeRoot,
logger,
maybeRecoverOnError,
allowRestrictedFeatures,
suppressWarningOptions
))
}

def process(
def processSources(
content: String,
extractedDirectives: ExtractedDirectives,
path: Either[String, os.Path],
scopeRoot: ScopePath,
logger: Logger,
maybeRecoverOnError: BuildException => Option[BuildException],
allowRestrictedFeatures: Boolean,
suppressWarningOptions: SuppressWarningOptions
): Either[BuildException, Option[ProcessingOutput]] = either {
Expand All @@ -246,13 +244,6 @@ case object ScalaPreprocessor extends Preprocessor {
suppressWarningOptions
))

value {
checkForAmmoniteImports(
afterStrictUsing.strippedContent.getOrElse(content0),
path
)
}

if (afterStrictUsing.isEmpty) None
else {
val allRequirements = Seq(afterStrictUsing.globalReqs)
Expand All @@ -274,70 +265,6 @@ case object ScalaPreprocessor extends Preprocessor {
}
}

private def checkForAmmoniteImports(
content: String,
path: Either[String, os.Path]
): Either[BuildException, Unit] = {

import fastparse.*

import scala.build.internal.ScalaParse.*

val res = parse(content, Header(_))

val indicesOrFailingIdx0 = res.fold((_, idx, _) => Left(idx), (value, _) => Right(value))

val indicesOrErrorMsg = indicesOrFailingIdx0 match {
case Left(failingIdx) =>
val newCode = content.take(failingIdx)
val res1 = parse(newCode, Header(_))
res1 match {
case f: Parsed.Failure =>
val msg = formatFastparseError(Util.printablePath(path), content, f)
Left(msg)
case s: Parsed.Success[Seq[(Int, Int)]] =>
Right(s.value)
}
case Right(ind) =>
Right(ind)
}

// TODO Report error if indicesOrErrorMsg.isLeft?

val importTrees = indicesOrErrorMsg
.toSeq
.iterator
.flatMap(_.iterator)
.flatMap {
case (start, end) =>
val code = content.substring(start, end) // .trim // meh
val importRes = parse(code, ImportSplitter(_))
importRes.fold((_, _, _) => Iterator.empty, (trees, _) => trees.iterator).map { tree =>
(start, tree.copy(start = start + tree.start, end = start + tree.end))
}
}
.toVector

val dependencyTrees = importTrees.filter { case (_, t) =>
val firstSegmentOpt = t.prefix.headOption
(firstSegmentOpt.contains("$ivy") || firstSegmentOpt.contains("$dep")) &&
t.prefix.lengthCompare(1) > 0
}

if (dependencyTrees.nonEmpty) {
val toFilePos = Position.Raw.filePos(path, content)
val exceptions = for {
(importStart, t) <- dependencyTrees
pos = toFilePos(Position.Raw(importStart, t.end))
dep = t.prefix.drop(1).mkString(".")
newImportText = s"//> using dep \"$dep\""
} yield new UnsupportedAmmoniteImportError(Seq(pos), newImportText)

Left(CompositeBuildException(exceptions))
}
else Right(())
}

private def processStrictUsing(
content: String,
extractedDirectives: ExtractedDirectives,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -84,46 +84,6 @@ class ActionableDiagnosticTests extends munit.FunSuite {
}
}

test("error on ammonite imports") {
val dependencyOsLib = "com.lihaoyi::os-lib:0.7.8"
val dependencyUpickleLib = "com.lihaoyi::upickle:1.4.0"
val ivyImport = s"import $$ivy.`$dependencyOsLib`"
val depImport = s"import $$dep.`$dependencyUpickleLib`"
val testInputs = TestInputs(
os.rel / "Foo.scala" ->
s"""$ivyImport
|$depImport
|
|object Hello extends App {
| println("Hello")
|}
|""".stripMargin
)
testInputs.withBuild(baseOptions, buildThreads, None, actionableDiagnostics = true) {
(_, _, maybeBuild) =>
expect(maybeBuild.isLeft)
val exceptions = maybeBuild match {
case Left(c: CompositeBuildException) => c.exceptions
case _ => Seq()
}

expect(exceptions.length == 2)
expect(exceptions.forall(_.isInstanceOf[UnsupportedAmmoniteImportError]))

expect(exceptions.head.textEdit.get.newText == s"//> using dep \"$dependencyOsLib\"")
expect(
exceptions.tail.head.textEdit.get.newText == s"//> using dep \"$dependencyUpickleLib\""
)

val filePositions = exceptions.flatMap(_.positions.collect {
case File(_, startPos, endPos) => (startPos, endPos)
})

expect(filePositions.head == ((0, 0), (0, ivyImport.length)))
expect(filePositions.tail.head == ((1, 0), (1, depImport.length)))
}
}

test("actionable actions suggest update only to stable version") {
val testInputs = TestInputs(
os.rel / "Foo.scala" ->
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -84,17 +84,19 @@ class BuildProjectTests extends munit.FunSuite {
)
)

val inputs = Inputs.empty("project")
val sources = Sources(Nil, Nil, None, Nil, options)
val logger = new LoggerMock()
val inputs = Inputs.empty("project")
val sources = Sources(Nil, Nil, None, Nil, options)
val logger = new LoggerMock()
val artifacts = options.artifacts(logger, Scope.Test).orThrow
val res = Build.buildProject(
inputs,
sources,
Nil,
options,
Some(Positioned(bloopJavaPath, bloopJvmVersion)),
Scope.Test,
logger
logger,
artifacts
)

val scalaCompilerOptions = res.fold(throw _, identity)
Expand Down Expand Up @@ -169,12 +171,13 @@ class BuildProjectTests extends munit.FunSuite {
LocalRepo.localRepo(scala.build.Directories.default().localRepoDir)
)
)
val inputs = Inputs.empty("project")
val sources = Sources(Nil, Nil, None, Nil, options)
val logger = new LoggerMock()
val inputs = Inputs.empty("project")
val sources = Sources(Nil, Nil, None, Nil, options)
val logger = new LoggerMock()
val artifacts = options.artifacts(logger, Scope.Test).orThrow

val project =
Build.buildProject(inputs, sources, Nil, options, None, Scope.Main, logger).orThrow
Build.buildProject(inputs, sources, Nil, options, None, Scope.Main, logger, artifacts).orThrow

expect(project.workspace == inputs.workspace)
}
Expand Down
26 changes: 0 additions & 26 deletions modules/build/src/test/scala/scala/build/tests/BuildTests.scala
Original file line number Diff line number Diff line change
Expand Up @@ -250,32 +250,6 @@ abstract class BuildTests(server: Boolean) extends munit.FunSuite {
simpleNativeTest()
}

test("should fail for ammonite imports - $ivy") {
val testInputs = TestInputs(
os.rel / "simple.sc" ->
"""import $ivy.`com.lihaoyi::geny:0.6.5`
|import geny.Generator
|val g = Generator("Hel", "lo")
|println(g.mkString)
|""".stripMargin
)
testInputs.withBuild(defaultOptions, buildThreads, bloopConfigOpt) { (_, _, maybeBuild) =>
expect(maybeBuild.isLeft)
}
}
test("should fail for ammonite imports - $dep") {
val testInputs = TestInputs(
os.rel / "simple.sc" ->
"""import $dep.`com.lihaoyi::geny:0.6.5`
|import geny.Generator
|val g = Generator("Hel", "lo")
|println(g.mkString)
|""".stripMargin
)
testInputs.withBuild(defaultOptions, buildThreads, bloopConfigOpt) { (_, _, maybeBuild) =>
expect(maybeBuild.isLeft)
}
}
test("dependencies - using") {
val testInputs = TestInputs(
os.rel / "simple.sc" ->
Expand Down
21 changes: 0 additions & 21 deletions modules/build/src/test/scala/scala/build/tests/SourcesTests.scala
Original file line number Diff line number Diff line change
Expand Up @@ -245,27 +245,6 @@ class SourcesTests extends munit.FunSuite {
}
}

test("should fail for ammonite imports in .sc - $ivy") {
val testInputs = TestInputs(
os.rel / "something.sc" ->
"""import $ivy.`org1:name1:1.1`
|import $ivy.`org2::name2:2.2`
|import $ivy.`org3:::name3:3.3`
|import scala.collection.mutable
|
|def a = 1
|""".stripMargin
)
testInputs.withInputs { (_, inputs) =>
expect(CrossSources.forInputs(
inputs,
preprocessors,
TestLogger(),
SuppressWarningOptions()
).isLeft)
}
}

test("should skip SheBang in .sc and .scala") {
val testInputs = TestInputs(
os.rel / "something1.sc" ->
Expand Down
3 changes: 2 additions & 1 deletion modules/cli/src/main/scala/scala/cli/ScalaCli.scala
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ import scala.cli.config.{ConfigDb, Keys}
import scala.cli.internal.Argv0
import scala.cli.launcher.{LauncherCli, LauncherOptions}
import scala.cli.publish.BouncycastleSignerMaker
import scala.cli.util.ConfigDbUtils
import scala.util.Properties

object ScalaCli {
Expand All @@ -36,7 +37,7 @@ object ScalaCli {
private val scalaCliBinaryName = "scala-cli"
private var isSipScala = {
lazy val isPowerConfigDb = for {
configDb <- ConfigDb.open(Directories.directories.dbPath.toNIO).toOption
configDb <- ConfigDbUtils.configDb.toOption
powerEntry <- configDb.get(Keys.power).toOption
power <- powerEntry
} yield power
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -206,11 +206,11 @@ abstract class ScalaCommand[T <: HasGlobalOptions](implicit myParser: Parser[T],
def maybePrintToolsHelp(options: T, buildOptions: BuildOptions): Unit =
for {
shared <- sharedOptions(options)
if shared.helpGroups.helpScaladoc || shared.helpGroups.helpRepl || shared.helpGroups.helpScalafmt
logger = shared.logger
artifacts <- buildOptions.artifacts(logger, Scope.Main).toOption
scalaArtifacts <- artifacts.scalaOpt
scalaParams = scalaArtifacts.params
if shared.helpGroups.helpScaladoc || shared.helpGroups.helpRepl || shared.helpGroups.helpScalafmt
} {
val exitCode: Either[BuildException, Int] = either {
val (classPath: Seq[os.Path], mainClass: String) =
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -77,7 +77,6 @@ object Bsp extends ScalaCommand[BspOptions] {

val inputs = argsToInputs(args.all).orExit(logger)
CurrentParams.workspaceOpt = Some(inputs.workspace)
val configDb = options.shared.configDb.orExit(logger)
val actionableDiagnostics =
options.shared.logging.verbosityOptions.actions

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ import scala.cli.commands.util.BuildCommandHelpers
import scala.cli.commands.{CommandUtils, ScalaCommand, SpecificationLevel, WatchUtil}
import scala.cli.config.{ConfigDb, Keys}
import scala.cli.util.ArgHelpers.*

import scala.cli.util.ConfigDbUtils
object Compile extends ScalaCommand[CompileOptions] with BuildCommandHelpers {
override def group: String = HelpCommandGroup.Main.toString

Expand Down Expand Up @@ -92,7 +92,7 @@ object Compile extends ScalaCommand[CompileOptions] with BuildCommandHelpers {
val threads = BuildThreads.create()

val compilerMaker = options.shared.compilerMaker(threads).orExit(logger)
val configDb = options.shared.configDb.orExit(logger)
val configDb = ConfigDbUtils.configDb.orExit(logger)
val actionableDiagnostics =
options.shared.logging.verbosityOptions.actions.orElse(
configDb.get(Keys.actions).getOrElse(None)
Expand Down