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

[FEATURE] Make Lombok compatible with JDK 16 #2681

Closed
ilgrosso opened this issue Dec 15, 2020 · 95 comments
Closed

[FEATURE] Make Lombok compatible with JDK 16 #2681

ilgrosso opened this issue Dec 15, 2020 · 95 comments

Comments

@ilgrosso
Copy link

After JDK 16 has started the rampdown phase, we could not get Lombok working any more.

The exception reported by Maven compiler is:

[ERROR] Failed to execute goal org.apache.maven.plugins:maven-compiler-plugin:3.8.1:compile (default-compile)
on project syncope-wa-starter: Fatal error compiling: java.lang.IllegalAccessError: 
class lombok.javac.apt.LombokProcessor (in unnamed module @0x3af91e7e) cannot access class
com.sun.tools.javac.processing.JavacProcessingEnvironment (in module jdk.compiler) because module jdk.compiler
does not export com.sun.tools.javac.processing to unnamed module @0x3af91e7e -> [Help 1]

No issues with earlier JDK 16 releases.

@Rawi01
Copy link
Collaborator

Rawi01 commented Dec 15, 2020

I think the reason why it doesn't work anymore is that they added JEP 396. Adding --illegal-access=permit should fix the problem.

@randakar
Copy link

randakar commented Dec 15, 2020 via email

@rspilker
Copy link
Collaborator

@JornVernee
Copy link

JornVernee commented Dec 20, 2020

For anyone using Maven, I was able to make it work with the following compiler plugin configuration:

<plugin>
    <groupId>org.apache.maven.plugins</groupId>
    <artifactId>maven-compiler-plugin</artifactId>
    <version>3.8.1</version>
    <configuration>
        <source>16</source>
        <target>16</target>
        <fork>true</fork>
        <compilerArgs>
            <arg>-J--add-opens=jdk.compiler/com.sun.tools.javac.code=ALL-UNNAMED</arg>
            <arg>-J--add-opens=jdk.compiler/com.sun.tools.javac.comp=ALL-UNNAMED</arg>
            <arg>-J--add-opens=jdk.compiler/com.sun.tools.javac.file=ALL-UNNAMED</arg>
            <arg>-J--add-opens=jdk.compiler/com.sun.tools.javac.main=ALL-UNNAMED</arg>
            <arg>-J--add-opens=jdk.compiler/com.sun.tools.javac.model=ALL-UNNAMED</arg>
            <arg>-J--add-opens=jdk.compiler/com.sun.tools.javac.parser=ALL-UNNAMED</arg>
            <arg>-J--add-opens=jdk.compiler/com.sun.tools.javac.processing=ALL-UNNAMED</arg>
            <arg>-J--add-opens=jdk.compiler/com.sun.tools.javac.tree=ALL-UNNAMED</arg>
            <arg>-J--add-opens=jdk.compiler/com.sun.tools.javac.util=ALL-UNNAMED</arg>
            <arg>-J--add-opens=jdk.compiler/com.sun.tools.javac.jvm=ALL-UNNAMED</arg>
        </compilerArgs>
        <annotationProcessorPaths>
            <path>
                <groupId>org.projectlombok</groupId>
                <artifactId>lombok</artifactId>
                <version>1.18.16</version>
            </path>
        </annotationProcessorPaths>
    </configuration>
</plugin>

Note that <fork>true</fork> is needed, otherwise Maven seems to (silently) ignore the -J options when invoking the compiler. Looks like jdk.compiler/com.sun.tools.javac.jvm also needs to be exported on top of what @rspilker linked.

@rzwitserloot
Copy link
Collaborator

Reproduced with jdk16ea on x64 mac.

@rzwitserloot
Copy link
Collaborator

We've found a solution that does not require all million users to add all these add-opens to their build scripts. We'll release it in time for the OpenJDK 16 release (In March 2021).

@namannigam
Copy link

@rzwitserloot that's good news. just curious though, if there is an open pull request for these changes?

@rspilker
Copy link
Collaborator

@namannigam No, there isn't. We'll explain the reasons in this issue once jdk16 has been officially released. Please don't speculate.

@pruidong
Copy link

pruidong commented Feb 2, 2021

Is there a solution to this problem?

I have a maven project on IntelliJ IDEA that needs to be run through tomcat. But when I run it, a similar error is prompted:

java: java.lang.IllegalAccessError: class lombok.javac.apt.LombokProcessor (in unnamed module @0xddf172a) cannot access class com.sun.tools.javac.processing.JavacProcessingEnvironment (in module jdk.compiler) because module jdk.compiler does not export com.sun.tools.javac.processing to unnamed module @0xddf172a

The environment I use:

  1. Java JDK 16
  2. IntelliJ IDEA 2020.3.2
  3. Apache Maven 3.6.3
  4. Lombok 1.18.19

thanks.

@namannigam
Copy link

@pruidong the temporary solution is mentioned under #2681 (comment)

@pruidong
Copy link

pruidong commented Feb 3, 2021

@namannigam thanks.

I found a better way to add to the Windows system environment variables:

Name:

_JAVA_OPTIONS

value:

--add-opens=jdk.compiler/com.sun.tools.javac.code=ALL-UNNAMED --add-opens=jdk.compiler/com.sun.tools.javac.comp=ALL-UNNAMED --add- opens=jdk.compiler/com.sun.tools.javac.file=ALL-UNNAMED --add-opens=jdk.compiler/com.sun.tools.javac.main=ALL-UNNAMED --add-opens=jdk. compiler/com.sun.tools.javac.model=ALL-UNNAMED --add-opens=jdk.compiler/com.sun.tools.javac.parser=ALL-UNNAMED --add-opens=jdk.compiler/com. sun.tools.javac.processing=ALL-UNNAMED --add-opens=jdk.compiler/com.sun.tools.javac.tree=ALL-UNNAMED --add-opens=jdk.compiler/com.sun.tools. javac.util=ALL-UNNAMED --add-opens=jdk.compiler/com.sun.tools.javac.jvm=ALL-UNNAMED

@drmaniac
Copy link

We've found a solution that does not require all million users to add all these add-opens to their build scripts. We'll release it in time for the OpenJDK 16 release (In March 2021).

I'm very excited to see what this solution looks like. I wonder why this is why such a secret is made.

@rzwitserloot
Copy link
Collaborator

@pruidong Our fix for jdk16 should also take care of that.

We held it back just in case us publishing this causes a chain of events that ends up removing this convenient workaround from the GA of JDK16. Given that GA is in about 10 days, I think we're good to go and will soon commit our patch to master publicly.

@pron
Copy link

pron commented Mar 5, 2021

As an OpenJDK developer, let me explain that we’ve intentionally decided (after a lot of debate) to leave some well-known loopholes in to give libraries more time to gradually prepare for the new reality, and explain it, and why it is absolutely essential, to their users. Those loopholes are scheduled for gradual removal, one at a time, starting with JDK 17, as 16 already has the first step of encapsulation. Publishing exploits won’t make them stay or be removed any sooner or later; their removal will not be hastened or postponed. They are being removed according to schedule, and there will be a few months’ notice before each. I would recommended library authors use that time to prepare themselves and their users for this better but different environment.

In a few words, the strategy is this: use of JDK internals by libraries is tolerated as long as they notify the user that they’re doing so with add-opens. This is because such libraries become tied to JDK versions, and so must be vigilant when updating, and their users need to be aware of maintenance issues libraries might impose on them (there are also security issues when it comes to libraries that do this at runtime). So, you can do what you like, but if you impose a potential maintenance burden on your users, you must let them know about it. When all loopholes are closed, libraries will need to decide whether to not use internals or acclimate their users to add-opens. In some months there will be no way — using Unsafe, attach, native code etc. — to hack JDK internals without express approval of the application.

Of course, this is not an issue for applications, that should embed their own runtime and launcher with whatever flags they like (or, when it comes to build time tools, you can create a Maven plugin that does that). The point is that introducing maintenance (and/or security) burden will not be possible without the user knowing about it and taking some action to explicitly allow it — by using a specific executable, a specific plugin, or adding flags. When users understand that this is necessary, they tend to be very accommodating.

@rzwitserloot
Copy link
Collaborator

there are also security issues when it comes to libraries that do this at runtime

@pron This keeps being said. It doesn't make any sense. I've asked for clarification before, but never got an answer, so let me take this opportunity to ask again: What are you on about? How does mandatory --add-opens address security concerns?

It makes sense to deny plugins (a.k.a. code you might not fully trust) the ability to just call and interact with private API. However, this add-opens restriction does not accomplish that goal: If there is no security manager, then the code you don't trust can call private API (as long as they do so in opened packages). They can also invoke C:\windows\format.exe and wipe out a user's home dir while they are at it. It makes no sense to me to make any claims about 'this is more secure in environments where no security manager is set'. That's like installing a vault in the middle of an area fenced off by velvet ropes and saying: There! Now nobody can enter! Whilst everybody just hops over the rope.

Clearly you must be talking about some other sense of 'more secure', but I'm not sure what, precisely, you mean.

as long as they notify the user that they’re doing so with add-opens.

This just isn't workable advice.

  • Until very recently, this was impossible on maven, because command line args to the VM were stored by maven in a hashmap, thus preventing the ability of using the same argument more than once, and --add-opens needs to be applied more than once.
  • The amount of --add-opens that is required here is enormous, the syntax is unwieldy and just looking at the full list of command line args required is unlikely to really make anything particularly clear to your casual java user. They have no idea what they are looking at - just 'uh, this site told me to copy/paste this gigantic batch of stuff into my script. No idea why or what it does, but, CMD+C CMD+V I guess'.

When suggesting new language features, generally the inclination of the JEP process is to do some research: What is the impact of this change? This impact analysis should presumably take into account how common various libraries are (in other words, the 'assert' debacle where 1.4 added assert as a keyword should never have happened, given that junit was, at the time, the single most commonly used library in the entire eco-system, and 1.4 broke it by introducing a backwards incompatible change: That was a mistake, and lessons were learned).

But evidently that yardstick is not being applied here. I'd love to see the OpenJDK team take a closer look at this suggested upgrade path, using similar ideas as what is often requested for language changes: Instead of thumping specs, consider actual impact.

Note that, even today, java8 is vastly more popular than java11. I'm pretty sure the attitude of the OpenJDK team in regards to the detrimental impact that the jigsaw project has had on compatibility between java8 and java11 is the primary cause of this.

In light of all these things, I implore you, take this to the OpenJDK team and reconsider the plans for --add-opens and the suggested upgrade path. Your stated aims are not going to be accomplished with your plans, and haven't for a while now. All you're doing is making it worse:

  • Java11 adoption has been set back by a decade because of your plans.
  • We just keep working around it, and I rather doubt that will change. We can always just make maven and gradle plugins, which may fit your stated technical upgrade path option (essentially, we are now an 'application'), but does not accomplish the goal of making more clear that internal API is being used at all; if anything, it makes it harder to figure that out, not easier, and it makes it more likely that java programmers get the headache of incompatibilities.

In your mission to ensure the java ecosystem has fewer issues with backwards incompatibilities, you're aggressively introducing them instead. You're adding to the problem instead of solving them!

@rzwitserloot
Copy link
Collaborator

For the lombok users following this thread: We have some less well known loopholes we can use to bridge a few gaps. We'll start work on gradle and maven plugins in the mean time, which will be a long-term fix.

@pron
Copy link

pron commented Mar 16, 2021

Encapsulation of internal implementation details is the mechanism by which OpenJDK will ensure conformance to the spec, which has been Java's strategy for backward compatibility since its inception; use of implementation details is not portable pretty much by definition, and has been found to be the main hurdle to backward compatibility and the overwhelming cause of the difficulty to upgrade from 8. Moreover, the platform's security will become increasingly reliant on the integrity of encapsulation. Libraries will have the option of either targeting the spec or requesting the application for permission to rely on internals via add-opens. As this is not the OpenJDK mailing list, I'm merely informing you of the strategic decisions that have been made. If you'd like to appeal them, feel free to contact the mailing lists.

@rzwitserloot
Copy link
Collaborator

rzwitserloot commented Mar 16, 2021

@pron wrote:

the overwhelming cause of the difficulty to upgrade from 8

That's a misleading statement.

JDK11 broke a bunch public API, such as moving javax.annotation.Generated around, in order to fix split packages. I think it was the right call, but it is disingenuous to suggest that 'libraries that do not automatically just work in JDK11, well, they messed up'. No, the compatibility differences between 8 and 11 are, relative to any other java version update, an order of magnitude or two larger, and that's not the fault of libraries. The responsibility lies with the OpenJDK. The vast majority of actual 'library X worked in 8 and no longer works in 11, and it is because they used internal APIs' cases are broken because --add-opens exists, and not because said internal API actually changed. That, too, is a situation where the blame falls at least partly on OpenJDK. Team OpenJDK made the active decision to cause these incompatibilities. It's incompatible only because OpenJDK11 broke .setAccessible. The goals for both of these breaks are perhaps defensible, but I take offense at your strong insinuation that the blame falls squarely on libraries who either used @Generated or other stuff that disappeared from core java in 11, or on those who used internal API. No, it doesn't. They shoulder part, maybe. But only a part, the rest falls on OpenJDK's shoulders.

Moreover, the platform's security will become increasingly reliant on the integrity of encapsulation.

In security analysis, a threat model is often the starting point. More or less a movie script: Spin a story about how a hacker may get access to stuff they shouldn't have gotten access to. Armed with a few of these movie scripts, you can actually 'test' security measures: Which script(s) does it stop, if any. How reasonable are the scripts that it stops.

I have absolutely no idea what you're talking about. Can you paint me a picture of a security compromise that --add-opens will mitigate?

Which mailing list would you suggest is the best vehicle to raise these points with the OpenJDK project?

@pron
Copy link

pron commented Mar 16, 2021

That's a misleading statement.

No, it is not. The overwhelming cause of migration difficulty, without a doubt, has been reliance on internal implementation details. There have also been changes to the spec, but those caused a very small percentage of the issues.

The vast majority of actual 'library X worked in 8 and no longer works in 11, and it is because they used internal APIs' cases are broken because --add-opens exists, and not because said internal API actually changed.

That is false, not to mention that module encapsulation has only been turned on in 16. In any event, I'm not here to debate decisions, merely to inform about them.

Which mailing list would you suggest is the best vehicle to raise these points with the OpenJDK project?

jigsaw-dev.

@siziyman
Copy link

siziyman commented Mar 16, 2021

@pron

The overwhelming cause of migration difficulty, without a doubt, has been reliance on internal implementation details

To be quite honest, phrase "without a doubt" sounds a little bit weird here. Is there public/private research (be it polls or anything else) that substantiates that, or is it just an educated guess of some sorts?

Thanks!

@filiphr
Copy link

filiphr commented Mar 16, 2021

For the lombok users following this thread: We have some less well known loopholes we can use to bridge a few gaps. We'll start work on gradle and maven plugins in the mean time, which will be a long-term fix.

As an annotation processor maintainer it would be interesting to see how an approach such as using maven / gradle plugins.


Some other general question, has there been an initiative to add the things that Lombok needs to the Java compiler, perhaps an additional APIs that would make it possible for Lombok to do what it does. There were some discussions in https://twitter.com/gunnarmorling/status/1370687166553161732.

I have no idea of the Lombok internals, but if the Java Compiler allows plugging into the AST creation and modifying it then I guess Lombok will not need to depend on Java internals anymore.

@pron
Copy link

pron commented Mar 16, 2021

@siziyman It is based on an internal analysis by Oracle on the hurdles various companies faced when upgrading. If and when we ever take the time to write a post-mortem, we'll discuss this in greater detail. But I shouldn't have gone into that at all, and for that I apologise, because this is an inappropriate venue for debating a completely different project, and I shouldn't interfere with someone venting frustrations, however misdirected, on their own project's site.

To turn in a more constructive direction, users need not be bothered with lots of add-opens. The relatively few libraries that need internals at all, need very few, while applications, which overall might need many, control the launch and need not bother their users with the command line at all. Lombok can, for example, ship as an application, lomboc, which breaks encapsulation as much or as little as it wishes to.

@filiphr
Copy link

filiphr commented Mar 16, 2021

@pron the problem that you are saying for Lombok to ship as an application is not that easy. From my understanding Lombok does not need any internals during the runtime of an application. It needs to access certain internals during the compilation phase, since the Annotation Processing API does not provide a way for them to do what they are doing.

@pron
Copy link

pron commented Mar 16, 2021

@filiphr That is precisely why it can ship as an application, just like javac. But that's just a suggestion. Build tool plugins are another.

@rspilker
Copy link
Collaborator

rspilker commented Mar 16, 2021

I agree that this might not be the place to discuss javac development, there are a few things that I would like to mention.

We decided to not create another executable, because the current ones are fine and well supported in build tools and IDE's.

Regarding the suggestion to create a binary like javac, I think no-one would benefit from that. The command line parameters of javac are not specified nor stable.

Lombok is compatible with all JDKs starting from 1.6. If we would release a new version of lombok, what javac version would we enforce on our users?

We chose use hook in as an annotation processor to make it as easy on our users as possible. We've taken on the burden to be compatible with 11 javac version, OpenJ9 and also several ecj-s. We support a wide range of build tools like ant, Maven, Gradle and Bazel, as well as IDE's like Eclipse, NetBeans, IntelliJ and Visual Studio Code.

What we offer is that our users download a jar that is smaller than 2 MB, put it on their classpath, optionally explicitly point to the annotation processor, and it just works.

It would be great if there was a way so we can express in the manifest what API's to open if we're running as an annotation processor. So far, I was not able to make this work, but possibly I missed something.

I personally would also think that "compiling an application" and "running an application in production at the customer" have different security concerns. And there are a lot of good arguments to give developers an easy way to get rid of these warnings during compilation.

Regarding the use of "private API's", as already mentioned, it would have been very nice if there had been a public AST manipulation API. Both Reinier and I have been talking about this already 10 years ago with the persons responsible for the compiler and annotation processor. The response from those in charge was: "Horrible, we're never going to do that. It would prevent the creation of new language features, and annotation processors are never going to be allowed to change the behavior of the program."

Regarding the "prevent creation of new language features", the idea was that adding methods to visitor interfaces would be backwards incompatible change. And while technically correct, the same arguments can be made regarding JDBC. And also the same "defense" applies: There are only a limited number of JDBC vendors. Well, there are only a limited number of compiler integrations.

Regarding annotations changing the behavior, this is also not longer as black and white.

Possibly, the ideas regarding public APIs and allowing annotation processors to modify the resulting bytecode has changed in past decade. Honestly, Reinier and I have given up that hope, but we're very willing to collaborate with those in charge of the compiler to see how we can improve the java ecosystem.

@pron
Copy link

pron commented Mar 16, 2021

It would be great if there was a way so we can express in the manifest what API's to open if we're running as an annotation processor.

The rule is that the library doesn't decide on encapsulation but that the application has the final say (and the defaults are provided by the target code). That principle is core to encapsulation. Internal implementation details can change at any time, even in patches, and the consumer of a library needs to know that the library is tied to a specific release.

Regarding the use of "private API's", as already mentioned, it would have been very nice if there had been a public AST manipulation API.

That's a whole other discussion, and a good one to have, but I am certain there would be some debate. I am not involved with that area of the platform at all, but I believe that annotation processors were intentionally limited, just as Java agents are (and there are libraries that don't like that, either). The Java platform and the Java language should be free to define the scope of their features and to disallow or not support certain uses. Those still have an avenue through the mechanism I described, even if it means more work.

Anyway, the plan for encapsulation was finalized three years ago but encapsulation wasn't turned on but just emitted warnings precisely to have time for such discussions. But, better late than never, and while I doubt new APIs will be added before the loopholes are closed, if you have some small suggestion, perhaps it could be done. compiler-dev is probably the right place to take this.

@A248
Copy link

A248 commented Mar 17, 2021

The question I have is, what is lombok's long-term strategy for easing away from reliance on JDK internals? Will it continue to implement additional workarounds and hacks to break into javac? Shipping as a maven plugin seems like another way of working around the strengthening of encapsulation brought by recent JDK updates. There will come a point when these JDK internals are no longer accessible, and the hacks will fail. Either that, or lombok will have to continue to use a mechanism such as add-opens to keep prying open internals.

Does lombok really plan to rely on JDK internals indefinitely?

gardner pushed a commit to gardner/random-cut-forest-by-aws that referenced this issue Jul 5, 2022
sudiptoguha pushed a commit to aws/random-cut-forest-by-aws that referenced this issue Jul 8, 2022
Co-authored-by: Gardner Bickford <gardner.bickford@fluxfederation.com>
tomitheninja added a commit to kir-dev/paybasz that referenced this issue Aug 5, 2022
bonita-ci pushed a commit to bonitasoft/bonita-engine that referenced this issue Oct 10, 2022
Update to latest version to-date that fixes [an issue](projectlombok/lombok#2681) with Java 17
psiotwo added a commit to psiotwo/owldiff that referenced this issue Nov 14, 2022
BenLocal added a commit to BenLocal/mini-agent that referenced this issue Jul 18, 2023
@yanxutao89
Copy link

We've found a solution that does not require all million users to add all these add-opens to their build scripts. We'll release it in time for the OpenJDK 16 release (In March 2021).

Sorry to disturbe you. I meet the same probolem when testing my antotation processor with JDK 16. The problem remained after following what you do in 9806e5c. It seems the add-opens does not pass to the comipler. Can you help me out with that?

leo-monus pushed a commit to au-research/cerium that referenced this issue Aug 15, 2023
JackPGreen added a commit to JackPGreen/hazelcast-code-samples that referenced this issue Apr 16, 2024
When building, the following warning is logged for every module:
```
[INFO] --- compiler:3.11.0:compile (default-compile) @ data-locality ---
[INFO] Changes detected - recompiling the module! :source
[INFO] Compiling 4 source files with javac [debug release 17] to target/classes
[WARNING] --add-opens has no effect at compile time
```

The `add-opens` workaround is [no longer required with newer versions of Lombok](projectlombok/lombok#2681) anyway and so should instead be removed.
JackPGreen added a commit to hazelcast/hazelcast-code-samples that referenced this issue Apr 16, 2024
When building, the following warning is logged for every module:
```
[INFO] --- compiler:3.11.0:compile (default-compile) @ data-locality ---
[INFO] Changes detected - recompiling the module! :source
[INFO] Compiling 4 source files with javac [debug release 17] to target/classes
[WARNING] --add-opens has no effect at compile time
```

The `add-opens` workaround is [no longer required with newer versions of Lombok](projectlombok/lombok#2681) anyway and so should instead be removed.
baulea added a commit to baulea/zerocode that referenced this issue Apr 19, 2024
extendreport uses lombok. Lombok is compatible with JDK 16/17 since version 1.18.22 (projectlombok/lombok#2681)

extentreports.version 5.1.1 -> org.projectlombok:lombok:1.18.26
extentreports.version 5.0.9 -> org.projectlombok:lombok:1.18.12
antonsviridov-src added a commit to sourcegraph/scip-java that referenced this issue Aug 1, 2024
A canonical usage for this is passing `--add-opens` flags to the _launcher_ of javac to make sure annotation processors work.

To pass these arguments to the launcher, they have to be perfixed with -J – but arguments like this cannot be passed through the argfile that we use for javacOptions (see https://docs.oracle.com/en/java/javase/17/docs/specs/man/javac.html#command-line-argument-files) so we need to pass them to the command itself. 

For that purpose, we add a `jvmOptions` field to scip-java.json config – these options will have `-J` added to them and passed to the `javac` command.

The test to verify this behaviour relies on an [old version of lombok that requires these options](projectlombok/lombok#2681 (comment))

Additionally, a hidden option `--strict-compilation` is added to the CLI, to prevent error recovery: sometimes scip-java can just ignore javac errors and produce semanticdb artifacts regardless. This complicates testing, so I need an escape hatch).

### Test plan

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

No branches or pull requests