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

Handle classfiles with too new versions. #3598

Merged
merged 1 commit into from
Mar 21, 2022

Conversation

jlahoda
Copy link
Contributor

@jlahoda jlahoda commented Feb 13, 2022

javac normally refuses to load classfiles that are too new (unlike old nb-javac). Classfile versions change fairly frequently, but javac can often load the newer classfiles anyway. So let's try to load them, as did nb-javac. (It will be a bit slower to read the too-new classfile, but I think that is mostly fine.)

Based on a report from here:
https://lists.apache.org/thread/gtv4nnhvjtzqrk8zmsojj70blw0n18yv

@jlahoda jlahoda added the Java [ci] enable extra Java tests (java.completion, java.source.base, java.hints, refactoring.java, form) label Feb 13, 2022
@jlahoda jlahoda added this to the NB14 milestone Feb 13, 2022
@mbien
Copy link
Member

mbien commented Feb 13, 2022

awesome :)
nitpick: typo in commit msg and PR title. two -> too

Copy link
Member

@mbien mbien left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ran some manual tests and I could open and edit ea JDK 19 loom/panama projects while using nb-javac and running NB on JDK 17. Auto completion, rename refactoring, jackpot scripts etc all worked with no exceptions in the log.

@mbien
Copy link
Member

mbien commented Feb 14, 2022

There will be another rc for NB 13 on Tuesday. From a certain perspective this could be seen as a bug fix. Might be worth to rebase to delivery.

@jlahoda jlahoda changed the title Handle classfiles with two new versions. Handle classfiles with too new versions. Feb 14, 2022
@jlahoda
Copy link
Contributor Author

jlahoda commented Feb 14, 2022

There will be another rc for NB 13 on Tuesday. From a certain perspective this could be seen as a bug fix. Might be worth to rebase to delivery.

I can rebase, I think, but I'll leave it up to @neilcsmith-net is he would be interested to include this in NB 13. To me, seems it could wait for the next version, though.

@jlahoda
Copy link
Contributor Author

jlahoda commented Feb 14, 2022

(Thanks for testing and spotting the typo, BTW!)

@neilcsmith-net
Copy link
Member

Well, I've no more say than anyone else what goes in NB13! However, I do have reservations about dropping this in so late. I'm also curious what the implications are with things like Compile on Save? Hard fail with workaround in NB13, and aiming to look at a better experience for this in NB14 seems a good move to me.

@matthiasblaesing
Copy link
Contributor

This was proposed to NB13, anyone objecting merging it?

@lkishalmi
Copy link
Contributor

Got approved, tests seems to be happy. I'm merging this one.

@lkishalmi lkishalmi merged commit c204921 into apache:master Mar 21, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Java [ci] enable extra Java tests (java.completion, java.source.base, java.hints, refactoring.java, form)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants