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

Java 8 in CI: the end is nigh #215

Open
armanbilge opened this issue Mar 18, 2022 · 12 comments
Open

Java 8 in CI: the end is nigh #215

armanbilge opened this issue Mar 18, 2022 · 12 comments
Labels

Comments

@armanbilge
Copy link
Member

armanbilge commented Mar 18, 2022

Maybe not tomorrow or even this year, but this stuckfast position will gradually grow more and more uncomfortable. E.g. our sbt-plugins are itching to leave us behind:

FWIW we can probably get along just fine without JDK 8 in CI. But perhaps I haven't heard the horror stories 😆

  1. The tlJdkRelease := Some(8) setting activates all the scalac and javac flags to safely build/release from a newer JDK.
  2. fs2 only runs CI on JDK 17 and besides the occasional bump is really a success story. Note that the tlJdkRelease setting would actually prevent those ByteBuffer-oopsies. In fact fs2 can't use the tlJdkRelease setting because it blocks access to JDK 9+ APIs!
@rossabaker
Copy link
Member

rossabaker commented Apr 2, 2022

Java 8 Active Support ended on March 31, which is historically the event where I become less of an curmudgeon about maintaining old things.

@armanbilge
Copy link
Member Author

Politely pinging @vasilmkd, do you have any opinions on this? Things are still a bit new with the tlJdkRelease flag on Scala 3 but actually so far it seems to be doing better than vanilla JDK 8 🤣

@vasilmkd
Copy link
Member

vasilmkd commented Apr 2, 2022

I wouldn't completely drop it in Cats Effect (and we would set that as a special case), but as a default, I think we should be nudging users towards the future and make JDK 17 the default.

@armanbilge armanbilge added this to the v0.5.0 milestone Apr 2, 2022
@armanbilge
Copy link
Member Author

Yup, this is definitely where things are headed. /cc @valencik

In v0.5.0 we'll set tlJdkRelease := 8, switch the default CI JDK to 17, and upgrade the nix flake in the g8 template as well.

@bpholt
Copy link
Member

bpholt commented Jul 5, 2022

FWIW, I think catbird probably ought to stay on Java 8 until Twitter supports Java 11 in twitter/util (twitter/util#266). (In my experience, twitter/util does seem to work on Java 11, but I don't have visibility to what's holding them back internally.)

@armanbilge
Copy link
Member Author

@bpholt thanks, that's helpful input. Projects such as Cats, Cats Effect, and Http4s will almost certainly want to keep the Java 8 builds as well.

So I guess we have to be careful to make sure sbt-typelevel still works on JDK 8, even if we switch the default CI to JDK 17 and set the release flag to Java 8 as proposed here.

We can keep thinking about this as we approach sbt-typelevel v0.5.x.

@armanbilge
Copy link
Member Author

armanbilge commented Nov 4, 2022

Another interesting idea:

So sbt has a setting for running scalac and forked tests on a different JDK than what sbt itself is running on (h/t @keynmol).
https://github.com/sbt/sbt/blob/5532af17c795e0e9981b542810f449035c25172e/main/src/main/scala/sbt/Keys.scala#L290

val javaHome = settingKey[Option[File]]("Selects the Java installation used for compiling and forking.  If None, uses the Java installation running the build.").withRank(ASetting)

If we can figure out how to configure this, then it would mean that our tooling can upgrade to newer JDKs (such as sbt plugins as mentioned in #215 (comment)) while we still keep compiling and testing our libraries on the ancient JDK of our choice.

Edit: I'm actually not sure if this setting is used for scalac. The setting description may be referring to javac. In any case, is the JDK where tests run that's important.

@bpholt
Copy link
Member

bpholt commented Nov 7, 2022

Another thing I just encountered: I don't think setting -release helps catch errors where a dependency is inadvertently updated to a version that requires a higher JDK version than the one being targeted by your project: e.g. logback 1.3 targets Java 8 but logback 1.4 targets Java 11.

I just had a CI run on Java 8 catch the inadvertent update (because tests failed due to the java.lang.UnsupportedClassVersionError). When I ran the tests with Java 11 locally, no error is raised.

If tests can continue to run on the old JDK like @armanbilge suggested that feels like solid middle ground. Or maybe there are other ways to catch this earlier?

@armanbilge
Copy link
Member Author

armanbilge commented Nov 7, 2022

I don't think setting -release helps catch errors where a dependency is inadvertently updated to a version that requires a higher JDK version than the one being targeted by your project

Yes that's right. -release is a compiler flag, so it only affects how your code compiles. It can prevent you from calling certain JDK APIs. But it won't check your dependencies, and it can't stop you from calling APIs in dependencies that need newer JDKs.


I'm currently leaning towards the javaHome idea. I like that it means we can upgrade our build tooling and relieve some of the upstream pressure while still running tests on the JDK of our choice.

It will however take some acrobatics to make this work with the existing download-java and setup-java actions. Basically we need a way to install at least two JDKs: one for sbt, and one for forked tests.

Edit: it would also be extremely useful e.g. for feral, where currently everything runs on an Amazon Coretto JDK which is pretty silly. Only the AWS JVM lambdas "need" that, everything else should use Temurin or whatever.

@armanbilge
Copy link
Member Author

armanbilge commented Jun 18, 2023

I still like the javaHome idea but it's not going to land in 0.5.0. I think we're just going to stick to Java 8 for now.

@armanbilge armanbilge removed this from the v0.5.0 milestone Jun 18, 2023
@keynmol
Copy link

keynmol commented Jun 22, 2023

With regards to java home, you can theoretically use something like this: https://github.com/sourcegraph/scip-java/blob/main/project/JavaToolchainPlugin.scala

I should warn you though that when using SBT BSP with Metals, some java compilation state gets corrupted and a lot of weird artifacts appear.

The concept replicates Gradle's toolchains, and apart from this weirdness works reasonably well.

@armanbilge
Copy link
Member Author

armanbilge commented Nov 7, 2023

mdoc just dropped Java 8 support.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants