[SPARK-58200][BUILD] Add a scala-3 build profile and make the version switch reversible#57343
[SPARK-58200][BUILD] Add a scala-3 build profile and make the version switch reversible#57343fangchenli wants to merge 13 commits into
Conversation
… switch reversible
Adds the plumbing needed to build the tree against Scala 3 (3.3.8) alongside 2.13,
using the mechanism Spark already used to cross-build 2.12/2.13: rewrite the POMs with
dev/change-scala-version.sh, then select the target with a Maven profile. Both Maven and
sbt follow, the latter through the existing -P passthrough into sbt-pom-reader.
Nothing here is expected to build the whole reactor under Scala 3 yet. The point is to
have a build target so progress can be measured module by module, which is how the
scala-2.13 profile was introduced in SPARK-29296 roughly two years before 2.13 worked.
Version handling. scala.version is now a reference (${scala213.version} or
${scala3.version}) that the switch flips, rather than a literal interpolated from
`mvn help:evaluate -Pscala-<v>`. Each full version is therefore defined once and the two
cannot drift; the previous scheme required a scala-<version> profile to exist purely to
be evaluated, and the commented-out scala-2.13 profile it read had drifted to 2.13.11.
Coordinates that must not follow scala.binary.version get their own property, which also
keeps the artifactId rewrite away from them: mvn-scalafmt (a build-time plugin with no _3
build), chill (SPARK-55152, consumed as chill_2.13 from Scala 3), and the Scala standard
library itself, whose coordinate differs between 2 and 3 (scala-library vs
scala3-library_3). The scala-3 profile keeps scala-library managed even though it no
longer names it, or a transitive 2.13 stdlib silently downgrades the one underneath
scala3-library_3.
Fixes to dev/change-scala-version.sh, most of which are latent today rather than new:
- the artifactId rewrite matched any line containing the text "artifactId", so it also
corrupted property declarations whose name merely ends in it; it is now anchored to
the <artifactId> element
- FROM_VERSION was hardcoded to 2.12, which made a switch to any newer version one-way
- the scala.binary.version pattern required a dot and so could never match a bare 3
- the scaladoc/mima branch injected -Pscala-2.13 when switching *to* 2.13, from when
2.12 was the default; 2.13 is now the default and that profile no longer exists, so
running the script corrupted dev/mima and docs/_plugins/build_api_docs.rb
genjavadoc is dropped under Scala 3 in the sbt build: it is a Scala 2 compiler plugin
published against the full Scala version, and leaving its -P: options behind makes the
Scala 3 compiler fail with "bad option".
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
Thank you for your passion, @fangchenli . However, I'm wondering if you are really tested this patch manually by yourselves. Please switch the version here in order to see the result for further discussion.
To @uros-b , one tip for Scala 3 support. You need to ask the contributor to pass CI with their Scala 3 change before giving your approval (like I did in the above).
Apache Spark repository is a world-widely shared repository. We should not accept a partial patch because it could give a misleading signal to the other audience.
…o the 2.13 line under scala-3
scala-reflect and scala-compiler have no Scala 3 release, so under -Pscala-3 the
managed ${scala.version} resolved to a nonexistent 3.3.8 artifact and dependency
resolution failed before the compiler ran. Pin both to ${scala213.version} in the
scala-3 profile, matching how chill and the stdlib are already handled.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…piler version
repl declared scala-compiler and scala-reflect with an explicit
<version>${scala.version}</version>, which overrides dependencyManagement. Under
-Pscala-3 that resolved to a nonexistent 3.x artifact even with the managed pin in
the scala-3 profile. Drop the explicit versions so repl inherits the managed one,
as every other module already does.
The 2.13 build is unaffected: the managed version is ${scala.version}, so the
effective POM is byte-identical there.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…ne under scala-3
kafka and classutil publish no Scala 3 build, so kafka_${scala.binary.version} and
classutil_${scala.binary.version} resolved to nonexistent _3 artifacts under
-Pscala-3. Pin both coordinates the way chill already is.
kafka is test scope only; the connectors compile against the Java kafka-clients.
Pinning it to 2.13 makes it drag in jackson-module-scala_2.13, which conflicts with
the jackson-module-scala_3 that core provides, so exclude it alongside the
jackson-core, jackson-databind and jackson-annotations exclusions already there.
The exclusion uses the pinned property rather than a literal _2.13 so
change-scala-version.sh does not rewrite it to a suffix that matches nothing.
The 2.13 build is unaffected: the resolved test classpath of sql-kafka-0-10 is
identical before and after, and jackson-module-scala_2.13 remains on it via core.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
### What changes were proposed in this pull request?
Two source changes in `ConfigBuilder.scala` so `common/utils` compiles under
the `scala-3` profile:
- `enum` is a keyword in Scala 3 and cannot be used as a binding name, so
`case Some(enum) => enum` becomes `case Some(e) => e`.
- Scala 3 requires parentheses around a typed lambda parameter, so
`{ v: T => v.toString }` becomes `{ (v: T) => v.toString }`.
### Why are the changes needed?
Without them `common/utils` fails with 17 errors under `-Pscala-3`, which also
blocks `unsafe`, `kvstore` and `variant` since they depend on it. That leaves
only 16 Scala files in the whole reactor actually reaching the Scala 3
compiler, too little for CI to prove anything about the profile.
### Does this PR introduce any user-facing change?
No.
### How was this patch tested?
Both changes are accepted by Scala 2.13, so the existing build is unaffected:
`common-utils/compile` and `common-utils/Test/compile` pass on 2.13.
Under `-Pscala-3` the 14 modules below now compile, emitting 112 `.tasty`
files (899 classes): tags, common-utils, common-utils-java, unsafe, kvstore,
variant, sketch, launcher, network-common, network-shuffle, connect-shims,
udf-worker-proto, udf-worker-core, udf-worker-grpc.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
### What changes were proposed in this pull request? Adds a `scala3` job to `build_and_test.yml`, modelled on the existing `java25` job. It runs three checks: 1. `dev/change-scala-version.sh 3` followed by `2.13` must leave the tree unchanged, guarding the reversibility of the version switch. 2. The 14 modules that currently build under Scala 3 are compiled with `-Pscala-3`. 3. Each module that has Scala sources must have emitted `.tasty` files. Only dotty produces those, so this rules out a silent fallback to the Scala 2 compiler. Like `java25`, the job is enabled on forks and disabled on scheduled `apache/spark` runs, so it reports on pull requests. ### Why are the changes needed? The `scala-3` profile added by SPARK-58200 had no CI coverage, so nothing stopped a later change from breaking the switch or the modules that compile. All four build defects fixed in this PR were the kind this job catches. The module list is deliberately explicit rather than derived: the rest of the reactor still needs source migration under SPARK-54150, and an allowlist that has to be extended as subtasks land is clearer than a filter that could pass for the wrong reason. ### Does this PR introduce any user-facing change? No. ### How was this patch tested? Locally, on the merged tree: the round-trip leaves 0 modified files, the 14 modules compile under `-Pscala-3` producing 899 classes and 112 `.tasty` files, and the `.tasty` assertion was checked against negative controls (a Java-only module and a missing directory both correctly fail it). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…rofile ### What changes were proposed in this pull request? Two fixes found by building with Maven rather than SBT: - `dev/change-scala-version.sh` now rewrites `<scala.library.artifact>` alongside `<scala.version>` and `<scala.binary.version>`. Previously the stdlib coordinate was only swapped inside the `scala-3` profile, so the rewritten POM claimed `scala.version` 3.3.8 while still naming `scala-library`, and resolution failed on the nonexistent `org.scala-lang:scala-library:3.3.8` unless `-Pscala-3` was also passed. - The `scala-3` profile sets `maven.scaladoc.skip`. `scala-maven-plugin` passes `-doc-format:html` and `-doc-title` to the `doc-jar` goal by default; Scala 3's scaladoc rejects both, and because `-doc-title` is dropped its argument is then read as a source file, failing the build at the first module with Scala sources. ### Why are the changes needed? Without them `./build/mvn -Pscala-3 ... install` fails immediately, even though the same modules build with SBT. The CI job added in this PR uses SBT, so neither defect would have been caught there. ### Does this PR introduce any user-facing change? No. `maven.scaladoc.skip` stays `false` outside the profile, so scaladoc still runs for Scala 2.13. ### How was this patch tested? `./dev/change-scala-version.sh 3` followed by `2.13` still leaves the tree unchanged. After switching to Scala 3, `dependency:resolve` without any profile now resolves `scala3-library_3:3.3.8` and `scala-library:2.13.18`. With `-Pscala-3 -Dmaven.test.skip=true`, all 14 modules that build under Scala 3 install successfully. Test sources are not migrated yet, so `-DskipTests` alone still fails in `udf-worker-core`; that belongs to SPARK-54150. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…t on the 2.13 line
### What changes were proposed in this pull request?
Points the root `dependencyManagement` entries for `scala-compiler` and
`scala-reflect` at `${scala213.version}` instead of `${scala.version}`, and drops
the duplicate declarations the `scala-3` profile added for them.
### Why are the changes needed?
Neither artifact is published for Scala 3, so `${scala.version}` resolves to a
nonexistent 3.x coordinate once the version switch has run. Pinning them in the
profile worked, but re-declaring the same groupId:artifactId there left two
managed entries for one dependency, and the profile copy carried no `<exclusions>`
element, so whether the base `scala-xml` exclusion survived depended on Maven's
profile-injection merge semantics.
Managing them on the 2.13 line in the base POM is unambiguous, keeps the exclusion
attached to the single declaration that has it, and makes the switched POM resolve
these artifacts with or without `-Pscala-3`.
### Does this PR introduce any user-facing change?
No. Under Scala 2.13 `scala.version` is `${scala213.version}`, so the managed
versions are unchanged.
### How was this patch tested?
The effective POM on Scala 2.13 is identical before and after apart from the
removed profile entries; both managed versions still resolve to 2.13.18.
After `./dev/change-scala-version.sh 3`, `scala-compiler` and `scala-reflect`
resolve to 2.13.18 and `scala3-library_3` to 3.3.8, both with and without
`-Pscala-3`. `-Pscala-3 -Dmaven.test.skip=true install` of the 14 Scala 3 modules
still succeeds.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…ild job ### What changes were proposed in this pull request? Turns the `scala3` job into a matrix over `sbt` and `maven`, with `fail-fast` disabled so one build tool failing still reports the other. The Maven leg runs `-Pscala-3 -Dmaven.test.skip=true ... install` over the same 14 modules and caches `~/.m2/repository` under its own key. `-Dmaven.test.skip` rather than `-DskipTests`, since the latter still compiles test sources, which are not migrated to Scala 3 yet. The `.tasty` verification now picks the output directory per build tool: SBT writes to `target/scala-<full version>`, Maven to `target/scala-<binary version>`. ### Why are the changes needed? The two paths diverge, and the SBT leg alone does not cover the difference. Testing Maven by hand found two defects that an SBT-only job could not have caught: the switched POM not resolving without `-Pscala-3`, and the `scala-maven-plugin` scaladoc options that Scala 3 rejects. Both are fixed in this PR, and both lived in configuration SBT never reads. ### Does this PR introduce any user-facing change? No. ### How was this patch tested? Both legs were run locally on the merged tree. SBT compiles all 14 modules; Maven installs all 14 plus the parent POM. The `.tasty` verification was exercised against both output layouts and reports the same 4 modules with Scala sources (`tags`, `common-utils`, `connect-shims`, `udf-worker-core`). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
… line ### What changes were proposed in this pull request? Reformats `SCALA3_MAVEN_MODULES` from a single long comma-separated string to one module per line, matching how `SCALA3_MODULES` is already written, and joins the entries with commas where Maven's `-pl` consumes them. The list has to be space separated in YAML: a folded scalar replaces newlines with spaces, so keeping the commas in the value would produce `a, b` rather than `a,b` and Maven would not match the module names. ### Why are the changes needed? The two module lists cover the same 14 modules and are easier to compare, and to extend as SPARK-54150 subtasks land, when they are formatted the same way. ### Does this PR introduce any user-facing change? No. ### How was this patch tested? `-Pscala-3 -Dmaven.test.skip=true -pl "$(echo $SCALA3_MAVEN_MODULES | tr ' ' ',')" -am install` was run with the reformatted value and installs all 14 modules plus the parent POM. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…module list ### What changes were proposed in this pull request? Replaces the two parallel module lists in the `scala3` job with one, given as directories. Maven consumes it directly; the SBT project names are read out of each module POM's `<sbt.project.name>` instead of being listed separately. If a POM does not declare the property, the step fails and names it. SBT would otherwise fall back to the artifactId and build the wrong project, or nothing, while the job still reported success. ### Why are the changes needed? SBT addresses projects by `<sbt.project.name>` and Maven by directory, and neither can be derived from the other by rule, so the two lists had to be kept in sync by hand with nothing detecting drift. Adding a module to one and not the other would have quietly narrowed what the job covers. ### Does this PR introduce any user-facing change? No. ### How was this patch tested? The derived command list is identical to the hand-written one it replaces, and `./build/sbt -Pscala-3` with the derived commands compiles all 14 modules with no errors. The failure path was checked against a POM with no `<sbt.project.name>`; every module in the tree currently declares it, so the guard is for modules added later. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
… Maven leg ### What changes were proposed in this pull request? Splits the Maven leg of the `scala3` job into two passes: one that packages the test-jars the module set depends on, and one that builds main sources only. ### Why are the changes needed? The single-pass `-Dmaven.test.skip=true` build failed in CI resolving `spark-tags_3:jar:tests`. Maven resolves test-scope dependencies even when it is not compiling tests, and `maven.test.skip` suppresses test-jar packaging along with test compilation, so nothing produced the jar. It passed locally only because a stale artifact from an earlier `-DskipTests` run was still in `~/.m2`. Only three modules publish test-jars the others depend on, and all three have Java test sources only, so packaging them cannot invoke the Scala 3 compiler. Building those first leaves the second pass free to skip test compilation entirely, which matches what the SBT leg compiles: main sources for the same 14 modules. The alternative, `-DskipTests` throughout, would compile every module's test sources. Those are not migrated to Scala 3 yet and fail in `udf-worker-core`. The second pass also passes `-DskipTests`, because scalatest-maven-plugin reads that property rather than `maven.test.skip` and would otherwise run any test classes left in the build directory. ### Does this PR introduce any user-facing change? No. ### How was this patch tested? Ran the step with both `~/.m2` and every `target/scala-3` directory removed first, since earlier runs had been passing against leftover state. Both passes report `BUILD SUCCESS`, all 14 modules plus the parent POM install, no tests execute, and the `.tasty` verification finds the same 4 modules with Scala sources. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
| def toEnum[E <: Enum[E]](s: String, enumClass: Class[E], key: String): E = { | ||
| enumClass.getEnumConstants.find(_.name().equalsIgnoreCase(s.trim)) match { | ||
| case Some(enum) => enum | ||
| case Some(e) => e |
There was a problem hiding this comment.
enum is a keyword in Scala 3.
|
|
||
| def this(parent: ConfigBuilder, converter: String => T) = { | ||
| this(parent, converter, { v: T => v.toString }) | ||
| this(parent, converter, { (v: T) => v.toString }) |
There was a problem hiding this comment.
Ditto. Is this a source-code-level breaking change of Scala 3?
There was a problem hiding this comment.
The old syntax is no longer valid in Scala 3.
Those two code changes are slightly out of scope for this PR. But without them, only a few Scala files can be compiled. Happy to split them into a separate PR if you prefer this PR to stay free of source changes.
There was a problem hiding this comment.
IIRC, there exists some PRs (before this PR) for this kind of breaking Scala 3 language change.
What changes were proposed in this pull request?
Adds a Scala 3 build target alongside 2.13: a scala-3 Maven profile (Scala 3.3.8) and a reversible dev/change-scala-version.sh (2.13 <-> 3). Coordinates with no Scala 3 build are pinned to their own property (chill as chill_2.13, mvn-scalafmt), and genjavadoc is disabled under Scala 3. It is not expected to compile the whole reactor yet; it is the target modules get ported against.
Why are the changes needed?
There is currently no way to build the project against Scala 3, since the cross-version tooling only targets 2.13. A Scala 3 build target is a prerequisite for porting modules and measuring progress one at a time, the same way the Scala-2.13 profile (SPARK-29296) was added well before 2.13 was actually built.
Does this PR introduce any user-facing change?
No.
How was this patch tested?
Manually tested.
Was this patch authored or co-authored using generative AI tooling?
Generated-by: Claude Code (Opus 4.8)