RFR: 8226754: FX build fails using gradle 5.6+ or 6

[Kevin Rushforth]

On Wed, 9 Oct 2019 12:38:09 GMT, Johan Vos <jvos at openjdk.org> wrote:

> On Wed, 9 Oct 2019 12:24:29 GMT, Kevin Rushforth <kcr at openjdk.org> wrote:
> 
>> On Wed, 9 Oct 2019 07:50:44 GMT, Johan Vos <jvos at openjdk.org> wrote:
>> 
>>> On Tue, 8 Oct 2019 13:54:22 GMT, Kevin Rushforth <kcr at openjdk.org> wrote:
>>> 
>>>> JBS issue: [JDK-8226754](https://bugs.openjdk.java.net/browse/JDK-8226754)
>>>> 
>>>> As noted in the JBS bug, the JavaFX build fails with gradle 6 (as well as not building correctly with 5.6 or later).
>>>> 
>>>> The existing JavaFX build uses two deprecated features that are removed in gradle 6, so the build fails when building with gradle 6. Additionally, some change that went into gradle 5.6 prevents all of our resource files (e.g., css files, images, shaders) from being included in the built artifacts, which causes JavaFX to be non-functional (our unit tests catch this failure).
>>>> 
>>>> The purpose of this bug fix is to allow JavaFX to build with gradle 6, which is needed to allow building with JDK 13. We will likely upgrade to gradle 6 in the near future. Additionally, this fixes the resource bug that was exposed (or introduced) in gradle 5.6 and also affects gradle 6.
>>>> 
>>>> The changes are as follows:
>>>> 
>>>> 1. Remove unneeded STABLE_PUBLISHING setting, which was transitional to allow gradle 4.x to continue working while we moved to gradle 5.x
>>>> 2. Use `ivy patternLayout ...` instead of `layout "pattern", ...`
>>>> 3. Specify no metadata for ivy repositories
>>>> 4. Set output.resourcesDir of sourceSet to match processResources.destinationDir
>>>> 5. Bump minimum gradle version to 5.3 (since it will no longer run with gradle 4.x after change 1)
>>>> 
>>>> I verified that the build artifacts produced by gradle 5.3 before and after this changes are identical (so it is behavior neutral for the supported version of gradle). After the fix, I also verified that the build artifacts produced by gradle 5.6.2 and 6.0 nightly match those produced by 5.3. I have tested this fully on Linux and Windows, and I will do a sanity test on Mac in parallel with the review.
>>>> 
>>>> ----------------
>>>> 
>>>> Commits:
>>>>  - bc6bd441: 8226754: FX build fails using gradle 5.6+ or 6
>>>> 
>>>> Changes: https://git.openjdk.java.net/jfx/pull/9/files
>>>>  Webrev: https://webrevs.openjdk.java.net/jfx/9/webrev.00
>>>>   Issue: https://bugs.openjdk.java.net/browse/JDK-8226754
>>>>   Stats: 28 lines in 4 files changed: 17 ins; 4 del; 7 mod
>>>>   Patch: https://git.openjdk.java.net/jfx/pull/9.diff
>>>>   Fetch: git fetch https://git.openjdk.java.net/jfx pull/9/head:pull/9
>>> 
>>> build.gradle line 1836:
>>> 
>>>> 1835:                 url JFX_DEPS_URL
>>>> 1836:                 metadataSources {
>>>> 1837:                     artifact()
>>> 
>>> From the JBS entry, I understood for now you wanted to keep layout (instead of patternLayout): https://bugs.openjdk.java.net/browse/JDK-8226754?focusedCommentId=14293009&page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel#comment-14293009
>>> 
>>> I understand the reasoning behind this (not using an incubating API), so I wonder why it is changed in this PR?
>> 
>> I think you meant to point to [this earlier comment](https://bugs.openjdk.java.net/browse/JDK-8226754?focusedCommentId=14273845&page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel#comment-14273845) that I made back in June. Yes, I had indicated that I wanted to wait until gradle 6 builds were available before switching from `layout` to `patternLayout`, in case they made any changes.
>> 
>> Your question does raise a good point, though: Should we wait until we actually want to switch to gradle 6 before making this change? If so, then it might make sense to split this change into two bugs: the `layout` --> `patternLayout` changes, which would wait until we switch to gradle 6, and the rest, which would be done now.
>> 
>> I could go either way. Which do you prefer?
> 
> https://docs.gradle.org/current/dsl/org.gradle.api.artifacts.repositories.IvyArtifactRepository.html still shows `patternLayout` to be incubating. Gradle doesn't have an excellent track record for finishing incubating API's (e.g. https://github.com/spring-projects/spring-boot/issues/11640). 
> Hence, I think it is indeed safer not to switch from a deprecated API to an incubating API (and as a result, split the PR so that the `layout` --> `patternLayout` is not included for now.
> 
> I don't have a very strong opinion on this though, so if you want to keep the changes, I don't object that.

OK, I'll go ahead and update this PR to split out the `layout` --> `patternLayout` changes. I'll file a new issue to upgrade to gradle 6, targeted to `tbd` for now, since we don't know when gradle 6 will be released. The `layout` --> `patternLayout` changes can be done as part of the actual upgrade.

PR: https://git.openjdk.java.net/jfx/pull/9