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

8227808: Make GTK3 libraries mandatory for building on Linux #61

Closed
wants to merge 2 commits into from

Conversation

arapte
Copy link
Member

@arapte arapte commented Dec 5, 2019

Need for the change [JDK-8227808]:
Currently GTK3 is the default on Linux. However, there is build logic that will skip building glassgtk3 if the gtk3 development libraries are not installed. This creates a situation where the build will succeed, but the resulting JavaFX library will fail to run unless you explicitly force GTK2 with "-Djdk.gtk.version=2".

How to verify the change:
On a Linux system without gtk2 and gtk3 packages,

  1. Clone jfx and run gradle sdk.
    -> Build should fail with an error for missing GTK3 packages.

  2. Install gtk3 packages, remove build directory and run gradle clean sdk.
    -> Build should fail with an error for missing GTK2 packages.

  3. Install gtk2 packages, remove build directory and run gradle clean sdk.
    -> Build should finish without any error.

Build verification:

  1. Run a JavaFX application with -Djdk.gtk.verbose=true
    Verify the verbose message, the default glassgtk3 library should be used.

  2. Run a JavaFX application with -Djdk.gtk.verbose=true -Djdk.gtk.version=2
    Verify the verbose message, glassgtk2 library should be used.

  3. Run a JavaFX application with -Djdk.gtk.verbose=true -Djdk.gtk.version=3
    Verify the verbose message, glassgtk3 library should be used.

Progress

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

Issue

JDK-8227808: Make GTK3 libraries mandatory for building on Linux

Approvers

  • Johan Vos (jvos - Reviewer)
  • Kevin Rushforth (kcr - Reviewer)

@bridgekeeper
Copy link

bridgekeeper bot commented Dec 5, 2019

👋 Welcome back arapte! 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 Ready for review label Dec 5, 2019
@mlbridge
Copy link

mlbridge bot commented Dec 5, 2019

Webrevs

def libsGTK3 = properties.getProperty("libsGTK3")
if (cflagsGTK3 && libsGTK3) {
gtk3CCFlags.addAll(cflagsGTK3.split(" "))
gtk3LinkFlags.addAll(libsGTK3.split(" "))
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Here only variable names are changed to make them similar to the names used for gtk2 code.

buildSrc/linux.gradle Outdated Show resolved Hide resolved
Copy link
Collaborator

@johanvos johanvos left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Works as expected. I recommend removing gtk-specific flags in the launcher.

@arapte
Copy link
Member Author

arapte commented Dec 10, 2019

Works as expected. I recommend removing gtk-specific flags in the launcher.

Hi Johan, The PR is updated with this change, please take a look.

@openjdk openjdk bot removed the rfr Ready for review label Dec 11, 2019
@openjdk
Copy link

openjdk bot commented Dec 11, 2019

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

8227808: Make GTK3 libraries mandatory for building on Linux

Reviewed-by: jvos, kcr
  • 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 have been 7 commits pushed to the master branch:

  • d2d44b4: 8207759: VK_ENTER not consumed by TextField when default Button exists
  • fc539b5: 8223296: NullPointerException in GlassScene.java at line 325
  • 71ca899: 8220722: ProgressBarSkin: adds strong listener to control's width property
  • 07af89a: 8221334: TableViewSkin: must initialize flow's cellCount in constructor
  • 4ddf554: 8234916: [macos 10.15] Garbled text running with native-image
  • 46338d0: 8232524: SynchronizedObservableMap cannot be be protected for copying/iterating
  • a68347c: 8235150: IosApplication does not pass the required object in _leaveNestedEventLoopImpl

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.

@kevinrushforth
Copy link
Member

This will also need a second review from @johanvos

@openjdk openjdk bot added the ready Ready to be integrated label Dec 11, 2019
@arapte
Copy link
Member Author

arapte commented Dec 18, 2019

/integrate

@openjdk openjdk bot closed this Dec 18, 2019
@openjdk openjdk bot added integrated Pull request has been integrated and removed ready Ready to be integrated labels Dec 18, 2019
@openjdk
Copy link

openjdk bot commented Dec 18, 2019

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

  • 1140d34: 8196587: Remove use of deprecated finalize method from JPEGImageLoader
  • d2d44b4: 8207759: VK_ENTER not consumed by TextField when default Button exists
  • fc539b5: 8223296: NullPointerException in GlassScene.java at line 325
  • 71ca899: 8220722: ProgressBarSkin: adds strong listener to control's width property
  • 07af89a: 8221334: TableViewSkin: must initialize flow's cellCount in constructor
  • 4ddf554: 8234916: [macos 10.15] Garbled text running with native-image
  • 46338d0: 8232524: SynchronizedObservableMap cannot be be protected for copying/iterating
  • a68347c: 8235150: IosApplication does not pass the required object in _leaveNestedEventLoopImpl

Your commit was automatically rebased without conflicts.

Pushed as commit 4e005e4.

@mlbridge
Copy link

mlbridge bot commented Dec 18, 2019

Mailing list message from Ambarish Rapte on openjfx-dev:

Changeset: 4e005e4
Author: Ambarish Rapte <arapte at openjdk.org>
Date: 2019-12-18 17:05:51 +0000
URL: https://git.openjdk.java.net/jfx/commit/4e005e4e

8227808: Make GTK3 libraries mandatory for building on Linux

Reviewed-by: jvos, kcr

! buildSrc/linux.gradle

@arapte arapte deleted the gtk3_mandatory_for_linux_build branch January 28, 2020 16:19
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
integrated Pull request has been integrated
Development

Successfully merging this pull request may close these issues.

3 participants