mbien commented on code in PR #4672:
URL: https://github.com/apache/netbeans/pull/4672#discussion_r977178060
##########
java/spi.java.hints/src/org/netbeans/modules/java/hints/spiimpl/Utilities.java:
##########
@@ -1083,7 +1083,7 @@ public synchronized ClasspathInfo createUniversalCPInfo()
{
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;
}
Review 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.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
For further information about the NetBeans mailing lists, visit:
https://cwiki.apache.org/confluence/display/NETBEANS/Mailing+lists
