Skip to content

Commit

Permalink
Update dependencies and improve build tool testing infrastructure, ad…
Browse files Browse the repository at this point in the history
…d JDK 21 (#692)

Additionally: 

* Refactor build tool tests to support running under any JDK

We introduce the concept of Tool, which has restrictions on
minimum/maximum JDK version it can run under.
We also use external JDK version to conditionally ignore tests.
  • Loading branch information
keynmol authored Apr 18, 2024
1 parent ff5891a commit 0fd665e
Show file tree
Hide file tree
Showing 35 changed files with 1,341 additions and 1,173 deletions.
10 changes: 5 additions & 5 deletions .github/workflows/ci.yml
Original file line number Diff line number Diff line change
Expand Up @@ -14,10 +14,10 @@ jobs:
# NOTE(olafurpg) Windows is not enabled because it times out due to reasons I don't understand.
# os: [windows-latest, ubuntu-latest]
os: [ubuntu-latest]
java: [8, 11, 17]
java: [8, 11, 17, 21]
steps:
- uses: actions/checkout@v2
- uses: actions/setup-java@v3
- uses: actions/checkout@v4
- uses: actions/setup-java@v4
with:
distribution: "temurin"
cache: "sbt"
Expand All @@ -29,8 +29,8 @@ jobs:
runs-on: ubuntu-latest
name: Benchmark tests
steps:
- uses: actions/checkout@v2
- uses: actions/setup-java@v3
- uses: actions/checkout@v4
- uses: actions/setup-java@v4
with:
distribution: "temurin"
cache: "sbt"
Expand Down
29 changes: 19 additions & 10 deletions build.sbt
Original file line number Diff line number Diff line change
Expand Up @@ -12,20 +12,20 @@ lazy val V =
val protobuf = "3.15.6"
val protoc =
"3.17.3" // the oldest protoc version with Apple M1 support, see https://github.com/scalapb/ScalaPB/issues/1024#issuecomment-860126568
val coursier = "2.1.5"
val coursier = "2.1.9"
val scalaXml = "2.1.0"
val bsp = "2.0.0-M13"
val moped = "0.1.11"
val moped = "0.2.0"
val gradle = "7.0"
val scala213 = "2.13.10"
val scala212 = "2.12.17"
val scala213 = "2.13.13"
val scala212 = "2.12.19"
val scala211 = "2.11.12"
val scala3 = "3.2.2"
val metals = "0.11.11"
val scalameta = "4.8.1"
val scala3 = "3.3.3"
val metals = "1.2.2"
val scalameta = "4.9.3"
val semanticdbKotlinc = "0.4.0"
val testcontainers = "0.39.3"
val requests = "0.6.5"
val requests = "0.8.0"
val minimalMillVersion = "0.10.0"
val millScipVersion = "0.3.6"
val kotlinVersion = "1.9.22"
Expand All @@ -35,8 +35,6 @@ inThisBuild(
List(
scalaVersion := V.scala213,
crossScalaVersions := List(V.scala213),
scalafixDependencies +=
"com.github.liancheng" %% "organize-imports" % "0.6.0",
scalafixCaching := true,
scalacOptions ++= List("-Wunused:imports"),
semanticdbEnabled := true,
Expand Down Expand Up @@ -406,6 +404,17 @@ lazy val minimized17 = project
.dependsOn(agent, javacPlugin)
.disablePlugins(JavaFormatterPlugin)

lazy val minimized21 = project
.in(file("tests/minimized/.j21"))
.settings(
javaOnlySettings,
minimizedSettings,
javaToolchainVersion := "21",
javacOptions ++= javacModuleOptions
)
.dependsOn(agent, javacPlugin)
.disablePlugins(JavaFormatterPlugin)

lazy val minimizedScala = project
.in(file("tests/minimized-scala"))
.settings(
Expand Down
2 changes: 1 addition & 1 deletion project/build.properties
Original file line number Diff line number Diff line change
@@ -1 +1 @@
sbt.version=1.9.7
sbt.version=1.9.9
4 changes: 2 additions & 2 deletions project/plugins.sbt
Original file line number Diff line number Diff line change
Expand Up @@ -3,8 +3,8 @@ addSbtPlugin("se.marcuslonnberg" % "sbt-docker" % "1.9.0")
addSbtPlugin("com.github.sbt" % "sbt-ci-release" % "1.5.10")
addSbtPlugin("com.eed3si9n" % "sbt-buildinfo" % "0.10.0")
addSbtPlugin("org.scalameta" % "sbt-scalafmt" % "2.4.6")
addSbtPlugin("org.scalameta" % "sbt-mdoc" % "2.2.24")
addSbtPlugin("ch.epfl.scala" % "sbt-scalafix" % "0.10.1")
addSbtPlugin("org.scalameta" % "sbt-mdoc" % "2.5.2")
addSbtPlugin("ch.epfl.scala" % "sbt-scalafix" % "0.12.0")
addSbtPlugin("com.thesamet" % "sbt-protoc" % "1.0.6")
addSbtPlugin("com.sourcegraph" % "sbt-sourcegraph" % "0.4.3")
addSbtPlugin("com.lightbend.sbt" % "sbt-java-formatter" % "0.6.1")
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -101,7 +101,7 @@ object Embedded {
.forEach { line =>
reporter.error(line)
}
Some(CommandResult(1, Nil))
Some(CommandResult(Nil, 1, Nil))
} else {
None
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -114,16 +114,16 @@ class GradleBuildTool(index: IndexCommand) extends BuildTool("Gradle", index) {
| import com.sourcegraph.gradle.semanticdb.SemanticdbGradlePlugin
|
| allprojects {
| project.extra["semanticdbTarget"] = "$targetroot"
| project.extra["javacPluginJar"] = "$pluginpath"
| project.extra["dependenciesOut"] = "$dependenciesPath"
| project.extra["javacAgentPath"] = "$agentpath"
| apply<SemanticdbGradlePlugin>()
| project.ext["semanticdbTarget"] = "$targetroot"
| project.ext["javacPluginJar"] = "$pluginpath"
| project.ext["dependenciesOut"] = "$dependenciesPath"
| project.ext["javacAgentPath"] = "$agentpath"
| apply plugin: SemanticdbGradlePlugin
| }
""".stripMargin.trim

Files.write(
tmp.resolve("init-script.gradle.kts"),
tmp.resolve("init-script.gradle"),
script.getBytes(StandardCharsets.UTF_8)
)
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -128,15 +128,15 @@ class ScipBuildTool(index: IndexCommand) extends BuildTool("SCIP", index) {
} catch {
case NonFatal(e) =>
e.printStackTrace(index.app.out)
CommandResult(1, Nil)
CommandResult(Nil, 1, Nil)
}
case ErrorResult(error) =>
error
.all
.foreach { d =>
index.app.error(d.message)
}
CommandResult(1, Nil)
CommandResult(Nil, 1, Nil)
}
}

Expand Down Expand Up @@ -186,7 +186,7 @@ class ScipBuildTool(index: IndexCommand) extends BuildTool("SCIP", index) {
s"doing nothing, no files matching pattern '$sourceroot/**.{java,scala,kt}'"
)
}
return CommandResult(0, Nil)
return CommandResult(Nil, 0, Nil)
}

val compileAttempts = ListBuffer.empty[Try[Unit]]
Expand All @@ -206,7 +206,7 @@ class ScipBuildTool(index: IndexCommand) extends BuildTool("SCIP", index) {
errors.foreach { error =>
index.app.reporter.log(Diagnostic.exception(error))
}
CommandResult(1, Nil)
CommandResult(Nil, 1, Nil)
} else {
if (errors.nonEmpty && isSemanticdbGenerated) {
index
Expand All @@ -221,7 +221,7 @@ class ScipBuildTool(index: IndexCommand) extends BuildTool("SCIP", index) {
index.app.reporter.info(error.getMessage())
}
}
CommandResult(0, Nil)
CommandResult(Nil, 0, Nil)
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ import com.sourcegraph.scip_java.BuildInfo
import org.gradle.api.DefaultTask
import org.gradle.api.Plugin
import org.gradle.api.Project
import org.gradle.api.artifacts.Configuration
import org.gradle.api.artifacts.component.ModuleComponentIdentifier
import org.gradle.api.provider.Property
import org.gradle.api.publish.PublishingExtension
Expand Down Expand Up @@ -65,15 +66,16 @@ class SemanticdbGradlePlugin extends Plugin[Project] {
triggers += "compileJava"
triggers += "compileTestJava"

val hasAnnotationPath = {
val apConfig = project
.getConfigurations()
.getByName("annotationProcessor")
if (apConfig.isCanBeResolved()) {
apConfig.getDependencies().size() > 0
} else
false
}
val hasAnnotationPath = Try(
project.getConfigurations().getByName("annotationProcessor")
).map(apConfig =>
if (apConfig.isCanBeResolved()) {
apConfig.getDependencies().size() > 0
} else
false
)
.toOption
.contains(true)

val compilerPluginAdded =
try {
Expand All @@ -93,22 +95,23 @@ class SemanticdbGradlePlugin extends Plugin[Project] {
// If the `compileOnly` configuration has already been evaluated
// by the build, we need to fallback on agent injected into javac
warn(
s"Failed to add compiler plugin to javac, will go through the agent route: ${exc.getMessage()}"
s"Failed to add compiler plugin to javac, will go through the agent route (${exc
.getClass()}): ${exc.getMessage()}"
)
false
}

project
.getTasks()
.withType(classOf[JavaCompile])
.configureEach { task =>
.all { task =>
// If we run on JDK 17, we need to add special flags to the JVM
// to allow access to the compiler.

// JDK 17 support was only introduced in 7.3 so
// we don't need to do it for earlier versions
// https://docs.gradle.org/current/userguide/compatibility.html
if (!gradle.is5 && !gradle.is6) {
if (!gradle.is3 && !gradle.is2 && !gradle.is5 && !gradle.is6) {
type JavaCompiler = {
type Metadata = {
type LangVersion = {
Expand All @@ -118,6 +121,7 @@ class SemanticdbGradlePlugin extends Plugin[Project] {
}
def getMetadata(): Metadata
}

type HasCompilerProperty = {
def getJavaCompiler(): Property[JavaCompiler]
}
Expand Down Expand Up @@ -320,7 +324,7 @@ class SemanticdbGradlePlugin extends Plugin[Project] {
// classpath is murky
//
// We also don't want to bundle kotlin plugin with this one as it
// can cause all sorts of troubles.
// can cause all sorts of troubles).
//
// Instead, we commit the sins of reflection for our limited
// needs.
Expand Down Expand Up @@ -368,7 +372,7 @@ class SemanticdbGradlePlugin extends Plugin[Project] {
}
}

tasks.register(
tasks.create(
"scipCompileAll",
{ task =>
triggers
Expand All @@ -380,31 +384,34 @@ class SemanticdbGradlePlugin extends Plugin[Project] {
}
)

tasks.register("scipPrintDependencies", classOf[WriteDependencies])
tasks.create("scipPrintDependencies", classOf[WriteDependencies])

}

}

class GradleVersion(ver: String) {
override def toString(): String = s"[GradleVersion: $ver]"
def is7 = ver.startsWith("7.")
def is8 = ver.startsWith("8.")
def is6 = ver.startsWith("6.")
// 6.7 introduced toolchains support https://blog.gradle.org/java-toolchains
// And javaCompiler property
def is6_7_plus = {
ver match {
case s"6.$x.$y" if x.toInt >= 7 =>
true
case s"6.$x" if x.toInt >= 7 =>
true
case _ =>
false
}
}

class GradleVersion(ver: String) {
override def toString(): String = s"[GradleVersion: $ver]"
def is7 = ver.startsWith("7.")
def is8 = ver.startsWith("8.")
def is6 = ver.startsWith("6.")
// 6.7 introduced toolchains support https://blog.gradle.org/java-toolchains
// And javaCompiler property
def is6_7_plus = {
ver match {
case s"6.$x.$y" if x.toInt >= 7 =>
true
case s"6.$x" if x.toInt >= 7 =>
true
case _ =>
false
}
def is5 = ver.startsWith("5.")
}
def is5 = ver.startsWith("5.")
def is3 = ver.startsWith("3.")
def is2 = ver.startsWith("2.")
}

class WriteDependencies extends DefaultTask {
Expand All @@ -424,6 +431,8 @@ class WriteDependencies extends DefaultTask {
val project = getProject()
val projectName = project.getName()

val gradle = new GradleVersion(project.getGradle().getGradleVersion())

// List the project itself as a dependency so that we can assign project name/version to symbols that are defined in this project.
// The code below is roughly equivalent to the following with Groovy:
// deps += "$publication.groupId $publication.artifactId $publication.version $sourceSets.main.output.classesDirectory"
Expand All @@ -448,7 +457,7 @@ class WriteDependencies extends DefaultTask {
warn(s"""
|Failed to extract Maven publication from the project `$projectName`.
$crossRepoBanner
|Here's the raw error message:
|Here's the raw error message (${exception.getClass()}):
| "${exception.getMessage()}"
|Continuing without cross-repository support.
""".stripMargin.trim())
Expand Down Expand Up @@ -500,10 +509,16 @@ class WriteDependencies extends DefaultTask {
}
}

def canBeResolved(conf: Configuration) =
if (gradle.is2)
!conf.isEmpty()
else
conf.isCanBeResolved()

project
.getConfigurations()
.forEach { conf =>
if (conf.isCanBeResolved()) {
if (canBeResolved(conf)) {
try {
val resolved = conf.getResolvedConfiguration()
resolved
Expand Down
4 changes: 2 additions & 2 deletions tests/buildTools/src/test/resources/example-maven-pom.xml
Original file line number Diff line number Diff line change
Expand Up @@ -14,8 +14,8 @@

<properties>
<project.build.sourceEncoding>UTF-8</project.build.sourceEncoding>
<maven.compiler.source>1.7</maven.compiler.source>
<maven.compiler.target>1.7</maven.compiler.target>
<maven.compiler.source>1.8</maven.compiler.source>
<maven.compiler.target>1.8</maven.compiler.target>
</properties>

<dependencies>
Expand Down
Loading

0 comments on commit 0fd665e

Please sign in to comment.