Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Update latest Scala 2 and Scala 3 versions #1636

Merged
merged 3 commits into from
Oct 25, 2024
Merged

Update latest Scala 2 and Scala 3 versions #1636

merged 3 commits into from
Oct 25, 2024

Conversation

WojciechMazur
Copy link
Contributor

@WojciechMazur WojciechMazur commented Oct 23, 2024 •
edited
Loading

Description

Updates to latest minor version of Scala 2 or patch version of Scala 3:

  • 2.12.19 -> 2.12.20
  • 2.13.14 -> 2.13.15
  • 3.3.3 -> 3.3.4
  • 3.5.0 -> 3.5.2

WojciechMazur
WojciechMazur commented Oct 23, 2024
View reviewed changes
scripts/create_repository.py
@@ -33,7 +33,7 @@ def select_root_artifacts(scala_version) -> List[str]:
scala_major = ".".join(scala_version.split(".")[:2])
scala_test_major = "3" if scala_major >= "3.0" else scala_major
scala_fmt_major = "2.13" if scala_major >= "3.0" else scala_major
kind_projector_version = "0.13.2" if scala_major < "2.13" else "0.13.3"
kind_projector_version = "0.13.2" if scala_major < "2.12" else "0.13.3"
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Kind projector 0.13.2 is no longer published for Scala 2.12, minimal version is 0.13.3

WojciechMazur
WojciechMazur commented Oct 23, 2024
View reviewed changes
scripts/create_repository.py
@@ -104,7 +104,9 @@ def map_to_resolved_artifacts(output) -> List[ResolvedArtifact]:

def resolve_artifacts_with_checksums_and_direct_dependencies(root_artifacts) -> List[ResolvedArtifact]:
command = f'cs resolve {' '.join(root_artifacts)}'
output = subprocess.run(command, capture_output=True, text=True, shell=True).stdout.splitlines()
proc = subprocess.run(command, capture_output=True, text=True, shell=True)
print(proc.stderr)
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I left it here, becouse if some artifact cannot be find (see kind-projector) we're left without any context

WojciechMazur
WojciechMazur commented Oct 23, 2024
View reviewed changes
third_party/repositories/scala_3_3.bzl
Comment on lines +147 to +148
"artifact": "org.scala-sbt:compiler-interface:1.9.6",
"sha256": "b986ad1cfcef14c8abeaa3e788df28be9150acfbf587b5f7aa92c1b043c4fa8d",
Copy link
Contributor Author

@WojciechMazur WojciechMazur Oct 23, 2024 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't know why the previously declared version was higher than minor version used in the compiler. (1.10.1)

@WojciechMazur
Copy link
Contributor Author

For reviewers:
The first commit contains most of important changes: changes to versions and SHA's
The next commits are fixing rest of the build to ensure usage of the latest version in tests

@WojciechMazur WojciechMazur mentioned this pull request Oct 23, 2024
Merged
mbland added a commit to mbland/rules_scala that referenced this pull request Oct 24, 2024
@mbland
Bump Scala 2 libs after bazelbuild#1636
c30f54b 
Brings all the Scalafmt 3.8.3 updates from bazelbuild#1631 up to date with the
Scala version bumps from bazelbuild#1636.
@WojciechMazur
Fix failing Scala 3 diagnostics tests after upgrade to 3.3.4 - this v…
28ca146 
…ersion backported propositions for missing identifiers
@simuons
Copy link
Collaborator

simuons commented Oct 25, 2024

Thanks, @WojciechMazur!

@simuons simuons merged commit 487c2fa into bazelbuild:master Oct 25, 2024
2 checks passed
@WojciechMazur WojciechMazur deleted the update/scala-versions branch October 25, 2024 18:33
mbland added a commit to mbland/rules_scala that referenced this pull request Oct 25, 2024
@mbland
Bump Scala 2 libs after bazelbuild#1636
9879592 
Brings all the Scalafmt 3.8.3 updates from bazelbuild#1631 up to date with the
Scala version bumps from bazelbuild#1636.
liucijus pushed a commit that referenced this pull request Oct 29, 2024
@mbland
Update to Scalafmt 3.8.3 (#1631)
190fbee 
* Update to Scalafmt 3.8.3

Ensures that all Scalafmt targets succeed under every supported Scala
version. Part of #1482.

Scala 2.11 remains on Scalafmt 2.7.5, since there's not a more recent
compatible version.

Between 3.0.0 and 3.8.3, Scalafmt's `FileOps` moved packages,
`Config.fromHoconFile` moved to `ScalafmtConfig`, and the methods now
take a `java.nio.file.Path` parameter. As a result, this required
extracting a thin `ScalafmtAdapter` object, with one implementation for
Scala 2.11 and Scalafmt 2.7.5, and one for Scalafmt 3.8.3.

This change also adds the file path to the `Scalafmt.format()` call,
allowing error messages to show the actual file path instead of
`<input>`.

Removed the `verticalMultiline.newlineBeforeImplicitKW = true` and
`unindentTopLevelOperators = false` options from `.scalafmt.conf`.
They don't appear to be available in Scalafmt 3.8.3.

Also removes some `@io_bazel_rules_scala` references to make the
internal implementation less dependendent on that name. This will allow
Bzlmod clients to use `rules_scala` in a `bazel_dep()` without setting
`repo_name = "io_bazel_rules_scala"`.

---

Scalafmt 3.0.0 works with the current default Scala version 2.12.19,
but breaks under Scala 2.13.14:

```txt
$ bazel test --repo_env=SCALA_VERSION=2.13.14 //test/scalafmt/...

ERROR: .../test/scalafmt/BUILD:49:20:
  ScalaFmt test/scalafmt/test/scalafmt/unformatted/unformatted-test.scala.fmt.output
  failed: (Exit 1): scalafmt failed: error executing command
  (from target //test/scalafmt:unformatted-test)
  bazel-out/darwin_arm64-opt-exec-2B5CBBC6/bin/scala/scalafmt/scalafmt
  '--jvm_flag=-Dfile.encoding=UTF-8' ... (remaining 1 argument skipped)

java.util.NoSuchElementException: last of empty IndexedSeq
    at scala.collection.IndexedSeqOps.last(IndexedSeq.scala:110)
    at scala.collection.IndexedSeqOps.last$(IndexedSeq.scala:105)
    at scala.meta.tokens.Tokens.last(Tokens.scala:29)
    at org.scalafmt.internal.Router.$anonfun$getSplitsImpl$90(Router.scala:707)
    at scala.Option.map(Option.scala:242)
    at org.scalafmt.internal.Router.getSplitsImpl(Router.scala:706)
    at org.scalafmt.internal.Router.getSplits(Router.scala:2314)
    at org.scalafmt.internal.BestFirstSearch.$anonfun$routes$1(BestFirstSearch.scala:38)
    [ ...snip... ]
```

This matches:

- scala/community-build#1680

Which mentions apparent fixes in:

- scalameta/scalameta#3235
- scalameta/scalafmt#3581

So the fix was to upgrade the Scalafmt version. That said, I held its
Scalameta dependencies to 4.9.9 per the following link, even though
4.10.2 is out now.

- https://mvnrepository.com/artifact/org.scalameta/scalafmt-core_2.13/3.8.3

---

This change updates Scalameta to version 4.9.9 because between 4.9.9 and
4.10.2, Scalafmt breaks Scala 3 file formatting by unindenting some code
that it shouldn't. Or, our usage of it breaks somehow; I can't find any
open or closed issues in the Scalameta project that matches what happes
in rules_scala. (Perhaps I can file one eventually.)

- https://github.com/scalameta/scalafmt/issues

The solution was to keep the Scalameta dependencies at 4.9.9. FWIW, this
was one of the most time consuming bugs to pinpoint and rectify in the
entire Bzlmodification process.

This was the failing command inside `test_cross_version`:

```txt
$ bazel run //scalafmt:unformatted-test3.format-test

INFO: Analyzed target //scalafmt:unformatted-test3.format-test
  (0 packages loaded, 0 targets configured).
INFO: From ScalaFmt scalafmt/scalafmt/unformatted/unformatted-test3.scala.fmt.output:
```

The `test_cross_version/scalafmt/unformatted/unformatted-test3.scala`
file formatted by this test target looks like this:

```scala
import org.scalatest.flatspec._

class Test extends
  AnyFlatSpec:

        "Test"  should  "be formatted" in  {
     assert(true)
        }
```

I hacked `ScalaWorker.format()` to print the `code` variable, and could
see that  after the first `Scalafmt.format()` pass, the code looks like
this:

```scala
import org.scalatest.flatspec._

class Test extends AnyFlatSpec:

"Test" should "be formatted" in {
  assert(true)
}
```

Since the result doesn't match the original code, it tries to call
`Scalafmt.format()` again on this "formatted" code with the incorrect
indentation. That's when we get the following, which doesn't look
anything like the original file:

```txt
Unable to format file due to bug in scalafmt
scalafmt/unformatted/unformatted-test3.scala:3:
  error: [dialect scala3 [with overrides]] `;` expected but `:` found
class Test extends AnyFlatSpec:
                              ^
```

---

As it turns out, bumping to com.google.protobuf:protobuf-java:4.28.2
alone breaks the bump to Scalafmt 3.8.3. Then bumping to rules_proto
6.0.2, with the separate protobuf v21.7, fixes it, presumably by
recompiling `protoc`.

The in-between breakage happened in the `test_cross_build` project:

```txt
$ bazel build //scalafmt:formatted-binary2

INFO: Analyzed target //scalafmt:formatted-binary2
  (4 packages loaded, 64 targets configured).
ERROR: .../test_cross_build/scalafmt/BUILD:59:22:
  ScalaFmt scalafmt/scalafmt/formatted/formatted-binary2.scala.fmt.output
  failed: Worker process did not return a WorkResponse:

---8<---8<--- Start of log, file at .../bazel-workers/worker-3-ScalaFmt.log ---8<---8<---
Exception in thread "main" java.lang.NoSuchMethodError:
  'void com.google.devtools.build.lib.worker.WorkerProtocol$WorkRequest.makeExtensionsImmutable()'
    at com.google.devtools.build.lib.worker.WorkerProtocol$WorkRequest.<init>(WorkerProtocol.java:1029)
    at com.google.devtools.build.lib.worker.WorkerProtocol$WorkRequest.<init>(WorkerProtocol.java:922)
    at com.google.devtools.build.lib.worker.WorkerProtocol$WorkRequest$1.parsePartialFrom(WorkerProtocol.java:2482)
    at com.google.devtools.build.lib.worker.WorkerProtocol$WorkRequest$1.parsePartialFrom(WorkerProtocol.java:2476)
    at com.google.protobuf.AbstractParser.parsePartialFrom(AbstractParser.java:192)
    at com.google.protobuf.AbstractParser.parsePartialDelimitedFrom(AbstractParser.java:232)
    at com.google.protobuf.AbstractParser.parseDelimitedFrom(AbstractParser.java:244)
    at com.google.protobuf.AbstractParser.parseDelimitedFrom(AbstractParser.java:249)
    at com.google.protobuf.AbstractParser.parseDelimitedFrom(AbstractParser.java:25)
    at com.google.protobuf.GeneratedMessage.parseDelimitedWithIOException(GeneratedMessage.java:367)
    at com.google.devtools.build.lib.worker.WorkerProtocol$WorkRequest.parseDelimitedFrom(WorkerProtocol.java:1438)
    at io.bazel.rulesscala.worker.Worker.persistentWorkerMain(Worker.java:81)
    at io.bazel.rulesscala.worker.Worker.workerMain(Worker.java:49)
    at io.bazel.rules_scala.scalafmt.ScalafmtWorker$.main(ScalafmtWorker.scala:12)
    at io.bazel.rules_scala.scalafmt.ScalafmtWorker.main(ScalafmtWorker.scala)
---8<---8<--- End of log ---8<---8<---
Target //scalafmt:formatted-binary2 failed to build
```

* Add attributes to .scalafmt.conf files

Per suggestions from @WojciechMazur in #1631.

* Bump Scala 2 libs after #1636

Brings all the Scalafmt 3.8.3 updates from #1631 up to date with the
Scala version bumps from #1636.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@simuons simuons simuons approved these changes

@liucijus liucijus Awaiting requested review from liucijus liucijus is a code owner

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@WojciechMazur @simuons