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.

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

2.12-ification #505

Closed
33 tasks done
mosesn opened this issue May 18, 2016 · 12 comments
Closed
33 tasks done

2.12-ification #505

mosesn opened this issue May 18, 2016 · 12 comments

Comments

@mosesn
Copy link
Contributor

mosesn commented May 18, 2016

In order to support 2.12, we need to add crossScalaVersions to Build.scala, and make sure everything compiles. Not every scala library will be built against the current snapshot, so we may have to support different versions from 2.11 to 2.12. If part of an API has churned (scalacheck in particular is susceptible to this) we may need to upgrade the 2.11 library version to make progress. If that happens, contact me (@mosesn) and I'll navigate the upgrade for you–it's a little complicated because we will need to upgrade all of our internal libraries, not just finagle. Please note that most of this upgrade will not be feasible until we have a portion of util already upgraded, and we won't be able to do the thrifty bits until scrooge is upgraded too.

  • finagle-benchmark
  • finagle-benchmark-thrift
  • finagle-cacheresolver
  • finagle-commons-stats
  • finagle-core
  • finagle-example
  • finagle-exception
  • finagle-exp
  • finagle-http
  • finagle-http-compat
  • finagle-http-netty4
  • finagle-http2
  • finagle-integration
  • finagle-kestrel
  • finagle-mdns
  • finagle-memcached
  • finagle-mux
  • finagle-mysql
  • finagle-native
  • finagle-netty4
  • finagle-netty4-http
  • finagle-ostrich4
  • finagle-redis
  • finagle-serversets
  • finagle-spdy
  • finagle-stats
  • finagle-stream
  • finagle-stress
  • finagle-test
  • finagle-testers
  • finagle-thrift
  • finagle-thriftmux
  • finagle-zipkin

If you want to claim a module, please let us know in this thread! I'll annotate different modules once they're done.

Probably a good way to start is for someone to try add 2.12 support for all of them, and then report back which ones still fail, so we know which ones are tricky and which ones work trivially.

@taylorleese
Copy link
Contributor

@jeffreyolchovy
Copy link
Contributor

@mosesn let me know if you still want any outside help with this. when i locally hacked around with ostrich to complete my dependencies to attempt a trial 2.12 upgrade for finagle there didn't seem to be many necessary finagle source changes.

off the top of my head, iirc, most things centered around dependencies:

  • upgrade jackson version to one that supports 2.12 (i.e. for jackson scala module)
  • twitter bijection needs to support 2.12 for finagle-memcached

however, the main issue that i noted was compilation of 2.12 barks with an ambiguous reference to overloaded definition for the compose operators on Filter:

[error] both method andThen in class Filter of type (f: Req => com.twitter.util.Future[Rep])Req => com.twitter.util.Future[Rep]
[error] and  method andThen in class Filter of type (service: com.twitter.finagle.Service[Req,Rep])com.twitter.finagle.Service[Req,Rep]
[error] match argument types (com.twitter.finagle.Service[Req,Rep]) and expected result type com.twitter.finagle.Service[Req,Rep]
[error]         service => filters.andThen(service)

hopefully, resolving this one will not need an API change, but I think decisions like this are best left to you guys :)

anyway, this was back at the end of May, so maybe all is moot and a non-issue now, but like i said, let me know if you'd like an outside set of hands on this.

@mosesn
Copy link
Contributor Author

mosesn commented Jul 27, 2016

@jeffreyolchovy yeah, we ran into that too. filed a ticket in the scala issue tracker here: https://issues.scala-lang.org/browse/SI-9871 and it's being worked on here: scala/scala-dev#157

we'd love some help with poking the bijection people to start supporting 2.12–I don't know if any of them are still at Twitter. jackson we're waiting on the same thing as in the ostrich ticket.

good to hear it won't require any source changes in finagle! and thanks so much for digging into this.

@jcrossley
Copy link
Contributor

We've done some further poking of the bijection folks; once that's in I think we'll be able to start 2.12-ification in earnest. @jeffreyolchovy are you still interested?

@jeffreyolchovy
Copy link
Contributor

Sure. Myself and a few other @Tapad engineers would like to contribute toward the effort :)

@jcrossley
Copy link
Contributor

@jeffreyolchovy that's great to hear, thank you! Will let you know once the bijection upgrade lands.

@clhodapp
Copy link

clhodapp commented Dec 1, 2016

Bijection 0.9.3 with support for 2.12 is on maven central.

@jeffreyolchovy
Copy link
Contributor

I'll start revisiting this weekend so that our team members are ready to contribute early next week.

@mosesn
Copy link
Contributor Author

mosesn commented Dec 2, 2016

@jeffreyolchovy bad news, there's a scala bug that's preventing us from upgrading now. https://issues.scala-lang.org/browse/SI-9871

I'm not sure what we can do until that gets resolved. @SethTisue do you know if there's anything we can do to work around this in the mean time?

@jeffreyolchovy
Copy link
Contributor

Ah yeah.. Definitely a blocker unless you want to refactor a bunch of core code...

@mosesn
Copy link
Contributor Author

mosesn commented Dec 21, 2016

Update on this–we ended up having to make a pretty painful breaking change in finagle to work around the scalac backwards incompatibility, since the scala team introduced new semantics in 2.12 that unbreaking our backwards incompatibility would have made backwards incompatible. Subsequently, we ran into several more compiler bugs, so @jcrossley ended up tackling it, and has migrated finagle to 2.12! Yay!! Marking this one as closed. Thanks for your help, @jeffreyolchovy!

@mosesn mosesn closed this as completed Dec 21, 2016
@codefromthecrypt
Copy link
Contributor

codefromthecrypt commented Dec 22, 2016 via email

@luciferous luciferous mentioned this issue Jul 16, 2019
28 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Development

No branches or pull requests

6 participants