On Sun, 27 Sep 2026 00:05:21 GMT, Nir Lisker <[email protected]> wrote:

>> Adds the [Java 
>> Toolchain](https://docs.gradle.org/current/userguide/toolchains.html#sec:using-java-toolchains)
>>  to replace manual executions:
>> 
>> | Old | New
>> |------|------|
>> | `JAVA` | 
>> [JavaLauncher](https://docs.gradle.org/current/javadoc/org/gradle/jvm/toolchain/JavaLauncher.html)
>>  |
>> | `JAVAC` | 
>> [JavaCompiler](https://docs.gradle.org/current/javadoc/org/gradle/jvm/toolchain/JavaCompiler.html)
>>  |
>> | `JAVADOC` | 
>> [JavadocTool](https://docs.gradle.org/current/javadoc/org/gradle/jvm/toolchain/JavadocTool.html)
>>  |
>> 
>> ## Instructions for reviewers
>> 
>> The build runs on Windows and Linux (Ubuntu), but the analysis was done on 
>> Windows.
>> 
>> ### JVM detection and selection
>> The toolchain selects the JDKs to use based on all those that are made 
>> available to it. As detailed in the link above, JDKs can come from various 
>> sources on the user machine, including being auto-provisioned. The daemon 
>> JDK is always available (usually `JAVA_HOME`), and `gradle.properties` made 
>> `JDK_HOME` available in #2240.
>> Selection of the toolchain build/worker JDK from all available ones is done 
>> internally (as detailed in the link) based on the restrictions configured in 
>> the toolchain. I have restricted the major version to that of 
>> `jdkVersionInfo` (exact version, not minimum), which is determined by the 
>> *build file's JDK resolution*:
>> 
>> def envJavaHome = cygpath(System.getenv("JDK_HOME"))
>> if (envJavaHome == null || envJavaHome.equals("")) envJavaHome = 
>> cygpath(System.getenv("JAVA_HOME"))
>> def javaHome = envJavaHome == null || envJavaHome.equals("") ? 
>> System.getProperty("java.home") : envJavaHome
>> 
>> 
>> An additional restriction we can add is by vendor so that only the JDKs that 
>> match `JDK_HOME` can be selected.
>> 
>> However **the selected toolchain JDK might not be the same as the resolved 
>> build's JDK**. The toolchain always prefers the daemon JVM to avoid forking, 
>> so if it meets the restrictions, it's selected. Consider the following 
>> scenarios (assuming also vendor matches if that helps):
>> 
>> | Scenario | `JAVA_HOME` | `JDK_HOME` | Selection                            
>>                                   |
>> |-----------|----------------|---------------|------------------------------------------------------|
>> | 1             | 25                  | 26                 | `JDK_HOME` - 
>> only it passes the restrictions       |
>> | 2             | 26                  | 25                 | `JDK_HOME` - 
>> only it passes the restrictions       |
>> | 3             | 25/26 Path A | 25...
>
> Nir Lisker has updated the pull request incrementally with one additional 
> commit since the last revision:
> 
>   Typo
>   
>   Co-authored-by: Marius Hanl <[email protected]>

> First, in 
> [build.properties](https://github.com/openjdk/jfx/blob/master/build.properties)
>  we have
> 
> ```ini
> jfx.build.jdk.version=26.0.2
> jfx.build.jdk.buildnum=10
> jfx.build.jdk.version.min=25
> jfx.build.jdk.buildnum.min=36
> jfx.jdk.target.version=25
> ```
> 
> but the comment there says
> 
> > jfx.build.jdk.buildnum.min should be set to the lowest version that
> > supports building FX (which must be <= jfx.build.jdk.buildnum)
> 
> 36 is not <= 10. @kevinrushforth To my understanding that comment is wrong. 
> The build number cannot be compared between version numbers. Am I reading 
> this right?

The comment is misleading. Build number comparisons are only done if the JDK 
version matches. So 25+34 would be below the minimum of 25+36, but 25.0.1+1 or 
26.0.2+10 are above the minimum.

I've started looking at this PR and done several tests. No blockers so far. 
I'll finish my review soon.

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

PR Comment: https://git.openjdk.org/jfx/pull/2294#issuecomment-6672587410

Reply via email to