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

Eclipse javaagent and mapstruct APT implementation #1359

Open
pgaschuetz opened this issue Apr 11, 2017 · 42 comments
Open

Eclipse javaagent and mapstruct APT implementation #1359

pgaschuetz opened this issue Apr 11, 2017 · 42 comments

Comments

@pgaschuetz
Copy link

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

@pgaschuetz pgaschuetz changed the title Eclipse Javaagent and mapstruct AST implementation Eclipse Javaagent and mapstruct APT implementation Apr 11, 2017
@pgaschuetz pgaschuetz changed the title Eclipse Javaagent and mapstruct APT implementation Eclipse javaagent and mapstruct APT implementation Apr 11, 2017
@rspilker
Copy link
Collaborator

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

@kanouken
Copy link

kanouken commented Jun 8, 2017

I got the same issues,

@polle
Copy link

polle commented Jun 8, 2017

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
Copy link

filiphr commented Jul 2, 2017

@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
Copy link

filiphr commented Jul 2, 2017

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
Copy link

polle commented Jul 3, 2017

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

@filiphr
Copy link

filiphr commented Jul 3, 2017

@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
Copy link

polle commented Jul 3, 2017

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
Copy link

filiphr commented Jul 3, 2017

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
Copy link

polle commented Jul 3, 2017

@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 mapstruct/mapstruct#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
Copy link

filiphr commented Jul 3, 2017

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
Copy link

lppedd commented Aug 27, 2017

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
Copy link

I have exactly the same problem

@nickheniser
Copy link

@eximius313
Copy link

You must use eclipse task in order to generate factoryPath for Eclipse because Buildship doesn't have this feature yet: eclipse/buildship#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
Copy link

v1nc3n4 commented Feb 8, 2018

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:

  • Copy the org.mapstruct.ap.spi.AstModifyingAnnotationProcessor class from the mapstruct-processor 1.2.0.Final JAR into the Lombok JAR, and update Eclipse installation.
  • Downgrade to Mapstruct 1.1.0.Final (may be obvious, but since I didn't know the issue was related to Lombok and Mapstruct 1.2.0.Final release...)

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
Copy link

evser commented Feb 8, 2018

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

@v1nc3n4
Copy link

v1nc3n4 commented Feb 8, 2018

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
Copy link

evser commented Feb 8, 2018

@Vicente69 yes, thanks!

@eximius313
Copy link

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

@filiphr
Copy link

filiphr commented Feb 8, 2018

@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
Copy link

v1nc3n4 commented Feb 8, 2018

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:

  • The Mapstruct implementations are generated correctly during Eclipse (re-)compilation, with Lombok 1.16.12 and Mapstruct 1.2.0.Beta1+.
  • Lombok 1.16.14+ is now tightly coupled with Mapstruct 1.2.0.Beta1+, with the use of the AstModifyingAnnotationProcessor interface, whose source was probably copied in the project from Mapstruct (note that this is the first revision of the source, not the 2nd one - and latest). But the interface, which is compiled during Lombok build process, is not included in the Lombok JAR, leading to a ClassNotFoundException exception at runtime in Eclipse log, and preventing Mapstruct annotation processing.

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
Copy link

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
Copy link

filiphr commented Feb 9, 2018

@eximius313 is this with or without the agent?

@eximius313
Copy link

How to check it?

@softdays
Copy link

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

@v1nc3n4
Copy link

v1nc3n4 commented Jun 6, 2018

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
Copy link
Collaborator

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 mapstruct/mapstruct#1159

@rzwitserloot
Copy link
Collaborator

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
Copy link

ft0907 commented Aug 29, 2018

@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
Copy link

mvmn commented Sep 8, 2018

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: mapstruct/mapstruct#1159 (comment)
Please refer to that comment if you need full stacktrace.

@alexweirig
Copy link

Hi,

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

  • Eclipse 2018.12
  • lombok 1.18.4 added as -javaagent in eclipse.ini
  • mapstruct (1.2.0-Final or 1.3.0-Beta2) added in the Properties -> Java Compiler -> Annotation Processing -> Factory Path in my Java project

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
Copy link

pavelhoral commented Mar 21, 2019

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
Copy link

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
Copy link

filiphr commented May 3, 2019

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
Copy link
Collaborator

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
Copy link

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
Copy link
Collaborator

rspilker commented May 14, 2020

@AxelHeathnet It would if you use the edge release.

@avanathan
Copy link

avanathan commented Sep 2, 2020

@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
Copy link
Collaborator

rspilker commented Sep 17, 2020

@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
Copy link

imod commented Sep 18, 2020

This works with the latest MapStruct Version 1.4.0.CR1

@filiphr
Copy link

filiphr commented Sep 18, 2020

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.

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