Skip to content
This repository has been archived by the owner on Jul 30, 2021. It is now read-only.

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

Open
wants to merge 4 commits into
base: master
Choose a base branch
from

Conversation

matteo-gallo-bb
Copy link

@matteo-gallo-bb matteo-gallo-bb commented Mar 27, 2018

@michaelkrog
Copy link

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

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

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

@anejnz
Copy link

anejnz commented Sep 5, 2018

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

@Shredder121
Copy link
Member

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

@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.

matteo-gallo-bb added 2 commits September 19, 2018 17:12
…ces. Adding maven compiler source and target Java 6.
…:file:/temp/boot.jar!/" indicating the Root folder of the Jar. Updated unit test.
@jkuhnert
Copy link

jkuhnert commented Oct 8, 2018

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

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

jkuhnert commented Oct 8, 2018

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

jkuhnert commented Oct 8, 2018

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

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

wjans commented Jul 29, 2019

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

@anejnz
Copy link

anejnz commented Aug 22, 2019

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

@matteo-gallo-bb
Copy link
Author

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

idosal commented Nov 25, 2019

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

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

@F43nd1r
Copy link
Member

F43nd1r commented Jul 8, 2021

@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.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

9 participants