Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

8226754: FX build fails using gradle 5.6+ or 6 #9

Closed

Conversation

@kevinrushforth
Copy link
Member

kevinrushforth commented Oct 8, 2019

JBS issue: 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.

NOTE: the replacement for one of the deprecated APIs, ivy layout, is an incubating API in gradle 5.x. Since there is no stable replacement that we can use during the transition, I will defer the change from layout "pattern", ... to patternLayout ... until the eventual update to 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.

Progress

  • Change must not contain extraneous whitespace
  • Commit message must refer to an issue
  • Change must be properly reviewed

Issue

JDK-8226754: FX build fails using gradle 5.6+ or 6

Approvers

  • Johan Vos (jvos - Reviewer)
1. Remove unneeded STABLE_PUBLISHING setting
2. Bump minimum gradle version to 5.3
3. Use ivy patternLayout(...) instead of layout("pattern", ...)
4. Specify no metadata for ivy repositories
5. Set output.resourcesDir of sourceSet to match processResources.destinationDir
@bridgekeeper
Copy link

bridgekeeper bot commented Oct 8, 2019

👋 Welcome back kcr! A progress list of the required criteria for merging this PR into master will be added to the body of your pull request (refresh this page to view it).

@openjdk openjdk bot added the rfr label Oct 8, 2019
@mlbridge
Copy link

mlbridge bot commented Oct 8, 2019

Webrevs

build.gradle Outdated Show resolved Hide resolved
@kevinrushforth kevinrushforth requested a review from johanvos Oct 9, 2019
@kevinrushforth kevinrushforth self-assigned this Oct 9, 2019
Copy link
Collaborator

johanvos left a comment

Looks good. However, automated testing on Windows failed. I believe this is an infrastructure issue, but I want to investigate it in detail tomorrow.

Copy link
Collaborator

johanvos left a comment

Works on Linux/Mac/Windows

@openjdk openjdk bot removed the rfr label Oct 11, 2019
@openjdk
Copy link

openjdk bot commented Oct 11, 2019

@kevinrushforth This change can now be integrated. The commit message will be:

8226754: FX build fails using gradle 5.6+ or 6

Remove obsolete STABLE_PUBLISHING; explicitly set output.resourcesDir; bump minimum gradle version to 5.3

Reviewed-by: jvos
  • If you would like to add a summary, use the /summary command.
  • To list additional contributors, use the /contributor command.

Since the source branch of this PR was last updated there has been 1 commit pushed to the master branch:

  • 63fe66c: 8231870: CrossLibs script for armv6hf toolchain download fails

Since there are no conflicts, your changes will automatically be rebased on top of the above commits when integrating. If you prefer to do this manually, please merge master into your branch first.

  • To integrate this PR with the above commit message, type /integrate in a new comment.
@openjdk openjdk bot added the ready label Oct 11, 2019
@kevinrushforth
Copy link
Member Author

kevinrushforth commented Oct 14, 2019

/summary Remove obsolete STABLE_PUBLISHING; explicitly set output.resourcesDir; bump minimum gradle version to 5.3

@openjdk
Copy link

openjdk bot commented Oct 14, 2019

@kevinrushforth Setting summary to Remove obsolete STABLE_PUBLISHING; explicitly set output.resourcesDir; bump minimum gradle version to 5.3

@kevinrushforth
Copy link
Member Author

kevinrushforth commented Oct 14, 2019

/integrate

@openjdk openjdk bot closed this Oct 14, 2019
@openjdk openjdk bot added integrated and removed ready labels Oct 14, 2019
@openjdk
Copy link

openjdk bot commented Oct 14, 2019

@kevinrushforth The following commits have been pushed to master since your change was applied:

  • 63fe66c: 8231870: CrossLibs script for armv6hf toolchain download fails

Your commit was automatically rebased without conflicts.

Pushed as commit 64aaeb8.

@kevinrushforth kevinrushforth deleted the kevinrushforth:8226754-gradle-depr branch Oct 21, 2019
@kevinrushforth kevinrushforth mentioned this pull request Nov 11, 2019
3 of 3 tasks complete
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

2 participants
You can’t perform that action at this time.