On Wed, 12 Jun 2024 20:03:24 GMT, Kevin Rushforth <k...@openjdk.org> wrote:

> This PR updates `build.gradle` to define javac lint options for three 
> different types of java compilation tasks: sdk classes, test classes 
> (including shims), and tool classes (including JSLC). The defaults for these 
> three groups of lint options are specified in `build.gradle`.
> 
> We also define three gradle properties that can be used at build time to 
> override the global lint options for each of the three categories of tasks as 
> follows:
> 
> * `LINT` - used when compiling sdk tasks : default = 
> "removal,missing-explicit-ctor"
> * `TOOL_LINT` - used when compiling tool tasks : default = ""
> * `TEST_LINT` - used when compiling test tasks : default = ""
> 
> Each property takes a string with a comma-separated list of lint options. For 
> example:
> 
> 
> gradle -PLINT="deprecation,removal" sdk
> 
> 
> I provided a build mechanism to add extra per-module lint options in 
> `build.gradle`. None are currently defined, but I tested the logic to show 
> that it can be. I did not provide a way to override the per-module lint 
> options via the command line. Once we start using them, we might consider 
> adding that.
> 
> While testing this PR, I ran into a few places where we need to suppress 
> removal warnings for use of our own internal classes that are terminally 
> deprecated, so I filed 
> [JDK-8334143](https://bugs.openjdk.org/browse/JDK-8334143) to track this, and 
> added the missing annotations in this PR.
> 
> I also tested with JDK 23 to verify that we are now getting the expected 
> warnings for the use of sun.misc.Unsafe, and filed two bugs 
> --[JDK-8334138](https://bugs.openjdk.org/browse/JDK-8334138) to (temporarily) 
> suppress those warnings in this PR and 
> [JDK-8334137](https://bugs.openjdk.org/browse/JDK-8334137) to eventually fix 
> them by replacing our use of sun.misc.Unsafe.

Looks good, tested several combination of options. Have one minor query, added 
as comment.
An observation to mention: 
In setupLintOptions the options are separated and provided to javac 
individually. though the comma separated list can be provided at once.
I tested that a single comma separated list works, but in case of any invalid 
option the error message is not clear (It prints all the options in list). But 
when options are provided individually then error points out exact invalid 
option.

build.gradle line 4139:

> 4137:             }
> 4138:         } else if (compile.name == "compileJslcJava" ||
> 4139:             compile.name == "compileToolJava") {

Should we include the _compileToolsJava_ task from media too ?

-------------

Marked as reviewed by arapte (Reviewer).

PR Review: https://git.openjdk.org/jfx/pull/1474#pullrequestreview-2125657996
PR Review Comment: https://git.openjdk.org/jfx/pull/1474#discussion_r1644548174

Reply via email to