Improved support for multiple scala versions

[daenenk]

improved-support-for-multiple-scala-versions
for ScalaIDE-4.6 and up
to support also scala-2.11 while preserving functionality for older versions,
i.e. support for scala-2.10 in ScalaIDE-4.0 and up.

This is a patch on top of commit 1bbf64c (Fix #239)
added EclipseKeys.defaultScalaInstallation
usage documentation added in README.md

[daenenk]

#239 provided a initial support for multiple scala versions for Scala-IDE4.0, but is the implementation looked for scala-2.10 specifically. As a consequence the issue raised again when Scala-IDE4.6.0 came out targeting scala-2.12 as default.
Some Issue reports and request have attempted to fix this: see #287 and #340.
With this pull I hope to provide a more generic solution. I refactored the code submitted in #239.

[benmccann]

I don't understand what the D represents here

[daenenk]

D stands for "default". 'xD.yD' represents the scala installation eclipse is using as default.

[daenenk]

I updated the description. I hope it is more clear now.

[benmccann]

Would it be accurate to call these variables installationVersion and projectVersion?

[daenenk]

fine for me.

[daenenk]

update as you suggested has been pushed

[benmccann]

@daenenk I just merged this, but now am wondering if this is the best approach and am thinking about reverting it. Do we really need to make the user set defaultScalaInstallation themselves or can we extract it from ScalaVersion like in #340

[daenenk]

Both #356 and #340 determine the version value for the properties scala.compiler.additionalParams, scala.compiler.installation and scala.compiler.sourceLevel (in org.scala-ide.sdt.core.prefs) from scalaVersion (in build.sbt). The delta is the criteria when to specify these project specific properties or not. #340 checks whether Xp.Yp (as derived from scalaVersion) is exactly 2.10; if yes, then the project specific parameters are generated. I could have taken the same approach, and check on 2.10 or 2.11 (further referred to a approach A), without introducing the key defaultScalaInstallation. This would have worked for Scala IDE 4.6 - 4.7, but would impact support for older Scala IDE releases, and would fail again when a future Scala IDE comes with scala 2.13 or 3.0 as default scala installation.

The introduction of defaultScalaInstallation, set to 2.12 as default (i.e. the approach of #356, further referred to a B), results in the same behavior as A for Scala IDE 4.6 - 4.7, while people could ignore defaultScalaInstallation. In addition, it gives the flexibility to set defaultScalaInstallation to 2.11 in order to support e.g. Scala IDE 4.5.0, as well as to e.g. 2.13 for futureScala IDE releases.

To be clear, defaultScalaInstallation reflects your Scala IDE, and is not related to the scalaVersion of your project. I don't like eclipse-specifc settings in my build.sbt file. The solution for this is to specify sbtsclipse as global plugin, and (if needed) to overwrite defaultScalaInstallation in a global ad hoc plugin. For sbt 0.13, you could write a file ~/.sbt/0.13/plugins/SbtEclipseSettings.scala:

import sbt._
import Keys._
import com.typesafe.sbteclipse.plugin.EclipsePlugin.EclipseKeys._

object SbtEclipseSettings extends Plugin {
  override def settings = Seq(
    defaultScalaInstallation := "2.11"
  )
}

This may look complex, but most people will not need it, and for those who need it, it is easier than being blocked or having to create a pull request on sbteclipse again.

I don't know exactly what all implications are of the setting scala.compiler.additionalParams=\ -Xsource\:2.11 -Ymacro-expand\:none. That's what Scala IDE itself generate when you select scala installation 2.11. An alternative solution (C) would be to always set scala.compiler.useProjectSettings=true and scala.compiler.additionalParams=\ -Xsource\:Xp.Yp -Ymacro-expand\:none, unless the latter would be overrruled by scalacOptions. Still, silently adding -Ymacro-expand\:none (also for scala 2.12 projects) may not be what people want/expect. In case there would be a need to overrrule scala.compiler.useProjectSettings=true, still a sbteclipse-specific key will be needed.

Up till now, I think approach B had the smallest impact on the sbteclipse code based and it's behaviour, and it is better than before #356, as well as approach A. It will help people out using scala 2.11 on a recent Scala IDE's, and don't break functionality for others.

A better solution may be possible in the future. It would be nice if the engineers of the eclipse Scala IDE would join this discussion.

[benmccann]

@wpopielarski @sschaef do you think that sbteclipse is taking the correct approach to generating eclipse files for use by scala IDE here?

[daenenk]

Defining a project in build.sbt, for me, is the best approach; then using sbt to resolve the dependencies. Finaly we need a way to import the project into an IDE. sbteclipse does a pretty good job: the eclipse task resolves the dependencies and generates a resolved project definition, in terms of producing the eclipse files.

The question is whether generating the latter directly in a (eclipse) Scala IDE format is the best approach. To decouple the dependencies on the IDE, maybe you could agree on an a generic resolved project definition format (something straight and simple, all you need in a single file: property-file or json formatted). The Scala IDE should than support importing the latter. During the import, warnings of incompatibility could be generated. This approach would also require an update-import feature in eclipse.

So far, I've no experience with https://github.com/scala-ide/sbt-integration. At first glance, it looks more complex to setup.

[benmccann]

Thinking about this more, I believe that setting scala.compiler.useProjectSettings=true would have been the better thing to try, so I've reverted this PR. It sounds like we should set -Ymacro-expand:none only if the user is using 2.10

[daenenk]

I agree with

I believe that setting scala.compiler.useProjectSettings=true would have been the better thing to try

That's exactly what the code does, in case the defaultScalaInstallation > projectScalaVersion

1. setting scala.compiler.useProjectSettings=true, and
2. setting scala.compiler.installation

The fact that also scala.compiler.additionalParams=-Xsource\:2.11 -Ymacro-expand\:none must be set is unfortunately. It is also unclear for me why exactly the ScalaIDE requires this, but it get sbteclipse working in all cases.

[benmccann]

Can we do it always so that this plugin does not to have to know about defaultScalaInstallation?

[daenenk]

Yes, but it has consequences.
The scala.compiler.installation parameter, sets the scala compiler. But that is not enough, since the IDE has a single presentation compiler. With the -Xsource param, the latter is forced in a backward compatibility mode. This mode however has some limitations; that's why the param -Ymacro-expand:none is needed as well. I'm not 100% sure whether this limitation is still there but I've not found evidence of this being solved.

So yes, we can apply these settings in all cases, which means we apply also -Ymacro-expand:none in all cases. Personally, I could live with that.

If some people would need macro-expand support in eclipse, sbt-eclipse could define a setting key useProjectSettings (iso defaultScalaInstallation). Only when this is true the settings as discussed are generated. You may not like again having a setting key, but at least this one does not reflect a version number.

[benmccann]

We may only need to do that when setting -Xsource:2.10?

Since whitebox macros have some incompatible improvements between 2.11 and 2.10, the IDE is not able to expand macros for you in compatibility mode. This probably will remain a long-term limitation of this mode. This is why the -Ymacro-expand:none setting works conjointly with the -Xsource:2.10 setting.

I'm not sure though when they say "This probably will remain a long-term limitation of this mode." whether that means we need to do it for newer versions as well. Maybe you're right that it always needs to be done

[benmccann]

Maybe a better option would be to have a boolean flag like setProjectScalaVersion. Then you can still choose to set it or not. The thing that gave me pause about defaultScalaInstallation is that it makes it really hard to work in a team environment where people upgrade their Scala IDE at different times

[daenenk]

I keep the sbt-eclipse plugin and its settings as global settings on my machine only, as other developers may not use it. I do share the concern that having to maintain a version number in the build config with defaultScalaInstallation may cause extra maintenance and version control issues in a team.

Based on your view I summarize the solution as:

• definition of a setting key setProjectScalaVersion
• when setProjectScalaVersion == false, no project specific parameters are configured
• when setProjectScalaVersion == true, the following settings will be configured:
  • scala.compiler.useProjectSettings=true
  • scala.compiler.installation
  • scala.compiler.additionalParams
    The latter will specify the scala version, and only in case this is 2.10.x, it also adds -Ymacro-expand:none

[benmccann]

Yes, sounds good to me

Sorry for the back-and-forth on this one and my taking so long. I no longer use sbt as we have migrated to Play on Gradle, so I've had a hard time finding time to devote to this