-
Notifications
You must be signed in to change notification settings - Fork 176
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
Remove test dependencies from runtime classpath #354
Comments
Encountered this problem while using |
We currently don't have the capacity to start this, but seeing that it is the highest voted issue, we will schedule it once we finish some of the other big items we currently have in flight. This will need some more design though, because we can't just hardcode This is simple for "Launch as Java Application", because there you launch exactly one class. It gets tricky for "Launch JUnit Test", because you can tell Eclipse to launch all tests in the whole project and these might come from different SourceSets (e.g. The nice thing about this solution is that it lines up with the proposed solution for "multiple compile classpaths" in the e4 incubator. So we could reuse their classpath attributes and get the dependency usage validation for free if the user has that plugin installed. |
For what its worth... I like your proposed solution. I think it will be great as it will:
It still won't fix the case where someone tries to run both integration and unit tests. But... well... yeah... that one is hard (is it even really fixable, you can't possibly run all those tests 'correctly' in a single classpath). Whatever the case may be. I'm glad you decided to try fix the 'easy cases', and you are even taking it a step further than just fixing the 'java main' case I had in mind. So... yeah... this sounds really good! |
I agree that this issue makes builds failed. |
I we are facing this issue as well. Eventually this means you cannot use a For now we just moved our tests into a separate project that depend on the real project |
Can I test this by switching my wrapper to a recent nightly? |
Not yet, unfortunately. What you see on the screenshot is really just a prototype that we used for validating our approach how to solve the problem. We'll post an update here once we have something to try. |
Pull request extending the Eclipse model in Gradle: gradle/gradle#2943. |
To @lukeu and everyone who wants to give this feature a try: I've prepared a snapshot update site from the development branch containing this feature. Here's how you can use it.
|
Great! I will give that a spin over the weekend. Quick question: will this also consider test resources? One of the issues we hit is that modules include a |
Thanks for trying it out! No, the current change separates binary dependencies only. As a next step, I'll look into removing the test resources from the main classpath. It's not that simple because Eclipse by default uses the same output folder for all source files. |
We should change that on the Gradle side and set a different output folder for each sourceSet. E.g. |
I agree, ultimately that will be the solution. Nonetheless, I want to prototype the solution in Buildship first because it's faster and this way users can try it out faster (by modifying their classpath definition in the |
@lukeu Today I had time to continue on this story. I prototyped a custom classpath provider on the feature branch so that project resources from external source sets are excluded too. I updated the update site URL in my previous comment. Because I haven't changed anything in Gradle core, you have to 'manually' adjust the source folder locations in your build script. It should look like this.
Note, that you might have to manually delete the run configuration being used and the content of the default project output dir ( |
OK I gave this a spin. Your minimal test works as described after creating a new Gradle project. However I didn't have much joy with a full project import. I already tweak the Eclipse output to ensure that Eclipse builds use a different output from the Gradle and Maven command-line builds. (We're still in transition). So I began with this:
I then tried changing I found that the test directories from each module were still in the classpath. (Confirmed from the logging behaviour, and by Debug view > right-click java.exe > Properties.) For reference our sourceSets are as follows:
Tip: it takes me a while to create a new run configuration (lots of settings and half a dozen environment vars). It seems to be sufficient to switch to the 'classpath' tab, and click "Restore Default Entries". |
@lukeu You don't have to create a run configuration manually. Let me explain the internals why I requested the deletion of the run configuration. As you probably know already, Eclipse creates a run configuration for you when you launch a java main method. This config contains a hidden property called the classpath provider. My feature branch replaces the default classpath provider with a custom one when a configuration is created or changed. The implementation doesn't handle all use cases yet, so you might have to delete the existing run config to have the custom classpath provider injected. The other thing I've seen is that even though you set custom output folders for your source folders and clean your projects, Eclipse might not clean up the default output directory. You might need to open the folder and delete the content manually. |
@donat
Do you think any of this was problematic (e.g. doing 2 after 1, or 7) and needs to be done again differently? |
It doesn't really matter what is the default output dir until it's different from the other source dir's output. Your steps in the previous comment seem fine to me. I don't completely understand what does step 7 do. In any case, I'll try to recreate your use case and see where Buildship fails. |
Feature announced in the milestone release: https://discuss.gradle.org/t/buildship-2-2-milestone-is-now-available/24551 |
@donat I tried my showcase project it with the latest milesone in oxygen The bean defined in the test config still seems to end up in the main classpath. Maybe this is actually beyond the scope of this bug as it does not come from a dependency but the test source |
Sources should also be filtered out. Are you using the latest Gradle 4.4 snapshot? |
@oehme you are right! works with |
OK I have:
And... no joy unfortunately. The (customised) test output dirs are still present on the classpath:
(Refer back to my comment on 24 Sep for the relevant parts of our The file that should configure logging is in
And uses that in preference to the desired:
|
@lukeu I have a couple of ideas what might have gone wrong. But first, a smaller thin that might be related. The latest
Buildship falls back to the original runtime classpath implementation if an old Gradle version is used for the project import or if one of the target source folders doesn't define dependency scopes. It's a bit tricky debug this because the UI hides some of the classpath information, but if you execute
Buildship expects a As of the run configurations, Buildship supports 2 types at the moment: java launches and JUnit tests. If you save the launch configuration to the disk (run configuration dialog -> common tab), you should see the following parameter in the file:
|
Szia @donat! Thanks for the tips. My It seems the issue is that the runtime classpath is only filtered to
Note how the main application (at the top, after the JDK) is not a parent of the other modules in the filesystem. I don't suppose that is significant?
We also have empty parent directories that group some sub-modules together, which are imported with |
It should not matter.
I might miss something here. So you have some projects in the workspace that you don't import with the At this point, I think it would be nice if you could just create a sample project exhibiting the problem. That way I'd be able to analyze the problem much faster. |
Hello, I still have this problem too. And for me it is a blocking issue. I use EmbeddedMongo database for test, and because this dependency is in the runtime, I can't launch my non-test classes without launching AND use this embedded databse which of course does not fit for production environnement. Can you help ? I precise that I just upgrade to gradle 4.6 |
@BetterWork4BetterWorld The latest Buildship should work for you. Just make sure you use Buildship 2.2+ and your project is actually imported with the proper Gradle version. |
I've now taken a 3rd crack at this (a couple of days) attacking it from both ends:
No joy :-( The minimal example still works as expected, and There may be a small clue from your suggestion to look for this:
If I delete that from the
I guess Buildship is "falling back" - but I'm at a loss as to why. Do you think it would be possible for me to debug Buildship to obtain some more clues? In anticipation I've started following the Oooph instructions to get set up. Any pointers where to stick a breakpoint etc.? (btw, would it be better to spin this out to a new issue - even if it turns out to be a non-issue? Or hold off until I have more concrete info) |
Huh, the key to making my minimal example fail was so simple in the end! Just run using JDK8 (I was previously using 9) and/or add:
Here is the minimal example: proj.zip. When proj-main is run, it prints:
Furthermore, if from the 'broken' state (after Gradle import) I then do:
Then the project runs as I would expect, without test resources on the CP. It prints:
|
I've had a bit of a dig around in the code. If you put a breakpoint on
then launching my test should catch the call to: A vague hunch - does something like what is done in |
Hi All, |
@alienisty Let's continue the conversation at #953. |
Context
The Buildship runtime classpath provided for Eclipse is different from the one used in Gradle. This leads to problematic classpath differences when the user executes a Java application from the IDE and via a Gradle build. It makes Spring Boot application development with STS and Buildship painful.
Original post: https://bugs.eclipse.org/bugs/show_bug.cgi?id=482315
Overview
Buildship should read the runtime classpath from the Gradle build and use it in Buildship.
In Gradle, all source folders and classpath entries should be marked with classpath attributes describing which source sets they belong.
In Eclipse the default JDT runtime classpath calculation should be overridden removing all elements from the classpath that don't belong to the same source set where the launched class is declared.
Implementation in Gradle
The
eclipse
plugin should define the following classpath attributes:<attribute name="gradle_source_sets" value="main,test"/>
<attribute name="gradle_source_sets" value="main,test"/>
The source folder attribute is easy because the
SourceFoldersCreator
instantiates source folders directly fromSourceSet
s.To calculate the attribute for the external dependencies we have to collect the runtime classpath entries for all source sets. Then,
EclipseDependenciesCreator
should look up which source sets an external dependency belong and add this info as a classpath attribute.Implementation in Buildship
Step 1 - Adjust
GradleClasspathContainerRuntimeClasspathEntryResolver
The classpath container should automatically pick up the new classpath attributes from Gradle.
The classpath entry resolver should only add an external dependency to the runtime classpath if the target source folder from the run configuration and dependency have a common source folder. If the source folder can't be determined (e.g. no run configuration is provided) then no such filtering is needed.
Step 2 - Provide custom JDT classpath provider
Along with external dependencies, Buildship should filter source folders that don't belong to the source set from where a JDT execution is launched. It can be done by contributing a custom runtime classpath provider via the
org.eclipse.jdt.launching.classpathProviders
extension point. Here's the required interface to implement:We should implement this extension point such that an entry is not added to the runtime classpath if it's not part of the current source set. The existing
GradleClasspathContainerRuntimeClasspathEntryResolver
class should be merged with this code.We should inject this custom provider via a launch configuration listener. The listener should check all launch configuration creation and if the target project is a Buildship project, then it should change the classpath provider to our implementation. Besides, if the Buildship nature is added/removed to a project, the classpath provider should be automatically updated accordingly.
Since there are a couple of different JDT launch types exist (java, junit, maybe testng) we should make sure that our custom runtime classpath injection supports all of them.
The text was updated successfully, but these errors were encountered: