Migrate to sbt 2#998
Merged
Merged
Conversation
Now that sbt-jcheckstyle 0.3.0 publishes an sbt2 cross-build, the sbt2 migration (superseding scala-steward PR #997) is unblocked. Bumps: - sbt: 1.12.13 -> 2.0.1 - sbt-jcheckstyle: 0.2.1 -> 0.3.0 (the actual blocker) - sbt-osgi: 0.10.0 -> 0.11.0-RC1 (only version with an sbt2 cross-build) - sbt-pgp, sbt-scalafmt, sbt-dynver already cross-built for sbt2 at their current versions build.sbt changes required by sbt 2's new task engine: - Wrap the jcheckStyle-dependsOn compile overrides in Def.uncached(...); CompileAnalysis has no JsonFormat to cache against - Add implicitConversions import and method-call dependsOn syntax to clear new Scala 3.8 strictness warnings sbt 2 itself requires JDK 17+ to run, while CI still needs to verify the library at runtime on JDK 8/11/17/21/24. Decouple the two via Test/fork + Test/javaHome (driven by a TEST_JAVA_HOME env var), and update CI.yml/release.yml/snapshot.yml to install a fixed modern JDK to launch sbt itself while forking tests onto the matrix JDK. Published bytecode is unaffected since it's already pinned to 1.8 via javacOptions.
CI's JDK8 lane failed: javac resolves API calls against whichever JDK actually runs it, not the -source/-target level, so compiling on JDK21 (now required to launch sbt 2) bound to JDK9+-only covariant overloads like ByteBuffer.flip(): ByteBuffer that don't exist on a real JDK8 at runtime (NoSuchMethodError). This is exactly what the pre-migration setup avoided by compiling and running each CI lane on the same single JDK. Restore that by scoping the existing TEST_JAVA_HOME-driven javaHome setting to the whole build instead of just Test, so sbt forks an external javac from that JDK for compilation too. Considered --release 8 instead, but that also hides the sun.misc.Unsafe/sun.nio.ch.DirectBuffer internals this code intentionally relies on, requiring extra flags for a more fragile setup than just compiling with the real target JDK. Wire release.yml/snapshot.yml the same way so published bytecode is compiled with a real JDK8/JDK11 again, matching their original intent.
The previous fix forked javac onto the target JDK, but missed that the test suite is Scala, and the Scala compiler always runs in-process inside the sbt JVM (it doesn't fork the way javac does). With sbt 2 forcing that JVM to be JDK17+, test code calling e.g. ByteBuffer.flip() directly (as MessageUnpackerTest.scala does) was still resolving to JDK9's covariant ByteBuffer.flip():ByteBuffer override, which doesn't exist on real JDK8 -> NoSuchMethodError at runtime, regardless of the javac fork fix. scalac's -release flag avoids this the same way --release does for javac, without needing an ignore-symbol-file escape hatch, since the test sources never touch JDK-internal APIs the way the main sources' sun.misc.Unsafe usage does. Verified via javap that the compiled call site now resolves to Buffer.flip():Buffer instead of the covariant override.
saveToTmpFile called File.createTempFile("testbuf", ".dat", new
File("target")) before tmp.getParentFile.mkdirs() — createTempFile
requires the parent directory to already exist, so under occasional
filesystem timing in CI sandboxes this threw "No such file or directory"
before the directory got created. Surfaced while validating the sbt 2
migration's CI matrix; unrelated to sbt 2 itself, but was intermittently
failing the JDK8 test lane via fail-fast cancellation of the rest of the
matrix.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
1.12.13->2.0.1, unblocked now thatsbt-jcheckstyle0.3.0publishes an sbt2 cross-build (supersedes Update sbt to 2.0.1 #997, which only bumpedbuild.propertiesand isn't sufficient on its own)sbt-jcheckstyle0.2.1->0.3.0andsbt-osgi0.10.0->0.11.0-RC1(the only version with a published sbt2 cross-build so far);sbt-pgp,sbt-scalafmt, andsbt-dynveralready have sbt2 cross-builds at their current versionsbuild.sbtfor sbt 2's new task engine: wrap thejcheckStyle-dependsOncompile overrides inDef.uncached(...)(CompileAnalysishas noJsonFormatto cache against), and clear new Scala 3.8 strictness warnings (implicitConversionsimport, method-calldependsOnsyntax)release.yml/snapshot.ymlpreviously launched./sbton JDK 8/11 directly. Decoupled "JDK running sbt" from "JDK under test" viaTest / fork+Test / javaHome(driven by aTEST_JAVA_HOMEenv var), and updatedCI.yml/release.yml/snapshot.ymlto install a fixed modern JDK (21) to launch sbt while forking tests onto the matrix JDK. Published bytecode is unaffected since it's already pinned to 1.8 viajavacOptions.Test plan
./sbt clean compile/Test/compile— succeeds, checkstyle passes./sbt clean test— full suite passes (193 tests, 0 failures/errors) on a freshly cleared sbt2 cache./sbt jcheckStyle/./sbt scalafmtCheckAll— pass./sbt package/./sbt publishLocal— succeed (exercises sbt-osgi, sbt-pgp, sbt-dynver)🤖 Generated with Claude Code