Update JDK version requirement warning

[mbien]

as previously discussed in #5852 (comment), NetBeans needs to inform the user if the runtime JDK does not meet the requirements. The current warning instructed the user to install Java 7, which is a bit outdated.

[mbien]

oh yeah I forgot that this will also cause all tests to fail which can't run on JDK 11 yet (#4904)

[neilcsmith-net]

oh yeah I forgot that this will also cause all tests to fail which can't run on JDK 11 yet (#4904)

I'd planned to look at a similar change before NB19, but was going to start a discussion because of the potential test issues! 😄

One answer might be to add a boolean system property onbootstrap.noJDKCheck ? Could be used for still pending tests, and also in any other case people are adamant they want to run the runtime container on JDK 8 and damn the consequences.

[neilcsmith-net]

... and, sorry, -0.5 to the update just added - whatever the right way to switch this is, I think it needs to be external logic not internal (whether that be system property, environment variable, or something else).

[mbien]

... and, sorry, -0.5 to the update just added - whatever the right way to switch this is, I think it needs to be external logic not internal (whether that be system property, environment variable, or something else).

The problem with flags is that they become cheat codes for running NB on unsupported configs. But I can change it to a property since it isn't working anyway.

We should remove this once tests are fixed.

[matthiasblaesing]

We have the system property netbeans.full.hack: https://netbeans.apache.org/wiki/DevFaqNetBeansFullHack. It might be the right choice to disable the checks.

[mbien]

let me think about this a bit. The netbeans.full.hack property isn't set everywhere, for example if I run BuildZipDistributionTest (apisupport/apisupport.ant) in the IDE on JDK 8, it won't work. Adding the property to more tests just to solve this issue here is risky since it changes NB behavior. So I would be slightly in favor for adding a new property which isn't used for anything else and remove it again once the tests are fixed on JDK 11.

as a sidenote:
the devhack is fairly problematic since it caused already issues more than once (most recent) during PR review where something worked when it was set, but not in release configuration. I would highly recommend to remove it from here when you run test builds or NB from the IDE.

[mbien]

updated to netbeans.full.hack, since only two tests needed adjustments when platform/o.n.bootstrap tests are not run on JDK 8. This should be ok since the tests are repeated on JDK 11 in the same job.

[neilcsmith-net]

The problem with flags is that they become cheat codes for running NB on unsupported configs.

That is literally what I'm suggesting though! 😄

Bear in mind I was originally in the "raise the default bytecode level across the codebase" camp! But what we agreed was that we'd raise bytecode level as and when necessary. I'm sure @matthiasblaesing made a comment about lower level things like o.n.bootstrap being later down the chain? I can't find it right now, but anyway ...

We won't support the runtime container on JDK 8 from NB19, and on JDK 11 from NB22. This code should by default error out at those levels with those releases. But, that doesn't mean we will bump the bytecode level of o.n.bootstrap with those releases as far as I'm aware. While the bootstrap will technically run, then we should provide an escape hatch to turn the error into a warning. That should be logged, but otherwise allow the launch to proceed. Yes, that might fail later, but that's the point.

That's my 2c on approach to this anyway - allows opt in for tests and any other nefarious purpose.

PS. I'm also against requiring netbeans.full.hack for this, although probably should also act to switch the error to warning, it also has other side effects we might not want.

[mbien]

The problem with flags is that they become cheat codes for running NB on unsupported configs.

That is literally what I'm suggesting though! smile

I don't advise doing this. NB either runs on JDK 8 or it doesn't. Adding a flag which can be used in production to run on unsupported/untested platforms doesn't sound like a good idea to me.

PS. I'm also against requiring netbeans.full.hack for this, although probably should also act to switch the error to warning, it also has other side effects we might not want.

The fullhack flag shouldn't be used in production (and in my opinion also not for manual tests, or tests where it is not needed). Turning off requirements checks should also not be done in production. I too am not a fan of the flag itself, but it does overlap somewhat with the "disable requirement checks for testing purposes" usecase.

[neilcsmith-net]

I don't advise doing this. NB either runs on JDK 8 or it doesn't. Adding a flag which can be used in production to run on unsupported/untested platforms doesn't sound like a good idea to me.

If it was aimed at IDE users I might agree with you, but this is about the core of the platform, and I'm thinking of IDE and platform developers. And primarily for development and testing time purposes, given we'll be bumping this up again to Java 17 in the next 12 months. If a platform developer really wanted to ship something in production with this flag that's their call - it's a bad idea, we obviously don't support that, but they should be grown up enough to understand the ramifications and make that call for themselves. Same as other flags that have no place in production code.

Shipping an IDE that requires a bunch of --add-opens and --add-exports into the internals of the JDK isn't exactly following what's supported either, but I'm for sure happy that the JDK has externally configurable escape hatches! 😄

[mbien]

should be working now. The mx.project build is failing since some dependencies just went offline.

e.g https://lafo.ssw.uni-linz.ac.at/pub/graal-external-deps/ninja_syntax-1.7.2.tar.gz

edit: looks like Uni Linz is online again

[neilcsmith-net]

@mbien current changes look good to me at a glance. Are you ready to move this out of draft? I really think this should be in NB19!

[mbien]

@neilcsmith-net renamed a method to make it clearer what it does. Ready to merge i think.

[mbien]

@matthiasblaesing @neilcsmith-net everyone ok with this?

[matthiasblaesing]

Looks sane to me.