Skip to content
This repository

HTTPS clone URL

Subversion checkout URL

You can clone with HTTPS or Subversion.

Download ZIP

Add ScalaIDE container to the Eclipse classpath for Scala projects #99

Closed
wants to merge 1 commit into from

5 participants

Ben Manes Szczepan Faber Hans Dockter Iulian Dragos Peter Niederwieser
Ben Manes

The ScalaIDE plugin is the standard approach for developing Scala projects in Eclipse. Previously an import of a Scala gradle project would initially fail to build due to the error 'Cannot find Scala library on the classpath. Verify your build path!'. This had to be resolved manually so that the ScalaIDE verifier could be run successfully.

The ScalaIDE is now added as a classpath container. In addition to the unit test, the change was verified on a private project. The container was added to Scala projects only (and not Java projects).

Further background details is provided in the forum post, Generate ScalaIDE Eclipse metadata for Scala projects.

Szczepan Faber
Collaborator

Hey!

Thanks for the pull request. Can you double check how this implementation behaves in this scenario:

  1. the gradle scala project already declares dependency to scala-library with a different version than the scala-library shipped with Scala IDE.
  2. same as above but let's say gradle project depends on matching version of scala-library.

AFAIK, the scala container only adds the 'scala-library' on the path. If it also adds other dependencies, we need to make sure the project imports smoothly if the Gradle project already declares similar dependencies explicitly.

Ben Manes

Can you please clarify the expectations between disjoint versions?

Scala only provides backwards compatibility for minor versions (e.g. 2.9.0 to 2.9.2). It does not guarantee compatibility between major revisions (e.g. 2.9.x to 2.10.x). While much of the core is expected to stabilize as the language ages, most cross-version mixing is unsafe. Using a version of the ScalaIDE that does not match the major version in the build file would be considered an invalid configuration. This is why the ScalaIDE includes a classpath validator (see faq), which seems to be all that the container adds. The compatibility problem is explained by Suereth and Odersky, with coding strategies to avoid issues.

I'll try your scenarios tomorrow.
Thanks!

Szczepan Faber
Collaborator

I want to make sure that the project imports cleanly to eclipse if it already has scala library. I don't have any specific versions in mind. Cheers!

Ben Manes
  1. gradle dependency v2.9.2 and ScalaIDE adding v2.9.1 to the referenced libraries.
    • Import works correctly. I see the mixed versions in the project, but no failures occur
    • Navigating into scala library code resolves to the ScalaIDE's library, which could cause developer confusion
  2. same versions
    • This was my project's default, so verified.

A little searching and an additional concern was raised on the ScalaIDE forum. I have not encountered the specs2 issue, but was able to verify that full rebuilds are being performed instead of incremental. What do you think the proper fix is for that issue?

Hans Dockter
Owner

Right now Gradle does not do incremental compiles for Scala. We are aware that this is an important issue. We have already successfully spiked a solution by integrating with the SBT compiler. Right now we are planning to tackle this feature in Gradle 1.3 with a high priority. So possibly there will be a solution in a couple of weeks.

Hans Dockter hansd closed this September 06, 2012
Hans Dockter hansd reopened this September 06, 2012
Ben Manes

The incremental compilation referred to is through the Eclipse Scala Builder, which is backed by an embedded copy of SBT. The generated Eclipse metadata causes the ScalaIDE to not perform incremental compilation. I think that it is separate from gradle native support. It may be a ScalaIDE bug depending on interpretation.

Szczepan Faber
Collaborator

Hey Ben,

I'm not convinced that having 2 conflicting versions of scala-library on the IDE build path is healthy. You've tested it and apparently it does not break anything. I had problems with that few months ago (I don't remember what version of scala IDE / scala-library I have used) - the build in IDE didn't work in that scenario (however, I don't remember what was the exact problem). Anyway, to fix the problem I needed to remove the container from the project.

Not sure what to do with the problem that the scala ide requires its very own version of scala library for the incremental compilation to work. Is this a bug that the scala IDE team wants to fix at some point?

I'm wondering about:

  1. Gradle only adds the container if scala-library is not already on build classpath for given project.
  2. Gradle removes from the build path the reference to scala-library (defined via regular gradle dependencies) and adds the container instead.

I need touch base with the team how we want to approach it.

Ben Manes

Hi Szczepan,

I suspect that you used disjoint versions (e.g. 2.9.x in Gradle, 2.10.x in Eclipse). That would be a really easy mistake to make by naively taking the latest version and not realizing the incompatibility issues. I haven't tested that scenario since it could lead to a lot of surprising failures.

I definitely agree with (2) and I can try to make those changes to my pull request if agreed to. I'm less convinced of (1) because I haven't seen the problem occur.

When your team decides on the preferred approach, we can ask on the Scala IDE mailing list for one of their developers to offer feedback.

Szczepan Faber
Collaborator

Hey,

We're leaning towards solution 2 but we haven't yet concluded :)

When your team decides on the preferred approach, we can ask on the Scala IDE mailing list for one of their developers to offer feedback.

Are you already on this mailing list? Can lead the effort and ask the Scala IDE guys?

Cheers!

Ben Manes

requested feedback from ScalaIDE team.

Iulian Dragos

I'm a Scala IDE committer. I'm very glad to see support from Gradle!

  • The incremental builder should not misbehave if you have a second (or a different) scala-library on the build path. If it does, it's a bug and we'll fix it. If you have a small project I can have a look.
  • If there's more than one library on the classpath, the first one is picked up (the order can be changed on the Order and Export tab of the build path window
  • Hyperlinking does not treat the scala library in any special way, so the order on the classpath applies here as well.

I don't know if Gradle already does it, but it would be great if it attached sources to jars, so in case the exported project definition does not have the Scala container, the scala-library.jar comes with sources and hyperlinking still works.

For reference, sbteclipse filters out the scala-library dependency and adds only the classpath container. People seem to be happy with this solution, and it has the advantage that if you have the wrong IDE version (2.9 instead of 2.10), it won't crash spectacularly: you would see compilation errors if you're using anything that's been added in 2.10), or for dependencies that are compiled against the wrong version.

Ben Manes

The ScalaIDE 2.9x nightly appears to work perfectly. This is with Scala 2.9.2 gradle dependency and Scala 2.9.3-(nightly #) as the Scala Library.

I attempted to revert back to a stable release and reproduce the problem, but forgetting my original version I was unable to confidently identify a problem in v2.0.2. I was assuming full vs. incremental compiles for a project based on the speed of the build. If the ScalaIDE and gradle dependency used the same version of Scala then the build was immediate. If they differed by a minor version then the build was much slower. The change was to modify a single character in a Scalatra route. Given the behavior of the nightly build, I don't consider this to be a problem anymore.

I am in favoring of being consistent the rest of the ScalaIDE ecosystem. The m2eclipse-scala plugin also removes the scala-library from its dependency list.

With respect to attaching sources to jars, the EclipseClasspath has downloadSources enabled by default ("Whether to download and add sources associated with the dependency jars. Defaults to true.").

Szczepan Faber
Collaborator

Thanks a lot for the research and feedback.

Let's go for the option with replacing the scala-library with the scala container.

@Ben, can you prepare a new pull request? :) Please remember about:

  1. Eclipse plugin already does something for the scala projects - can you keep your code close to it?
  2. Unit test should cover that we're adding the container for scala projects and not adding for some other project, let's say java. It should also cover that the order of applying eclipse/scala plugin does not matter (in your test, just apply eclipse before scala and that will do it).
  3. We should have an integration test (say a new method in EclipseClasspathIntegrationTest) with a build file that has eclipse/scala plugins applied at the top of the script and dependencies declared later on, including scala library. Use fake maven repo (you'll find examples in the existing tests). Implementation-wise you probably want to use 'whenMerged' hook to remove scala library from the classpath.
  4. Can you document in the user guide (eclipse plugin I think) what we do for scala projects?
  5. Can you add a short note to the release notes ('notes.md') file about the feature?

Thanks a lot!

Ben Manes

Thanks Iulian and Szczepan!

I'd like to keep this pull request open until I have the code changes ready. I'll then close this request and start the new one so that the discussions can focus on the code review. That way these improvements don't fall off the radar as we work on the coding.

I will be participating in college career fair events next week so my changes may be delayed. Please note that since I am unaffiliated with Gradle or Scala teams, feel free to push forward if my progress becomes too slow. I'll work on the five points opportunistically, but may have other commitments that take precedence.

Szczepan Faber
Collaborator

Hey @Ben,

There's no rush. Feel free to work on the items in whatever order / time you like. We won't push on the implementation because of other priorities on our end. Bear in mind that we'd rather pull the request if all the items are completed (e.g. integ tests, docs, etc :)

It would be really helpful if you used fresh fork so that we don't have to review old commits.

Thanks a lot and good luck!

Ben Manes

I finally got back to this and have a partial solution. The only way I could find to remove the dependencies within EclipsePlugin.gradle was to use an whenMerged block (see changes). This removes it from the generated .classpath as desired.

However, when importing with the Gradle STS plugin the dependencies are still listed. I suspect that it is using the Gradle Tooling API instead to retrieve the list of external dependencies.

I'm unsure how best to proceed so that the removal of the external dependencies can be seen by gradle tooling.

Ben Manes

I realized a better approach that works is effectively,
eclipse.classpath.minusConfigurations += configurations.scalaTools

The only problem with this is that the scalaTools includes different dependencies than the ones that the ScalaIDE adds, so I have to dynamically create a configuration object. I'm trying to figure out the best way now.

It would be very helpful if a Gradle dev was available to chat for guidance. The freenode irc channel is unresponsive.

Ben Manes

@szczepiq

I have finished all items, rebased my branch, and squashed all changes into a single commit. Please let me know if you'd still prefer that we close this pull request and open a new one to focus on the code. I'm happy to continue this one.

I decided to add a scalaIde configuration to allow projects to exclude the scala library dependencies provided by the ScalaIDE. This was after a few failed attempts:

  • It appears that whenMerged changes are not made visible to the Gradle STS plugin. This approach had the benefit of automatic filtering, similar to the Maven plugin's approach linked above. I suspect that the changes are too late and opaque that they are not communicated out via the Gradle Tooling API.
  • The Scala plugin's scalaTools configuration could not be used instead of the scalaIde. If used directly it would exclude additional dependencies not provided by the ScalaIDE.
  • Due to non-deterministic ordering, I was unable to find a reliable way to inspect dependencies to infer the scala version. This meant that I could not dynamically create a detached configuration to exclude the libraries via a minusConfiguration.
  • Because of the above, I also attempted to specify the dependencies as a version range (e.g. 2.+). I had hoped that a minusConfiguration might have this be treated like a filter, but this did not appear to work.

The only two solutions remaining was to either have the build file specify the Scala version or add a new configuration type. I am unsure why the Scala plugin preferred the latter approach, but it set the precedent and expectation of how the Scala project should be configured. This is similar to how the idea-scala-facet plugin added a scalaApi configuration in order to perform some of the same integration magic.

A benefit of the approaches taken is that I was able to add them as build customizations into my projects until Gradle includes the improved support. The build file now emulates these fixes by including,

eclipse.classpath {
  minusConfigurations += configurations.scalaIde
  containers += ["org.scala-ide.sdt.launching.SCALA_CONTAINER"]
}
Szczepan Faber
Collaborator

Thanks for the investigation. I need to think about it a bit more.

Ben Manes

Please let me know if there is anything else that I can do so that v1.3 contains ScalaIDE support. That could be either the proposed code changes, documenting only the build customizations, or another alternative. I'd like to see a documented fix in v1.3 due to the awesome work towards native incremental compilation support. The build customizations has been a positive improvement for my engineering team by making Eclipse much more usable.

Szczepan Faber
Collaborator

Hey Ben,

Due to non-deterministic ordering, I was unable to find a reliable way to inspect dependencies to infer the scala version

I've discussed this issue with Peter. We think it should be possible to configure the minusConfigurations with a detached configuration that contains the same scala library dependency as the declared by the user. What exactly was the problem?

In general we would like to follow above path. E.g. detached configuration with scala library, assigned to minusConfigurations.

Ben Manes

Updated to use a detached configuration by waiting until all projects were evaluated before inspecting the dependencies. In addition to the tests, I verified with a scala project by inspecting the .classpath generated by ./gradlew eclipse as well as importing the project with Gradle STS. Both show the desired behavior.

This is thanks for Peter's answer to a similar problem on StackOverflow, where he advised using project.gradle.projectsEvaluated { ... } to resolve the ordering issue that I encountered previously.

Ben Manes Added ScalaIDE support to the Eclipse plugin.
The ScalaIDE classpath container is now added when to projects using
the Scala plugin. This resolve build problems after importing a Scala
project due to the ScalaIDE not being able to add its dynamically
computed entries to the classpath of the project.

The ScalaIDE adds the Scala libraries to the classpath, which
duplicates the dependencies provided by Gradle. In the ScalaIDE v2.0.2,
having multiple entries for scala-library results in full builds
instead of incremental). A detatched configuration allows projects to
exclude the dependencies from being provided by Gradle, which matches
the SBT and Maven behavior.
73b68b5
Peter Niederwieser
Collaborator

Thanks for the pull request. It's part of Gradle 1.3.

Peter Niederwieser pniederw closed this November 05, 2012
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Showing 1 unique commit by 1 author.

Nov 04, 2012
Ben Manes Added ScalaIDE support to the Eclipse plugin.
The ScalaIDE classpath container is now added when to projects using
the Scala plugin. This resolve build problems after importing a Scala
project due to the ScalaIDE not being able to add its dynamically
computed entries to the classpath of the project.

The ScalaIDE adds the Scala libraries to the classpath, which
duplicates the dependencies provided by Gradle. In the ScalaIDE v2.0.2,
having multiple entries for scala-library results in full builds
instead of incremental). A detatched configuration allows projects to
exclude the dependencies from being provided by Gradle, which matches
the SBT and Maven behavior.
73b68b5
This page is out of date. Refresh to see the latest.
5  subprojects/docs/src/docs/release/notes.md
Source Rendered
@@ -104,6 +104,10 @@ Groovy: [`ScalaCompile.fork = true`](dsl/org.gradle.api.tasks.scala.ScalaCompile
104 104
 activates external compilation, and [`ScalaCompile.forkOptions`](dsl/org.gradle.api.tasks.scala.ScalaCompile.html#org.gradle.api.tasks.scala.ScalaCompile:forkOptions)
105 105
 allows to adjust memory settings.
106 106
 
  107
+### ScalaIDE integration
  108
+
  109
+The [Eclipse Plugin](http://gradle.org/docs/current/userguide/eclipse_plugin.html) has improved Scala support by generating classpath entries for the [Scala IDE](http://scala-ide.org).
  110
+
107 111
 ## Promoted features
108 112
 
109 113
 Promoted features are features that were incubating in previous versions of Gradle but are now supported and subject to our backwards compatibility policy.
@@ -210,5 +214,6 @@ We would like to thank the following community members for making contributions
210 214
 * Gerd Aschemann - fixes for `application` plugins generated shell scripts (GRADLE-2501)
211 215
 * Cruz Fernandez - fixes to the properties sample project
212 216
 * Fadeev Alexandr - fixes for Gradle Daemon on Win 7 when `PATH` env var is not set (GRADLE-2461)
  217
+* Ben Manes - ScalaIDE integration
213 218
 
214 219
 We love getting contributions from the Gradle community. For information on contributing, please see (gradle.org/contribute)[http://gradle.org/contribute]
2  subprojects/docs/src/docs/userguide/eclipsePlugin.xml
@@ -44,7 +44,7 @@
44 44
             <td><link linkend="groovy_plugin">Groovy</link></td><td>Adds Groovy configuration to <filename>.project</filename> file.</td>
45 45
         </tr>
46 46
         <tr>
47  
-                <td><link linkend="scala_plugin">Scala</link></td><td>Adds Scala support to <filename>.project</filename> file.</td>
  47
+            <td><link linkend="scala_plugin">Scala</link></td><td>Adds Scala support to <filename>.project</filename> and <filename>.classpath</filename> files.</td>
48 48
         </tr>
49 49
         <tr>
50 50
             <td><link linkend="war_plugin">War</link></td><td>Adds web application support to <filename>.project</filename> file.
43  subprojects/ide/src/integTest/groovy/org/gradle/plugins/ide/eclipse/EclipseClasspathIntegrationTest.groovy
@@ -631,4 +631,47 @@ dependencies {
631 631
         libraries[1].assertHasJar(file('unresolved dependency - i.dont Exist 1.0'))
632 632
         libraries[2].assertHasJar(localJar)
633 633
     }
  634
+
  635
+    @Test
  636
+    void classpathIsConfiguredForScalaIDE() {
  637
+        //given
  638
+        def scalaCompilerJar = mavenRepo.module('org.scala-lang', 'scala-compiler', '2.9.2').publish().artifactFile
  639
+        def scalaLibraryJar = mavenRepo.module('org.scala-lang', 'scala-library', '2.9.2').publish().artifactFile
  640
+        def scalaSwingJar = mavenRepo.module('org.scala-lang', 'scala-swing', '2.9.2').publish().artifactFile
  641
+        def scalaDbcJar = mavenRepo.module('org.scala-lang', 'scala-dbc', '2.9.2').publish().artifactFile
  642
+        def scalaJlineJar = mavenRepo.module('org.scala-lang', 'jline', '2.9.2').publish().artifactFile
  643
+
  644
+        //when
  645
+        runEclipseTask """
  646
+apply plugin: 'scala'
  647
+apply plugin: 'eclipse'
  648
+
  649
+repositories {
  650
+    maven { url "${mavenRepo.uri}" }
  651
+    mavenCentral()
  652
+}
  653
+
  654
+dependencies {
  655
+    def libraries = [
  656
+        scala_compiler: "org.scala-lang:scala-compiler:2.9.2",
  657
+        scala_library: "org.scala-lang:scala-library:2.9.2",
  658
+        scala_swing: "org.scala-lang:scala-swing:2.9.2",
  659
+        scala_dbc: "org.scala-lang:scala-dbc:2.9.2",
  660
+        scala_jline: "org.scala-lang:jline:2.9.2",
  661
+    ]
  662
+
  663
+    scalaTools libraries.scala_compiler
  664
+    scalaTools libraries.scala_library
  665
+    scalaTools libraries.scala_jline
  666
+
  667
+    compile libraries.scala_library
  668
+    compile libraries.scala_swing
  669
+    compile libraries.scala_dbc
  670
+}
  671
+"""
  672
+
  673
+        //then
  674
+        assert classpath.libs.isEmpty()
  675
+        assert classpath.containers == ['org.eclipse.jdt.launching.JRE_CONTAINER', 'org.scala-ide.sdt.launching.SCALA_CONTAINER']
  676
+    }
634 677
 }
15  subprojects/ide/src/main/groovy/org/gradle/plugins/ide/eclipse/EclipsePlugin.groovy
@@ -16,6 +16,7 @@
16 16
 package org.gradle.plugins.ide.eclipse
17 17
 
18 18
 import org.gradle.api.Project
  19
+import org.gradle.api.artifacts.Dependency
19 20
 import org.gradle.internal.reflect.Instantiator
20 21
 import org.gradle.api.plugins.GroovyBasePlugin
21 22
 import org.gradle.api.plugins.JavaBasePlugin
@@ -143,6 +144,20 @@ class EclipsePlugin extends IdePlugin {
143 144
                         project.sourceSets.main.output.dirs + project.sourceSets.test.output.dirs
144 145
                     }
145 146
                 }
  147
+                project.plugins.withType(ScalaBasePlugin) {
  148
+                    classpath.containers 'org.scala-ide.sdt.launching.SCALA_CONTAINER'
  149
+
  150
+                    //remove the dependencies also provided by ScalaIDE
  151
+                    project.gradle.projectsEvaluated {
  152
+                        def provided = ["scala-library", "scala-swing", "scala-dbc"] as Set
  153
+                        def dependencies = project.configurations.collectMany {
  154
+                            it.getAllDependencies()
  155
+                        }.grep{ it.name in provided }.unique().toArray([] as Dependency[])
  156
+                        if (dependencies != []) {
  157
+                            classpath.minusConfigurations += project.configurations.detachedConfiguration(dependencies)
  158
+                        }
  159
+                    }
  160
+                }
146 161
             }
147 162
         }
148 163
     }
17  subprojects/ide/src/test/groovy/org/gradle/plugins/ide/eclipse/EclipsePluginTest.groovy
@@ -45,8 +45,8 @@ class EclipsePluginTest extends Specification {
45 45
 
46 46
     def applyToJavaProject_shouldOnlyHaveProjectAndClasspathTaskForJava() {
47 47
         when:
48  
-        project.apply(plugin: 'java-base')
49 48
         eclipsePlugin.apply(project)
  49
+        project.apply(plugin: 'java-base')
50 50
 
51 51
         then:
52 52
         assertThatCleanEclipseDependsOn(project, project.cleanEclipseProject)
@@ -63,28 +63,31 @@ class EclipsePluginTest extends Specification {
63 63
     }
64 64
 
65 65
     def applyToScalaProject_shouldHaveProjectAndClasspathTaskForScala() {
  66
+        def scalaIdeContainer = ['org.scala-ide.sdt.launching.SCALA_CONTAINER']
  67
+
66 68
         when:
67  
-        project.apply(plugin: 'scala-base')
68 69
         eclipsePlugin.apply(project)
  70
+        project.apply(plugin: 'scala-base')
  71
+        project.gradle.buildListenerBroadcaster.projectsEvaluated(project.gradle)
69 72
 
70 73
         then:
71 74
         assertThatCleanEclipseDependsOn(project, project.cleanEclipseProject)
72 75
         assertThatCleanEclipseDependsOn(project, project.cleanEclipseClasspath)
73 76
         checkEclipseProjectTask([new BuildCommand('org.scala-ide.sdt.core.scalabuilder')],
74 77
                 ['org.scala-ide.sdt.core.scalanature', 'org.eclipse.jdt.core.javanature'])
75  
-        checkEclipseClasspath([])
  78
+        checkEclipseClasspath([], scalaIdeContainer)
76 79
 
77 80
         when:
78 81
         project.apply(plugin: 'scala')
79 82
 
80 83
         then:
81  
-        checkEclipseClasspath([project.configurations.testRuntime])
  84
+        checkEclipseClasspath([project.configurations.testRuntime], scalaIdeContainer)
82 85
     }
83 86
 
84 87
     def applyToGroovyProject_shouldHaveProjectAndClasspathTaskForGroovy() {
85 88
         when:
86  
-        project.apply(plugin: 'groovy-base')
87 89
         eclipsePlugin.apply(project)
  90
+        project.apply(plugin: 'groovy-base')
88 91
 
89 92
         then:
90 93
         assertThatCleanEclipseDependsOn(project, project.cleanEclipseProject)
@@ -134,14 +137,14 @@ class EclipsePluginTest extends Specification {
134 137
         assert eclipseProjectTask.outputFile == project.file('.project')
135 138
     }
136 139
 
137  
-    private void checkEclipseClasspath(def configurations) {
  140
+    private void checkEclipseClasspath(def configurations, def additionalContainers = []) {
138 141
         GenerateEclipseClasspath eclipseClasspath = project.tasks.eclipseClasspath
139 142
         assert eclipseClasspath instanceof GenerateEclipseClasspath
140 143
         assert project.tasks.eclipse.taskDependencies.getDependencies(project.tasks.eclipse).contains(eclipseClasspath)
141 144
         assert eclipseClasspath.sourceSets == project.sourceSets
142 145
         assert eclipseClasspath.plusConfigurations == configurations
143 146
         assert eclipseClasspath.minusConfigurations == []
144  
-        assert eclipseClasspath.containers == ['org.eclipse.jdt.launching.JRE_CONTAINER'] as Set
  147
+        assert eclipseClasspath.containers == ['org.eclipse.jdt.launching.JRE_CONTAINER'] + additionalContainers as Set
145 148
         assert eclipseClasspath.outputFile == project.file('.classpath')
146 149
         assert eclipseClasspath.defaultOutputDir == new File(project.projectDir, 'bin')
147 150
     }
Commit_comment_tip

Tip: You can add notes to lines in a file. Hover to the left of a line to make a note

Something went wrong with that request. Please try again.