On Wed, 9 Oct 2019 12:38:09 GMT, Johan Vos <j...@openjdk.org> wrote: > On Wed, 9 Oct 2019 12:24:29 GMT, Kevin Rushforth <k...@openjdk.org> wrote: > >> On Wed, 9 Oct 2019 07:50:44 GMT, Johan Vos <j...@openjdk.org> wrote: >> >>> On Tue, 8 Oct 2019 13:54:22 GMT, Kevin Rushforth <k...@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
