Avoid using an invalid JavaPlatform #4672
Conversation
| final JavaPlatformManager man = JavaPlatformManager.getDefault(); | ||
| if (select.getSpecification().getVersion() != null) { | ||
| for (JavaPlatform p : JavaPlatformManager.getDefault().getInstalledPlatforms()) { | ||
| if (!"j2se".equals(p.getSpecification().getName()) || p.getSpecification().getVersion() == null) continue; | ||
| if (!p.isValid() || !"j2se".equals(p.getSpecification().getName()) || p.getSpecification().getVersion() == null) continue; | ||
| if (p.getSpecification().getVersion().compareTo(select.getSpecification().getVersion()) > 0) { | ||
| select = p; | ||
| } |
There was a problem hiding this comment.
the fix makes sense. But I believe this whole section needs some work.
E.g if the default platform has no spec version, it would simply select the default platform without searching further?
how about:
final JavaPlatformManager man = JavaPlatformManager.getDefault();
JavaPlatform select = Stream.of(man.getInstalledPlatforms())
.filter(JavaPlatform::isValid)
.filter((p) -> "j2se".equals(p.getSpecification().getName()))
.filter((p) -> p.getSpecification().getVersion() != null)
.max((p1, p2) -> p1.getSpecification().getVersion().compareTo(p2.getSpecification().getVersion()))
.orElse(JavaPlatform.getDefault());edit: maybe we should even add a cap to not pick a version which goes beyond (nb-)javac. Getting the feature version of (nb-javac) is easy: SourceVersion.latest().ordinal(). The SpecificationVersion is just a bit limited at the moment.
SpecificationVersion maxVersion = new SpecificationVersion(SourceVersion.latest().ordinal()+".99");
...
.filter((p) -> p.getSpecification().getVersion().compareTo(maxVersion) < 0)this would filter out JDK 20 for example on master, but pick 19
There was a problem hiding this comment.
f the default platform has no spec version, it would simply select the default platform without searching further?
Yes, I see that; but is it wrong?
I'm not familiar with this code. Your suggestion makes sense in principle; but I don't know, and can't readily discern, if there might be some significance to a default platform with no spec.version. Clearly the original author made a decision about how to handle this. I'm uncomfortable changing the behavior of code paths I don't grok.
not pick a version which goes beyond (nb-)javac
What would be the implication here if jdk-20 was picked and nb-javac only supported 19?
About the Stream.concat: isn't it required that the default platform be an installed platform?
Once I new how to fix the hints for my usage, it became a puzzle. I tried to see if I could find the spot without a lot of debugger action. I got lucky. I'm OK with closing this PR if you want to play with it, and would certainly watch for any discussion/changes. But I'm not qualified, without lots of effort, to go beyond avoiding a !valid platform
BTW, thanks for the Stream implementation. I've done little work with them. The more I work through some examples the better.
There was a problem hiding this comment.
f the default platform has no spec version, it would simply select the default platform without searching further?
Yes, I see that; but is it wrong?
my inspector gadget senses tell me that the loop appears to be there to select the most recent JDK. The null check might have been added to prevent a NPE in the loop, I can't imagine that it is supposed to ignore all configured platforms if the platform NB was started on doesn't have a spec version for whatever reason - but I might be wrong.
not pick a version which goes beyond (nb-)javac
What would be the implication here if jdk-20 was picked and nb-javac only supported 19?
Its an untested configuration and usually breaks things (netbeans expects having a javac of a certain version). NB also doesn't work very well if the runtime JDK is newer than nb-javac. In this case its better to uninstall nb-javac and hope for best. This situation would be similar. If nb-javac is uninstalled and NB would be started on JDK 20, SourceVersion.latest().ordinal() would return 20 and it would be selected. With nb-javac, it would cap at 19 which is probably safer.
Once I new how to fix the hints for my usage, it became a puzzle. I tried to see if I could find the spot without a lot of debugger action. I got lucky. I'm OK with closing this PR if you want to play with it,
nono don't close - you did all the work. I can open a followup PR to add the upper bound check later.
About the Stream.concat: isn't it required that the default platform be an installed platform?
the original code sets JavaPlatform select = JavaPlatform.getDefault(); first. After that it checks if any of the getInstalledPlatforms() are better choices. The Stream picks the best installed platform first, and if there is none it falls back to default platform. Same result just other way around. Btw I might have edited the snipped while you replied - sorry for the confusion.
mbien
added
Java
This fixes #4651 in my case.