error-prone icon indicating copy to clipboard operation
error-prone copied to clipboard

[Discussion] Increasing minimum supported JDK (and dropping support for JDK 11)

Open cushon opened this issue 2 years ago • 28 comments

Error Prone will eventually drop support for running on JDK 11. This bug is intended to collect input from the community on the timing of that decision. There aren't immediate plans to do this, but there are also reason to look forward to using Java > 11 language features in the implementation.

Note that using a newer JDK version to run javac during the build doesn't prevent building code that is deployed to earlier versions, the compiler just has to be configured with --release (or -source/-target/-bootclasspath). For example, it's supported to use the JDK 17 javac and pass --release 8 to compile Java 8 code that is deployed to a JDK 8 runtime.

Questions:

  • What would the next minimum supported version be? (I think a likely answer is 17, since it's the next LTS after 11.)

  • If you're currently running Error Prone on JDK 11, do you have a timeline for upgrading to a newer version?

cushon avatar Feb 28 '23 16:02 cushon

Great topic. Some n=1 feedback: all Picnic-internal code targets JDK 17. For Error Prone Support we kept the baseline at JDK 11, but that does impede the use of text blocks in unit tests which is a shame, because they would be a great quality of life improvement i.c.w. CompilationTestHelper and BugCheckerRefactoringTestHelper (PicnicSupermarket/error-prone-support#198). (Given Error Prone's reliance on JDK internals we do want to run the tests against all supported JDKs, so testing only on JDK 15+ doesn't seem appropriate.)

Stephan202 avatar Feb 28 '23 17:02 Stephan202

Dropping JDK 11 in 2023 would be premature. Where I work, we have not migrated everything off of JDK 8 yet. Any place that has a large codebase to maintain, especially libraries that are still in use by JDK 8 / 11 applications, will have a long road ahead of them to get to 17. Getting 95% of apps and libraries to 17 won't happen for us for 2+ years.

RobertTheBlair avatar Mar 01 '23 20:03 RobertTheBlair

Thanks! That sort of input is why I opened the issue. When we eventually do start requiring JDK 17 to run Error Prone I don't want that to be too onerous. It'll still be possible to continue to use older versions of Error Prone to get compatibility with older JDK versions, but we also don't want to do that too soon and cause fragmentation.

One thing to note is that using JDK 17 to run javac during the build doesn't prevent building code that is deployed to JDK 8, the compiler just has to be configured with --release 8 (or -source/-target/-bootclasspath).

At Google we aren't using JDK 8 anymore, and are using the latest javac to build while still supporting deployments to JDK 11.

cushon avatar Mar 01 '23 20:03 cushon

True about older versions. As long as it is a major version update to require min jdk 17, should be clear what to expect.

RobertTheBlair avatar Mar 03 '23 05:03 RobertTheBlair

I think it's too early to drop support for Java 11. Even the upcoming version 3 of the Quarkus framework still has Java 11 as the baseline.

ksilz avatar Mar 14 '23 10:03 ksilz

We use Java 11 extensively and would prefer support is retained until it is out of LTS (2026?).

alexec avatar Aug 15 '23 15:08 alexec

We use Java 11 extensively and would prefer support is retained until it is out of LTS (2026?).

@alexec do you know if using a newer javac version and passing --release 11 to target Java 11 would be an option for you (there's a note about that in https://github.com/google/error-prone/issues/3803#issue-1603438135)? Or is that not something you're likely to do?

cushon avatar Sep 14 '23 15:09 cushon

@cushon using --release doesn't prevent from using Java api of later versions. So, you also need to use --system. I wouldn't say it's convenient for Error Prone users

remal avatar Sep 14 '23 16:09 remal

We have 1000s of applications on Java 11. It's not feasible to migrate them.

alexec avatar Sep 14 '23 16:09 alexec

@cushon using --release doesn't prevent from using Java api of later versions. So, you also need to use --system. I wouldn't say it's convenient for Error Prone users

That is not accurate, --release configures the language level and APIs, passing just --release 11 is equivalent to passing -source 11 -target 11 --system <path to JDK 11>.

(There are some situations --release doesn't support, e.g. I think it still can't be used together with --add-exports=, so --system is necessary for compilations that rely on that.)

We have 1000s of applications on Java 11. It's not feasible to migrate them.

Understood, to be clear that is not what I was asking. It's possible to use a newer version of javac at compile-time and still use an older JDK version in production, javac has support for targeting old JDK versions. E.g.:

$JAVA_17_HOME/bin/javac --release 11 Foo.java
$JAVA_11_HOME/bin/java Foo

cushon avatar Sep 14 '23 16:09 cushon

@cushon have you tried it yourself? I have, and in my case it didn't work without --system. Which is logical, as javac don't know the path of JDK 11. And it doesn't search it.

remal avatar Sep 14 '23 18:09 remal

@remal I'm guessing that you're thinking of (e.g.) -source 8, which has exactly the problem you are describing. --release 8 was introduced largely because of that problem, as discussed in the Motivation section of JEP 247.

cpovirk avatar Sep 14 '23 18:09 cpovirk

@cushon have you tried it yourself?

Yes

in my case it didn't work without --system

Do you have an example of what didn't work?

javac don't know the path of JDK 11. And it doesn't search it.

It doesn't need a JDK 11, recent versions package data on the APIs for different JDK versions to allow cross-compilation.

Demo:

$ javac -fullversion
javac full version "17.0.2+8-86"
$ cat T.java
import java.util.List;

class T {
  List<Integer> xs = List.of(42);
}

Compilation with -source 8 -target 8 succeeds, even though the compilation is using an API that isn't available

$ javac -source 8 -target 8 T.java
warning: [options] bootstrap class path not set in conjunction with -source 8

Compilation with --release 8 fails, because it also configures the corresponding API version

$ javac --release 8 T.java
T.java:4: error: cannot find symbol
  List<Integer> xs = List.of(42);
                         ^
  symbol:   method of(int)
  location: interface List
1 error
$ 

cushon avatar Sep 14 '23 18:09 cushon

@cushon I think as a corner case one might not be 100% safe with --release if code uses JDK-internal APIs? See, e.g., the issues we ran into with Error Prone checkers and internal javac APIs changing in JDK 17 (https://github.com/google/error-prone/issues/4026). Does --release catch these issues as well?

msridhar avatar Sep 14 '23 18:09 msridhar

(There are some situations --release doesn't support, e.g. I think it still can't be used together with --add-exports=, so --system is necessary for compilations that rely on that.)

@cushon I think as a corner case one might not be 100% safe with --release if code uses JDK-internal APIs? See, e.g., the issues we ran into with Error Prone checkers and internal javac APIs changing in JDK 17 (#4026). Does --release catch these issues as well?

--release doesn't allow access to internal APIs, and javac doesn't allow using --add-exports= together with --release to export packages from system modules. So there's no risk of missing issues at compile-time that would fail at runtime, but it does mean code using internal APIs is stuck using -source/-target/--system instead of --release.

cushon avatar Sep 14 '23 18:09 cushon

@cushon, I just checked --release without --system. I have to admit - you're right - it checks the public API. Don't know why it didn't work in my project.

remal avatar Sep 14 '23 19:09 remal

BTW I found out why it didn't work in my project.

I used Gradle and set sourceCompatibility and targetCompatibility only. Without modifying JavaCompile.options.release. So, Gradle set -source and -target without setting --release.

So, I simply didn't use --release. That's why I needed to set --system. Looks like I can simplify the code a lot here.

Thanks!

remal avatar Sep 14 '23 21:09 remal

Going to join the chorus of voices here saying this is likely premature...

It's not just a matter of the target source code/bytecode being accepted/generated. Upgrading the java compiler binary itself can be an involved process which interacts with other aspects of our monorepo build system, development environment (and CI) containers, and tooling (e.g. other Javac Plugins). Currently, for us (Uber), the work to migrate to JDK 17 is planned for next year and it's dependent on first migrating build systems (to bazel from buck, which I don't believe supports Java 17). It's extremely unlikely we would be able to get there within the next couple Error Prone releases and thus this would likely mean keeping the internal Error Prone version at whichever is the latest to support JDK 11.

cc: @msridhar @kageiit

lazaroclapp avatar Sep 21 '23 21:09 lazaroclapp

Thanks @lazaroclapp. At Google we switched to using the latest version of javac around JDK 19, and have been able to continue to track the latest releases for javac, even though we're still using JDK 11 extensively in production. I definitely appreciate that upgrading just the javac version isn't effortless, though.

cushon avatar Sep 22 '23 00:09 cushon

@cushon does this mean tracking for example java 20 and 21 as new versions of java come out? But specifically with --release 11? Is there a specific reason (or reasons) to prefer this route?

Sineaggi avatar Sep 25 '23 19:09 Sineaggi

@cushon does this mean tracking for example java 20 and 21 as new versions of java come out? But specifically with ``--release 11`? Is there a specific reason (or reasons) to prefer this route?

We've been using the latest versions of javac as they're released at Google, but to be clear Error Prone will not require using the latest JDK release. If it eventually drops support for running on JDK 11 the next minimum supported version will probably be 17.

(And in terms of why Google is using recent versions of javac: we're using the same version of javac for everything in our monorepo, and doing smaller upgrades more frequently is less work, and having a recent javac version is a prerequisite for being able to use newer language features.)

cushon avatar Sep 26 '23 23:09 cushon

@cushon I'm not sure if I should open a separate issue for this, but one thing that impacts NullAway is that Error Prone does not yet have a BugChecker.SwitchExpressionTreeMatcher interface that we can implement to do checking of switch expressions. Is addition of such a matcher blocked on increasing the minimum supported JDK?

msridhar avatar Sep 27 '23 00:09 msridhar

@cushon I'm not sure if I should open a separate issue for this, but one thing that impacts NullAway is that Error Prone does not yet have a BugChecker.SwitchExpressionTreeMatcher interface that we can implement to do checking of switch expressions. Is addition of such a matcher blocked on increasing the minimum supported JDK?

Filing a separate issue for that SGTM, I don't think that is being tracked anywhere yet. I am not sure what all of the options are and am open to ideas. Increasing the minimum supported JDK would certainly make things like that easier, but there might be alternatives we could look at that used reflection etc. and avoided direct references to SwitchExpressionTree when running on older JDK versions.

cushon avatar Sep 27 '23 00:09 cushon

We are currently using latest errorprone version to check code that targets Java 5, 6, and 7 (don't ask why).

Java 5 code already requires some hacks (bumping --release to Java 6 as Java 5 is no longer supported by javac 11 (javac --help):

  --release <release>
        Compile for a specific VM version. Supported targets: 6, 7, 8, 9, 10, 11

Switching to javac 17 would drop support for Java 6:

  --release <release>
        Compile for the specified Java SE release. Supported releases: 7, 8, 9, 10, 11, 12, 13, 14, 15, 16, 17

Switching to javac 21 would drop support for Java 7:

  --release <release>
        Compile for the specified Java SE release.
        Supported releases:
            8, 9, 10, 11, 12, 13, 14, 15, 16, 17, 18, 19, 20, 21

So switching to Java 17 could be a bit painful for those that are still stuck at Java 6. In our case I think applying the same hack (compiling all Java 5 and 6 code a second time with --release 7 for errorprone) should do the trick.

quijote avatar Dec 16 '23 20:12 quijote

Thank you to everyone who has provided feedback so far!

I reviewed some of the input provided on the timeline for this:

I think it's too early to drop support for Java 11. Even the upcoming version 3 of the Quarkus framework still has Java 11 as the baseline.

It looks like Quarkus 3.7 will use Java 17 as the minimum JDK: https://quarkus.io/blog/java-17/

Currently, for us (Uber), the work to migrate to JDK 17 is planned for [2024]

@lazaroclapp are you able to provide any updates on that migration?

cushon avatar Feb 15 '24 18:02 cushon

@cushon

We (Uber) have recently finished the migration of the build system from buck to bazel and some initial JDK 17 support is being rolled out. However, the mass migration will probably still take some time and likely won't finish before the end of this half.

yuxincs avatar Feb 16 '24 18:02 yuxincs