Cross-build Scala 2.13.0

[yufangong]

Scala 2.13 is released and finagle and twitter-server is close to finishing cross-building 2.13. It would be great to have finatra prepare for enabling cross-building 2.13, and @blueway first opened an issue to ask for 2.13 support. We want to do a similar fashion to twitter/finagle#771.

Library dependencies: Twitter-Server, Finagle

Subprojects: They are ordered by dependency depth, the first layer (httpAnnotations and injectSlf4j) should have the least dependencies. Note each subproject may have a TestJarSources with them that can be upgraded at the same time (for example: httpclient and httpclientTestJarSources.

If you start working on one of these subprojects, please add a comment on this thread and I'll add your name next to it so people know not to duplicate your work.

---

• httpAnnotations (@chrisbenincasa)
• injectSlf4j (@chrisbenincasa)

---

• injectCore (@chrisbenincasa)

---

• injectModules (@chrisbenincasa)
• injectUtils (@chrisbenincasa)
• injectPorts (@chrisbenincasa)

---

• injectApp (@chrisbenincasa)
• injectThrift (@chrisbenincasa)
• injectDtab (@sideshowcoder)

---

• thrift (@chrisbenincasa)
• injectRequestScope (@chrisbenincasa)
• injectServer (@chrisbenincasa)

---

• utils (@@chrisbenincasa, @sideshowcoder)
• httpclient (@chrisbenincasa)

---

• validation (@chrisbenincasa)
• http (@chrisbenincasa)
• mustache (@sideshowcoder)
• kafka (@yufangong)
• kafkaStreamsQueryableThriftClient (@sideshowcoder)

---

• jackson (@chrisbenincasa)
• injectThriftClient (@sideshowcoder)
• injectLogback (@sideshowcoder)
• httpMustache(@sideshowcoder)
• kafkaStreams(@yufangong)
• kafkaStreamsStaticPartitioning (@yufangong)

---

• benchmarks(@sideshowcoder)
• injectThriftClientHttpMapper(@sideshowcoder)
• kafkaStreamsPrerestore (@yufangong)
• kafkaStreamsQueryableThrift (@yufangong)

---

finatraExamples (@yufangong)

[chrisbenincasa]

I believe all of Finagle is properly cross built for 2.13 now. I think I should have some time to help out here. I've just opened 3 dependent PRs:

• upgrade test libraries to versions compatible with cross-building for 2.13 (root: Upgrade test library versions in preparation for 2.13 support #527)
• cross-building httpAnnotations (and the dependent jsonAnnotations) (httpAnnotations/jsonAnnotations: Cross-build for 2.13 #528)
• cross-building inject-slf4j (inject-slf4j: Cross-build for Scala 2.13 #529)

I can start taking a look at injectCore as well too, to help unblock other submodules.

EDIT:
I've also opened up PRs for inject-core (#530), inject-utils (#532), inject-modules (#531), and inject-ports (#533).

[chrisbenincasa]

Also of note: some core test library dependencies must have their versions bumped in order to make the entire repo eligible for cross-building to 2.13. It seems like this will be the biggest hurdle since I imagine that these versions must be updated on the internal monorepo version. However, until those versions are updated (see #527), it seems that none of the other cross-building changes will be able to be merged in.

[yufangong]

@chrisbenincasa Thanks a lot! We will soon get them reviewed!

[ihasfrozen]

I'd be happy to take a shot at validation if it's next in line!

[cacoco]

Note that we use net.codingwell#scala-guice and we are currently on version 4.2.0. This is a central library and we need to get to at least version 4.2.5 which supports Scala 2.13. Unfortunately the library switches from using Manifests to TypeTags for building Guice Key types from a TypeLiteral which is a breaking change for Finatra. We are currently working on upgrading this library (along with the other libraries which need to move in order to be able to cross compile for Scala 2.13). Thanks!

[chrisbenincasa]

@cacoco thanks for the update!

[chrisbenincasa]

I think validation will have to moved to the utils level, since validation seems to depend on utils.

Additionally, jackson depends on both validation and utils so I believe that'll have to be regrouped as well.

[yufangong]

I think validation will have to moved to the utils level, since validation seems to depend on utils.

Additionally, jackson depends on both validation and utils so I believe that'll have to be regrouped as well.

Updated accordingly, thanks!

[chrisbenincasa]

I'm starting to look at converting some of the heavier modules (where performance is likely a concern) and am bumping up against the concerns posed in twitter/finagle#818 . There is a lot of usages of ArrayBuffer and other mutable Seqs that have breaking changes in 2.13 and where the recommended solutions could have a performance impact. I'm wondering if any thought has gone into this internally

[chrisbenincasa]

@yufangong I'm hitting an unusual issue while attempting to convert http to the new version. Here is a gist of the compiler output:

https://gist.github.com/chrisbenincasa/ae5f6816d7c0394792c1607c1d208730

What's interesting is that javax.servlet is no longer a dependency on the project (I see it was removed over a year ago). http is compiling just fine on 2.11 and 2.12 with no changes. I've been digging around to see why 2.13 would affect this (and have double-checked various things, like my Java version, etc). Do you have any ideas?

[chrisbenincasa]

I was able to track this down a bit more... it seems like there is a transitive dependency on javax.servlet-api from apache commons, which is used here:

finatra/http/src/main/scala/com/twitter/finatra/http/fileupload/FinagleRequestFileUpload.scala

Line 8 in 39fd964

class FinagleRequestFileUpload extends FileUploadBase {

This dep is marked as provided in apache commons-fileupload: https://github.com/apache/commons-fileupload/blob/commons-fileupload-1.4/pom.xml#L234-L239

I'm unclear on why this ends up throwing when compiling 2.13 only, however.

[yufangong]

@chrisbenincasa That's odd, I have no clue... I found a similar issue at sbt/sbt#2958, it might be worth trying the suggested workarounds/tests about coursier/clean-up jars and consider filing an sbt issue if it is reproduced consistently.

[chrisbenincasa]

@yufangong interestingly, bumping from 2.13.1 to 2.13.2 resolves the problem. Perhaps we should target that version?

[yufangong]

I'm starting to look at converting some of the heavier modules (where performance is likely a concern) and am bumping up against the concerns posed in twitter/finagle#818 . There is a lot of usages of ArrayBuffer and other mutable Seqs that have breaking changes in 2.13 and where the recommended solutions could have a performance impact. I'm wondering if any thought has gone into this internally

Thanks for bringing this up! At this point, 2.12 performance has a more significant impact, so I think compromising 2.13 perf is acceptable. Moving forward, we will evaluate the plausible solutions and any other proposed solution for both 2.12/2.13. There are a few benchmarks sitting in finatra/benchmarks for http/jackson/validation that could be leveraged.

[cacoco]

@chrisbenincasa RE: javax.servlet-api from apache commons being marked as provided in apache commons-fileupload: https://github.com/apache/commons-fileupload/blob/commons-fileupload-1.4/pom.xml#L234-L239. It is confusing why this would work at all with sby/ivy. We should likely declare the dependency explicitly in finatra/http for correctness. Provided is intended to signal that it is required to compile but that you need to bring this dependency (or a binary compatible version) yourself -- it was likely a mistake for us to remove the javax.servlet-api dependency in this case.

[mosesn]

@chrisbenincasa just a heads up since your name is next to "utils"–someone is looking at "utils" internally too, have you already tackled it? Might be good to hold off if you haven't started yet

[chrisbenincasa]

@mosesn yes, #535 converts utils

[mosesn]

@chrisbenincasa ack, we'll work something out so that the two diffs don't collide.

[chrisbenincasa]

@mosesn sounds good. FWIW, the change there simply adds the cross version. Not sure if the internal changes are meant as rewrites to use features from 2.13, but there shouldn't be any overlap since I'm simply trying to get versions published for 2.13 for immediate use.

[sideshowcoder]

Hey @chrisbenincasa I'm making the same change to utils as #535 to get the cross build, plus resolving some warnings which came with 2.13, the line in build.sbt is the same and as far as I can see #535 only makes that change to utils, so I don't think there should be a conflict there.

[chrisbenincasa]

Sounds good!

[sideshowcoder]

Took a look through the build.sbt to check what the current dependencies look like, maybe helpful for others looking into this. If people don't mind I would tackle kafkaStreamsQueryableThriftClient next as it is currently not blocked by anything.

Unblocked

• injectThriftClient
• injectLogback
• httpMustache
• injectThriftClientHttpMapper
• benchmarks
• kafka (Cross-build Scala 2.13.0 #522 (comment))

Blocked

• kafkaStreamsPrerestore | kafkaStreamsQueryableThrift
  • kafkaStreamsStaticPartitioning
    • kafkaStreams
      • kafka

[yufangong]

Thank you @sideshowcoder! You got it.

[chrisbenincasa]

Awesome!

I started looking at Kafka, this may be another dependency issue. It seems the 2.4.0 release was the first to support Scala 2.13. Finatra is currently on 2.2.0. Do we have to wait for an internal repo version bump?

[yufangong]

Awesome!

I started looking at Kafka, this may be another dependency issue. It seems the 2.4.0 release was the first to support Scala 2.13. Finatra is currently on 2.2.0. Do we have to wait for an internal repo version bump?

It looks like some internal work started to upgrade kafka: 3c78c34, a skim of that I assume we will need to have scala 2.12/2.13 diverged in kafka projects, I will follow up internally and let you know. Thanks!

[sideshowcoder]

injectDtab was missing from the list as I think it was created after this issue was first opened, it builds fine so I'm going add the cross build setting.

[mosesn]

Thanks @sideshowcoder! added it to the list at the top

[sideshowcoder]

@yufangong I had a look and it looks like injectThriftClient, injectLogback, httpMustache, injectThriftClientHttpMapper, and benchmarks all build fine agains 2.13.1. There are a bunch of deprecation warnings so which are probably worth fixing. Mind if I take those on? I see that kafka is already worked on.

[yufangong]

Nice! Thank you @sideshowcoder. Added to the items.

[chrisbenincasa]

@yufangong do you have an update on the internal work with Kafka? Wondering if the remaining modules listed above are ready to be converted or not.

[yufangong]

@chrisbenincasa Sorry! I haven't look into it. I'm planning to allocate sometime next week to evaluate the work.

[yufangong]

Hey @chrisbenincasa, I have some updates:

The finatra/kafka targets are a bit complicated regards build, after talking with the supporting team we think currently the most efficient way is to fork the scala 2.13/2.12 on the finatra-kafka-stream projects.
Forcing 2.13 to use kafka 2.5 and 2.12/2.11 to use kafka 2.2.
kafka 2.5 should have all things build up equally supported with 2.2 but as beta, I suspect some hiccups can occur in tests.

The directories are already split into kafka-2.2 and kafka-2.5 within the related packages.

We should probably weigh in more opinions/suggestions before totally settle with this solution, in the meantime, feel free to poke around, first-hand experience will be great!

[mucahitkantepe]

If it was a blocker, Kafka is published for Scala 2.13 via 2.7.0

[cacoco]

Thanks to everyone for your help (especially @yufangong, @chrisbenincasa, and @sideshowcoder) we're now fully cross-building with Scala 2.13 as of 7deb115! ✅

The upcoming February release should be cross-published with Scala 2.13.

[chrisbenincasa]

Woohoo!

[cacoco]

2.13 artifacts now published with release 21.2.0: https://repo1.maven.org/maven2/com/twitter/

[sideshowcoder]

AWESOME! 🎉