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

Support Jigsaw projects #658

Open
HoldYourWaffle opened this issue Feb 19, 2018 · 22 comments
Open

Support Jigsaw projects #658

HoldYourWaffle opened this issue Feb 19, 2018 · 22 comments

Comments

@HoldYourWaffle
Copy link

HoldYourWaffle commented Feb 19, 2018

Importing a java 9 project without any additional configuration simply doesn't work. Certain attributes have to be set on several classpath entries to make it work:

  • Dependencies have to be marked as modules[1] (not sure where I found this piece of configuration, can't seem to find it anywhere now)
  • To make tests work both the src/test/java source folder and testCompile-scoped dependencies have to be marked as 'tests' (this does not seem to work correctly in the latest Eclipse Oxygen as of writing, it works perfectly in Photon 20180202-1214)

I currently use the following workaround in all my java 9 projects to generate a fully-functional (as far as I've tested) Java 9 eclipse project:

eclipse {
	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'
				}
			}
		}
	}
}

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 don't know enough about gradle to be sure the methods at the bottom work for every use-case, they worked fine for me 🤷‍♂️

Some background information on the test configuration from the eclipse bugtracker
This configuration seems pretty trivial to me, I suggest buildship automatically does this for you (if that doesn't break compatability with java 8 in some way) so you don't have to clutter your build files with trivial configuration.

I'm sorry if my way of writing seems a bit aggressive, I'm way too tired after figuring this out for several hours 😅
@oehme
Copy link
Contributor

oehme commented Feb 20, 2018

Thanks for taking the time to investigate how to make it work in Eclipse!

Gradle doesn't provide much support for Jigsaw at this point out of the box. We have an experimental plugin that you can use in the meantime. Your configuration above should probably be added to that plugin. A pull request would be welcome!

@HoldYourWaffle
Copy link
Author

HoldYourWaffle commented Feb 20, 2018

Upon further testing I encountered some issues and inconsistencies in the way buildship handles this. For some reason it broke overnight 😕. I'm still investigating, but this is what I've got so far:

Java 8

The following configuration makes everything work perfectly as intended:

eclipse {
	classpath {
		file {
			whenMerged {
				entries.findAll { isSource(it) && isTestScope(it) }.each { it.entryAttributes['test'] = 'true' }	
				entries.findAll { isLibrary(it) && isTestScope(it) }.each { it.entryAttributes['test'] = 'true' }
			}
		}
	}
}

boolean isLibrary(entry) { return entry.properties.kind.equals('lib') }
boolean isTestScope(entry) { return entry.entryAttributes.get('gradle_used_by_scope').equals('test'); }
boolean isSource(entry) { return entry.properties.kind.equals('src'); }

Although technically not required to make things work, I think it's still wise to put this in (it can't do any harm as far as I know).

Java 9

What does work:

  • When generating the project with gradle eclipse and importing it using the Existing Projects into Workspace wizard, the test flags are set correctly

This is where things start to break apart

Using the same configuration as above.
(This doesn't include properly compiling modules I think, not sure if the additional configuration for that is still required.)

  • When using Gradle > Refresh gradle project inside eclipse the gradle_used_by_scope property is apparently unknown and null, causing the test flags to not get set
  • The Existing Gradle project import wizard overrides the .classpath of a project. This is a problem because buildship doesn't set the test correctly flags because of the above issue.
  • When adding a gradle nature to a project generated by the way that does work, the test flags get overwritten again.
  • I had some issues with running JUnit tests from eclipse yesterday, it seems to have fixed itself although I'm not quite sure if that's because of buildship or because of me experimenting with class/modulepaths. Probably the latter so more research is needed.

Things I still have to look into

  • Actually do some proper research on the difference between class/modulepath because my current idea is apparently wrong considering some test results
  • Properly setting the class/modulepath in eclipse including dependencies
  • Properly shade implementation dependencies from the java-library plugin (this needs a lot more research and is probably a subject for a different issue)

Summary

The core issue is that buildship doesn't know the gradle_used_by_scope property. Another minor (side) issue is the fact that buildship overrides the .classpath when importing/adding gradle nature, but that's probably unavoidable.

@oehme
Copy link
Contributor

oehme commented Feb 20, 2018

The core issue is that buildship doesn't know the gradle_used_by_scope property.

That property is added by a whenMerged hook in the eclipse plugin, so it should definitely be there before your code runs. A little reproducible example would help here. It's not related to Buildship, the code path through the eclipse plugin is the same.

Another minor (side) issue is the fact that buildship overrides the .classpath when importing/adding gradle nature, but that's probably unavoidable.

This is intentional. Everything can be configured in the whenMerged {} hook, so there is no need for manual classpath modifications.

HoldYourWaffle added a commit to HoldYourWaffle2/buildship-658-sample that referenced this issue Feb 20, 2018
@HoldYourWaffle
Copy link
Author

Sample project

Running gradle eclipse from the commandline gives the following result:

> Task :eclipseClasspath
SOURCE: main,test
SOURCE: test
LIB: test
LIB: test


BUILD SUCCESSFUL in 0s
3 actionable tasks: 3 executed

Importing the project into eclipse outputs:

CONFIGURE SUCCESSFUL in 0s
SOURCE: null
SOURCE: null
LIB: null
LIB: null

@HoldYourWaffle HoldYourWaffle changed the title Automatically configure Java 9 "stuff" Eclipse Java 9 configuration Feb 21, 2018
@HoldYourWaffle HoldYourWaffle changed the title Eclipse Java 9 configuration Java 9 configuration Feb 23, 2018
@donat
Copy link
Contributor

donat commented Mar 5, 2018

@HoldYourWaffle If I import the project with Gradle 4.4+ into Eclipse then the output is:

SOURCE: main,test
SOURCE: test
LIB: test
LIB: test

The project contains the wrapper executable (gradlew) but the gradle-wrapper.properties file is missing. This causes Buildship to fall back to the default Gradle version defined by the Tooling API which is 4.3 for the latest release.

@HoldYourWaffle
Copy link
Author

HoldYourWaffle commented Mar 12, 2018

There was indeed some weird stuff going on with my wrapper, no idea how that happened and why it continued on fresh projects.

Doing some investigation on java 9 configuration for eclipse again, stuff is being really weird and I have no idea why. It's like the module flag suddenly has no meaning. Stuff that worked before just magically broke for some reason although I'll need to investigate further.

@HoldYourWaffle
Copy link
Author

HoldYourWaffle commented Mar 13, 2018

Alright I think I've sorted this out now.

plugins {
	id 'java'
	id 'eclipse'
}
sourceCompatibility = targetCompatibility = '1.9'

eclipse {
	classpath {
		file {
			whenMerged {
				//Define a module as being either a library or a project dependency.
				//Test sources are excluded because eclipse wants them on the classpath for some reason (1)
				entries.findAll { (it instanceof org.gradle.plugins.ide.eclipse.model.Library || it instanceof org.gradle.plugins.ide.eclipse.model.ProjectDependency) && !it.entryAttributes.get('gradle_used_by_scope').equals('test') }.each {
					it.entryAttributes['module'] = 'true'
				}
				
				//Test-scoped stuff should set the appropriate flag
				entries.findAll { (it.properties.kind.equals('src') || it.properties.kind.equals('lib')) && it.entryAttributes.get('gradle_used_by_scope').equals('test') }.each {
					it.entryAttributes['test'] = 'true'
				}
			}
		}
	}
}

(1)

That's all the configuration that's necessary to get a working Java 9 Jigsaw project inside eclipse. This works perfect concerning the scope of buildship.
Until you start unit testing 😕
Running unit tests from inside eclipse does weird stuff, it loads & runs the tests from the src/main/java folder (which it shouldn't) and tries to load the classes from src/test/java, but fails spitting out exceptions like this:

WARNING: package my.awesome.project.tests not in imy.awesome.project
Class not found my.awesome.project.tests.SimpleUnitTestTest
java.lang.ClassNotFoundException: my.awesome.project.tests.SimpleUnitTestTest
	at java.base/jdk.internal.loader.BuiltinClassLoader.loadClass(BuiltinClassLoader.java:582)
	at java.base/jdk.internal.loader.ClassLoaders$AppClassLoader.loadClass(ClassLoaders.java:185)
	at java.base/java.lang.ClassLoader.loadClass(ClassLoader.java:496)
	at org.eclipse.jdt.internal.junit.runner.RemoteTestRunner.loadClass(RemoteTestRunner.java:770)
	at org.eclipse.jdt.internal.junit.runner.RemoteTestRunner.loadClasses(RemoteTestRunner.java:499)
	at org.eclipse.jdt.internal.junit.runner.RemoteTestRunner.runTests(RemoteTestRunner.java:522)
	at org.eclipse.jdt.internal.junit.runner.RemoteTestRunner.runTests(RemoteTestRunner.java:760)
	at org.eclipse.jdt.internal.junit.runner.RemoteTestRunner.run(RemoteTestRunner.java:460)
	at org.eclipse.jdt.internal.junit.runner.RemoteTestRunner.main(RemoteTestRunner.java:206)

I have literally no idea how to fix this or what causes it. Test libraries are on the classpath like eclipse asked so I'm not even sure this is a buildship issue. Any help would be greatly appreciated 😄

@donat
Copy link
Contributor

donat commented Mar 14, 2018

Can you update your sample project so that it contains your latest findings? Maybe the Eclipse Java9 support doesn't play nicely with the Buildship classpath customization.

@HoldYourWaffle
Copy link
Author

HoldYourWaffle commented Mar 14, 2018

I have updated my sample project.

@HoldYourWaffle
Copy link
Author

Any update on this?

@DJViking
Copy link

Isn't this something that has to be fixed in the Gradle eclipse plugin?

@HoldYourWaffle
Copy link
Author

Well..... yeah.
That's why we're here.

@DJViking
Copy link

DJViking commented Jun 19, 2018

We are at eclipse buildship. That is why I asked.

https://github.com/gradle/gradle/blob/master/subprojects/ide/src/main/java/org/gradle/plugins/ide/eclipse/EclipsePlugin.java
Which is not this project.

Is there an reported issue on the Eclipse Plugin project for this problem? I can't find any.

@HoldYourWaffle
Copy link
Author

Damn you just blew my mind.

@johnjaylward
Copy link

From what I understand the EclipsePlugin under gradle is to handle things that are "one-off" (i.e. I use it to ignore warnings on generated source folders). It's not meant to be full support for Gradle in Eclipse. Buildship should be handling this classpath/module stuff by default without any major modifications to the gradle EclipsePlugin.

So while the fix @HoldYourWaffle found is done through the GradlePlugin, the proper place for the fix is to get it done in Buildship.

@Klaboe
Copy link

Klaboe commented Feb 7, 2019

Any news on this? Since #708 was closed as a dupe of this.

I concur with @johnjaylward ; the eclipse-gradle-plugin is not included in any of my gradle-projects, but that should not stop buildship from working with modular java?

@DJViking
Copy link

DJViking commented Feb 7, 2019

Really hope a fix is coming soon. I have begun using Buildship with gradle instead of the Eclipse plugin, but it dows not work with Java 9+. Everytime the build configuration change I have to move "Project and External Dependencies" to Classpath to Modulepath manually.

@oehme oehme changed the title Java 9 configuration Support Jigsaw projects Feb 8, 2019
@oehme
Copy link
Contributor

oehme commented Feb 8, 2019

Changing the title, as this is not related to "Java 9", but jigsaw. There's no short-term plan to work on jigsaw support, so please don't hold your breath.

From what I understand the EclipsePlugin under gradle is to handle things that are "one-off" (i.e. I use it to ignore warnings on generated source folders). It's not meant to be full support for Gradle in Eclipse. Buildship should be handling this classpath/module stuff by default without any major modifications to the gradle EclipsePlugin.

Nope, everything is done in Gradle's EclipsePlugin, Buildship just gets the data from there. Buildship does not do any significant work itself. It automatically applies the EclipsePlugin to your build if you haven't yourself. To properly support Jigsaw, that support has to be built into Gradle first.

@Klaboe
Copy link

Klaboe commented Feb 8, 2019

@oehme Thank you, this made this alittle clearer for me at least. Could we just close this then, and redirect our efforts to the gradle-github?

@oehme
Copy link
Contributor

oehme commented Feb 8, 2019

It'll still need some work on Buildship to take the additional information into account once Gradle provides it, so I'd leave this open.

@A248
Copy link

A248 commented Apr 23, 2021

Gradle 7 has improved support for JPMS. Are there plans to work on this issue in buildship? This is a blocker for projects whose maintainers are (rightfully) reluctant to add module descriptors due to the lack of tooling support.

@fithisux
Copy link

fithisux commented Dec 12, 2023

I have problem importing the controlsfx project I use the "partial fix" (move deps from classpath to modulepath, kudos eclipse team) but still other parts need possible the --add-opens ... that make it to jvmArgs.
I need to figure it out. But still not clear to me why it is not imported as is.

MacOSX x64
Eclipse 4.30

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

8 participants