Skip to content

sbt-dotty: Add a setting to exclude a configuration from the Dotty IDE, and exclude the optimized projects from the Dotty build #3255

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.

Sign up for GitHub

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

Merged
merged 2 commits into from
Oct 5, 2017
Merged

sbt-dotty: Add a setting to exclude a configuration from the Dotty IDE, and exclude the optimized projects from the Dotty build #3255

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
c250f66
sbt-dotty: Add a setting to exclude a configuration from the Dotty IDE
smarter Oct 4, 2017
1c6c5c6
Build.scala: Exclude the optimized projects from the IDE
smarter Oct 4, 2017
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 9 additions & 1 deletion project/Build.scala
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -228,7 +228,15 @@ object Build {


// Bootstrap with -optimise
lazy val commonOptimisedSettings = commonBootstrappedSettings ++ Seq(scalacOptions ++= Seq("-optimise"))
lazy val commonOptimisedSettings = commonBootstrappedSettings ++ Seq(
scalacOptions ++= Seq("-optimise"),

// The *-bootstrapped and *-optimised projects contain the same sources, so
// we only need to import one set in the IDE. We prefer to import the
// non-optimized projects because optimize is slower to compile and we do
// not trust its output yet.
excludeFromIDE := true
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I read the above comment as "*-bootstrapped projects must be skipped, so why isn't this in commonBootstrappedSettings instead? My guess is that it's because dotty-language-server exists only in bootstrapped-mode and shouldn't be skipped?

Interestingly, commonBootstrappedSettings contains:

EclipseKeys.skipProject := true

It would probably be less surprising if kept close to excludeFromIDE := true.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There are three kinds of projects with the same set of sources:

  • Non-bootstrapped (compiled with Scala 2.12)
  • Bootstrapped (compiled with Dotty non-bootstrapped, default options)
  • Optimized (compiled with Dotty non-bootstrapped, with -optimise added)

For the Dotty IDE, the non-bootstrapped don't matter: they're not Dotty-compiled projects so they will be skipped by the plugin, so we only need to decide if we want the bootstrapped or the optimized one, we prefer the bootstrapped non-optimized for the reason given by the comment.

For Eclipse, the situation is different: the only projects which we want to import are the non-bootstrapped one, because the Scala IDE can only deal with Scala 2-compiled projects, so we skip everything else (commonBootstrappedSettings is also part of the settings used for optimized projects).

Hope this clears things up :).

)

lazy val commonBenchmarkSettings = Seq(
mainClass in (Jmh, run) := Some("dotty.tools.benchmarks.Bench"), // custom main for jmh:run
Expand Down
36 changes: 23 additions & 13 deletions sbt-dotty/src/dotty/tools/sbtplugin/DottyIDEPlugin.scala
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,16 @@ import scala.collection.mutable.ListBuffer
import DottyPlugin.autoImport._

object DottyIDEPlugin extends AutoPlugin {
object autoImport {
val excludeFromIDE = settingKey[Boolean]("If true, do not import this project or configuration in the Dotty IDE")
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not sure if it's a good idea, is it worth considering naming this key ide-skip-project? To match https://github.com/JetBrains/sbt-ide-settings/blob/750b993453fb3d1f31f371968d06e3fc792870a1/src/main/scala/sbtide/Keys.scala#L11-L12

Also, by convention, keys from the same plugin should ideally use the same prefix:

  • ideLaunch
  • ideExclude
  • ideCommand
  • ideRun

This convention makes it easy to explore available plugin keys from the sbt shell.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

is it worth considering naming this key ide-skip-project?

Using the same key name as another plugin seems like a bad idea in fact, because then they will conflict with each other, I have no idea what that does in the sbt repl but in build files you'll get errors due to ambiguity.

Also, by convention, keys from the same plugin should ideally use the same prefix

Maybe yes, feel free to open an issue for that. I have no idea what the good practices for sbt plugins are :).

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Instead of defining a new key, can't we just use skip in launchIDE := true?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That wouldn't be accurate since launchIDE just calls configureIDE and I don't think we can do skip in configureIDE since configureIDE is a command and not a task. It also wouldn't be complete since we also want to disable the config for compileForIDE. All in all, it seems better to have a specific key for this purpose :).


val codeCommand = taskKey[Seq[String]]("Command to start VSCode")
val runCode = taskKey[Unit]("Start VSCode, usually called from launchIDE")
val launchIDE = taskKey[Unit]("Configure and run VSCode on this project")
}

import autoImport._

// Adapted from scala-reflect
private def distinctBy[A, B](xs: Seq[A])(f: A => B): Seq[A] = {
val buf = new mutable.ListBuffer[A]
Expand Down Expand Up @@ -111,14 +121,20 @@ object DottyIDEPlugin extends AutoPlugin {
}
}

/** Run task `key` in all configurations in all projects in `projRefs`, using state `state` */
private def runInAllConfigurations[T](key: TaskKey[T], projRefs: Seq[ProjectRef], state: State): Seq[T] = {
/** Run task `key` in all configurations in all projects in `projRefs`, using state `state`,
* configurations where `excludeFromIDE` is `true` are skipped. */
private def runInAllIDEConfigurations[T](key: TaskKey[T], projRefs: Seq[ProjectRef], state: State): Seq[T] = {
val structure = Project.structure(state)
val settings = structure.data
val joinedTask = projRefs.flatMap { projRef =>
val project = Project.getProjectForReference(projRef, structure).get
project.configurations.flatMap { config =>
key.in(projRef, config).get(settings)
excludeFromIDE.in(projRef, config).get(settings) match {
case Some(true) =>
None // skip this configuration
case _ =>
key.in(projRef, config).get(settings)
}
}
}.join

Expand Down Expand Up @@ -151,20 +167,12 @@ object DottyIDEPlugin extends AutoPlugin {

private val projectConfig = taskKey[Option[ProjectConfig]]("")

object autoImport {
val codeCommand = taskKey[Seq[String]]("Command to start VSCode")
val runCode = taskKey[Unit]("Start VSCode, usually called from launchIDE")
val launchIDE = taskKey[Unit]("Configure and run VSCode on this project")
}

import autoImport._

override def requires: Plugins = plugins.JvmPlugin
override def trigger = allRequirements

def configureIDE = Command.command("configureIDE") { origState =>
val (dottyVersion, projRefs, dottyState) = dottySetup(origState)
val configs0 = runInAllConfigurations(projectConfig, projRefs, dottyState).flatten
val configs0 = runInAllIDEConfigurations(projectConfig, projRefs, dottyState).flatten

// Drop configurations that do not define their own sources, but just
// inherit their sources from some other configuration.
Expand Down Expand Up @@ -192,7 +200,7 @@ object DottyIDEPlugin extends AutoPlugin {

def compileForIDE = Command.command("compileForIDE") { origState =>
val (dottyVersion, projRefs, dottyState) = dottySetup(origState)
runInAllConfigurations(compile, projRefs, dottyState)
runInAllIDEConfigurations(compile, projRefs, dottyState)

origState
}
Expand Down Expand Up @@ -234,6 +242,8 @@ object DottyIDEPlugin extends AutoPlugin {
override def buildSettings: Seq[Setting[_]] = Seq(
commands ++= Seq(configureIDE, compileForIDE),

excludeFromIDE := false,

codeCommand := {
Seq("code", "-n")
},
Expand Down