Experience of migrating to 2.13.x

#1

We are trying to migrate from Scala 2.12.8 to 2.13.0-RC1 for several twitter libraries, util/scrooge/finagle/twitter-server/finatra, currently finished util.
(Moses did the last experiment to try out 2.13.0-M4 for util here before.) We would like to share some feedback about this time migration:

  1. CanBuild[A, C] has been replaced by Factory[A, C] + BuildFrom[From, A, C].

We have troubles to construct a Builder[A, Seq[A]] from scratch. If we use Factory, there’s no implicit SeqFactory can be summoned but only ArrayFactory, however, we did notice there is an implicit iterableFactory, but failed to call it. If we use BuildFrom, it seems BuildFrom cannot build from Nothing anymore.
e.g.,

  1. In mutable.Queue, +=/+=: became final, also enqueue method does not accept varargs. Can we add the backward compatible APIs here?

  2. Seq.unapply() cannot infer types anymore; users need to add annotations, is this intentional?
    https://github.com/twitter/util/compare/develop...yufangong:yg-oss/scala-2.13#diff-4d3d63d2f0fe2cab12515cdd7503022fL63

  3. We have many many many toSeqs need to be added in order to compile due to the new collection lib change. We could change our library APIs to take immutable Seq, but it won’t help a lot for our users’ existing code. Does it make sense to have some implicit methods in the collection lib to do the transform?

  4. scala-collection-compat migration tool seems a bit limited at this time; it fixes most breakOut changes and JavaConverters migration. But to make our code compile with 2.13.0-RC1, we still need manual work to transform between the collection.Seq with immutable.Seq, adding many toSeq to the ArrayBuffer and other structures used to implement Seq.

Migration tool rewrites:

We would like to hear more insights about these API changes, and suggestions to help us migrate in the future. And we will continue this thread to add more observations when finish migrating other libraries.

3 Likes
#2

If you want minimum adjustments, and most of your problems seem to relate to scala.Seq, the easiest is to replace Seq with scala.collection.Seq, because then it means the same in Scala 2.12 and 2.13.

By the way, since you mention 2.13.0-RC1 – you should probably move already on to 2.13.0-RC3!

1 Like
#3

IMO many toSeq's should be dealt with differently. twitter/util uses ArrayBuffers in many places as a mutable builder that is then returned as a Seq. Using a builder of an immutable collection and returning the result, using primitives that already use a builder under the hood, or wrapping a mutable structure in an immutable facade is IMO in many places preferable over using toSeq.

A big issue I encountered with the builder infrastructure is that some of it currently depends on undefined behaviour (see this comment of my attempt to port back in april, or rely on the return type of the builder in scope happening to be a Seq (see this comment of the same port attempt)

1 Like
#4
  1. The polymorphic factory types for different kinds are not available implicitly. If you want to build a (default) Seq you can just do Seq.newBuilder. If you want to abstract over different collection types, it is better to request an implicit Factory for the proper type (implicitly[Factory[Int, Seq[Int]]]). Abstracting over type constructors like Seq restricts your code to work with collections of a single kind. This was a problem in the old collections library where other kinds like Map did not have the same level of support (e.g. no to[Map], no way to get a MapFactory from a Map).

  2. enqueue should look and behave the same way when used with varargs. If you want to pass an explicit collection object, use enqueueAll. The old version was very inefficient for enqueuing single elements. Keeping a deprecated overload around for another version is unfortunately not possible in this case. Instead of += you need to override addOne. All symbolic methods are final aliases now. Implementing collections is generally not expected to be source compatible. Except in very simple cases you will need separate source files.

  3. Can you open a ticket for this? This is probably a bug caused by the changes in the pattern matcher.

Others have already covered Seq. Using collection.Seq everywhere is the simplest option but not recommended for idiomatic code on 2.13. For this reason there is no automatic migration.

#5

I think the problem here is the same as the other Seq issues. Seq is now immutable.Seq and schedules is a mutable.Buffer[(Runnable, Long, Long, TimeUnit)].

#6

I’ve brought the other port from RC1 to RC3, and it’s now green in CI.

An issue that I have no idea where to start looking for the root cause is that some tests stop compiling with an error similar to the following. Pulling the test body into a separate method avoids the error, but I don’t understand why

java.lang.IllegalArgumentException: Could not find proxy for val rw: com.twitter.io.Pipe in List(value rw, method $anonfun$new$15, value , class PipeTest, package io, package twitter, package com, package ) (currentOwner= value stabilizer$2 )

full stack trace at https://gist.github.com/martijnhoekstra/4fe3bd2fbeefceb3b29ee1366aac5c89

#7

value stabilizer$2 suggests that there’s some unexpected feature interaction with [edit: by-name implicits (https://github.com/scala/scala/pull/6050)] https://github.com/scala/scala/pull/5999. If you have a step-by-step procedure to reproduce the issue, please file a bug. Even if it’s not self-contained / minimized, see for example https://github.com/scala/bug/issues/11524#issuecomment-491265486.

#8

will do!

done at https://github.com/scala/bug/issues/11556