Conversation
The plugin now cross-publishes for both sbt 1.x and sbt 2.x. The build keeps an sbt 1.x launcher and produces both artifacts via crossScalaVersions [2.12, 3] + pluginCrossBuild/sbtVersion (2.12 -> sbt 1.x, Scala 3 -> sbt 2.0.0), matching the sbt-protoc / sbt-assembly pattern. sbt-projectmatrix has no sbt 2 release, so the "build with an sbt 2 launcher" path is not viable yet. Key changes: - build.sbt: split scala3LTS (3.3.7, runtime stays on LTS) from scala3 (3.8.4, codegen + plugin Scala-3 axis); add plugin crossScalaVersions, pluginCrossBuild/sbtVersion, scriptedSbt; add sbt2-compat 0.1.0. - Bump sbt-protoc 1.0.7 -> 1.1.0-RC1 (only version with an sbt-2 variant) in both the plugin module and project/plugins.sbt. - Migrate scalapb to 1.0.0-alpha.5: stable 0.11.x compilerplugin_3 is compiled against protoc-bridge_2.13, whose ProtocCodeGenerator.$init$ ABI is absent in the protoc-bridge_3 that sbt-protoc's sbt-2 build pulls (NoSuchMethodError at codegen time). Only scalapb 1.0.0-alpha.4+ is natively protoc-bridge_3. Exclude scalapb-json4s's transitive scalapb-runtime (json4s only ships alpha.1) so consumers get a single coherent alpha.5. - Source fixes for Scala 3 / -Werror: -Xfatal-warnings -> -Werror on Scala 3; drop private[this]; drop explicit Seq[Def.Setting[_]] return type; explicit InclExclRule element type in excludeDependencies. - e2e scripted test: pin scalaVersion 2.13 (resolves exact-match artifacts on both axes); delete project/build.properties so it runs on both sbt axes; @unchecked on partial pattern bindings; drop now-useless @nowarn. - CI: JDK 8 -> 17 (sbt 2.0.0 requires 17); run `+test +publishLocal +plugin/scripted` (bare scripted only covers sbt 1.x). - Docs: note dual sbt support, JDK 17, and the corrected test command. Verified: +test (50 tests, all axes) and +plugin/scripted (e2e passes under both sbt 1.11.7 and sbt 2.0.0) green; scalafmtCheckAll clean. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
Code Review
This pull request updates the project to support cross-building the twinagle-scalapb-plugin for both sbt 1.x and sbt 2.x, which includes upgrading Scala, sbt, and ScalaPB dependencies, adjusting compiler options, and making compatibility fixes. The review feedback highlights an existing bug in build.sbt where the -Xlint option is discarded, and suggests refactoring the scalacOptions block. Additionally, it recommends replacing refutable pattern bindings with idiomatic Specs2 throwA matchers in HaberdasherSpec.scala to avoid the use of @unchecked annotations.
- build.sbt: the scalacOptions block discarded the inner `scalacOptions ++=
Seq("-Xlint")` Setting, so -Xlint was never actually applied on Scala 2.
Refactor to return a single Seq per axis; -Xlint is now genuinely active on
2.12/2.13 (sources verified lint-clean) and -Werror on Scala 3.
- HaberdasherSpec: replace the `: @unchecked` refutable pattern bindings with
Specs2 `throwA[TwinagleException].like { case ex: TwinagleException => ... }`
(the explicit pattern type is required so `ex.code` resolves). Drops the now
unused `Throw` import.
Verified: +test (50 tests, all axes) and +plugin/scripted (e2e passes under both
sbt 1.11.7 and sbt 2.0.0) green; scalafmtCheckAll clean.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
|
@gemini-code-assist re-review |
| "com.thesamet.scalapb" %% "scalapb-runtime" % "1.0.0-alpha.5", | ||
| // scalapb-json4s has only published 1.0.0-alpha.1, which pulls scalapb-runtime alpha.1. | ||
| // Exclude its transitive scalapb-runtime so only our alpha.5 above remains: sbt's | ||
| // early-semver check otherwise flags the alpha.1 -> alpha.5 eviction as a hard conflict, | ||
| // and a per-dependency exclude (unlike dependencyOverrides) serializes into the published | ||
| // POM/ivy so downstream consumers inherit the coherent single version. | ||
| ("com.thesamet.scalapb" %% "scalapb-json4s" % "1.0.0-alpha.1") | ||
| .exclude("com.thesamet.scalapb", "scalapb-runtime_2.12") | ||
| .exclude("com.thesamet.scalapb", "scalapb-runtime_2.13") | ||
| .exclude("com.thesamet.scalapb", "scalapb-runtime_3"), |
There was a problem hiding this comment.
this is probably unacceptable but hopefully will be fixed once 1.0 scalapb is released
There was a problem hiding this comment.
Code Review
This pull request introduces cross-building support for both sbt 1.x and sbt 2.x, updating Scala versions, dependencies, and compiler options accordingly. Feedback points out that the specified Scala version 3.8.4 does not exist on Maven Central and will cause compilation failures. Additionally, it is recommended to simplify dependency exclusions in both build.sbt and Twinagle.scala by using ExclusionRule directly.
…ndencies
- Twinagle.scala: use ExclusionRule("org", "name") instead of the verbose
`Seq[sbt.librarymanagement.InclExclRule](org % name)` form.
- build.sbt: keep the explicit _2.12/_2.13/_3 suffixes on the scalapb-json4s
scalapb-runtime exclude, and document why. Gemini suggested
CrossVersion.binary, but that serializes to the suffix-less artifact id in
the published POM and fails to match scalapb-runtime_<v> in a downstream
consumer — the scripted e2e (consumer's-eye view) reproduces the version
conflict with it, so the explicit suffixes are required.
- Note: Gemini's "Scala 3.8.4 does not exist" finding is a stale-knowledge
false positive; CI resolves 3.8.4 and passes.
Verified: +test (50, all axes), +plugin/scripted (e2e passes under sbt 1.11.7
and sbt 2.0.0), scalafmtCheckAll all green.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Summary
Cross-builds
twinagle-scalapb-pluginfor both sbt 1.x and sbt 2.x, and migrates the scalapb stack as required to make the sbt-2 artifact actually work end-to-end.The build keeps an sbt 1.x launcher and cross-publishes both plugin artifacts via
crossScalaVersions = [2.12, 3]+pluginCrossBuild / sbtVersion(Scala 2.12 → sbt 1.x, Scala 3.8.4 → sbt 2.0.0). This is the pattern used by sbt's own reference plugins (sbt-protoc, sbt-assembly, sbt-buildinfo) per the sbt 2 plugin cross-building guide.Produces:
twinagle-scalapb-plugin_2.12_1.0— for sbt 1.x userstwinagle-scalapb-plugin_sbt2_3— for sbt 2.x usersKey decisions
1. sbt 1.x launcher, not sbt 2 — even though we target sbt 2.
The guide's "build from sbt 2" path requires
sbt-projectmatrix, which has no sbt-2 release (only_2.12_1.0exists). Building from an sbt-1 launcher withpluginCrossBuild / sbtVersionis the only way to produce both artifacts today, and is exactly what sbt-protoc/sbt-assembly do. "Build with sbt 2" here means the Scala-3 axis compiles against sbt 2.0.0 — the_sbt2_3artifact runs under sbt 2.0.0 and passes its e2e test.2. Split the Scala-3 version: runtime stays LTS, codegen+plugin move to 3.8.4.
sbt 2.0.0's build DSL runs on Scala 3.8.4, so the plugin (and
codegen, which itdependsOn) must compile on that axis.twinagle-runtime— the end-user library — stays on LTS 3.3.7. sbt can't cross-build one project at two Scala-3 versions (both produce_3), hence the split.3. Migrate scalapb to 1.0.0-alpha.5 — the hard blocker.
sbt-protoc's only sbt-2 variant (_sbt2_31.1.0-RC1) depends onprotoc-bridge_3(a native Scala-3 trait). But every stable scalapb 0.11.xcompilerplugin_3is compiled againstprotoc-bridge_2.13, whoseProtocCodeGenerator.$init$ABI is absent inprotoc-bridge_3→NoSuchMethodErrorat code-generation time. Only scalapb 1.0.0-alpha.4+ ships acompilerplugin_3that is nativelyprotoc-bridge_3. So a working sbt-2 plugin requires the alpha line. (The repo already started this in #494.)scalapb-json4shas only published1.0.0-alpha.1(pulls scalapb-runtime alpha.1); its transitive scalapb-runtime is excluded so consumers get a single coherent alpha.5 (sbt's early-semver check otherwise rejects the alpha.1→alpha.5 eviction). The exclude is per-dependency so it serializes into the published POM/ivy and downstream users inherit it.4. Scala 3 /
-Werrorsource fixes.-Xfatal-warnings→-Werroron Scala 3 (the alias is itself a fatal deprecation); droppedprivate[this]; dropped the explicitSeq[Def.Setting[_]]return type (_wildcard is fatal on 3.x,?won't compile on 2.12); explicitInclExclRuleelement type where sbt 2 no longer applies the implicit conversion.5. e2e scripted test runs on both axes.
Pinned
scalaVersion := 2.13(resolves exact-match published artifacts on both sbt axes). Deleted itsproject/build.propertiesso scripted runs it on both axes (sbt treats that file as a per-axis filter).@uncheckedon partial pattern bindings + dropped now-useless@nowarn(2.13 lint differences).6. CI: JDK 8 → 17 (sbt 2.0.0 requires 17), and
+test scripted→+test +publishLocal +plugin/scripted— barescriptedonly exercised the sbt-1 axis, and the 2.13-pinned e2e needstwinagle-runtime_2.13published first.Test plan
sbt +test— 50 tests pass across Scala 2.12 / 2.13 / 3.3.7sbt +publishLocal— both_2.12_1.0and_sbt2_3plugin artifacts publishsbt +plugin/scripted—generator/e2epasses under sbt 1.11.7 (2.12 axis) and sbt 2.0.0 (3.8.4 axis), 10 examples eachsbt scalafmtCheckAll— clean🤖 Generated with Claude Code