Support for requires.nb.javac modules with javac.source/target > 8.

[lahodaj]

Currently, when a module sets requires.nb.javac, it cannot have javac.source/javac.target > 8, and the use of --release is disabled for it. This is because the requires.nb.javac is implemented using -Xbootclasspath/p:, which is incompatible with both javac.source > 8 and --release.

As NetBeans now practically only supports JDK 17 as the build/runtime JDK, this is becoming increasingly troublesome. JDK 17 includes things like Text Blocks, which would be immensely useful for writing tests, but we cannot use that because javac.source is stuck on 8 for the javac-based modules.

There are multiple ways out of this, but the one in this patch is that we run the compilation with --limit-modules setup in such a way that java.compiler and jdk.compiler are disabled. The correct javac is then pulled from the classpath as any ordinary library. This is a bit tricky, as it means we need to list modules that should be enabled. This is achieved using a custom Ant task, which may run a probe on the target JDK, determining modules that are neither java.compiler nor jdk.compiler, and do not depend either of these.

Some alternatives:

• using --patch-module, and replace the content of the module (which may be tricky, as this does not remove the all module content)
• having more precise dependencies on JDK modules inside project.xml, and specifically setup the system module paths to only include these modules. This would work, but is a lot of more work. So, while we may do that in the long run, the patch here still seems reasonable for now to me.

There is an obvious relation to #7188 - either this or that patch will need tweaks to accommodate changes from the other. I am completely fine with doing that here, assuming there's a reasonable timeline.

[mbien]

sounds like this would fix #6817? awesome! Thanks for looking into this.

[mbien]

I like this solution. It has a very small footprint and is only active for modules which request nb-javac.

As mentioned in the linked issue I investigated a bit the option of making a java module out of nb-javac and patch the jdk with it, which sounded like the closest thing to a boot classpath, but this approach is more elegant.

[mbien]

is the input stream intentionally left open?

[ebarboni]

make sense for NB22 ?

[mbien]

I suggest the following merge order:
NB 22:

• NB Build: Basic support for javac.release property. #7188
• Support for requires.nb.javac modules with javac.source/target > 8. #7201 (this PR)

NB 23:

• Use javac release option where possible #7247 (or some similar PR)

This gives devs a chance to update to NB 22 before starting to use the option (since the build would work but NB would break since it would load everything with 1.4 lang level).

cc @matthiasblaesing @neilcsmith-net

[lahodaj]

FWIW - I've merged with master (to get javac.release), and tried adjust this patch for that:
a7e81d3

Also tried to fix apisupport.ant to support the --limit-modules. The effective way is that is asks nb-javac about modules for the target source level, which also includes the data for --release, and hence this is to a degree independent on the runtime platform. This is:
3e88fb0

Please let me know what you think!

[mbien]

the diff below would make the tests pass. It moves the tests to the build-tools job which is JDK 17 and disables some. Feel free to apply to your changes if you want.

diff --git a/.github/workflows/main.yml b/.github/workflows/main.yml
index 8c1fcf4..ac9f4ec 100644
--- a/.github/workflows/main.yml
+++ b/.github/workflows/main.yml
@@ -922,6 +922,10 @@
         if: ${{ matrix.java == '17' }}
         run: ant $OPTS -f extide/o.apache.tools.ant.module test
 
+      - name: apisupport.ant
+        if: ${{ matrix.java == '17' }}
+        run: ant $OPTS -f apisupport/apisupport.ant test
+
       - name: Create Test Summary
         uses: test-summary/action@v2
         if: failure()
@@ -1544,9 +1548,6 @@
       - name: Extract
         run: tar --zstd -xf build.tar.zst
 
-      - name: apisupport.ant
-        run: ant $OPTS -f apisupport/apisupport.ant test
-
       - name: apisupport.project
         run: ant $OPTS -f apisupport/apisupport.project test
 
diff --git a/apisupport/apisupport.ant/nbproject/project.properties b/apisupport/apisupport.ant/nbproject/project.properties
index 03ab15a..eb26886 100644
--- a/apisupport/apisupport.ant/nbproject/project.properties
+++ b/apisupport/apisupport.ant/nbproject/project.properties
@@ -21,39 +21,16 @@
 
 requires.nb.javac=true
 javac.compilerargs=-Xlint -Xlint:-serial
-javac.source=1.8
-javac.release=17
+javac.source=11
+javac.target=11
 javadoc.arch=${basedir}/arch.xml
 
 test-unit-sys-prop.test.nbroot=${nb_all}
 test-unit-sys-prop.test.netbeans.dest.dir=${netbeans.dest.dir}
 test-unit-sys-prop.java.awt.headless=true
 
-test.config.stableBTD.includes=**/*Test.class
-test.config.stableBTD.excludes=\
-    **/AccessibilityQueryImplTest.class,\
-    **/AntArtifactProviderImplTest.class,\
-    **/ApisupportAntUtilsTest.class,\
-    **/AvoidModuleListInProjectConstructorTest.class,\
-    **/BrandingSupportTest.class,\
-    **/BuildZipDistributionTest.class,\
-    **/ClassPathProviderImplTest.class,\
-    **/CustomizerLibrariesTest.class,\
-    **/EvaluatorTest.class,\
-    **/GlobalJavadocForBinaryImplTest.class,\
-    **/GlobalSourceForBinaryImplTest.class,\
-    **/Issue167725DeadlockTest.class,\
-    **/JavadocForBinaryImplTest.class,\
-    **/ModuleActionsTest.class,\
-    **/ModuleListTest.class,\
-    **/ModuleTypePanelTest.class,\
-    **/NbModuleProjectTest.class,\
-    **/ProjectXMLManagerTest.class,\
-    **/SingleModulePropertiesTest.class,\
-    **/SourceForBinaryImplTest.class,\
-    **/SourceLevelQueryImplTest.class,\
-    **/SubprojectProviderImplTest.class,\
-    **/SuiteOperationsTest.class,\
-    **/TestEntryTest.class,\
-    **/UnitTestForSourceQueryImplTest.class,\
-    **/UpdateTrackingFileOwnerQueryTest.class
+test.config.default.includes=**/*Test.class
+test.config.default.excludes=\
+    **/ExternalBuildDirTest.class,\
+    **/UseHtml4JavaTest.class
+
diff --git a/apisupport/apisupport.ant/test/unit/src/org/netbeans/modules/apisupport/project/CompilationDependencyTest.java b/apisupport/apisupport.ant/test/unit/src/org/netbeans/modules/apisupport/project/CompilationDependencyTest.java
index 597b38d..ef778b0 100644
--- a/apisupport/apisupport.ant/test/unit/src/org/netbeans/modules/apisupport/project/CompilationDependencyTest.java
+++ b/apisupport/apisupport.ant/test/unit/src/org/netbeans/modules/apisupport/project/CompilationDependencyTest.java
@@ -88,8 +88,9 @@
         assertFalse("Successfully compiled when is invalid specification version",
                 testingProject.getModuleJarLocation().exists());
     }
-    
-    public void testCompileAgainstPublicPackage() throws Exception {
+
+    // TODO fixme
+    public void fails_on_11_testCompileAgainstPublicPackage() throws Exception {
         NbModuleProject testingProject = TestBase.generateStandaloneModule(getWorkDir(), "testing");
         testingProject.open();
         FileObject buildScript = findBuildXml(testingProject);

[lahodaj]

the diff below would make the tests pass. It moves the tests to the build-tools job which is JDK 17 and disables some. Feel free to apply to your changes if you want.

Thanks! I've applied your patch, except for the change in source/target for apisupport.ant:
ea4246c
Lets see is tests will pass.

[mbien]

@lahodaj although I do like the changes here, I am a bit worried that this could cause unexpected issues. Would it make more sense to merge this early in the NB 23 dev cycle rather than at the end of NB 22?

One nitpick I have is that I am no fan of duplicated code in the repo, we might find a way to copy SetupLimitModulesProbe.java around before merging this.