Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Fix #38 JSPC compile fails on JDK11 without compilerSourceVM and compilerTargetVM options. #39

Merged
merged 4 commits into from
May 9, 2022

Conversation

…eVM and compilerTargetVM options.

Signed-off-by: kaido207 <kaido.hiroki@fujitsu.com>
@joakime
Copy link
Member

joakime commented Dec 17, 2021

Skip all of the minimum jvm checks, they are wrong for Java 11+. Best to just let javac fail.

Food for thought.

If you are on Java 11 runtime, which this version of wasp has set as a minimum support level, then you only have Java source/target of 6 (or 1.6) as a minimum.
If you are on Java 17 runtime, which this version of wasp has to certify via the TCK on, then you only have Java source/target of 7 (or 1.7) as a minimum.

I'd be more curious as to what would happen if we instead had NO predefined or hard-coded source/target defined.
Make it use the runtime source/target (just like javac does).
This doesn't mean remove support for defining source/target if the user wants to, but why hard code it in the first place?

… java spec version as default value

Signed-off-by: kaido207 <kaido.hiroki@fujitsu.com>
@kaido207
Copy link
Contributor Author

@joakime Completely agree with you. I'd changed my proposal.

…getVM.

Signed-off-by: kaido207 <kaido.hiroki@fujitsu.com>
@kaido207
Copy link
Contributor Author

@joakime Is there anything else I can do?

@arjantijms
Copy link
Contributor

It looks good to me, if @joakime doesn't reply we can merge it by lazy consensus.

@joakime
Copy link
Member

joakime commented Jan 13, 2022

I'm in a mental fog (covid). Do what you want here.

@kaido207
Copy link
Contributor Author

Are there any other suggestions to merge this patch?

@arjantijms
Copy link
Contributor

@kaido207

Are there any other suggestions to merge this patch?

There's a merge conflict now. Sorry, can you reapply the patch?

Signed-off-by: kaido207 <kaido.hiroki@fujitsu.com>
@kaido207
Copy link
Contributor Author

kaido207 commented May 9, 2022

@arjantijms Sure, I done.

@arjantijms
Copy link
Contributor

@kaido207 thanks!

@arjantijms arjantijms merged commit 18445b1 into eclipse-ee4j:master May 9, 2022
@arjantijms arjantijms added this to the 3.1.0 milestone May 9, 2022
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

JSPC compile fails on JDK11 without compilerSourceVM and compilerTargetVM options.
3 participants