[WIP] Cross-compile Akka with Scala 3 #29929

andreaTP · 2021-01-05T09:09:54Z

This PR aims to kick start the work and conversations on cross-compiling Akka with, upcoming, Scala 3.

Current status:
Modules akka-actor, akka-testkit, akka-actor-tests are compiling and all of the tests are passing locally (but TypedMultiMapSpec which is commented) starting sbt with sbt -Dakka.build.scalaVersion=3.0.
Started cross-compiling akka-stream which is down to ~50 compile-time errors, but I have a problem with variance (i.e. @uncheckedVariance) that needs further attention.

There are a number of small changes that are simple refactorings and I will follow up with separate PRs to current master to reduce the diff (e.g. extra import to enable the bang operator).

I'll comment on the diff a bunch of additional points that came out and try to ping relevant ppl for help 🙏

Note: This is a personal initiative started during the Christmas break 🙂

akka-ci · 2021-01-05T09:11:22Z

Thank you for your pull request! After a quick sanity check one of the team will reply with 'OK ΤO ΤESΤ' to kick off our automated validation on Jenkins. This compiles the project, runs the tests, and checks for things like binary compatibility and source code formatting. When two team members have also manually reviewed and (perhaps after asking for some amendments) accepted your contribution, it should be good to be merged.

For more details about our contributing process, check out CONTRIBUTING.md - and feel free to ask!

andreaTP · 2021-01-05T09:17:39Z

akka-actor-tests/src/test/scala/akka/AkkaExceptionSpec.scala

@@ -16,6 +16,9 @@ import akka.actor._
 */
 class AkkaExceptionSpec extends AnyWordSpec with Matchers {

+  // TODO DOTTY, yes I know it's wrong
+  implicit val pos: org.scalactic.source.Position = new org.scalactic.source.Position(fileName = "", filePathname = "", lineNumber = 1)


This is a workaround, ScalaTest is usable with Scala 3, but I found a few rough edges.
The implicit position is provided from a macro in Scala 2.X and is not available in Scala 3.

I see that there is some ongoing work to cross-compile ScalaTest (e.g. scalatest/scalatest@e8201d4 ) and I expect that this issue is solved there, maybe @bvenners or @cheeseng can chip in?

Reference issue:
scalatest/scalatest#1950

andreaTP · 2021-01-05T09:20:14Z

akka-actor-tests/src/test/scala/akka/actor/DynamicAccessSpec.scala

+  implicit val pos: org.scalactic.source.Position = new org.scalactic.source.Position(fileName = "", filePathname = "", lineNumber = 1)
+
+  // TODO DOTTY
+  protected override def runTest(testName: String, args: org.scalatest.Args): org.scalatest.Status = akka.testkit.ScalatestRunTest.scalatestRunTest(testName, args)


This is again regarding ScalaTest, I would love to know if there is a better way of doing it.
cc. @bvenners @cheeseng

andreaTP · 2021-01-05T09:22:36Z

akka-actor-tests/src/test/scala/akka/actor/ActorRefSpec.scala

@@ -466,8 +466,8 @@ class ActorRefSpec extends AkkaSpec("""
        def receive = { case name: String => sender() ! context.child(name).isDefined }
      }), "parent")

-      assert(Await.result((parent ? "child"), timeout.duration) === true)


assert is a macro and is not usable "as is", for the moment I switched to require, but I was expecting this to work out of the box, maybe @nicolasstucki or @smarter can shed a light?

What was the issue with assert?

Since assert is a macro Scala 3 rejects it.

@nicolasstucki there's no actual issue, this project is just using a scala 2 version of scalatest instead of the scala 3 version: #29929 (comment)

andreaTP · 2021-01-05T09:24:02Z

akka-actor-tests/src/test/scala/akka/actor/DynamicAccessSpec.scala

@@ -51,7 +59,7 @@ class DynamicAccessSpec extends AnyWordSpec with Matchers with BeforeAndAfterAll
      instantiateWithDefaultOrStringCtor("akka.actor.TestClassWithDefaultConstructor").get.name shouldBe "default"
      instantiateWithDefaultOrStringCtor("akka.actor.foo.NonExistingClass") match {
        case Failure(t) =>
-          t shouldBe a[ClassNotFoundException]


ScalaTest related type check is done via a macro.
cc. @bvenners @cheeseng

andreaTP · 2021-01-05T09:27:55Z

akka-actor-tests/src/test/scala/akka/util/TypedMultiMapSpec.scala

-    }
-
-  }
+  // "A TypedMultiMap" must {


I guess that TypedMultiMap types signatures are pretty bound to Scala 2, anyone I can ping on the subject?
maybe @raboof or @johanandren can point me to the right person?

It's our own impl so not sure anyone is more or less suitable to say anything about it without looking at in which way it does not work on Scala 3. Perhaps it would be possible to implement a source compatible version specifically for Scala 3?

I will take a look and give it a spin.

andreaTP · 2021-01-05T09:31:50Z

akka-actor/src/main/java/akka/actor/dungeon/AbstractActorCell.java

-              ActorCell.class.getDeclaredField(
-                  "akka$actor$dungeon$Dispatch$$_mailboxDoNotCallMeDirectly"));
+              // TODO DOTTY
+              ActorCell.class.getDeclaredField("_mailboxDoNotCallMeDirectly"));


Here I need some guidance on how to proceed, this AFAIU prevents today to use Akka Actor in a Scala 3 project.
But, at the same time, rely on the internals of the compiler, should we implement fallbacks or tackle the problem differently? cc. @raboof @johanandren @patriknw

@eloots did you not hit this in https://github.com/eloots/akka-typed-on-scala-3 ?

I assume that @eloots was using exclusively Akka Typed, probably not hitting the untyped ActorCell. But this needs to be proved ...

The offsets here is used by the underlying impl also for typed actors (those are on top of "classic" actors), so that should be involved regardless. But Erics sample just used the pre-compiled 2.13 Akka artifacts rather than compiling them with Scala 3 himself, so maybe that keeps the encoding.

I guess this would have to be separate logic for Scala 2 and 3 somehow?

I'll double-check that this is needed after the rest of the more "trivial" changes get merged, in case I'll go for the switch case. (Do we already have available the scalaVersion in the runtime, such as BuildInfo or something like Version?)

Neither Scala 2 nor Scala 3 give any guarantee about the naming scheme of private fields, so while this might work on a particular version of a compiler, it might not work on the next.

While I understand there is no such guarantee/promise, it has been stable in Scala 2 at least since 2012 (and Scala 2.10.0-M7). This will likely be tricky to solve if the internal encoding in Scala 3 is expected to change often.

Perhaps moving on to VarHandle from Unsafe could solve it, but we can't do that as long as we also support Java 8 (I have a feeling that there was performance concerns around that as well).

akka-actor/src/main/scala/akka/actor/ActorPath.scala

andreaTP · 2021-01-05T09:36:23Z

akka-actor/src/main/scala/akka/actor/ActorPath.scala

-@SerialVersionUID(1L)
+// TODO DOTTY
+// @SerialVersionUID does nothing on a trait
+// @SerialVersionUID(1L)


I'm not sure why we add an annotation and the silencer to prevent the compiler to say that it's not going to work, any help @raboof @johanandren ?

Both the concrete subclasses have their own @serialversionuid, I think it is completely fine to remove here as it doesn't do anything anyway?

andreaTP · 2021-01-05T09:37:39Z

akka-actor/src/main/scala/akka/actor/LightArrayRevolverScheduler.scala

@@ -128,7 +128,10 @@ class LightArrayRevolverScheduler(config: Config, log: LoggingAdapter, threadFac
        }
      }

-      @tailrec final def cancel(): Boolean = {
+      // TODO DOTTY
+      // Cannot rewrite recursive call: it targets a supertype


I'm puzzled by this error message, is it the expected behavior @nicolasstucki, @smarter ?

The error message refers toc.cancel() which is an overridden method of the current cancel() method, dotty correctly tells you that it cannot rewrite it, scala 2 doesn't say anything here, it's not a bug, just a difference of what @tailrec checks.

done in #29939

andreaTP · 2021-01-05T09:41:18Z

akka-actor/src/main/scala/akka/actor/RepointableActorRef.scala

-  @silent @volatile private var _cellDoNotCallMeDirectly: Cell = _
-  @silent @volatile private var _lookupDoNotCallMeDirectly: Cell = _
+  // TODO DOTTY
+  @silent @volatile protected var _cellDoNotCallMeDirectly: Cell = _


Here is a real "issue", e.g. something that is ok for a POC but is not going to be merged on master.

My understanding is that private unused variables are erased by the Scala 3 compiler, is there a way to preserve them(with an annotation or something) since in this case they are used entirely through Unsafe?

cc. @nicolasstucki @smarter

Make a def: ohze@26394d4

done in #29930

andreaTP · 2021-01-05T09:43:33Z

akka-actor/src/main/scala/akka/dispatch/AbstractDispatcher.scala

-final case class Envelope private (message: Any, sender: ActorRef)
+final case class Envelope private (message: Any, sender: ActorRef) {
+  // TODO DOTTY -> bug? copy method becomes private with private constructor
+  def copy(message: Any, sender: ActorRef) = Envelope(message, sender)


In Dotty the copy method of case classes have the same visibility as the constructor AFAIU.
Is it a deliberate change from Scala 2 to Scala 3 or is it accidental? @nicolasstucki @smarter

see scala/scala3#5472

Thanks @SethTisue ! Means we have to declare them explicitly.

andreaTP · 2021-01-05T09:47:27Z

akka-actor/src/main/scala/akka/event/ActorClassificationUnsubscriber.scala

@@ -15,7 +15,8 @@ import akka.util.unused
 *
 * Watches all actors which subscribe on the given event stream, and unsubscribes them from it when they are Terminated.
 */
-protected[akka] class ActorClassificationUnsubscriber(bus: ManagedActorClassification, debug: Boolean)
+// TODO DOTTY -> Something wrong is happening in the compiler
+protected[akka] class ActorClassificationUnsubscriber(bus: String, unsubscribe: ActorRef => Unit, debug: Boolean)


I'm not sure why, but the compiler go completely nuts on this detail, the pattern is something like:
file1.scala

class A { val b = new B(this) def foo() = {} }

file2.scala

class B(a: A) { def bar() = { a.foo() } }

@nicolasstucki @smarter

This is not enough details for us to say anything (what does "completely nuts" mean?).

The compiler rejects the build saying that one of the class file is missing.
It builds although using incremental compilation by removing dependencies on the other compilation unit and re-adding them back.
I can try to to provide a branch as a repro, sorry for the lack of details.

andreaTP · 2021-01-05T09:50:04Z

akka-stream/src/main/boilerplate/akka/stream/javadsl/GraphCreate.scala.template

      block: function.Function2[GraphDSL.Builder[M], S1, S]): Graph[S, M] =
-    scaladsl.GraphDSL.create(g1) { b => s => block.apply(b.asJava, s) }
+    scaladsl.GraphDSL.create(g1) { b => (s: Shape @uncheckedVariance) => block.apply(b.asJava, s.asInstanceOf[S1]) }


this solves a bunch of errors but is probably wrong

In general, when Scala 3 requires @uncheckedVariance where Scala 2 doesn't, it's entirely plausible that the Scala 2 compiler was wrongly accepting unsound code. (But that's in general; I haven't dug into this particular case.)

I think the original sin here is the definition of Shape with @uncheckedVariance:

akka/akka-stream/src/main/scala/akka/stream/Graph.scala

Line 23 in e8e62e2

type Shape = S @uncheckedVariance

This breaks soundness as demonstrated by https://scastie.scala-lang.org/smarter/Cno5sLZyTSybDC80uDt2hQ, meaning that we can't really guarantee anything about type-safety in akka.

andreaTP · 2021-01-05T09:51:12Z

akka-testkit/src/main/scala/akka/testkit/TestActorRef.scala

@@ -23,7 +23,22 @@ import akka.pattern.ask
 * @since 1.1
 */
 @silent // 'early initializers' are deprecated on 2.13 and will be replaced with trait parameters on 2.14. https://github.com/akka/akka/issues/26753


Should we move this implementation to the scala-3 folder and use proper trait parameters in Scala 3?
@raboof @johanandren

If that works with minimal changes it sound fine by me

andreaTP · 2021-01-05T09:53:39Z

project/AkkaDisciplinePlugin.scala

@@ -64,10 +64,14 @@ object AkkaDisciplinePlugin extends AutoPlugin {
    "akka-testkit")

  lazy val silencerSettings = {
-    val silencerVersion = "1.7.1"
+    val silencerVersion = "1.5-SNAPSHOT"
+    // TODO: FIXME DOTTY (using a locally published version of silencer...)


I simply published locally an "empty version" of the silencer, any plan to port it to Scala 3 already on-going @ghik ?

Does Scala 3 support @nowarn? I would expect that it does. All @silent annotations should be migrated to @nowarn - silencer would then be necessary only in Scala 2.12 (it understands @nowarn to some extent).

Thanks @ghik for the answer!
The question then goes to the Akka Team ( @johanandren @raboof ): can we migrate all the annotations to @nowarn in the entire code-base, and tweak compiler flags to be more lenient in Scala 2.12?
Or, we can define a local annotation @nowarn in the scala-212 folder that simply rebind silence (have to test this option).

@nowarn is already available for Scala 2.12 in scala-collection-compat. It is partially interpreted by silencer.

@nowarn will be in Scala 2.12.13 (scala/scala#9248), so I think it'd make sense to wait for that.

Do we need to make our flags more lenient in order to move to @nowarn?

Do we need to make our flags more lenient in order to move to @nowarn?

Don't know, will test!

@nowarn is not implemented in Scala 3 yet: scala/scala3#8908

andreaTP · 2021-01-05T09:54:54Z

project/Dependencies.scala


    val lmdb = "org.lmdbjava" % "lmdbjava" % "0.7.0" // ApacheV2, OpenLDAP Public License

    val junit = "junit" % "junit" % junitVersion // Common Public License 1.0

    // For Java 8 Conversions
-    val java8Compat = Def.setting { "org.scala-lang.modules" %% "scala-java8-compat" % java8CompatVersion.value } // Scala License
+    // TODO DOTTY
+    val java8Compat = Def.setting { DottyCompatModuleID("org.scala-lang.modules" %% "scala-java8-compat" % java8CompatVersion.value).withDottyCompat(getVersion()) } // Scala License


This is a library that probably we want to, eventually, cross-compile as well for Scala 3, cc. @SethTisue

yeah, scala/scala-java8-compat#200 ... we're hoping someone will volunteer to tackle it. if you have any problems using it with withDottyCompat, let me know, I would consider that high priority to investigate

smarter · 2021-01-05T12:53:50Z

project/Dependencies.scala

-      val scalatest = "org.scalatest" %% "scalatest" % scalaTestVersion % "test" // ApacheV2
-      val scalacheck = "org.scalacheck" %% "scalacheck" % scalaCheckVersion % "test" // New BSD
+      // TODO DOTTY
+      val scalatest = DottyCompatModuleID("org.scalatest" %% "scalatest" % scalaTestVersion % "test").withDottyCompat(getVersion()) // ApacheV2


All your problems related to scalatest stem from the fact that you're using the Scala 2 version of scalatest, so none of the macros are usable. You should drop the withDottyCompat for all dependencies which are published for Scala 3, this includes scalatest: https://repo1.maven.org/maven2/org/scalatest/scalatest_3.0.0-M3/

Amazing didn't know about it! Thanks a lot @smarter !
Unfortunately, Scala 3 is not listed by Maven central:
https://mvnrepository.com/artifact/org.scalatest/scalatest

How do we know if a library is available or we should use the compatibility mode?

scaladex can help: https://index.scala-lang.org/scalatest/scalatest/scalatest/3.2.3 (see the "version matrix" button on the right), or just browse https://repo1.maven.org/maven2 manually.

Is Scala 3.0.0-M3 still missing in Scaladex?
And, found that, probably, this is a good starting point:
https://index.scala-lang.org/search?q=depends-on:lampepfl/scala3-library&page=1

done in #29930

smarter · 2021-01-05T20:24:12Z

FYI, there's a wip dotty branch for akka in https://github.com/ohze/akka/commits/dotty/wip by @ohze / @giabao, you might want to collaborate.

patriknw · 2021-01-25T12:09:05Z

Superseded by #29956, I assume

andreaTP added 6 commits December 29, 2020 19:47

Testkit is passing

31224e7

more

b30b450

Actor tests running

155d1d7

one more

9d19992

590 errors to stream

2fe61c8

down to ~50 in stream but dsl is broken, possibly

0f31adc

andreaTP commented Jan 5, 2021

View reviewed changes

akka-actor/src/main/scala/akka/actor/ActorPath.scala Show resolved Hide resolved

andreaTP commented Jan 5, 2021

View reviewed changes

smarter reviewed Jan 5, 2021

View reviewed changes

SethTisue mentioned this pull request Jan 5, 2021

dotty build #28835

Closed

This was referenced Jan 6, 2021

Scala3 support patch 1 #29930

Closed

Move to nowarn instead of silencer #29932

Closed

Explicit bang import #29938

Closed

Scala 3 compatible tailrec #29939

Closed

andreaTP mentioned this pull request Jan 17, 2021

Scala 3 cross compatible akka-actor / akka-testkit / akka-actor-tests #29956

Merged

patriknw closed this Jan 25, 2021

[WIP] Cross-compile Akka with Scala 3 #29929

[WIP] Cross-compile Akka with Scala 3 #29929

Conversation

andreaTP commented Jan 5, 2021

akka-ci commented Jan 5, 2021

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

andreaTP Jan 5, 2021 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

andreaTP Jan 5, 2021 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

andreaTP Jan 5, 2021 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

SethTisue Jan 5, 2021 • edited

Choose a reason for hiding this comment

smarter Jan 5, 2021 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

andreaTP Jan 5, 2021 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

smarter commented Jan 5, 2021

patriknw commented Jan 25, 2021

andreaTP Jan 5, 2021 •

edited

andreaTP Jan 5, 2021 •

edited

andreaTP Jan 5, 2021 •

edited

SethTisue Jan 5, 2021 •

edited

smarter Jan 5, 2021 •

edited

andreaTP Jan 5, 2021 •

edited