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
10Fixed
Related Reports
Blocks :  
JDK-8193450 - Port changes to java and native files from JDK-8192806 to FX 8u
Relates :  
JDK-8170730 - javac always generates JNI header regardless of source change
Relates :  
JDK-8182758 - JEP 313: Remove the Native-Header Generation Tool (javah)
Relates :  
JDK-8170591 - Modification of single java file in javafx.web causes recompilation of all other files
Description
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.
Comments
fixed by http://hg.openjdk.java.net/openjfx/jfx-dev/rt/rev/90773bfb2a58
05-12-2017
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
05-12-2017
The .02 version looks good. +1
05-12-2017
Sorry, this is fixed in http://cr.openjdk.java.net/~jvos/8192806/webrev.02/
05-12-2017
You are missing the import of java.lang.annotation.Native in the two .java files, so it won't compile as is.
05-12-2017
new webrev is at http://cr.openjdk.java.net/~jvos/8192806/webrev.01/ This adds the @Native annotation and fixes the whitespace issue.
05-12-2017
Looks good to me. Thanks.
04-12-2017
Yes, that would be best.
01-12-2017
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?
01-12-2017
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.
01-12-2017
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.
01-12-2017
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.
01-12-2017
webrev is uploaded to http://cr.openjdk.java.net/~jvos/8192806/webrev.00/
01-12-2017
[~arajkumar] Can you also review this, once it is posted?
01-12-2017
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.
01-12-2017
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?
01-12-2017
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.
30-11-2017