Re: [Approved] RFR: 8226754: FX build fails using gradle 5.6+ or 6
On Tue, 8 Oct 2019 13:54:22 GMT, Kevin Rushforth 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 Works on Linux/Mac/Windows Approved by jvos (Reviewer). PR: https://git.openjdk.java.net/jfx/pull/9
Re: [Rev 01] RFR: 8226754: FX build fails using gradle 5.6+ or 6
On Wed, 9 Oct 2019 17:16:10 GMT, Kevin Rushforth wrote: > The pull request has been updated with additional changes. > > > > Added commits: > - a928b41f: Revert ivy layout pattern changes per review comments > > Changes: > - all: https://git.openjdk.java.net/jfx/pull/9/files > - new: https://git.openjdk.java.net/jfx/pull/9/files/bc6bd441..a928b41f > > Webrevs: > - full: https://webrevs.openjdk.java.net/jfx/9/webrev.01 > - incr: https://webrevs.openjdk.java.net/jfx/9/webrev.00-01 > > Issue: https://bugs.openjdk.java.net/browse/JDK-8226754 > Stats: 21 lines in 2 files changed: 0 ins; 15 del; 6 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 Looks good. However, automated testing on Windows failed. I believe this is an infrastructure issue, but I want to investigate it in detail tomorrow. PR: https://git.openjdk.java.net/jfx/pull/9
Re: [Rev 01] RFR: 8226754: FX build fails using gradle 5.6+ or 6
The pull request has been updated with additional changes. Added commits: - a928b41f: Revert ivy layout pattern changes per review comments Changes: - all: https://git.openjdk.java.net/jfx/pull/9/files - new: https://git.openjdk.java.net/jfx/pull/9/files/bc6bd441..a928b41f Webrevs: - full: https://webrevs.openjdk.java.net/jfx/9/webrev.01 - incr: https://webrevs.openjdk.java.net/jfx/9/webrev.00-01 Issue: https://bugs.openjdk.java.net/browse/JDK-8226754 Stats: 21 lines in 2 files changed: 0 ins; 15 del; 6 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 PR: https://git.openjdk.java.net/jfx/pull/9
Re: RFR: 8226754: FX build fails using gradle 5.6+ or 6
On Wed, 9 Oct 2019 12:38:09 GMT, Johan Vos wrote:
> On Wed, 9 Oct 2019 12:24:29 GMT, Kevin Rushforth wrote:
>
>> On Wed, 9 Oct 2019 07:50:44 GMT, Johan Vos wrote:
>>
>>> On Tue, 8 Oct 2019 13:54:22 GMT, Kevin Rushforth 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=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=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:
Re: RFR: 8226754: FX build fails using gradle 5.6+ or 6
On Wed, 9 Oct 2019 12:24:29 GMT, Kevin Rushforth wrote:
> On Wed, 9 Oct 2019 07:50:44 GMT, Johan Vos wrote:
>
>> On Tue, 8 Oct 2019 13:54:22 GMT, Kevin Rushforth 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=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=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.
PR: https://git.openjdk.java.net/jfx/pull/9
Re: RFR: 8226754: FX build fails using gradle 5.6+ or 6
On Wed, 9 Oct 2019 07:50:44 GMT, Johan Vos wrote:
> On Tue, 8 Oct 2019 13:54:22 GMT, Kevin Rushforth 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=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=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?
PR: https://git.openjdk.java.net/jfx/pull/9
Re: RFR: 8226754: FX build fails using gradle 5.6+ or 6
On Tue, 8 Oct 2019 13:54:22 GMT, Kevin Rushforth 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=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?
PR: https://git.openjdk.java.net/jfx/pull/9
RFR: 8226754: FX build fails using gradle 5.6+ or 6
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 PR: https://git.openjdk.java.net/jfx/pull/9
