Closed felixbr closed 2 years ago
I've thought a bit more about what we can do with all the code using Manifest
:
Manifest
has been deprecated (and later "temporarily" undeprecated) a while ago and split into ClassTag
and TypeTag
. ClassTag
is also available in Scala3 but it only gives you information that isn't erased at runtime. TypeTag
can give you full type information (e.g. also type-parameters) but it is not available in Scala3.
For cases where Manifest
was only used to get access to the runtime class (via classTag[T].runtimeClass
) we can probably just switch to ClassTag
and don't lose any safety or features.
For cases where "deeper" type information is extracted, we have to find a different solution because TypeTag
is not available. First we will have to collect all of those cases and then we have to discuss what kind of breaking changes are tolerable.
My main goal is to unblock twitter/twitter-server
and twitter/finagle
for Scala3 migrations, so technically I only need the modules migrated which are used there. I haven't checked yet but I suspect the answer is "almost everything and especially the reflection stuff" 😅
Thanks, this is great! Just as a heads up, we will probably want to merge this stuff in incrementally, rather than as a big-bang patch, so that we can review it carefully (especially where we have to break APIs w/ Manifest / TypeTag). However, I think keeping a bunch of ideas on a single big branch is also good. We'll just need to break it up later.
One other thing is that we're considering having a couple of interns take a stab at some of this work too, so we may be able to put together a crew to work on this.
Re: Manifest
/ ClassTag
, I guess we have to switch to ClassTag
? What's the alternative, use Java Class
types?
We have something similar to twitter/util
internally and for the 2.12->2.13 migration we set up the sbt build with root
and root2_13
as aggregations, while adding migrated modules incrementally to root2_13
.
I could imagine a similar strategy here as it allows us to migrate modules one-by-one, split up work between people, and maybe even cross-publish the already migrated subset (not all modules might be needed for downstream projects).
Once #288 is merged, I can try and open a PR for just the cross-build setup and we go from there.
I'll leave this PR up for now if you don't mind, as the diff might be useful to others working on it just like I used the previous work by @martijnhoekstra .
Re: Manifest / ClassTag, I guess we have to switch to ClassTag? What's the alternative, use Java Class types?
Scala 3 will only have ClassTag
for now (no plans for TypeTag
yet). Alternatives for Manifest
depend on what it's currently used for. Passing the Class
explicitly might work but I'm not sure about erasure of type-parameters. The safest route is not using reflection at all and using typeclasses (or explicit parser-combinators) but that's probably also the biggest API change.
We will have to test how libraries like Jackson work with type-parameters. For example if something like decodeJson(classOf[Map[String, Foo]])
could work despite erasure.
Thank you so much for looking into this! I was imagining that if there wasn't a way to create binary compatible I did some work to tease apart what could be done in parallel, I'll add the table below. I suspected we'd have to have some 2.13 and 3 specific code for the cross-build until we can figure out blockers.
The information you provided about ClassTags is really useful, thank you!
Layer |
Stream 1 | Stream 2 | |
---|---|---|---|
Layer 1 | util-core is a dependency for everything else | util-core done together (paired work) | |
Layer 2 | Things that only depend on util-core / layer 1 | util-lint, util-registry, util-app-lifecycle, util-hashing | util-mock, util-reflect, util-cache, util-codec |
Layer 3 | Things that depend on layer 2 | util-app, util-stats, util-zk-test | util-thrift, util-cache-guava, util-slf4j-api |
Layer 4 | Things that depend on layer 3 and above | util-jvm, util-logging, util-tunable, | util-slf4j-jul-bridge, util-validator, util-routing |
Layer 5 | Things that depend on layer 4 and above | util-security, util-zk, util-test | util-jackson |
Layer 6 | Things that depend on layer 5 and above | util-benchmark |
I've opened a Scala3 bug regarding Java-API not being able to call overridden trait-methods (e.g. Duration.fromSeconds(1);
): https://github.com/lampepfl/dotty/issues/12753
Scala 3 will only have ClassTag for now (no plans for TypeTag yet).
To be clear: TypeTag and everything else from scala-reflect will never come back in Scala 3, Scala 3 has its own meta-programming mechanisms detailed in https://dotty.epfl.ch/docs/reference/metaprogramming/toc.html. But depending on what you're using TypeTag for you might not need any of that and could either use ClassTag (if all you need is a convenient way to get the erased class of a type) or the new TypeTest mechanism: http://dotty.epfl.ch/docs/reference/other-new-features/type-test.html
@smarter Thanks for the clarification. It wasn't my intention to imply TypeTag could potentially come back, I worded that poorly.
I didn't know about TypeTest
, though (there is so much to read up on for Scala 3) 👍
I just confirmed that in Scala 3.0.2 the compile errors for the Java-API will be fixed. 🙂
(I tried 3.0.2-RC1-bin-20210621-4aa7f90-NIGHTLY
and it works)
I rebased the changes onto the last commit before the (twitter-internal?) Scala3 migration by @hamdiallam as there's a bunch of conflicts now.
The Scala 3.0.2-RC1 version works fine, so that's good.
If I can help in any way by splitting things from this PR into a separate PR which can actually be merged into develop
, please let me know 🙂
@felixbr Hey, I realized I dropped the ball on this. We decided to have our interns work on Scala 3.x directly to reduce their overhead so they could make as much progress during their internship as possible. I’m really sorry for not keeping you in the loop earlier! Your work greatly inspired their intern project, so I feel especially bad that I didn’t do a good job of communicating it with you. We’ll work with you to split out the rest of the patches you’ve made so you can finish the job after their internship ends next week. I owe you a beer (or the beverage of your choice), please look me up if you’re ever in NYC!
Hi @felixbr,so sorry we dropped the ball on this! Let's talk about breaking up some of this work so we can start merging it! We like to merge subproject by subproject, so if you take a look at the build.sbt file, you will see that we have two settings: sharedSettings
and sharedScala3EnabledSettings
. sharedSettings
doesn't include the cross-build with scala 3. Could you please start with breaking out the changes in util-logging and enable cross-building with scala 3 in just that subproject?
Sorry for not answering sooner, I was on vacation.
@mosesn No need to apologize. I honestly don't care too much about attribution if omitting it speeds up anything. I just want to use Scala 3 with Finagle, any progress on that is good news to me.
I currently don't have much capacity to work on this migration and since the PR code here now has greatly diverged from develop
, I'd first have to solve that before making meaningful progress. In other words: If you're waiting for me to do the rest, you're probably waiting for quite a while.
@dotordogh The two settings are great, that'll make things easier for sure. 🙂
As mentioned before I don't have much capacity but I will try to take a look at util-logging
tomorrow.
IMPORTANT: This is WIP, I just want to put it up for discussion and to avoid duplication of efforts. There are lots of things that still need to be fixed. I've prefixed them with
// TODO Scala3
so it's easy to find.I took the initial work by @martijnhoekstra and tried to push it as far as possible. Many things work with little or no modification but anything that touches reflection is likely a blocker at this point. There are also one or two things that might be compiler-bugs and we should probably open tickets for the Scala 3 team.
Blockers so far:
Manifest
. In some cases replacing it withClassTag
might be fine but it can be unsafe when used with the wrong types. The alternative would be source-incompatible changes. Both options aren't great.scala-reflect
. I haven't looked into it yet bututil-reflect
needsscala-reflect
and is used by other modules (util-benchmark
,util-validator
)Duration
andTime
is not compiling with Scala3. This could be a compiler-bug. In any case we have to solve it or provide the Java-API separately (which would be a breaking change).util-mock
depends onmockito-scala
which uses macros and doesn't have a Scala3 version. I haven't checked yet if we can port it withoutmockito-scala
macros, but I doubt it will work without breaking changes.A lot of things are already working for both 2.13.6 and 3.0.0. Most importantly
util-core
. I had to replacesun.misc.Unsafe
usage because in Scala3 it caused weird crashes. But withAtomicInteger
andAtomicReference
it works fine and if there are no performance issues, it also makes the code a bit cleaner.There's probably more to be said but I'm hungry and the most important part is discussing the blockers. If anyone wants to collaborate directly on this PR, I'll gladly give you commit-access to the fork.
Cheers ~ Felix