Skip to content

Commit

Permalink
Pass SemanticDB compiler options in a separate list
Browse files Browse the repository at this point in the history
  • Loading branch information
Jaden Peterson committed Nov 12, 2024
1 parent 8786cdd commit 4c0cc32
Show file tree
Hide file tree
Showing 3 changed files with 58 additions and 23 deletions.
17 changes: 14 additions & 3 deletions rules/private/phases/phase_semanticdb.bzl
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,16 @@ load(
)

def _semanticdb_directory_from_file(file):
return file.path[:file.path.find("META-INF") - 1]
"""
This is janky, but we're limited in what we can do in this function. From the
[documentation](https://bazel.build/rules/lib/builtins/Args#add_all) on `Args#add_all`:
To avoid unintended retention of large analysis-phase data structures into the execution phase,
the `map_each` function must be declared by a top-level `def` statement; it may not be a
nested function closure by default.
"""

return "{}/{}".format(file.root.path, file.short_path[:file.short_path.find("META-INF") - 1])

#
# PHASE: semanticdb
Expand Down Expand Up @@ -40,18 +49,20 @@ def phase_semanticdb(ctx, g):

if scala_configuration.version.startswith("2"):
arguments.add("--compiler_option=-P:semanticdb:failures:error")
arguments.add("--compiler_option_referencing_path=-P:semanticdb:sourceroot:${workDir}")
arguments.add_all(
[outputs[0]],
format_each = "--compiler_option=-P:semanticdb:targetroot:%s",
format_each = "--compiler_option_referencing_path=-P:semanticdb:targetroot:${path} %s",
map_each = _semanticdb_directory_from_file,
)
else:
arguments.add_all(
[outputs[0]],
format_each = "--compiler_option=-semanticdb-target:%s",
format_each = "--compiler_option_referencing_path=-semanticdb-target:${path} %s",
map_each = _semanticdb_directory_from_file,
)

arguments.add("--compiler_option_referencing_path=-sourceroot:${workDir}")
arguments.add("--compiler_option=-Ysemanticdb")

g.out.providers.append(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ import common.sandbox.SandboxUtil
import net.sourceforge.argparse4j.impl.{Arguments => ArgumentsImpl}
import net.sourceforge.argparse4j.inf.{Argument, ArgumentParser, ArgumentType, Namespace}
import java.util.{Collections, List as JList}
import scala.annotation.nowarn
import scala.collection.mutable.Buffer
import scala.jdk.CollectionConverters.*
import java.io.File
Expand All @@ -17,6 +18,18 @@ class CommonArguments private (
val compilerBridge: Path,
val compilerClasspath: List[Path],
val compilerOptions: List[String],

/**
* With [[https://bazel.build/remote/multiplex#multiplex_sandboxing multiplex sandboxing]], Bazel generates a separate
* sandbox directory for each worker invocation, which means the paths to build artifacts won't be known until the
* execution phase. However, compiler options are usually generated during the analysis phase.
*
* Some compiler options (currently, only those regarding SemanticDB) reference paths to build artifacts and need to
* be adjusted to be relative to the sandbox directory. It's more performant for those options to be passed in a
* separate list so we don't have to scan through every compiler option and potentially modify it. In our experience,
* this resulted in a ~10% worker speedup.
*/
val compilerOptionsReferencingPaths: List[String],
val classpath: List[Path],
val debug: Boolean,
val javaCompilerOptions: List[String],
Expand Down Expand Up @@ -49,22 +62,22 @@ object Analysis {
}

object CommonArguments {
private val scala2SemanticDbTargetRootRegex = """-P:semanticdb:targetroot:(.*)""".r
private val scala3SemanticDbTargetRootRegex = """-semanticdb-target:(.*)""".r

private def adjustCompilerOptions(workDir: Path, options: List[String]) = {
def adjustStringPath(path: String) =
SandboxUtil.getSandboxPath(workDir, Paths.get(path)).toString

options.flatMap {
case scala2SemanticDbTargetRootRegex(path) =>
List(s"-P:semanticdb:sourceroot:${workDir.toString}", s"-P:semanticdb:targetroot:${adjustStringPath(path)}")
private def adjustCompilerOptions(workDir: Path, options: List[String]) = options.map { option =>
val i = option.lastIndexOf(' ')
val withPathReplaced = if (i == -1) {
option
} else {
val template = option.slice(0, i)
val path = option.slice(i + 1, option.length)

case scala3SemanticDbTargetRootRegex(path) =>
List(s"-semanticdb-target:${adjustStringPath(path)}", s"-sourceroot:${workDir.toString}")

case option => List(option)
template.replace(
"${path}": @nowarn("cat=lint-missing-interpolator"),
SandboxUtil.getSandboxPath(workDir, Paths.get(path)).toString,
)
}

withPathReplaced
.replace("${workDir}": @nowarn("cat=lint-missing-interpolator"), workDir.toString)
}

/**
Expand Down Expand Up @@ -95,6 +108,11 @@ object CommonArguments {
.help("Compiler option")
.action(ArgumentsImpl.append)
.metavar("option")
parser
.addArgument("--compiler_option_referencing_path")
.help("Compiler option referencing the paths to build artifact(s)")
.action(ArgumentsImpl.append)
.metavar("option")
parser
.addArgument("--classpath")
.help("Compilation classpath")
Expand Down Expand Up @@ -195,9 +213,14 @@ object CommonArguments {
analyses = analyses,
compilerBridge = SandboxUtil.getSandboxPath(workDir, namespace.get[Path]("compiler_bridge")),
compilerClasspath = SandboxUtil.getSandboxPaths(workDir, namespace.getList[Path]("compiler_classpath")),
compilerOptions = adjustCompilerOptions(
compilerOptions = Option(namespace.getList[String]("compiler_option"))
.map(_.asScala.toList)
.getOrElse(List.empty),
compilerOptionsReferencingPaths = adjustCompilerOptions(
workDir,
Option(namespace.getList[String]("compiler_option")).map(_.asScala.toList).getOrElse(List.empty),
Option(namespace.getList[String]("compiler_option_referencing_path"))
.map(_.asScala.toList)
.getOrElse(List.empty),
),
classpath = SandboxUtil.getSandboxPaths(workDir, namespace.getList[Path]("classpath")),
debug = namespace.getBoolean("debug"),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -214,10 +214,11 @@ object ZincRunner extends WorkerMain[ZincRunnerWorkerConfig] {
.withClassesDirectory(classesOutputDir)
.withJavacOptions(workRequest.javaCompilerOptions.toArray)
.withScalacOptions(
Array.concat(
workRequest.plugins.map(p => s"-Xplugin:$p").toArray,
workRequest.compilerOptions.toArray,
),
(
workRequest.plugins.map(p => s"-Xplugin:$p") ++
workRequest.compilerOptions ++
workRequest.compilerOptionsReferencingPaths
).toArray,
)

val compilers = {
Expand Down

0 comments on commit 4c0cc32

Please sign in to comment.