JDK-8192806 : Building javafx.web module still requires javah
  • Type: Bug
  • Component: javafx
  • Sub-Component: build
  • Affected Version: 10
  • Priority: P3
  • Status: Resolved
  • Resolution: Fixed
  • Submitted: 2017-11-30
  • Updated: 2017-12-13
  • Resolved: 2017-12-05
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 10
Related Reports
Blocks :  
Relates :  
Relates :  
Relates :  
The build.gradle for OpenJFX still requires javah, in order to generate header files for java.lang.Character and java.net.IDN

Since javah might be removed from JDK 10, this will make it impossible to build OpenJFX using Java 10 as the bootstrap java.
fixed by http://hg.openjdk.java.net/openjfx/jfx-dev/rt/rev/90773bfb2a58

Changeset: 90773bfb2a58 Author: jvos Date: 2017-12-05 16:11 +0100 URL: http://hg.openjdk.java.net/openjfx/jfx-dev/rt/rev/90773bfb2a58 8192806: Building javafx.web module still requires javah Summary: Add required fields to internal sources and use javac -h Reviewed-by: kcr, arajkumar

The .02 version looks good. +1

Sorry, this is fixed in http://cr.openjdk.java.net/~jvos/8192806/webrev.02/

You are missing the import of java.lang.annotation.Native in the two .java files, so it won't compile as is.

new webrev is at http://cr.openjdk.java.net/~jvos/8192806/webrev.01/ This adds the @Native annotation and fixes the whitespace issue.

Looks good to me. Thanks.

Yes, that would be best.

I'll create a new webrev, but I'll wait for Arun's comments first. I guess it is better to commit this after Monday, in the new repository?

Testing looks good (all three platforms). I think this ready to go pending Arun's comments and the fixes for the other two issues I mentioned above.

Testing is in progress. Here are my comments on the patch itself: 1. As mentioned above, please add the '@Native' annotation to all of the new constant fields. 2. I would like Arun to comment on putting the new constants in what used to be a generated file, but is now checked into the repo. It won't cause any problems now, but I'd like his take on whether it might in the future. I am OK with the location if he is. 3. You have trailing whitespace in CharacterDataImpl.java that will cause jcheck to fail (note that it has to be run manually, currently). You can either setup and run jcheck or you can run "bash tools/scripts/checkWhiteSpace" to verify.

I'll test it out shortly. One quick comment is that all of the new fields should be annotated with '@Native' which we now do for other constants that are needed by native code.

webrev is uploaded to http://cr.openjdk.java.net/~jvos/8192806/webrev.00/

[~arajkumar] Can you also review this, once it is posted?

This approach and your proposed locations for the constants looks fine to me. Once you post the webrev I can test it on all three platforms.

The following approach fixes this: * remove the javah exec in build.gradle * add (by reference) the required public static values from Character.java and IDN.java into classes that have their source in OpenJFX and that are not public. * modify UnicodeJava.h to replace java_lang_Character.h with the replacement file * modify IDNJava.cpp in a similar way The remaining question is where to add the constants. I added the Character constants to modules/javafx.web/src/main/native/Source/WebCore/bindings/java/dom3/java/com/sun/webkit/dom/CharacterDataImpl.java and the IDN constant (singular) to modules/javafx.web/src/main/java/com/sun/webkit/network/URLLoader.java Both of these files are already used for javac -h compilation, and the generated headers are already in the include path for the native compilation. I tested this approach on Linux, and it works. I'll create a webrev unless there are objections about the locations of the (referenced) constants?

Given that javah is deprecated for removal, and planned to be removed in JDK 10, I am raising the priority to P3 as this will block upgrading the boot JDK once the removal of javah is pushed into JDK 10.