Do not include jdk.jshell in test mod list by default.

[sdedic]

After PR #3974, the Apache NBLS distribution is unbuildable:

• the IDE build now requires JDK11 (which might be OK, but was not communicated on mailing list in advance, so it is a sort of bad surprise since the build started to fail between vote start and vote end)
• when the build switched to JDK11, it turned out that it still fails

The failure is because of:

 [junit] Test org.netbeans.modules.java.mx.project.SdkSuiteTest FAILED (crashed)
    [junit] Error occurred during initialization of boot layer
    [junit] java.lang.module.FindException: Module jdk.internal.opt not found, required by jdk.jshell

in java/java.mx.project, but essentially in all modules that require nb-javac for propert functionality (requires.nb.javac in their project.properties)

The reason why master builds OK is just that those tests for all those modules (i.e. java/java.editor) are not, and were never run on JDK11. We are now in a peculiar situation, when JDK "x" is required to build the product, but most part of the product is not only not tested on JDK "x", but if the test would execute on JDK "x", they would 100% fail :)

This PR changes the default commandline for junit test so that it excludes jms-config/tools.flags (currently just the jdk.jshell); modules that need tool modules during test may use test.jms.flags property to add them.

This is because --limit-modules is used to exclude most of JDK contents, but specifically jdk.compiler that overlaps with nb-javac classes. As most tests do not run module system (because it's a way slower and more complicated) the classpath-located nbjavac would not be used if jdk.compiler was observable and loaded by JDK 9+. And if jdk.compiler is not observable, --add-module jdk.jshell causes the VM failure during boot layer init.

[neilcsmith-net]

the IDE build now requires JDK11 (which might be OK, but was not communicated on mailing list in advance,

It was communicated ad infinitum in advance. We officially dropped support for building with JDK 11 with NetBeans 13, and the IDE binaries have been built with JDK 11 since 12.6. There were loose ends to tidy up to get all of our CI on JDK 11, hence the delay in that PR which was originally meant for NB13. I should however have mentioned on list that it was merged. It was the point where it became a technical reality.

It looks like there are still a few loose ends to tidy up too!

The NB13 VSCode support was built with JDK 11 too - https://ci-builds.apache.org/job/Netbeans/job/netbeans-TLP/job/netbeans/job/release130/20/artifact/dist/vsix/ And that build job has support for VSCode plugin now - https://ci-builds.apache.org/job/Netbeans/job/netbeans-TLP/job/netbeans/job/vsnetbeans_preview_1303/lastSuccessfulBuild/artifact/dist/vsix/

TL;DR if this is needed to fix tests on JDK 11 👍

[neilcsmith-net]

Possibly rebase to delivery?

[sdedic]

It was communicated ad infinitum in advance. We officially dropped support for building with JDK 11 with NetBeans 13, and the IDE binaries have been built with JDK 11 since 12.6. There were loose ends to tidy up to get all of our CI on JDK 11, hence

The local build using JDK8 actually stopped working only last Friday (?). To that time, I could happily build master using JDK8 environment. As I (+ certain colleagues) and certainly some other people developing netbeans are totally lazy (I call that "conservative") and won't change the work environment (which is often used for other projects, too) unless necessary, as each change is for worse, obviously :)

Yes, the plan was set up. But the timing to actually publish plan's implementation was a little uncoordinated, IMHO. So if pioneering a change that affects the whole, it's better to be louder about it than just merge.

The NB13 VSCode support was built with JDK 11 too - https://ci-builds.apache.org/job/Netbeans/job/netbeans-TLP/job/netbeans/job/release130/20/artifact/dist/vsix/ And that build job has support for VSCode plugin now - https://ci-builds.apache.org/job/Netbeans/job/netbeans-TLP/job/netbeans/job/vsnetbeans_preview_1303/lastSuccessfulBuild/artifact/dist/vsix/

That's interesting !
Anyway, if I just switched JDKs on my local machine, or for the vscode dev build ot Apache CI, the vscode build just failed as recorded in the link above. Most probably because changes that added --add-modules jdk.jshell for all unit tests were added past NB13

[ppisl]

I have checkout this branch and build NetBeans, no problem now. I have run tests in java/java.editor, they are failing on JKD 11, but not due the issue mentioned here.

[mbien]

travis should be working again. Feel free to merge when ready. I left a comment though.

[mbien]

this would mean that running NB from the IDE would set fewer JMS flags than running a NB dist started with the typical netbeans.conf. Do we really want to do that?

[mbien]

in other words: the only difference between metabuild.jms-flags and metabuild.jms-flags.jvm is that one has a -J prefix for each flag the other has not. IMO: both flag list should not diverge in content.

Maybe we shouldn't have two properties, it might be better to have just one, and build a second one (taking the first as input) just before its inserted into netbeans.conf to make the relationship more clear.

[sdedic]

@mbien yes we do want that: with real module system in action (as it is the case at runtime), the correct classes (from NBJavac) are loaded despite the JDK modules present.

[mbien]

@sdedic the problem is that metabuild.jms-flags.jvm will be pasted into maven templates too in future, while the other property with -J is pasted to the conf.

example: apache/netbeans-mavenutils-archetype-nbm-archetype#9

Do we need a third property only for testing? Originally, there was only supposed to be one property, the only reason there are two is the -J prefix - this already is a fairly ugly workaround IMO.

one for CI, one for nb.conf, one for maven run config?

[sdedic]

@mbien what purpose is/will be this "maven run config" used for: this 'special exclusion' should affect surefire plugin, but not exec plugin.

[mbien]

@sdedic it is intended to be used as an initial set of flags which allow users to press play on a newly created platform project and it is going to work even if they added a few features. Users can then optimize the list and only include what is actually required to run their application.

If surefire will need a different set of flags than the exec plugin... well that complicates things even further. I hoped we could use one big list for everything - as initial default. How is the user supposed to know what to remove for the surefire plugin list?

I guess we could setup two lists by subtracting a few items from the first list? Maybe we could add the -J just before text substitution so that we don't need a third list.

as mentioned in the readme (https://github.com/apache/netbeans/blob/master/nbbuild/jms-config/README). "There should be hopefully fewer flags over time, devs are encouraged to shrink the list if possible."
I hope the list won't grow at least :)

[sdedic]

Anyway, the current state is fundamentally broken: it can not add jdk.jshell module while using --limit-modules at the same to exclude jdk.compiler and other jdk.jshell depenendencies. The set of flags is different already: the --limit-modules is only used for runnign tests.

[mbien]

sounds like we need another list for tests which use --limit-modules, or as mentioned, it might be possible to simply remove jdk.shell flags from the list before use in tests, assuming its the only special case.

[mbien]

but looking at the actual list https://github.com/apache/netbeans/blob/master/nbbuild/jms-config/tools.flags

its probably ok to exclude it by default, most platform apps won't notice anyway. Lets merge this PR :)

[mbien]

"--add-modules=jdk.jshell" shouldn't technically be in the list anyway, since its a list of encapsulation exceptions, not a list of dependencies.