projectlombok / lombok

Very spicy additions to the Java programming language.
https://projectlombok.org/
Other
12.73k stars 2.36k forks source link

Eclipse javaagent and mapstruct APT implementation #1359

Open pgaschuetz opened 7 years ago

pgaschuetz commented 7 years ago

Hi,

there seems to be a classloader issue when using lomboks javaagent with Eclipse and mapstructs annotation processing. There has been some discussion including observations and the relevant stacktrace in mapstruct/mapstruct#1159

Without fully understanding the custom integration between lombok and mapstruct, it seems, that lomboks javaagent references a wrong classloader when working with mapstruct.

Can you shed some light into the issue?

Many thanks Philipp

rspilker commented 7 years ago

I'm wondering what changed. From the discussion on https://github.com/mapstruct/mapstruct/issues/510 I think it used to work. Or did nobody ever use mapstruct from Eclipse?

kanouken commented 7 years ago

I got the same issues,

polle commented 7 years ago

I'm having the same issue after upgrading to Mapstruct 1.2 Beta3 and Lombok 1.16.16. Yes, it used to work with Mapstruct (and Lombok) in Eclipse.

filiphr commented 7 years ago

@polle did this work for you with 1.2.0.Beta1? Maybe we need to use a different classloader for the Service Loader.

Currently MapStruct does this

ServiceLoader.load(AstModifyingAnnotationProcessor.class, AnnotationProcessorContext.class.getClassLoader())

AnnotationProcessorContext is the class that collects the AstModifyingAnnotationProcessor(s) within MapStruct.

filiphr commented 7 years ago

We (@pgaschuetz) tried with using the Current thread class loader and this error comes

org.mapstruct.ap.spi.AstModifyingAnnotationProcessor: Provider lombok.launch.AnnotationProcessorHider$AstModificationNotifier not a subtype

Can there be some problem in the agent? Do you perhaps shade the AstModifyingAnnotationProcessor in the agent?

polle commented 7 years ago

@filiphr I did not try with 1.2.0.Beta1. I can try it out if you want?

filiphr commented 7 years ago

@polle you said that it used to work before. Which versions of Lombok and MapStruct were you using? Are you using the correct version of the jar?

polle commented 7 years ago

I'm currently using the following versions, which works fine with Eclipse:

Project dependencies:

org.mapstruct:mapstruct-jdk8:1.0.0.Final
org.mapstruct:mapstruct-processor:1.0.0.Final
org.projectlombok:lombok:1.16.2

Eclipse Lombok installation:

> java -jar lombok.jar --version
v1.16.16 "Dancing Elephant"

I probably used to use 1.16.2 for Eclipse as well, but it seems to still work after I upgraded to 1.16.16 while trying to upgrade both Mapstruct and Lombok.

filiphr commented 7 years ago

The AstModifyingAnnotationProcessor which cannot be found has only been introduced in 1.2.0.Beta1 from MapStruct and Lombok is using it from 1.16.14. See this FAQ on the MapStruct website.

polle commented 7 years ago

@filiphr the versions I listed above work fine in Eclipse, but not with most other tools (at least IntelliJ and Gradle) which is the main reason I wanted to upgrade to the latest versions. But with the latest versions in Eclipse I get the exception mentioned here.

Rereading this issue and https://github.com/mapstruct/mapstruct/issues/510 I guess when @rspilker asked whether this used to work, he meant in 1.2.0.Beta1 and onwards, not the old versions I listed above, which I guess are irrelevant for this discussion. Sorry for the confusion.

filiphr commented 7 years ago

OK now I get it. Thanks for the clarification. The old versions worked with Eclipse and the agent because most probably the agent forced the annotation processing rounds and did something so the others can work.

It seems that the problem with the missing class is from the beginning of 1.2.0.Beta1. If the guys from Lombok can figure out why exactly the classloaders are not working, and whether we need to use something else we would be happy to do any needed changes.

lppedd commented 6 years ago

As of Lombok 1.16.18 and MapStruct 1.2.0.CR1 the Javaagent problem is still present. It's a pity because it forces manual Maven clean/compiles, or, even worse, switching to a different IDE. If Lombok guys need users to try out something I'd be happy to do that.

eximius313 commented 6 years ago

I have exactly the same problem

nickheniser commented 6 years ago

Workaround: https://stackoverflow.com/a/45525100/503764

eximius313 commented 6 years ago

You must use eclipse task in order to generate factoryPath for Eclipse because Buildship doesn't have this feature yet: https://github.com/eclipse/buildship/issues/486

Unfortunatelly this doesn't solve the problem. I just checked it:

  1. I deleted .apt_generated manually
  2. I refreshed the project (F5) - .apt_generated folder was created but it was empty
  3. I run Project->Clean, Hibernate Metamodel classes were generated but not the Mapstruct
  4. I run gradle eclipse and refreshed/cleaned the project - nothing changed ;(
v1nc3n4 commented 6 years ago

Hi,

I'm experiencing this issue on a new project with Eclipse Oxygen 2 + Lombok 1.16.20 + Mapstruct 1.2.0.Final, I'm glad to read this discussion. I can confirm the 2 workarounds below for current projects that ambition to upgrade these dependencies:

It's not clear for me where the issue is (Lombok or Mapstruct?), and if a fix is in progress. I understand both Mapstruct and Lombok teams worked together to implement a working solution about the javaagent, annotation processors... As mentioned by @pgaschuetz, the issue is also pending Mapstruct-side (mapstruct/mapstruct#1159) since 9 months. Both workarounds are not really satisfactory: developers shall not tweak Lombok by hand, or miss important features with Mapstruct 1.2.0.

What is the current status of this issue? Thanks in advance for your answer (and also thanks for this amazing library!). BR

evser commented 6 years ago

@Vicente69 what do you mean by "update Eclipse installation"?

v1nc3n4 commented 6 years ago

Hi @evser,

I mean double-clicking on the Lombok JAR (under Windows) and use the dialog to update the JAR deployed in root directory of Eclipse. It can also be: copying the tweaked Lombok JAR directly in the root directory of Eclipse. Is it clear for you?

evser commented 6 years ago

@Vicente69 yes, thanks!

eximius313 commented 6 years ago

All of these workarounds are ugly :( I'd love to see Lombok & Mapstruct finally coopertating..

filiphr commented 6 years ago

@eximius313 and @Vicente69 I completely agree that the workarounds are ugly and are not nice for Eclipse users with the agent.

I am not sure where exactly is the issue, I would say that it is something linked to the Lombok javagent, as everything works fine when the agent is not there. Unfortunately I don't know the lombok codebase and have no experience with agents. If there is something that we (I am member of the MapStruct team) can do regarding the usage of classloaders we will do it.

@rspilker you asked at the beginning, but I didn't quite address that. Using MapStruct and Lombok works fine within Eclipse (when the agent is not there). The only time a problem occurs is when the agent is used. Therefore, I suspect that the issue is linked to that

v1nc3n4 commented 6 years ago

Hi @filiphr. and thank you for your support. I understood that the Lombok project shall implement the interface AstModifyingAnnotationProcessor, after the co-working between Lombok and Mapstruct teams. It seems to be the case since Lombok 1.16.14.

I ran several tests with the same Eclipse Oxygen 2 instance, and different Lombok + Mapstruct combinations in a Maven project, plus the corresponding javaagent in Eclipse. What I can tell is:

Why the interface is not included in the built JAR? This I cannot tell. What about Lombok team? To me, the issue could be resolved simply by inserting the line 261 <fileset dir="build/stubs" includes="org/mapstruct/ap/spi/AstModifyingAnnotationProcessor.class" /> in the build.xml script (version 1.16.14, I didn't check the latest releases). But it is too simple to me, and I guess there may be some arguments why the compiled interface was not included in the JAR.

I have rebuilt Lombok 1.16.14 with this line in the Ant script, and the JAR built is working with Mapstruct 1.2.0.Final and Eclipse.

Thanks in advance for any comment from the Lombok team. BR

eximius313 commented 6 years ago

I'm using Eclipse Version: Oxygen.2 Release (4.7.2) Lombok: 1.16.20 Mapstruct: mapstruct-jdk8 1.2.0.Final together with mapstruct-processor 1.2.0.Final Gradle: 4.5 and still have problems where Lombok classes (as well as Hibernate Metamodel classes) are generated, but Mapstruct classes are not.

filiphr commented 6 years ago

@eximius313 is this with or without the agent?

eximius313 commented 6 years ago

How to check it?

softdays commented 6 years ago

Could anyone of the Lombok team give a feedback about @Vicente69 suggestion?

v1nc3n4 commented 6 years ago

Sorry, I added an incorrect comment about upgrading to Lombok 1.18.0. The implementations of Mapstruct mappers are still prevented with Lombok 1.18.0.

rzwitserloot commented 6 years ago

I'm confused. I would say that including the org.mapstruct.ap.spi.AstModifyingAnnotationProcessor into lombok.jar itself could not possibly work and will definitely break when using lombok in module mode (java9/jigsaw), as split packages aren't allowed.

The only thing I can think of, is that mapstruct somehow runs on top of our classloader, in which case the solution 'throw the AstModifyingAnnotationProcessor.class file into lombok.jar' would indeed fix things. So, I'm going to assume for now that this is indeed how it works.

I have updated lombok to try to deal with this; we now load this class, but only if parent didn't already load this. I haven't been able to reproduce this problem, so, hopefully one of the many commenters in this thread can help? Download lombok.jar from https://projectlombok.org/download-edge and give it a shot.

NB: We really need signoff from someone running lombok.jar in module mode, because I'm really afraid of getting the 'no split packages' error. To do that, see the JDK9 section on this page: https://projectlombok.org/setup/javac

Thanks for all the attention, reporters & team mapstruct!

NB: Crossposted to https://github.com/mapstruct/mapstruct/issues/1159

rzwitserloot commented 5 years ago

There are now like 20 threads with 80 different workarounds and opinions. The prevailing workaround of just tossing the marker interface into lombok.jar is as of yet unacceptable to us because it seems like it cannot work in module-land due to the split packages rule and we don't want to introduce a fix that's already broken in future-standard java.

Next steps are to create a probably dockerized test environment so we can play around, reproduce properly, etc. That's gonna take a while, we need some serious free time before we can get into this. Sorry folks.

It would help if someone can confirm that running it all in module mode with the patched lombok.jar fails as I predict it will.

ft0907 commented 5 years ago

@rzwitserloot @filiphr I used Gradle to configure mapstruct in STS. Grade compiled incorrectly. I confirmed that I had upgraded to the latest version of Lombok (1.18.2), but it still didn't work. Did I miss the unknown configuration, please tell me! This is the mistake of build: java.lang.IllegalStateException: endPosTable already set

My permissible environment: STS 3.9.5 gradle 4.6 mapstruct 1.3.0.Beta1 Lombok 1.18.2

@pgaschuetz What is your solution? Can you tell me? thanks

mvmn commented 5 years ago

It's hard for me to understand the source of the problem. Does anyone have an idea why it happens at all?

This code in org.mapstruct.ap.internal.util.AnnotationProcessorContext fails to load org.mapstruct.ap.spi.AstModifyingAnnotationProcessor class:

ServiceLoader<AstModifyingAnnotationProcessor> loader = ServiceLoader.load(
     AstModifyingAnnotationProcessor.class, 
     AnnotationProcessorContext.class.getClassLoader()
);

But I don't understand why. The class AstModifyingAnnotationProcessor is in the same JAR as AnnotationProcessorContext itself, so AnnotationProcessorContext.class.getClassLoader() should return a class loader that should be able to load both AnnotationProcessorContext and AstModifyingAnnotationProcessor equally since they're in the same JAR.

How come this fails?

P.S. The stack trace I have (using Eclipse Oxygen) is very similar to this one: https://github.com/mapstruct/mapstruct/issues/1159#issuecomment-318050333 Please refer to that comment if you need full stacktrace.

alexweirig commented 5 years ago

Hi,

is there any progress on this topic? I'm currently trying to use mapstruct and lombok with the following setup:

No maven or other stuff added or updated.

When I run Eclipse without the -javaagent, mapstruct is working fine. When I add the -javaagent, mapstruct fails with the CNF: Caused by: java.lang.ClassNotFoundException: org.mapstruct.ap.spi.AstModifyingAnnotationProcessor at java.net.URLClassLoader.findClass(URLClassLoader.java:381) at java.lang.ClassLoader.loadClass(ClassLoader.java:424) at sun.misc.Launcher$AppClassLoader.loadClass(Launcher.java:338) at java.lang.ClassLoader.loadClass(ClassLoader.java:357)

Thank you

Alex

pavelhoral commented 5 years ago

I peeked inside Eclipse (2019-03) to check what is going on: MapStruct fails in the ServiceLoader call on NoClassDefFoundException for lombok.launch.AnnotationProcessorHider$AstModificationNotifier. I am going to try to dig deeper, but I am not sure how far I can get as I have close to zero knowledge when it comes to internal workings of Eclipse, MapStruct or Lombok :D.

Edit: I am on JDK 10 (default for current Ubuntu distros), so maybe my issue is connected to #1572

~Edit 2: Lombok's class is not found because JDT's APT class-loader chain contains this fella (lookup is not delegated to parent class-loader). The remaining mistery for me is why is ServiceLoader even trying to get Lombok's class.~

Edit 3: TIL Lombok actually contains implementation of MapStruct's annotation processor. So easy workaround (possibly a bit cleaner than copying classes) is to simply detelete that service registration from lombok.jar/META-INF/services.

~Edit 4: App class-loader actually tries to load Lombok's MapStruct annotation processor from a correct JAR. When debugging I got to ClassLoader#defineClass1 (native call) that ends up with NoClassDefFoundException. Not sure where to go from that point... it kind of seem like JVM issue.~

Edit 5: I feel silly for overanalyzing such obvious issue. Of course I get NoClassDefFoundException => the issue is that the interface org.mapstruct.ap.spi.AstModifyingAnnotationProcessor can not be loaded by the application class-loader (as kind of pointed out by other commenters)... duh. ~I am not sure what else is here to solve - this can never work as is. That interface can never be part of lombok.jar, because that would lead to ClassCastException down the class-loader line. This approach is pretty much flawed in its core.~

pavelhoral commented 5 years ago

TL;DR of my previous comment: IMO when Lombok is used as instrumentation javaagent, there is no need for MapStruct annotation processor integration. So lombok.jar used in Eclipse should not register AstModificationNotifier as this can never work across class-loader hierarchy.

filiphr commented 5 years ago

This is an interesting observation @pavelhoral. Reading this comments it would mean that the lombok.jar would always run first when it runs as an agent in Eclipse.

@rspilker is the Lombok Eclipse Agent and the Lombok jar the same jar? Is there some way to not have the AstModificationNotifier registered as a service when running it as an agent?

rspilker commented 4 years ago

We are in the process of splitting off the normal annotation processor, and the part the does the communication with MapStruct.

In addition to your existing Lombok and MapStruct dependencies, from the next version you should also add the following dependency:

<dependency>
    <groupId>org.projectlombok</groupId>
    <artifactId>lombok-mapstruct-binding</artifactId>
    <version>0.1.0</version>
    <scope>provided</scope>
</dependency>

You can already test this and give feedback, by using the edge release.

AxelHeathnet commented 4 years ago

So, including this dependency now (with lombok being at 1.18.12) would not solve the issue with eclipse? For now I'm using a "modded" version of the lombok jar with the mapstruct dependency being removed from it and it works but, of course, it is not the clean way

rspilker commented 4 years ago

@AxelHeathnet It would if you use the edge release.

avanathan commented 3 years ago

@rspilker I downloaded edge release version of lombok & saved in ./build folder. Here is what I have in my pom.xml . I am getting same error - i.e mapstruct not finding Builders annotated in my dtos.

I am noticing this on terminal app running mvn clean compile and in Build -> Rebuild Project in IntelliJ as well.

Am I missing something?

<!--        <dependency>-->
<!--            <groupId>org.projectlombok</groupId>-->
<!--            <artifactId>lombok</artifactId>-->
<!--            <version>1.18.12</version>-->
<!--        </dependency>-->
        <dependency>
            <groupId>org.projectlombok</groupId>
            <artifactId>lombok</artifactId>
            <version>edge-SNAPSHOT</version>
            <scope>system</scope>
            <systemPath>${basedir}/build/lombok-edge-20200822.021431-1.jar</systemPath>
        </dependency>
        <dependency>
            <groupId>org.projectlombok</groupId>
            <artifactId>lombok-mapstruct-binding</artifactId>
            <version>0.1.0</version>
        </dependency>
...
...
        <build>
        <finalName>${artifactId}</finalName>
        <plugins>
            <plugin>
                <groupId>org.apache.maven.plugins</groupId>
                <artifactId>maven-compiler-plugin</artifactId>
                <version>3.8.1</version>
                <configuration>
                    <source>${java.source.version}</source>
                    <target>${java.target.version}</target>
                    <annotationProcessorPaths>
                        <path>
                            <groupId>org.projectlombok</groupId>
                            <artifactId>lombok</artifactId>
                            <version>edge-SNAPSHOT</version>
                            <scope>system</scope>
                            <systemPath>${basedir}/build/lombok-edge-20200822.021431-1.jar</systemPath>
                        </path>
                        <path>
                            <groupId>org.mapstruct</groupId>
                            <artifactId>mapstruct-processor</artifactId>
                            <version>${org.mapstruct.version}</version>
                        </path>
                    </annotationProcessorPaths>
                </configuration>
            </plugin>
....
rspilker commented 3 years ago

@avanathan Can you at least see if lombok is triggered?

I have a vague recollection that annotation processors on "system" do not work in maven. That's why we recommend using our repository, as documented on the edge download page.

imod commented 3 years ago

This works with the latest MapStruct Version 1.4.0.CR1

filiphr commented 3 years ago

Starting from 1.4 MapStruct will ignore exceptions thrown when trying to load the AstModifyingAnnotationProcessor(s). This means that this is no longer a problem for MapStruct.

@rspilker I think that you can close this ticket. From MapStruct side everything is working OK now.