search

querydsl / codegen

Java/Scala Code generation tool
Apache License 2.0
23 stars 22 forks source link

Code generation support for application running in a fat JAR #43

Open matteo-gallo-bb opened 6 years ago

matteo-gallo-bb commented 6 years ago

28 Fixed classpath build when the classloader contains jar URIs. MemFileManager can now list Java class files that are packaged in a JAR.

This PR aims to fix the bugs:

Side note Most of the help for this code came from here: http://atamur.blogspot.ie/2009/10/using-built-in-javacompiler-with-custom.html

michaelkrog commented 6 years ago

@Shredder121 Is there anything holding this PR back from being merged? (The failed check seems to be a build setup issue related to JDK 7)

matteo-gallo-bb commented 6 years ago

I have the same question. When I created the PR I added @Shredder121 as a reviewer, but I didn't have any feedback from him. As you said the failed check is not related to the code that I wrote, because the company where I'm working is using the jar created from this branch and it's solving the issues described in the PR description.

michaelkrog commented 6 years ago

@matteo-gallo-bb It can confirm that this PR fixes #42.

anejnz commented 6 years ago

Can someone please merge this and release a new version, I have tested the snapshot as well, and it works :)

Shredder121 commented 6 years ago

The only thing I'm not comfortable with is the "Copyright The Querydsl Team", while the code is taken from the article.

In fact I just took the code from that article,

Which is taken from another article.

It's downloadable from IBM developerWorks, and can be freely modified, by the looks of it, but I haven't seen any copyright headers in the java source files. (I assume this means it inherits the same terms.) Would it be much to ask to at least mention the name of the original author, and to indicate that this is a derivative work?

matteo-gallo-bb commented 5 years ago

@Shredder121 The code is not exactly copied actually, there are small changes, especially one to make it work with Spring Boot with a jar in a jar scenario. Anyways for sure I'll mention the original author and that it's a derivative work. I'll do the changes and commit.

jkuhnert commented 5 years ago

Two small notes on this PR:

1) I tried this out on my own spring boot project and it still failed for me not using an uber-jar. (Said it couldn't open a class file with a path that is clearly valid and should be resolved) 2) Making spring-boot use jetty instead of tomcat fixed the classpath problem for me when running integration tests https://www.mkyong.com/spring-boot/spring-boot-jetty-as-embedded-server/

Shredder121 commented 5 years ago

but are you using the spring-boot plugin, or how do you package it up? Might be the tomcat webapp classloader not supporting nested jars or laying them out differently when, as a lib, you want to ask for the classes, and the next classloader, etc. Do you have an idea why jetty does work? Or could you find out?

jkuhnert commented 5 years ago

Gah, never mind. I just ran the uber-jar manually from command-line using java -jar and it still failed the same way with jetty. Oh well.

jkuhnert commented 5 years ago

Actually ok, so the final verdict is that this PR does fix this issue for me when running in uber-jar now. I only needed jetty to make it work during integration test runs for some reason. I guess I'll try to convince my employer to deploy this random jar I got from a random github repo fork for now until you guys figure out what you want to do here. =)

michaelkrog commented 5 years ago

@Shredder121 Is there anything holding this back now? It would be nice to not to have to maintain a custom build just for this. 😄

wjans commented 5 years ago

@matteo-gallo-bb @Shredder121 Any update on this one? I'm having the exact same issue.

anejnz commented 5 years ago

Any update on this one? Can someone merge this to master?

matteo-gallo-bb commented 4 years ago

I can't merge this PR and I applied all the changes that were suggested. Should I just close the PR at this point?

idosal commented 4 years ago

I'd say leave it open for now. We'll look into it for 5.X. Thank you for bringing this to our attention and for all the hard work!

viktorzub commented 3 years ago

Any updates here? @idosal mb you know somebody to merge it?)

F43nd1r commented 3 years ago

@viktorzub codegen now lives at https://github.com/querydsl/querydsl/tree/master/querydsl-codegen. You can test if the issue exists in 5.0.0-M1, and if it does port over the PR and we'll try to get it into the next release.