JDK-8161704 : Switch to Jigsaw-aware boot JDK for compiling FX 9
  • Type: Bug
  • Component: javafx
  • Sub-Component: build
  • Affected Version: 9
  • Priority: P2
  • Status: Resolved
  • Resolution: Fixed
  • Submitted: 2016-07-19
  • Updated: 2017-01-18
  • Resolved: 2016-11-08
The Version table provides details related to the release that this issue/RFE will be addressed.

Unresolved : Release in which this issue/RFE will be addressed.
Resolved: Release in which this issue/RFE has been resolved.
Fixed : Release in which this issue/RFE has been fixed. The release containing this fix may be available for download as an Early Access Release or a General Availability Release.

To download the current JDK release, click here.
JDK 9
9Fixed
Related Reports
Blocks :  
Blocks :  
Blocks :  
Blocks :  
Blocks :  
Blocks :  
Blocks :  
Blocks :  
Blocks :  
Relates :  
Relates :  
Relates :  
Relates :  
Relates :  
Relates :  
Relates :  
Relates :  
Relates :  
Relates :  
Relates :  
Relates :  
Relates :  
Description
We currently use JDK 9 build 109 as our boot JDK to build FX 9, and use a separate Jigsaw JDK to build the javapackager and to run tests in "Jigsaw" mode. We need to upgrade to building with a Jigsaw-aware boot JDK for several reasons.

1) This will remove the restriction in the current upgrade process for the Jigsaw-based JDK used by the packager where we have to disable the packager for one week (this cases all SQE tests to fail and makes the packager unavailable for a week).

2) We cannot use any APIs that have changes since JDK 9 build 109. In some cases we can work around this with reflection, but in some cases we are stuck.

3) We are blocked from fixing any bugs that depend on a newer JDK (e.g., JDK-8157508).

Related to the above, we cannot catch many errors at compile time today; rather we only hit the problems at runtime.

4) Testing in "legacy mode" with jfxrt.jar can mask problems that are introduced (our mitigation for this has been to build the JDK locally and do some testing in "Jigsaw" mode).

Comments
Changeset: 7da620ef965e Author: ddhill Date: 2016-11-08 09:32 -0500 URL: http://hg.openjdk.java.net/openjfx/9-dev/rt/rev/7da620ef965e 8161704: Switch to Jigsaw-aware boot JDK for compiling FX 9 Reviewed-by: kcr, ckyang, vadim
08-11-2016

All looks good now. +1
08-11-2016

+1
07-11-2016

One new issue found: there is a latent issue in the module-info.java file of jdk.packager. It is missing a "requires java.logging" statement even thought jdk.packager uses packages from this module. A recent change to javac to not automatically add a module to the module graph just because it appears in a qualified export statement has exposed this bug. Here is the patch that will fix this. diff --git a/modules/jdk.packager/src/main/java/module-info.java b/modules/jdk.p ackager/src/main/java/module-info.java --- a/modules/jdk.packager/src/main/java/module-info.java +++ b/modules/jdk.packager/src/main/java/module-info.java @@ -30,6 +30,7 @@ requires java.xml; requires java.desktop; + requires java.logging; exports com.oracle.tools.packager; exports com.sun.javafx.tools.packager;
07-11-2016

Webrev: http://cr.openjdk.java.net/~ddhill/8161704.1 Note: this is just the review changes.
07-11-2016

Vadim, > There seems to be a leftover FIXME I think Kevin got these out in his follow up review, converting some to JBS. > Re-running gradle on freshly built sdk still calls :graphics:compileJava, Will have to look at that. must have a loose files issue in gradle > WARNING: module-info.class not allowed in patch: C:\Vadim\fx9\rt\modules\javafx.graphics\build\classes\shims\javafx.graphics Nothing we can do about these, javac creates them, but update-module-patch needs to warn that they are getting ignores. Rather annoying though. > Gradle 3.1 folder is a hudson build artifact, allowing us do download a copy instead of installing it on each of the slaves > apps/toys/Industrial/ argh, I thought I had reverted those.
07-11-2016

This looks good, although there are some minor cosmetic issues. There seems to be a leftover FIXME at the first line of build.gradle and netbeans/systemTests/nbproject/project.properties You've removed JAVAH from the comment at the line 292 but it's still possible to specify it and it's apparently is still used so it's slightly confusing. Re-running gradle on freshly built sdk still calls :graphics:compileJava, I think at least performance enhancement should be filed. :controls:test WARNING: module-info.class not allowed in patch: C:\Vadim\fx9\rt\modules\javafx.graphics\build\classes\shims\javafx.graphics WARNING: module-info.class not allowed in patch: C:\Vadim\fx9\rt\modules\javafx.controls\build\classes\shims\javafx.controls WARNING: module-info.class not allowed in patch: C:\Vadim\fx9\rt\modules\javafx.base\build\classes\shims\javafx.base I don't see gradle-3.1 folder in the root, is it platform-dependent? It's mentioned in the hgignore. There's something fishy in the apps/toys/Industrial/nbproject/build-impl.xml, there're changes from "Industrial" to "TouchSuite" Also the apps/toys/Industrial/nbproject/project.properties has strange "file.reference.TouchSuite-src=src" Seems to be the only project which was opened and generated NB files were rebuilt.
07-11-2016

All my testing and code review looks good. There have been a few cleanup fixes pushed to the sandbox, so my final approval is pending a sanity check of the final patch and delta-webrev on Monday.
04-11-2016

+1.
04-11-2016

The IOS files need more work anyway, so they were intentionally skipped. Thanks for catching the leftover jfxrt.jar in netbeans/systemTests -- I had put a "FIXME" comment in, but hadn't gotten to it yet.
03-11-2016

Overall looks good to me. I was able to do a complete clean dev-build-full on Windows and Mac with your change. I have just one minor comment in case you have missed this one. There is still a reference to jfxrt.jar in systemTests project: ./rt/netbeans/systemTests/nbproject/project.properties:file.reference.jfxrt.jar=../../build/sdk/lib/jfxrt.jar I also noticed similar jfxrt.jar reference in iOS projects but I believe that was intentionally not to fix: ./rt/tools/ios/Maven/NetBeansMobileCenter/pom.xml: <exists>${java.home}/lib/ext/jfxrt.jar</exists> ./rt/tools/ios/Maven/NetBeansMobileCenter/pom.xml: <jfxrt.jar>${java.home}/lib/ext/jfxrt.jar</jfxrt.jar> ./rt/tools/ios/Maven/NetBeansMobileCenter/pom.xml: <exists>${java.home}/lib/jfxrt.jar</exists> ./rt/tools/ios/Maven/NetBeansMobileCenter/pom.xml: <jfxrt.jar>${java.home}/lib/jfxrt.jar</jfxrt.jar> ./rt/apps/toys/iOS/iPodAccessTest/nbproject/project.properties:file.reference.jfxrt.jar=../../../../build/ios-sdk/lib/jfxrt.jar ./rt/apps/toys/iOS/SensorsTest/nbproject/project.properties:file.reference.jfxrt.jar=../../../../build/ios-sdk/lib/jfxrt.jar
03-11-2016

Webrev: http://cr.openjdk.java.net/~ddhill/8161704/ Kevin, Chien, Vadim, There is a lot of change in this webrev, but there are a lot of common patterns (or sets of changes) ./apps - the majority of the change is altering the NB project files to use rt/build/compile.args and run.args via a relative path. This is a replacement action for using jfxrt.jar (and something I will address in a follow on email) Also, a new pattern for apps that need --add-exports lines to run properly (like Robot), we have added a file: apps/tests/HelloTest/addExports that will used as an @argfile with the run command. This allows greater visibility into the added command than hiding them in the project.properties files. Under ./modules/ there is two classes of renames module-info/module-info.java --> java/module-info.java test/java/com --> shims/java/com Module-info.java is needed by javac, and so moving into the "main" set makes sense. It does mean however that IDEs may get upset because this is JDK9 syntax. The "shim" classes were mixed in with the general test classes. No more. Now they are in the "shim" set to allow us to build them as part of a rebuild of the main module classes to create a shim-ed module for testing. It also means we don't have to play include/exclude games when building. Under packager, there is a different class of move/renames, creating " src/antplugin". These sources are used to build a separate jar for use with ant. We needed to move these out of the main set because the would cause javac failure - but gained an additional benefit of a clear separation of sources. What is left is changes related to: ripping out build/sdk adding in a modular build creating rt/build/*.args argfiles to ease development. A big notes: we had to change the build target to 1.9 javac.source=1.9 javac.target=1.9 because in many of the places javac would baulk at using the JDK9 commands without it. It is expected that the current IDEs may not like that.
03-11-2016