Upgrade to sbt 1 #3441

Duhemm · 2017-11-07T14:39:46Z

Projects from the community build to port to sbt1:

dottybot

Hello, and thank you for opening this PR! 🎉

All contributors have signed the CLA, thank you! ❤️

Have an awesome day! ☀️

smarter · 2017-11-07T15:00:52Z

Awesome! :)

smarter · 2017-11-07T15:01:14Z

@ttreyer Could you please sign the Scala CLA at https://www.lightbend.com/contribute/cla/scala ? That way we'll finally be able to merge your code 😄

smarter · 2017-11-07T15:04:21Z

project/Build.scala

@@ -150,8 +151,8 @@ object Build {

    // Avoid having to run `dotty-sbt-bridge/publishLocal` before compiling a bootstrapped project
    scalaCompilerBridgeSource :=
-      (dottyOrganization %% "dotty-sbt-bridge" % dottyVersion % Configurations.Component.name)
-      .artifacts(Artifact.sources("dotty-sbt-bridge").copy(url =
+      (dottyOrganization %% "dotty-sbt-bridge" % "NOT_PUBLISHED" % Configurations.Component.name)


A change was accidentally reverted here: we stopped using NOT_PUBLISHED in 78ac17a

smarter · 2017-11-07T15:20:13Z

Until we're ready to merge this PR, we should enable running all the sbt scripted tests on every push to make sure everything passes.

smarter · 2017-11-07T15:21:28Z

sbt/zinc#444 should be ported here too.

smarter · 2017-11-07T15:24:22Z

sbt-dotty/src/dotty/tools/sbtplugin/DottyPlugin.scala

-            moduleID
-        }
-    }
+     implicit class DottyCompatModuleID(moduleID: ModuleID) {


extra space in indentation

smarter · 2017-11-07T15:24:36Z

sbt-dotty/src/dotty/tools/sbtplugin/DottyPlugin.scala

  }

  import autoImport._

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

-  // Adapted from CrossVersionUtil#sbtApiVersion
+  // Adapted from CrossVersioconstant nUtil#sbtApiVersion


smarter · 2017-11-07T15:29:35Z

project/Build.scala

-        ).map(_.withDottyCompat())
+          dottyOrganization % "dotty-library_2.11" % dottyNonBootstrappedVersion % Configurations.ScalaTool.name,
+          dottyOrganization % "dotty-compiler_2.11" % dottyNonBootstrappedVersion % Configurations.ScalaTool.name
+        )//.map(_.withDottyCompat(scalaVersion.value))


This should be reverted now that withDottyCompat has been ported.

Duhemm · 2017-11-08T09:38:04Z

I addressed the comments so far. I've ported sbt/zinc#444 here.

All the scripted tests are already being run on every push for this PR.

I've also finished upgrading all the projects of the community build to sbt 1.0.3 and proposed to upstream the changes.

To test the community build:

checkout the branch sbt1 in dotty-community-build
DOTTY_REFERENCE="+refs/pull/3441/merge" ./run.sh

allanrenucci · 2017-11-08T10:37:27Z

project/Build.scala

-     // By default scripted tests use $HOME/.ivy2 for the ivy cache. We need to override this value for the CI.
-      ScriptedPlugin.scriptedLaunchOpts ++= ivyPaths.value.ivyHome.map("-Dsbt.ivy.home=" + _.getAbsolutePath).toList,
-      ScriptedPlugin.scripted := ScriptedPlugin.scripted.dependsOn(Def.task {
+      version := "0.1.6-SNAPSHOT",


smarter · 2017-11-08T15:46:09Z

project/Build.scala

@@ -199,7 +199,7 @@ object Build {
        Seq(
          dottyOrganization % "dotty-library_2.11" % dottyNonBootstrappedVersion % Configurations.ScalaTool.name,
          dottyOrganization % "dotty-compiler_2.11" % dottyNonBootstrappedVersion % Configurations.ScalaTool.name


This line and the line above also need to be reverted to not use "_2.11"

smarter · 2017-11-08T15:47:31Z

compiler/src/dotty/tools/dotc/sbt/ExtractAPI.scala

+          underlying :: ancestorTypes0
+        } else
+          ancestorTypes0
+      } catch {


The try/catch should only be needed around the call to linearizedAncestorTypes(cinfo).

First draft at fixing the sbt bridge to match the Zinc 1.0 API.

After sbt/zinc#101, SimpleType simply doesn't exist anymore.

Some of sbt's callbacks need the full name of the class that can only be accessed during the first step of the GenBCode pipeline. So we call the callbacks in the first step, generating at the same time the class files, and keep those files through the whole pipeline so the last step can write the bytecode in them.

A FileConflictException may occure when creating a class file. Add check for this error and refactor a bit of code around it.

smarter · 2017-11-22T15:35:45Z

project/Build.scala

@@ -862,7 +862,7 @@ object Build {
      unmanagedSourceDirectories in Compile +=
        baseDirectory.value / "../language-server/src/dotty/tools/languageserver/config",
      sbtPlugin := true,
-      version := "0.1.6-SNAPSHOT",
+      version := "0.1.8",


This is an incompatible change, I would bump to 0.2.0

smarter · 2017-11-22T15:38:52Z

compiler/src/dotty/tools/dotc/sbt/ExtractDependencies.scala

+      def apply(x: Option[Symbol], t: Tree)(implicit ctx: Context) =
+        if (x.isDefined) x
+        else t match {
+          case moduleDef: Thicket =>


I don't think this case is ever going to be hit, objects in Dotty after Typer are represented by a TypeDef and a ValDef, the TypeDef case below will be used.

smarter · 2017-11-22T15:40:33Z

compiler/src/dotty/tools/dotc/sbt/ExtractDependencies.scala

+    else {
+      val file = Option(sym.associatedFile)
+
+      Option(sym.associatedFile).flatMap {


I don't understand what this is for, if the symbol comes from source then sym.associatedFile will never be NoSourceFile.

smarter · 2017-11-22T15:44:08Z

compiler/src/dotty/tools/dotc/sbt/ExtractDependencies.scala

-        }
-      } else if (depFile.file != currentSourceFile) {
-        ctx.sbtCallback.classDependency(extractedName(dep.enclosingClass), extractedName(currentClass), context)
+    val onSource = dep.sourceFile


Why was this changed? This is also very likely to do the wrong thing. If dep was loaded from TASTY, dep.sourceFile will return the source filename stored in the SoureFile attribute of the TASTY section. But here we're really interested in recording the dependency on the classfile where dep comes from, because if this classfile changes we'll have to recompile the current unit.

smarter · 2017-11-22T15:45:00Z

compiler/src/dotty/tools/dotc/sbt/ExtractDependencies.scala

-          builder.append(" in [")
-          otherScopes.forEach(new java.util.function.Consumer[UseScope]() {
-            def accept(scope: UseScope): Unit =
+          // Pickling tests fail when this is turned in an anonymous class


Could you open an issue so we don't forget about this?

smarter · 2017-11-22T15:45:49Z

compiler/src/dotty/tools/dotc/sbt/ExtractDependencies.scala

@@ -182,13 +246,13 @@ private final class NameUsedInClass {
 *  specially, see the subsection "Dependencies introduced by member reference and
 *  inheritance" in the "Name hashing algorithm" section.
 */
-private class ExtractDependenciesCollector(implicit val ctx: Context) extends tpd.TreeTraverser { thisTreeTraverser =>
+private class ExtractDependenciesCollector(responsibleForImports: Symbol)(implicit val ctx: Context) extends tpd.TreeTraverser { thisTreeTraverser =>


Please add some documentation for this parameter

smarter · 2017-11-22T15:46:24Z

compiler/src/dotty/tools/dotc/sbt/ExtractDependencies.scala

@@ -199,15 +263,18 @@ private class ExtractDependenciesCollector(implicit val ctx: Context) extends tp
   *  because it refers to these classes or something defined in them.
   *  This is always a superset of `topLevelInheritanceDependencies` by definition.
   */


Documentation needs to be updated since this is now a set of pair, same for the def just below.

smarter · 2017-11-22T15:46:43Z

compiler/src/dotty/tools/dotc/sbt/ExtractDependencies.scala


  private def addUsedName(enclosingSym: Symbol, name: Name) = {
-    val enclosingName = extractedName(enclosingSym)
+    val enclosingName = enclosingSym match {
+      case sym if sym == defn.RootClass => ExtractDependencies.extractedName(responsibleForImports)


Why use a pattern match and not just an if/else here?

smarter · 2017-11-22T15:49:02Z

compiler/src/dotty/tools/dotc/sbt/ExtractDependencies.scala

@@ -276,19 +349,20 @@ private class ExtractDependenciesCollector(implicit val ctx: Context) extends tp
            addImported(name)
          case Thicket(Ident(name) :: Ident(rename) :: Nil) =>
            addImported(name)
-            if (rename ne nme.WILDCARD)
+            if (rename ne nme.WILDCARD) {


Unnecessary brackets

smarter · 2017-11-22T15:49:26Z

compiler/src/dotty/tools/dotc/sbt/ExtractDependencies.scala


-  private object PatMatDependencyTraverser extends ExtractTypesCollector {
+  private class PatMatDependencyTraverser(ctx0: Context) extends ExtractTypesCollector(ctx0) {


Why not implicit ctx: Context ? And at the same time, the implicit ctx: Context argument to method in this class should be removed.

smarter · 2017-11-22T15:51:04Z

compiler/src/dotty/tools/dotc/sbt/ThunkHolder.scala

-    SafeLazyWrapper.strict(t)
-}
-
-/** Wrapper around SafeLazy implementations.


So great that we can get rid of this stuff 😄 🔥

smarter · 2017-11-22T15:56:21Z

compiler/src/dotty/tools/dotc/sbt/ExtractAPI.scala

+      name, acc, modifiers, anns, defType, api.SafeLazy.strict(selfType), api.SafeLazy.strict(structure), Constants.emptyStringArray,
+      childrenOfSealedClass, topLevel, tparams)
+
+    // if (name.toString.contains("DottyPredef")) {


Remove debug code

smarter · 2017-11-22T15:57:20Z

compiler/src/dotty/tools/dotc/sbt/ExtractAPI.scala

+
+    allNonLocalClassesInSrc += cl
+
+    val javaPlatform = ctx.platform.asInstanceOf[JavaPlatform]


Please avoid the cast here, we don't want the compiler to crash when run with the Scala.js backend. I think we should just move hasJavaMainMethod outside of JavaPlatform

Yes, especially since Scala.js supports exactly the same main methods as Scala/JVM!

smarter · 2017-11-22T15:59:38Z

compiler/src/dotty/tools/dotc/sbt/ExtractDependencies.scala

+    // }
+}
+
+private final class NameUsedInClass {


All the added classes/objects in this PR should get a documentation comment.

jvican · 2017-11-28T18:16:25Z

Is there any chance this can be merged and released together with 0.5.0-RC1? I'd like to try cross-compiling bloop to Dotty. For now, I'm doing it locally, but ideally I'd have an official version to depend on so that the CI passes my changes.

smarter · 2017-11-28T18:19:11Z

Is there any chance this can be merged and released together with 0.5.0-RC1?

No, that would require delaying the release too much. But once this PR is merged you'll be able to use our nightly builds in your project, using a nightly build of Dotty is just as easy as using a release.

allanrenucci · 2017-12-21T10:57:18Z

We had a regression with sbt-pack #3124: the generated jar contained the non-bootstrapped version. Since you are updating the version, it would be good to know if the issue is still present.
/cc @liufengyun

allanrenucci · 2017-12-21T12:52:17Z

sbt-dotty/src/dotty/tools/sbtplugin/DottyPlugin.scala

        else
-          (incOptions in Compile).value
+          scalaBridge
      },

      scalaBinaryVersion := {


Can this be remove?

allanrenucci · 2017-12-21T13:24:10Z

.drone.yml

+    image: lampepfl/dotty:2017-10-20
+    commands:
+      - cp -R . /tmp/5/ && cd /tmp/5/
+      - ./project/scripts/sbt "sbt-dotty/scripted source-dependencies/*2of2"


It would be great if we did not introduce this huge regression (3x) in the time needed to run the scripted tests.
@Duhemm says it is because the bridge is recompiled for each scripted test. @smarter do you have an idea how this could be fixed?

Complain to the sbt people: sbt/sbt#3469 Maybe ask someone who has a lightbend subscription? :)

Alternatively, we could try copy-pasting the custom implementation of scripted used by zinc, it even has parallelism and fancy colors contributed by @jvican :P. https://github.com/sbt/zinc/blob/1.x/project/Scripted.scala https://github.com/sbt/zinc/tree/1.x/internal/zinc-scripted/src/test/scala/sbt/internal/inc

liufengyun · 2017-12-21T13:33:56Z

Good catch @allanrenucci : 0.9.1 is the version where we had the regression. The latest version for sbt-pack is 1.0.4. We can run dist-bootstrapped/pack, if the generated jars are good, then there's no worry.

allanrenucci · 2017-12-21T14:23:00Z

Unfortunately dist-bootstrapped/pack generates the non bootstrapped jars

xerial · 2018-01-11T22:26:23Z

@allanrenucci @liufengyun Released sbt-pack 0.10.0, which fixes an issue of including too many modules. xerial/sbt-pack#67

xerial · 2018-01-13T01:10:10Z

Released sbt-pack 0.10.1, which fixes a bug if you have multiple nested projects in build.sbt. Please use this version. Thanks.

allanrenucci · 2018-01-15T10:49:30Z

Thanks @xerial! I can confirm this fixes our issue

OlivierBlanvillain · 2018-01-18T17:43:04Z

I'm closing this PR given that it's been rebased in #3687. @allanrenucci next time just merge master into the existing PR, rebasing doesn't scale to multiple committers and long standing PRs...

This partially reverts 06a3d47 to bring back Dotty.js from the dead. Many things are broken still, but this is enough to do some experiments: - The backend was initially removed because of scala#1574, I don't see why GetClass has a requirement on FunctionalInterfaces, so I just removed it ¯\_(ツ)_/¯ - Scala.js was upgraded to 0.6.19. More recent versions of Scala.js cannot be used because they require sbt 0.13.16 and we're blocked by sbt/sbt#3460, this won't be an issue anymore once we switch to sbt 1 (see scala#3441) - GenSJSIR was hacked to compile with 0.6.19 but is missing most of the changes to the Scala.js backend phase for Scala 2 that happened since it was initially ported to Dotty from Scala.js 0.6.8 - scalajs-ir does not compile with Dotty anymore, see comments in Build.scala - The Scala.js backend explodes when extending a Scala 2 trait because of the way LinkScala2Impls transforms supercalls to these traits. Luckily, we don't need to extend js.JSApp anymore to make a Hello World :P. I verified that the following works: sbt sjsSandbox/run But I don't know if anything else does!

dottybot reviewed Nov 7, 2017

View reviewed changes

smarter reviewed Nov 7, 2017

View reviewed changes

allanrenucci reviewed Nov 8, 2017

View reviewed changes

smarter reviewed Nov 8, 2017

View reviewed changes

smarter and others added 17 commits November 13, 2017 07:49

sbt 1.0.0-M6

6aafb69

Upgrade to sbt 1.0.0-M6

4ce83b8

Fix sbt bridge

c91171d

First draft at fixing the sbt bridge to match the Zinc 1.0 API.

Add method SymDenotation.isTopLevelClass

024f5fb

Remove SimpleType

e1018d1

After sbt/zinc#101, SimpleType simply doesn't exist anymore.

Update bridge for compatibility with Zinc 1.0.0-X15

7865186

Fix for zinc 1.0.0-X16

3f8a49c

ExtractAPI: fix children of sealed classes

848e41b

ExtractAPI: add missing startSource callback

bfd2b0f

Fix various issues

d3f50a4

Catch FileConflictException creating class files

d7dba97

A FileConflictException may occure when creating a class file. Add check for this error and refactor a bit of code around it.

Adapt to latest Zinc

fb9c611

Compile only the bridge, not the whole compiler

aef68c2

Adapt to new main class detection in Zinc 1

0f13174

Re-enable dist-* projects

323b451

Fix bin/dot* scripts

c05353a

smarter reviewed Nov 22, 2017

View reviewed changes

smarter requested changes Nov 22, 2017

View reviewed changes

smarter reviewed Nov 22, 2017

View reviewed changes

This was referenced Nov 23, 2017

Is there a version of sbt-dotty for sbt 1.x? #3091

Closed

Migrate build to sbt 1.0 scalacenter/scalafix#281

Closed

smarter removed this from the 0.5 Tech Preview milestone Nov 24, 2017

jvican mentioned this pull request Nov 28, 2017

Add partial Dotty support scalacenter/bloop#74

Closed

allanrenucci added this to the 0.6 Tech Preview milestone Nov 30, 2017

allanrenucci added the stat:wip label Dec 18, 2017

allanrenucci reviewed Dec 21, 2017

View reviewed changes

allanrenucci mentioned this pull request Dec 21, 2017

Upgrade to sbt 1 #3687

Closed

allanrenucci modified the milestones: 0.6 Tech Preview, 0.7 Tech Preview Jan 18, 2018

OlivierBlanvillain assigned allanrenucci Jan 18, 2018

OlivierBlanvillain closed this Jan 18, 2018

allanrenucci mentioned this pull request Jan 19, 2018

Upgrade to sbt 1 #3872

Merged

Duhemm deleted the topic/sbt1 branch November 30, 2018 07:56


		private object PatMatDependencyTraverser extends ExtractTypesCollector {
		private class PatMatDependencyTraverser(ctx0: Context) extends ExtractTypesCollector(ctx0) {


		allNonLocalClassesInSrc += cl

		val javaPlatform = ctx.platform.asInstanceOf[JavaPlatform]

Upgrade to sbt 1 #3441

Upgrade to sbt 1 #3441

Uh oh!

Conversation

Duhemm commented Nov 7, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

dottybot left a comment

Choose a reason for hiding this comment

Uh oh!

smarter commented Nov 7, 2017

Uh oh!

smarter commented Nov 7, 2017

Uh oh!

Choose a reason for hiding this comment

Uh oh!

smarter commented Nov 7, 2017

Uh oh!

smarter commented Nov 7, 2017

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Duhemm commented Nov 8, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

smarter Nov 22, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

jvican commented Nov 28, 2017

Uh oh!

smarter commented Nov 28, 2017

Uh oh!

allanrenucci commented Dec 21, 2017

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Duhemm commented Nov 7, 2017 •

edited

Loading

Duhemm commented Nov 8, 2017 •

edited

Loading

smarter Nov 22, 2017 •

edited

Loading