On Wed, 12 May 2021 17:30:20 GMT, Crazyjavahacking <github.com+1445818+crazyjavahack...@openjdk.org> wrote:
>> This PR fixes the gradle deprecation warnings described in >> [JDK-8240336](https://bugs.openjdk.java.net/browse/JDK-8240336) and updates >> the JavaFX build to use gradle 7.0.1 as described in >> [JDK-8263760](https://bugs.openjdk.java.net/browse/JDK-8263760). The minimum >> version of gradle is set to 6.3. >> >> In addition to keeping gradle up to date, updating to gradle 7 will allow >> building and testing JavaFX with JDK 16. >> >> I have done a full build and test on all three platforms, comparing the >> artifacts produced before and after this change. >> >> ### Notes to Reviewers: >> >> The PR branch has two separate commits, one for each fix, in case reviewers >> want to look at them separately. As always, they will be squashed into a >> single commit when integrated. Both bug IDs will be listed in the commit. >> >> The following changes were done for >> [JDK-8240336](https://bugs.openjdk.java.net/browse/JDK-8240336) to eliminate >> the use of deprecated features removed in gradle 7: >> >> 1. Replaced `compile` with either `implementation` or `compileClasspath` as >> needed >> 2. Replaced obsolete `archiveName` and `destinationDir` properties in >> archive tasks with `archiveFileName` and `destinationDirectory` >> 3. Added missing `@Input` annotation to custom Groovy task properties >> 4. Bumped the minimum version of gradle to 6.3 (which we have been using for >> more than 1 year) >> >> >> The following changes were done for >> [JDK-8263760](https://bugs.openjdk.java.net/browse/JDK-8263760) to update to >> gradle 7.0.1: >> >> 1. Ran `bash ./gradlew wrapper --gradle-version=7.0.1` >> 2. Updated the gradle version in `build.properties` to `7.0.1` >> 3. Updated the SHA-256 checksum in `gradle/wrapper/gradle-wrapper.properties` > > buildSrc/src/main/groovy/com/sun/javafx/gradle/CCTask.groovy line 34: > >> 32: @Optional @Input List<String> linkerOptions = new >> ArrayList<String>(); >> 33: @Optional @InputDirectory File headers; >> 34: @Optional @Input Closure eachOutputFile; // will be given a File and >> must return a File > > Is this field actually used anywhere? > > As this field is not initialized + there is the `@Optional` annotation, > Gradle will not verify this is fine. Gradle docs is specifying that: > 1. `@Input` can be used on Closure if it is a provider of `File` (which means > no param in closure). > 2. `@Input` can be used for any `Serializable` which `Closure` actually is. > > It's not obvious if Gradle will correctly handle `Closure` taking argument. I'll take a look, but I will likely want to do any cleanup of this in a follow-on issue. As you note the `eachOutputFile` property is unused...as is the `exe` property. They were formerly used by one of the `javapackager` tasks, which has since been removed. > buildSrc/src/main/groovy/com/sun/javafx/gradle/NativeCompileTask.groovy line > 44: > >> 42: @Optional @Input String matches; // regex for matching input files >> 43: @Input List<String> params = new ArrayList<String>(); >> 44: @Input List sourceRoots = new ArrayList(); > > Shouldn't this be `@Internal`?: > > 1. `updateFiles()` uses `sourceRoots` and maps that into the `allFiles` > field, which has already `@InputFiles`. If the `sourceRoots` is changed in a > way that does not affect `allFiles`, this task can remain `UP_TO_DATE` and > save task execution. Perhaps, but I wouldn't want to make this change as part of this bug fix. I'd rather not have to do the analysis of whether there is any case where `sourceRoots` can change in a way that wouldn't affect the outputs. If not, then this is only a theoretical concern, and if so, it seems better to err on the side of correctness unless you can prove that all similar cases won't affect the outputs. ------------- PR: https://git.openjdk.java.net/jfx/pull/498
