Code generation fails with Java 17 #1339
Comments
lenaschoenburg
changed the title
|
@SimY4 can you pls take a look at this issue ? Let me know if you don't have time. |
|
@oleschoenburg seems to be working fine in github actions pipeline: https://github.com/SimY4/immutables/actions/runs/1362104867 Do you know what could be different between immutables fixtures set up and your build pipeline set up? |
|
Interesting, thanks for looking into it! The only difference I can think of is that I'm running on arm64 (the JDK is x86 though). I'll try to confirm that this doesn't happen on a x86 machine first. |
|
I've tested a couple of different combinations now:
This means that the title is incorrect, this occurs for Java 16 too. |
|
@oleschoenburg thank you for your inputs. I was able to reproduce it on another project of mine. |
|
@oleschoenburg apparently the difference that fixes this issue is this one: value fixture uses error-prone that requires all these additional opens. Apparently, some of them are also needed for an immutables annotation processor to work. The working theory that I'm currently working on is that if you define a module-info.java for your module using immutables you don't have to declare opens. Otherwise, we can work on figuring out the absolute minimum of required opens for immutavles. |
|
@SimY4 Thank you, I can confirm that adding the exports fixes the problem! 🥳 I didn't test adding |
|
Follow up Narrowed down list of opens that will make immutables work for this case is: |
|
done in 2.9.0 ? |
|
@elucash yes, with everything I had on this was merged to master. The final version requires only one additional export: |
|
Hey @SimY4! Do you have any more information on the root cause on why the export is required here, and if there are any possible workarounds? In my company, we have a several rather large projects making heavy use of immutables, and forking the maven compiler causes a ~10x slowdown in build times, so would be quite a blocker for moving onto JDK 17. I was hoping that the final version would not require any exports at all. I did find https://bugs.openjdk.java.net/browse/JDK-8229535, which I wonder is related to the underlying issue at hand. If you had any pointers to investigating deeper, that would be greatly appreciated! |
|
@dwragge In immutables, there are features that are compiler-specific. To be clear - not every feature of immutables is compiler-specific so you may not need to include this in your build. In my personal projects I somehow completely avoided the need to have it set. When you have a feature that requires Immutables to look beyond shared public compiler APIs of Hotspot JDK, this is where you would need to set this open. |
|
One way how Immutables can attempt to mitigate that is by specifying required opens in jar manifest: But this would require Immutables to get onboard with JPMS (AKA module system). |
|
Turned out to be easy: #1354 Now I wish to come up with it earlier. |
|
That's awesome, thanks a lot @SimY4 ! |
|
I'm not sure if this is related to this, as our issues were with It turned out that problem was using As example project didn't use |
|
I'm seeing this issue on 2.9.3 with |
|
TBH I do observe this issue in one of my projects also. I can't reproduce it in any other repo except one so I can't blame it specifically on Java 17. I believe there might be a race condition or something that in some circumstances doesn't let Immuables to calculate the future reference for not yet existent type (like |
|
Sorry for not contributing much here, but I don't know how to approach this. |
|
It's not a runtime issue though, it's a compile time one. I've got an example that reproduces here: https://github.com/dylanbaroody/reproduce-immutables-issue This does actually work if I generate the dependent files ( I can confirm that this example builds with java 11.0.15, but fails with java 17.0.5 |
|
Sorry, I only now got to the repro. But I wish I would see the actual error earlier (yep, it was in the original report) Where instead of class we have string |
|
Ok, so here's my understanding so far:
At some point we might consider doing Let's discuss how we can fix Jackson integration in light of this. What do we need for this? |
|
I can confirm that adding The problem with my previous suggestion of adding opens to jar manifest is that it won't work for annotation processors. This trick will only work for executable jars. I wonder if we can do something with programmatically opening modules at runtime. I think I saw JUnit 5 or Mockito (ByteBuddy in particular) doing something funky with modules to access classes. |
|
Also, this looks like a JEP that should solve this: https://openjdk.org/jeps/8280389 |
|
Idk, looks more like a "standard" alternative to asm bytecode lib |
It's been 7 months since the 5.10.0.RELEASE. The current release documentation states that that release is compatible with Spring Boot release versions up to 2.6.x. Meanwhile, Spring Boot 3.1.x is the current release, with Spring Boot 3.2.x nearing GA. There's not much that needed to change to prepare a version of the cf-java-client to be compatible with Spring Boot 3.1.x, but it does require setting target to Java 17. That required a change to the maven-compiler-plugin compileArgs configuration to facilitate source code generation and data-binding when using Immutables. I also took the test dependencies forward, so that in the future, the tests could be refactored to leverage JUnit 5 APIs. The rest of the changes are pretty straight-forward, just upgrades to dependencies and plugins to the latest available as of the date of this pull request. Default build with Maven 3.9.4 targets Java 8 and upgrades Spring Boot to 2.7.16. Maven profiles get activated based on the installed version of the JDK detected. These profiles update the dependencies and plugins to support compilation and testing on Java 17 and 21 with Spring Boot 3.1.4 dependency management. A Github Action workflow was added to provide signal on "good builds". Currently, tests fail when targeting either Java 17 or 21. See this open issue for more details: immutables/immutables#1339. > This PR was based upon earlier work in cloudfoundry#1198.
Employed OpenRewrite to do perform all changes save for one. AbstractRestTest.java had a reference to org.junit.ComparisonFailure which itself is a subclass of java.lang.AssertionError. I followed public documentation here: * https://docs.openrewrite.org/running-recipes/popular-recipe-guides/migrate-from-junit-4-to-junit-5 * https://docs.openrewrite.org/running-recipes/multi-module-maven Tests pass for Java 8 and Java 11. But we still observe the following issue targeting Java 17+: immutables/immutables#1339.
|
Is there a way to get this to work without forking the compiler? Forking the compiler slows stuff WAY down |
|
@dylanbaroody if I understand the question correctly (I didn't re-read the whole thread): yes, the alternative is to pass the relevant flags to the Maven process itself, e.g. by placing them in This is a project that follows that approach. (It doesn't use Immutables, but it does use a compiler plugin that heavily relies on compiler internals that are not exposed by default; see also their documentation where the non-fork approach is mentioned.) |
Over at Zeebe we are having some issues with
immutablesin combination withjackson-databindwhen upgrading from Java 11 to Java 17. We are usingimmutablesversion2.9.0-rc1.I'll link a minimal reproducing example below, but the basic issue is this:
Given two classes:
and
Compilation with Java 11 works as expected but when switching to Java 17 it fails:
Full build log
The offending generated code looks like this:
Full project to reproduce is here: https://github.com/oleschoenburg/immutables-bug-reproducer
I think this could be a new instance of #360?
The text was updated successfully, but these errors were encountered: