8335218: Eclipse Config: Remove Gradle Integration

[andy-goryachev-oracle]

trafficstars

This might be controversial. I am proposing to remove the Gradle integration in the Eclipse config files.

Problem

Eclipse Gradle integration (Buildship) cannot import the OpenJFX build.gradle cleanly. Every time the project is imported into a new workspace (or re-opened after being closed) it executes Gradle, creates and modifies a number of Eclipse .project and .classpath files, all of which need to be reverted for Eclipse workspace to become usable again.

Solution

Remove Gradle nature from the Eclipse project files. This change only affects Eclipse config files and does not impact build.gradle or other IDEs.

Advantages

1. The multiple nested projects in the repo will get imported cleanly on the first attempt, will not require additional steps to clear the Buildship changes.
2. completely removes the dependency on the Eclipse Buildship and its idiosyncrasies.

NOTES:

• even though the reverse was done for IntelliJ, but its gradle import still does not import tests cleanly, see JDK-8223373
• this improvement contradicts JDK-8223374 as without Eclipse files in the repo, it will be impossible to use Eclipse in a meaningful way without the fully functional Buildship support, and that is a big IF.
• once integrated, Eclipse users would only need to re-import the main project with 'search for nested projects' enabled

---

Progress

• [x] Change must not contain extraneous whitespace
• [x] Commit message must refer to an issue
• [ ] Change must be properly reviewed (2 reviews required, with at least 1 Reviewer, 1 Author)

Issue

• JDK-8335218: Eclipse Config: Remove Gradle Integration (Enhancement - P4)

Reviewing

Using git

Checkout this PR locally:
$ git fetch https://git.openjdk.org/jfx.git pull/1491/head:pull/1491
$ git checkout pull/1491

Update a local copy of the PR:
$ git checkout pull/1491
$ git pull https://git.openjdk.org/jfx.git pull/1491/head

Using Skara CLI tools

Checkout this PR locally:
$ git pr checkout 1491

View PR using the GUI difftool:
$ git pr show -t 1491

Using diff file

Download this PR as a diff file:
https://git.openjdk.org/jfx/pull/1491.diff

Webrev

Link to Webrev Comment

[bridgekeeper[bot]]

:wave: Welcome back angorya! A progress list of the required criteria for merging this PR into master will be added to the body of your pull request. There are additional pull request commands available for use with this pull request.

[openjdk[bot]]

@andy-goryachev-oracle This change now passes all automated pre-integration checks.

ℹ️ This project also has non-automated pre-integration requirements. Please see the file CONTRIBUTING.md for details.

After integration, the commit message for the final commit will be:

8335218: Eclipse Config: Remove Gradle Integration

Reviewed-by: nlisker, jhendrikx

You can use pull request commands such as /summary, /contributor and /issue to adjust it as needed.

At the time when this comment was updated there had been 12 new commits pushed to the master branch:

• 1c6ef3638057e05565d83c79acbfad83046b12c7: 8335934: Change JavaFX release version to 24
• a41dcf3cb7259af0b5feac404aa94c3c1b247460: 8336110: Update copyright header for files modified in 2024
• e0fdb42b9fc0c09d8d7bf751f5adb9d8f07c0c4c: 8331603: Cleanup native AbstractSurface methods getRGBImpl, setRGBImpl
• dbda2cce0352692c622e6cc1bc664d779c013fcb: 8295945: Revert unintended change to copyright start year
• 9723a9c4ce3a4490df962b83da0df71b1e5145f1: 8322619: Parts of SG no longer update during rendering - overlapping - culling - dirty
• 56e07dbc9b04af06fc8cd1970595d19cb77fea4a: 8335216: [windows] Missing error check for GetSystemDirectory in glass
• 72701e6cb4095b8f5042f54ae6bb2c0cec446bcf: 8335633: Missing @Override in Dimension
• 459b15fc61e7a78f29c16fe665370657c1236b3f: 8326712: Robot tests fail on XWayland
• 5656b80f8a584a2d4d791f9fab83f1719b29f986: 8326619: Stage.sizeToScene() on maximized/fullscreen Stage breaks the Window
• 6c3fb5f557597a5a226d4a7bd41d84b7feb4a162: 8289115: Touch events is not dispatched after upgrade to JAVAFX17+
• ... and 2 more: https://git.openjdk.org/jfx/compare/0d2a0a462e500b9bb7ac4fc4f45beff9a378ea60...master

As there are no conflicts, your changes will automatically be rebased on top of these commits when integrating. If you prefer to avoid this automatic rebasing, please check the documentation for the /integrate command for further details.

➡️ To integrate this PR with the above commit message to the master branch, type /integrate in a new comment.

[mlbridge[bot]]

Webrevs

• 00: Full (0f50d81e)

[kevinrushforth]

Reviewers: @nlisker

This will need discussion among and review by the Eclipse users on the list.

/reviewers 2

[openjdk[bot]]

@kevinrushforth The total number of required reviews for this PR (including the jcheck configuration and the last /reviewers command) is now set to 2 (with at least 1 Reviewer, 1 Author).

[lukostyra]

+1 for this change from me.

I use VS Code for development, on the marketplace most popular and recommended Java support extension is the Red Hat extension which is essentially repackaged Eclipse. It was always frustrating to try and open JFX only to find out that something doesn't load correctly because the plugin overwrote .project and .classpath files.

I checked this PR and with default extension settings it loads properly. All regular features also work fine, so the plugin definitely works correctly after this PR. I only noticed that there are still modified .project files in apps directory - I wonder if it's just VS Code plugin deal, but it doesn't seem to interrupt the build process for the plugin.

[andy-goryachev-oracle]

@lukostyra thank you for testing! there should be no changes made to the .project or any other files. could you reset your workspace and maybe try again?

if the changes do appear, what are they?

[nlisker]

Continuing the mailing list discussion with more technical details.

Gradle tries to configure Eclipse based on its build file. This includes things like source folders, compiler settings, dependencies and others. In the cases that I have tried in my projects, this works well. One problem that still remains with this configuration is that it's not aware of the locations of the modules. This causes missing module errors when launching, which require -add-modules to remedy. The eclipse plugin solves this by resolving the paths locally for your gradle cache.

This tends to be the preferred way to do things from my experience since you don't need to push eclipse-specific files to the repo. The gradle files need to be there anyway, so it makes sense to configure eclipse from them and you maintain uniformity between the different development environments. Where this fails is with OpenJFX projects. The monolithic and old gradle file uses conventions from old gradle versions. This causes faulty generation of the eclipse files with and without the eclipse plugin. For example, all the source folders (src/main/java, src/test/java...) are prepended with the project/module name (base/src/main/java) because of manual source set configurations that shouldn't exist, at least not the way they are now.

The problem presented in the issue has 2 solutions:

1. Opt out: Include the Gralde nature by default, do all the initial building, then revert the Eclipse files. If you want to disable the Gradle integration, you can do so as well be removing the nature within the IDE.
2. Opt in: Don't include the Gradle nature by default and do all the initial building. If you want the Gradle integration, you can add it within the IDE. Both of these are one-time actions.

I'm fine with either since I find the one-time action to be cheap.

By the way, with this change you will also have to update the wiki page to a state where the Java import is the standard and there's an opt-in for the Gradle integration.

@hjohn Do you have any remarks?

[hjohn]

Do you have any remarks?

I personally don't launch build tools from the IDE (neither Gradle, nor Maven) but only use such tools to import projects; after the import I expect the IDE to be able to build and run tests without intervention of any build tool. I will use the build tooling (usually on the command line) only as a last resort if I can't get it to work that way (which is sometimes the case for a complicated project like JavaFX, especially when I need to run a non-headless test). Importing and working this way works for almost any project, but never has worked for me with JavaFX (which was a big barrier for me when I first tried contributing to FX -- I even tried switching IDE's, but it wasn't much better there either).

What annoys me the most with the FX project is the inability to run certain tests directly from the IDE by right clicking and selecting Run as... JUnit test. I work around this by running the test once, then changing its configuration and adding a standard incantation to its command line (-Djavafx.toolkit, -Djava.library.path, some --add-modules) -- I can live with that, although it is less than ideal (right clicking and running a test would be nice to have working out of the box for JavaFX).

I doubt however this change will make this any better or worse :)

So, I'm fine with either solution here. As I don't use Gradle integration, I'd lean slightly to just removing it, or alternatively, fixing the problems in the gradle build so Buildship recognizes it properly. Since I suspect the latter is not an easy task, removing the Gradle nature seems like a good alternative.

[lukostyra]

if the changes do appear, what are they?

@andy-goryachev-oracle I tried it again after clearing the workspace folder and they do appear again.

I also had some old git index settings applied which hid most of .project changes. Turns out that basically all .project files are modified, but I don't know if we can really help that - even when you tell VSCode Java plugin to not populate the workspace with project files, it sees they exist in the repo and uses/modifies them anyway. I saw a few issues on GitHub regarding this with no real resolution.

All of .projects have the same bit added (seems like something internal for the plugin):

       <filteredResources>
               <filter>
                       <id>1720448829339</id>
                       <name></name>
                       <type>30</type>
                       <matcher>
                               <id>org.eclipse.core.resources.regexFilterMatcher</id>
                               <arguments>node_modules|\.git|__CREATED_BY_JAVA_LANGUAGE_SERVER__</arguments>
                       </matcher>
               </filter>
       </filteredResources>

Regardless, the change does prevent the plugin from modifying .classpath files, which lets the project index and build properly. I don't expect to ever have to tinker with .project files for any reason, so it's good enough for me.

[andy-goryachev-oracle]

Thank you @lukostyra for checking!

All of .projects have the same bit added (seems like something internal for the plugin):

This change looks innocuous enough - at least it does not break the build. Why VSCode does it is a mystery, but I suppose that's a bug in VSCode plugin.

[andy-goryachev-oracle]

What annoys me the most with the FX project is the inability to run certain tests directly from the IDE by right clicking and selecting Run as... JUnit test.

Yep, same issue here, and same workaround.

I wonder if it is possible to create a separate project(s) for tests that will have all the necessary dependencies added (as a separate issue).

For example, headless tests require addition of these vm args:

-Djava.library.path="../../../build/sdk/lib"
-Djavafx.toolkit="test.com.sun.javafx.pgstub.StubToolkit"
-ea

while system tests are slightly different:

--add-modules=javafx.base,javafx.graphics,javafx.controls,javafx.swing
--add-opens javafx.controls/test.com.sun.javafx.scene.control.test=javafx.base
--add-exports javafx.base/com.sun.javafx=ALL-UNNAMED
-Djava.library.path="../../../../build/sdk/lib"

[andy-goryachev-oracle]

thank you @nlisker for your thoughts!

The eclipse plugin solves this by resolving the paths locally for your gradle cache.

so this is a non-standard setup, meaning it is not described in the https://wiki.openjdk.org/display/OpenJFX/Using+an+IDE#UsinganIDE-UsingEclipse wiki page.

Still, I would like to propose we proceed with this change even if it is a temporary one (until the Buildship plug-in gets mature enough to be able to import the gradle configuration cleanly) because it prevents the said Buildship from messing up the project files and breaking the build every time the project(s) are imported or reopened.

[nlisker]

I work around this by running the test once, then changing its configuration and adding a standard incantation to its command line (-Djavafx.toolkit, -Djava.library.path, some --add-modules)

The --add-modules could be taken care of by the eclipse plugin. The other 2 are javafx specific.

fixing the problems in the gradle build so Buildship recognizes it properly. Since I suspect the latter is not an easy task, removing the Gradle nature seems like a good alternative.

Yes, https://github.com/openjdk/jfx/pull/1232 should be part of the effort to simplify the gradle build file. It will take several iterations on that file to modernize OpenJFX's build.

so this is a non-standard setup, meaning it is not described in the https://wiki.openjdk.org/display/OpenJFX/Using+an+IDE#UsinganIDE-UsingEclipse wiki page.

Yes, it's not described in the wiki because it can't read the gradle file properly to begin with.

Still, I would like to propose we proceed with this change even if it is a temporary one (until the Buildship plug-in gets mature enough to be able to import the gradle configuration cleanly) because it prevents the said Buildship from messing up the project files and breaking the build every time the project(s) are imported or reopened.

I'm fine with this. Just noting that we're not waiting on Buildship to catch up, we need our build.gradle to catch up :)

[andy-goryachev-oracle]

thank you for reviewing! will integrate after the fork.

[andy-goryachev-oracle]

/integrate

[openjdk[bot]]

Going to push as commit 0ce4e6f9540178ddf4761de21a86c769d57497fa. Since your change was applied there have been 12 commits pushed to the master branch:

• 1c6ef3638057e05565d83c79acbfad83046b12c7: 8335934: Change JavaFX release version to 24
• a41dcf3cb7259af0b5feac404aa94c3c1b247460: 8336110: Update copyright header for files modified in 2024
• e0fdb42b9fc0c09d8d7bf751f5adb9d8f07c0c4c: 8331603: Cleanup native AbstractSurface methods getRGBImpl, setRGBImpl
• dbda2cce0352692c622e6cc1bc664d779c013fcb: 8295945: Revert unintended change to copyright start year
• 9723a9c4ce3a4490df962b83da0df71b1e5145f1: 8322619: Parts of SG no longer update during rendering - overlapping - culling - dirty
• 56e07dbc9b04af06fc8cd1970595d19cb77fea4a: 8335216: [windows] Missing error check for GetSystemDirectory in glass
• 72701e6cb4095b8f5042f54ae6bb2c0cec446bcf: 8335633: Missing @Override in Dimension
• 459b15fc61e7a78f29c16fe665370657c1236b3f: 8326712: Robot tests fail on XWayland
• 5656b80f8a584a2d4d791f9fab83f1719b29f986: 8326619: Stage.sizeToScene() on maximized/fullscreen Stage breaks the Window
• 6c3fb5f557597a5a226d4a7bd41d84b7feb4a162: 8289115: Touch events is not dispatched after upgrade to JAVAFX17+
• ... and 2 more: https://git.openjdk.org/jfx/compare/0d2a0a462e500b9bb7ac4fc4f45beff9a378ea60...master

Your commit was automatically rebased without conflicts.

[openjdk[bot]]

@andy-goryachev-oracle Pushed as commit 0ce4e6f9540178ddf4761de21a86c769d57497fa.

:bulb: You may see a message that your pull request was closed with unmerged commits. This can be safely ignored.