andygrove opened a new pull request, #6448: URL: https://github.com/apache/datafusion-comet/pull/6448
## Which issue does this PR close? No issue filed; the problem is described below. ## Rationale for this change `make format` fails on the default profile: ``` Could not resolve artifact org.scalameta:semanticdb-scalac_2.13.17:4.13.6 ``` `semanticdb-scalac` is published separately for each Scala patch version. The pom pins 4.13.6 for every Scala 2.13 profile, and 4.13.6 was never published for Scala 2.13.17 (Spark 4.1, the default) or 2.13.18 (Spark 4.2). Later releases are: 4.13.9 and newer exist for 2.13.16, 2.13.17 and 2.13.18. The pom comment and two workflow comments still say `semanticdb-scalac_2.13.17` "is not yet published", and because of that: - `make format`, which the contributor guide and the `implement-comet-expression` and `optimize-comet-expression` skills tell contributors to run before a PR, fails on the default profile, so the semantic scalafix rules such as `RemoveUnused` never run locally. - CI leaves Spark 4.1 and 4.2 out of the Lint Java matrix, so sources that only those profiles compile never get the semantic rules. A separate compile-only `Build Spark 4.1` job stands in for it. - The benchmark check is pinned to Spark 4.0, so the 4.1-only benchmark sources are never linted. ## What changes are included in this PR? - `semanticdb.version` goes from 4.13.6 to 4.13.10 in every Scala 2.13 profile, with a note that it has to be published for each of their Scala versions. The Scala 2.12 profiles keep 4.8.8. - The Lint Java matrix gains Spark 4.1 and Spark 4.2 (JDK 17). The `Build Spark 4.1` job only compiled 4.1's main sources, because 4.1 could not join the lint matrix; the 4.1 lint entry compiles those and the tests, so the job is removed. Net CI cost is one more lint job per run. - The benchmark check lints the default profile instead of pinning Spark 4.0. - Linting 4.1 surfaced one violation: `RemoveUnused` flags `sink` in `CometTimeExtractBenchmark` because it is written but never read. That is deliberate, since the volatile store keeps the JIT from dropping the measured work, so the line is marked `// scalafix:ok RemoveUnused` rather than deleted. No 4.2-only source had a violation. - Docs: "Adding a New Spark Version" now says to check `semanticdb.version` for a new Scala patch version and to add the profile to the Lint Java matrix (the `build-spark` job it pointed at does not exist). The CI docs now say Lint Java compiles every Spark profile. ## How are these changes tested? Locally on macOS with JDK 17: - The Lint Java command, `./mvnw -B package -DskipTests scalafix:scalafix -Dscalafix.mode=CHECK -Psemanticdb -Pspark-X`, passes for `spark-4.0`, `spark-4.1` and `spark-4.2`, compiling with `semanticdb-scalac` 4.13.10 for Scala 2.13.16, 2.13.17 and 2.13.18, and scalafix processes each profile's source roots. - `make format` on the default profile completes and changes nothing beyond this PR. - `dev/ci/check-ci-config.py` and `actionlint` pass. The Lint Java entries run on this PR. The `run-benchmark-check` label runs the benchmark check on the default profile. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected] --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
