openjdk-jfx
openjdk-jfx copied to clipboard
Remove IDE specific files from the source code repository (JDK-8198795)
Support for Gradle is pretty advanced in most modern IDEs, which means they can import the modules based just on Gradle project definition. So I propose to remove IDE specific files from the repo. This is the best practice anyway.
https://bugs.openjdk.java.net/browse/JDK-8198795
This is relatively "low hanging fruit" so the process of reviewing and pushing fixes back to OpenJDK repo can be proved here.
My college @streifi and I wanted to remove all IDE specific code and we found out that the IDEs(Netbeans, Eclipse and IntelliJ) couldn´t import the project from gradle. We had a look at the problem and we have found the problem in the gradle.build has a few errors(the dependencies are not correctly specified). We fixed it and now Netbeans and IntelliJ can import openjfx without a problem. Eclipse cannot import gradle/jigsaw projects yet.
We would like to push first this build fix and then remove the IDE files(and ignore them).
Pull request: ~~https://github.com/javafxports/openjdk-jfx/pull/393~~ New one: https://github.com/javafxports/openjdk-jfx/pull/401
We fixed it and now Netbeans and IntelliJ can import openjfx without a problem. Eclipse cannot import gradle/jigsaw projects yet.
Eclipse can import Gradle projects just fine with Eclipse Gradle Buildship. I have no problem with using Eclipse with Gradle on Eclipse 2018-12, Gradle 5.x and Buildship 3.0. Though I have an eclipse.gradle with additional eclipse configuration to get it working with jigsaw.
eclipse {
project {
natures 'org.eclipse.buildship.core.gradleprojectnature'
}
classpath {
file {
whenMerged {
entries.findAll { isModule(it) }.each { //(1)
it.entryAttributes['module'] = 'true'
}
entries.findAll { isSource(it) && isTestScope(it) }.each {
it.entryAttributes['test'] = 'true'
}
entries.findAll { isLibrary(it) && isTestScope(it) }.each {
it.entryAttributes['test'] = 'true'
}
}
}
downloadSources = true
downloadJavadoc = true
}
}
boolean isLibrary(entry) { return entry.properties.kind.equals('lib') }
boolean isTestScope(entry) { return entry.entryAttributes.get('gradle_used_by_scope').equals('test'); }
boolean isModule(entry) { isLibrary(entry) && !isTestScope(entry); }
boolean isSource(entry) { return entry.properties.kind.equals('src'); }
I just tried with openjdk-jfx, while I have no problem with my own projects, this one did not import well. There was a few problems. Let me just take one for example. base, module-info.java complained about "The package com.sun.javafx.runtime does not exist or is empty".
Have anyone talked to the Eclipse team on fixing the jigsaw support. Seems there needs fix for both Eclipse Gradle Buildship and Gradle Eclipse plugin: https://github.com/eclipse/buildship/issues/658
For jigsaw I would anyway recommend a newer version of Gradle as it has better support for Java 11. Currently this project uses Gradle 4.8. However updating the gradle wrapper on this project was not hazzle free.
@DJViking Can you have a look to the pull request? I think if you want to try to import openjfx to eclipse, you need to use our pull request.
Cheers!
I cloned the repository from the PR #393 and tried to import to Eclipse. First I deleted all existing Eclipse IDE files (.project, .classpath), then imported by Gradle Buildship.
There is some shortcomings from the Eclipse Gradle support. Gradle Buildship does not set test=true for the test sources, thus it complains about cannot resolve junit
<attribute name="test" value="true"/>
Gradle Buildship needs the "Project and External Dependencies" under Libraries on Classpath, and not Modulepath (not sure why).
I added the following to build.gradle to fix these issues:
apply plugin: "eclipse"
apply(from: rootProject.file('eclipse.gradle'))
There is only one showstopper left from getting javafx.base error free in eclipse:
exports com.sun.javafx.runtime to
javafx.graphics;
Eclipse error: "The package com.sun.javafx.runtime does not exist or is empty"
Needed by test.com.sun.javafx.runtime.VersionInfoTest import com.sun.javafx.runtime.VersionInfo;
Somehow this class is under src/main/version-info https://github.com/javafxports/openjdk-jfx/blob/jfx-11/modules/javafx.base/src/main/version-info/VersionInfo.java
If the pull request https://github.com/javafxports/openjdk-jfx/pull/401 is merged, IntelliJ and Netbeans will open openjfx without a problem. Eclipse seems to have a problem with Gradle 4.x and jigsaw.
If we create a pull request removing all IDE specific files, does it block eclipse developers? Do eclipse IDE files work(they did not work for us)?
I think this eclipse problematic should be handled in a different issue(@DJViking can create a new one with his findings).
I have explained the Eclipse issue on the mailing list.
If we create a pull request removing all IDE specific files, does it block eclipse developers?
Yes. Eclipse files are not to be touched until Gradle can fully reproduce them. Have a look at the .classpath files for each project to see what Gradle needs to recreate.
Do eclipse IDE files work(they did not work for us)?
Yes. The .classpath files in the repo were specifically configured and are kept up to date. What does not work for you?
Yes. Eclipse files are not to be touched until Gradle can fully reproduce them.
I completely agree with this.
I would go further and say that there needs to be general consensus among the users of NetBeans and IntelliJ that the generated-from-gradle project files can be used with no issues before any of those IDE files are removed.
The IntelliJ and Netbeans project files have not worked for me.
With the pullrequest JDK-8220222: specify clearly gradle project dependencies #401 we fixed the dependencies in the gradle file. With this change, IntelliJ now can be used only with the gradle file. No IntelliJ project files are needed anymore.
Currently, Netbeans has an issue open with gradle and jigsaw (https://issues.apache.org/jira/browse/NETBEANS-2004 and kelemen/netbeans-gradle-project#417).
With the pull request, NetBeans and IntelliJ do not have problems importing the project. Please, could anybody else test this? It would be great.
The problem in Netbeans is not that important, because the project is correctly imported but NetBeans, as @christianheilmann already mentioned, does not work 100% perfect with gradle and jigsaw. There is a false error shown but you can work without a problem.
We don't need to remove the eclipse files at this moment if this will block eclipse developers. But as soon as somebody check that the fix in the gradle.build is right, we could remove NetBeans files and IntelliJ files (and add them to the .gitignore). And as soon as eclipse works fine, we could consider removing them too.
@nlisker about Eclipse, I do not have a special interest in working with it but had a look at the options there were. Currently, with the change, we can use IntelliJ and NetBeans. Thank you for your interest. :-)
@kevinrushforth Does it make sense to ask about it in the mailing list?
@Ciruman I was just explaining the situation because this was a discussion about removing all IDE files. :) You also said that the files don't work for you, and they should, so I was wondering why.
@DJViking I think having Gradle recreate the Eclipse files is possible. It was done in this project and It works in terms of modulepath/classpath configurations. What needs to be addressed, though, is explicitly specifying reads/exports commands for each module that requires it.
Does it make sense to ask about it in the mailing list?
@Ciruman Yes, that's what I would recommend.
@DJViking I think having Gradle recreate the Eclipse files is possible. It was done in this project and It works in terms of modulepath/classpath configurations.
Yes, I have done something very similar in my own projects as I posted in https://github.com/javafxports/openjdk-jfx/issues/4#issuecomment-468737934
Trying to import into Eclipse from #401 Import > Gradle > Existing Gradle Project With Gradle Buildship I get these projects in Eclipse:
apps
base
controls
fxml
graphics
media
openjdk-jfx
swing
swt
systemTests
web
However importing through Gradle Buildship does not work completely, because it uses the Gradle Eclipse plugin and it recreates the .project and .classpath files and the are missing some important module and test attributes. Unless you add in build.gradle my eclipse code snippet from https://github.com/javafxports/openjdk-jfx/issues/4#issuecomment-468737934 then it will work.
So if using the existing eclipse project files, then should use this import method instead:
Import > General > Existing Projects into Workspace
But need to import from modules directory, and not project root, or else you only get the root project openjdk-jfx.
Still there are problems with importing into Eclipse (Eclipse 2018-12). This is for the javafx.base module:
-
Problem with the existing eclipse project .classpath file. Project 'base' is missing required source folder: 'build/gensrc/java'Build Path Problem This source path does not exist Removed this from Java Build Path > Source
-
Problem with module-info.java and a misplaced java class. One Error in src/main/java/module-info.java The package com.sun.javafx.runtime does not exist or is empty The test in src/test/java/test/com/sun/javafx/runtime/VersionInfoTest.java requires this also.
The class VersionInfo does exists, but lies under src/main/version-info/VersionInfo.java
@DJViking But these are errors that arise from bad .classpath or configuration. After using your Gradle code, do you get the same .classpath files that currently exist?
No, not quite the same .classpath files. Some additional Gradle properties are added; gradle_scope, gradle_used_by_scope
This is the changes for base:
It removed source that does not exist:
- <classpathentry kind="src" path="build/gensrc/java"/>
Reading build.gradle I think I understand the purpose of version-info and build/gensrc/java The VersionInfo.java is processed and placed in build/gensrc/java/com/sun/javafx/runtime Gradle does not run processVersionInfo when importing the project. Running ./gradlew processVersionInfo manually fixes that. This can be fixed by adding to project(":base")
eclipseClasspath.dependsOn processVersionInfo
The main source gets additional output for shims:
- <classpathentry kind="src" path="src/main/java"/>
+ <classpathentry kind="src" output="bin/shims" path="src/main/java">
+ <attributes>
+ <attribute name="gradle_scope" value="shims"/>
+ <attribute name="gradle_used_by_scope" value="test,shims"/>
+ </attributes>
+ </classpathentry>
Not sure why, the src/main/java should actually be more like this:
<classpathentry kind="src" output="bin/main" path="src/main/java">
<attributes>
<attribute name="gradle_scope" value="main"/>
<attribute name="gradle_used_by_scope" value="main,test"/>
</attributes>
</classpathentry>
It is missing test attribute for source src/shims/java, and output is different:
- <classpathentry kind="src" output="testbin" path="src/shims/java">
- <attributes>
- <attribute name="test" value="true"/>
- </attributes>
- </classpathentry>
+ <classpathentry kind="src" output="bin/shims" path="src/shims/java">
+ <attributes>
+ <attribute name="gradle_scope" value="shims"/>
+ <attribute name="gradle_used_by_scope" value="test,shims"/>
+ </attributes>
+ </classpathentry>
Missing optional attribute from src/test/java, and output is different:
- <classpathentry kind="src" output="testbin" path="src/test/java">
- <attributes>
- <attribute name="test" value="true"/>
- <attribute name="optional" value="true"/>
- </attributes>
- </classpathentry>
+ <classpathentry kind="src" output="bin/test" path="src/test/java">
+ <attributes>
+ <attribute name="gradle_scope" value="test"/>
+ <attribute name="gradle_used_by_scope" value="test"/>
+ <attribute name="test" value="true"/>
+ </attributes>
+ </classpathentry>
The output directory has changed:
- <classpathentry kind="output" path="bin"/>
+ <classpathentry kind="output" path="bin/default"/>
The JRE is missing the module attribute.
- <classpathentry kind="con" path="org.eclipse.jdt.launching.JRE_CONTAINER">
- <attributes>
- <attribute name="module" value="true"/>
- </attributes>
- </classpathentry>
+ <classpathentry kind="con" path="org.eclipse.jdt.launching.JRE_CONTAINER/org.eclipse.jdt.internal.debug.ui.launcher.StandardVMType/JavaSE-11/"/>
The JUNIT is missing:
- <classpathentry kind="con" path="org.eclipse.jdt.junit.JUNIT_CONTAINER/4">
- <attributes>
- <attribute name="test" value="true"/>
- </attributes>
- </classpathentry>
This can be added by setting extra container in gradle eclipse configuration, but it is actually not needed since Gradle Buildship adds the junit4 dependency.
With the following additions to build.gradle I have been getting javafx.base module error free when imported with Gradle Buildship.
apply plugin: "eclipse"
apply(from: rootProject.file('eclipse.gradle'))
eclipseClasspath.dependsOn processVersionInfo
Only modules graphics and web that have remaining errors in eclipse.
What are the Gradle attributes "gradle_scope" and "gradle_used_by_scope"?
I don't think output="bin/shims" needs to be specified for path="src/main/java".
Only modules graphics and web that have remaining errors in eclipse.
Graphics will require an add-exports:
<classpathentry kind="con" path="org.eclipse.jdt.launching.JRE_CONTAINER">
<attributes>
<attribute name="module" value="true"/>
<attribute name="add-exports" value="java.base/sun.security.util=javafx.graphics"/>
</attributes>
</classpathentry>
But controls also requires add-exports, so I'm not sure how it is working for you.
What are the Gradle attributes "gradle_scope" and "gradle_used_by_scope"?
I'm not sure what these are for. They are added by Gradle Eclipse plugin.
Graphics will require an
add-exports:<classpathentry kind="con" path="org.eclipse.jdt.launching.JRE_CONTAINER"> <attributes> <attribute name="module" value="true"/> <attribute name="add-exports" value="java.base/sun.security.util=javafx.graphics"/> </attributes> </classpathentry>But controls also requires
add-exports, so I'm not sure how it is working for you.
Will check how to get this add-exports into the JRE container automatically from the Gradle Eclipse Plugin. Perhaps some missing configuration in build.gradle. Wiil look into this further.
Got the following add-exports in graphics .classpath
<classpathentry combineaccessrules="false" kind="src" path="/base">
<attributes>
<attribute name="module" value="true"/>
<attribute name="add-exports" value="javafx.base/com.sun.javafx.property=javafx.graphics"/>
</attributes>
</classpathentry>
Got the following add-exports in controls .classpath
<classpathentry kind="src" path="/graphics">
<attributes>
<attribute name="module" value="true"/>
<attribute name="add-exports" value="javafx.graphics/test.com.sun.javafx.pgstub=javafx.controls"/>
</attributes>
</classpathentry>
<classpathentry combineaccessrules="false" kind="src" path="/base">
<attributes>
<attribute name="module" value="true"/>
<attribute name="add-exports" value="javafx.base/test.com.sun.javafx.binding=javafx.controls"/>
</attributes>
</classpathentry>
The add-exports you got for graphics and controls are correct. They apply to the other modules that this module depends on. Graphics indeed relies on javafx.base/com.sun.javafx.property. So, somehow the correct add-exports are applied to the dependent projects, but not the JRE.
Our pull-request https://github.com/javafxports/openjdk-jfx/pull/401 checkin enables full working OpenJFX project in IntelliJ IDEA Version 20.18.3 or higher with the checked in build.gradle.
Simply open the build.gradle in IntelliJ and you are ready to code.
The checked in IntelliJ project files should be removed, because they do not work and are misleading!
I just approved PR #401 which can unblock this. As a next step, JDK-8198795 should be split into 3 separate issues, one for each of IntelliJ, Eclipse, and Netbeans. You will need general consensus from the users of an IDE before removing the checked in files for that IDE. By splitting it into three separate issues, you can take care of each IDE separately when the users of that IDE are happy with it.
I created the following tickets for Eclipse, Netbeans, and IntelliJ. I will create a PR to remove IntelliJ files. @nlisker, if Eclipse is able to load the projects from the gradle files and therefore, Eclipse files are not needed anymore, I could also remove them in another PR. https://bugs.java.com/bugdatabase/view_bug.do?bug_id=JDK-8223374 https://bugs.java.com/bugdatabase/view_bug.do?bug_id=JDK-8223375 https://bugs.java.com/bugdatabase/view_bug.do?bug_id=JDK-8223373
@Ciruman Eclipse is not able to create its files from Gradle yet. @DJViking worked on an attempt, but it's incomplete. Once we have a working solution I will remove the files.
@DJViking Have you had a chance to figure out the JRE container add-exports issue?
@nlisker Sorry I didn't reply. I was away for summer vacation and I simply forgot looking into this issue further. I haven't had much the time to look more into this. I think I will take a look again soon, maybe this weekend and see what the current status is with openjfx and eclipse on Gradle.
@DJViking Gradle was updated to version 5, I think now should work better in Netbeans and Eclipse
Gradle is getting full JPMS support in Gradle 6.4
I see that the RC is out. Maybe it's worth testing it already. @DJViking Can you give it a go?
Yes I can give it a go.