Use release 8 scalac flag to allow building on JDK11+ #187
Conversation
mdedetrich
requested review from
jrudolph,
gmethvin,
He-Pin,
nvollmar,
seglo and
pjfanning
|
This change shouldn't have any effect.
|
Those changes definitely have an effect if you build with JDK 11, they allow you to build with JDK 11 and produce completely compatible JKD 8 bytecode. With JDK 8 there is no effect, it's detailed in the PR. |
| "-Wconf:msg=reached max recursion depth:s") ++ | ||
| (if (isJdk8) Seq.empty | ||
| else if (scalaBinaryVersion.value == "2.12") Seq("-target:jvm-1.8") | ||
| else Seq("-release", "8")), |
There was a problem hiding this comment.
Here -release 8 was added before. Are you saying that it makes a difference whether -release:8 or -release 8 is used?
There was a problem hiding this comment.
Yes the flag was actually updated and improved, the previously linked PR provides all of the details.
|
I see, it only makes a difference for Scala 2.12 where this PR moves from |
This PR is the equivalent of apache/pekko#258 but for pekko-http. The context behind these changes are complicated (read the linked pekko PR for more detail) but long story short the
releasescalac flag for Scala 2 changed at some point. The original version ofreleaseflag (which the code currently in pekko-http was written for) only makes scalac produce JDK 8 bytecode but it ignores the JDK 8 runtimert.jar. This is almost never what you want, so the same flag was updated in later versions of Scala (see scala/scala#9982) which also brings in the correctrt.jar, i.e. if you build a project using JDK 11 with the-release:8flag then not only will it produce JDK8 bytecode but it will also bring in JDK 8'srt.jar.That part about bringing in the
rt.jar8 jar is critical because, because there was a change from JDK 8 to JDK9's stdlib where if you compile a codebase with JDK9+, because of a newly added overloaded method that is only available in JDK9+ the produced bytecode will reference that new method rather than the JDK8 one. This is what was documented in https://github.com/apache/incubator-pekko/pull/258/files#diff-c11a10bd9e773399e4272cd1ee5dff72545afb2f8bed23f2d1bbc49d4e1fc03fL61-L66.tl;dr Since we are using the most modern versions of scalac, we can just use
release:8and it handles the corner cases described above. pekko-core had code in sbt to prevent building without an available JDK 8, pekko-http had the same core issue with the deprecated flag but it seems that this same check wasn't added.To verify that this PR does in fact produce the same bytecode as if the project was compiled under JDK 8 https://github.com/lightbend-labs/jardiff was used. Its fairly easy to verify this yourself, clone incubator-pekko into a new directory (lets call it
incubator-pekko-2) and compile it under JDK 8 usingsbt ++compile. Then compile this branch/PR in anincubator-pekkofolder using JDK 11 and create a new folder calleddiff-temp. Then you can just runWhich creates a git repository in
diff-tempshowing the bytecode diff. Using a git diff viewer (I use Intellij) you get the followingAs you can see while there are some difference its not anything of consequence (i.e.
Versionhas changed but thats because the project version which is derived from the git has changed).If we however run the jardiff tool without the changes in this PR/branch we get the following
Notice how we now have differences in
HttpHeaderParser/CustomCharsetByteStringRenderer, and if we view what the difference is we getWhich is precisely the problem talked about earlier.