Bump to sbt 1.0.0-M6 by eed3si9n · Pull Request #323 · sbt/zinc

eed3si9n · 2017-06-28T04:43:08Z

eed3si9n · 2017-06-28T05:03:14Z

[info] Non-compiled module 'compiler-bridge_2.10' for Scala 2.10.5. Compiling...
275s
2541
[info] Loading project definition from /tmp/sbt_cd9bdb55/deb10309809912fbf38cf891af2ac61342024632/project
278s
2542
[info] xsbt.ExtractUsedNamesPerformanceSpecification *** ABORTED ***
279s
2543
[info]   java.lang.NoClassDefFoundError: scala/Product$class
279s
2544
[info]   at xsbti.TestCallback$TestUsedName.<init>(TestCallback.scala:11)
279s
2545
[info]   at xsbti.TestCallback.usedName(TestCallback.scala:60)
279s
2546
[info]   at xsbt.ExtractUsedNames.$anonfun$extractAndReport$5(ExtractUsedNames.scala:148)
279s
2547
[info]   at xsbt.ExtractUsedNames.$anonfun$extractAndReport$5$adapted(ExtractUsedNames.scala:138)
279s
2548
[info]   at xsbt.JavaUtils$JavaForEach$.foreach$extension(JavaUtils.scala:16)
279s
2549
[info]   at xsbt.ExtractUsedNames.$anonfun$extractAndReport$4(ExtractUsedNames.scala:138)
279s
2550
[info]   at xsbt.ExtractUsedNames.$anonfun$extractAndReport$4$adapted(ExtractUsedNames.scala:136)
279s
2551
[info]   at xsbt.JavaUtils$JavaMapForEach$.foreach$extension(JavaUtils.scala:27)
279s
2552
[info]   at xsbt.ExtractUsedNames.extractAndReport(ExtractUsedNames.scala:136)
279s
2553
[info]   at xsbt.API$ApiPhase.processScalaUnit(API.scala:46)
279s
2554
[info]   ...
279s
2555
[info]   Cause: java.lang.ClassNotFoundException: scala.Product$class
279s
2556
[info]   at java.net.URLClassLoader.findClass(URLClassLoader.java:381)
279s
2557
[info]   at java.lang.ClassLoader.loadClass(ClassLoader.java:424)
279s
2558
[info]   at sun.misc.Launcher$AppClassLoader.loadClass(Launcher.java:331)
279s
2559
[info]   at java.lang.ClassLoader.loadClass(ClassLoader.java:357)
279s
2560
[info]   at xsbti.TestCallback$TestUsedName.<init>(TestCallback.scala:11)
279s
2561
[info]   at xsbti.TestCallback.usedName(TestCallback.scala:60)
279s
2562
[info]   at xsbt.ExtractUsedNames.$anonfun$extractAndReport$5(ExtractUsedNames.scala:148)
279s
2563
[info]   at xsbt.ExtractUsedNames.$anonfun$extractAndReport$5$adapted(ExtractUsedNames.scala:138)
279s
2564
[info]   at xsbt.JavaUtils$JavaForEach$.foreach$extension(JavaUtils.scala:16)
279s
2565
[info]   at xsbt.ExtractUsedNames.$anonfun$extractAndReport$4(ExtractUsedNames.scala:138)
279s
2566
[info]   ...

jvican · 2017-06-30T07:45:29Z

zinc/src/main/java/xsbti/compile/ZincCompilerUtil.java

 * of the Scala incremental compiler if only {@link IncrementalCompiler} is required.
 */
-public interface ZincCompilerUtil {
+public class ZincCompilerUtil {


Can you add a comment to the Scaladoc that explains the reason why this is turned into a class instead of being an interface and linking to the issue?

jvican · 2017-06-30T07:46:12Z

zinc/src/main/scala/sbt/internal/inc/ZincUtil.scala

  }
-}
+
+  def constantBridgeProvider(


I think this is not passing formatting.

On both my local machine and on Drone box, `ZincComponentCompilerSpec` fails to compile 2.10 compiler bridge with the following error: ``` /T/sbt_1234/xsbti/compile/CompilerBridgeProvider.java:33: error: `;' expected but `{' found. static CompilerBridgeProvider constant(File file, ScalaInstance scalaInstance) { ^ ``` I suspected that this is due to Java `interface` defining a static method, a change introduced in only in Java 8. After making this change, the spec passed the test.

typesafe-tools · 2017-06-30T08:38:44Z

dbuild has checked the following projects against Scala 2.12:

Project	Reference	Commit
sbt	pull/3295/head	sbt/sbt@`0774a24`
zinc	pull/323/head	`eea9135`
io	1.0	sbt/io@`177ba25`
librarymanagement	pull/120/head	sbt/librarymanagement@`6953cb0`
util	1.0	sbt/util@`54cf4a4`
website	1.0.x	sbt/website@`5c32a0a`

✅ The result is: SUCCESS
(restart)

jvican

LGTM. Just some small comments to polish this, but looks great overall!

Lemme know when you've addressed feedback and I'll merge.

P.S. I wish we had a sbt release with the logs fixes. We'll be missing stack traces... Please, as soon as RC is out, upgrade the sbt version here. Future contributors in sprees will be affected by those old bugs.

jvican · 2017-07-03T13:58:09Z

build.sbt

-lazy val compilerBridgeScalaVersions = Seq(scala212, scala211, scala210)
+lazy val compilerBridgeScalaVersions = List(scala212, scala211, scala210)

 val scalafmtCheck = Command.command("scalafmtCheck") { state =>


This can be removed.

jvican · 2017-07-03T13:58:32Z

build.sbt

 //   format from which Java sources are generated by the sbt-contraband plugin.
 lazy val compilerInterface = (project in internalPath / "compiler-interface")
  .enablePlugins(ContrabandPlugin)
-  .disablePlugins(com.typesafe.sbt.SbtScalariform)


Context: sbt house rules has removed support for Scalariform.

jvican · 2017-07-03T13:59:26Z

build.sbt

    // javaOnlySettings,
    name := "Compiler Interface",
-    crossScalaVersions := Seq(scala212),
+    // Use the smallest Scala version in the compilerBridgeScalaVersions


It would be good to make this comment more descriptive and explain why this is the case so that future readers understand that the bridge has to be as much compatible as possible.

tbh I am not sure how much this had effect, but I did expand the comment.

I thought you did this so that the regression doesn't happen again. Forcing the latest Java version will always guarantee that our bridge compiles for all the Java versions supported, I presumed that was what you had in mind.

jvican · 2017-07-03T14:00:03Z

build.sbt

-    publishLocal := (),
+    publish := {},
+    publishLocal := {},
+    skip in publish := true,


If we already have skip in publish, why are publish and publishLocal definitions still required?

skip in publish AFAIK is only respected by sbt-pgp.

Okay, I thought that you had already merged the skip in publish functionality in sbt 1.0.x.

jvican · 2017-07-03T14:00:46Z

build.sbt

    s"${compilerInterface.id}/publishLocal" ::
      compilerBridgeScalaVersions.flatMap { (bridgeVersion: String) =>
-      s"wow $bridgeVersion" ::
+      s"++ $bridgeVersion!" ::


The good thing about this is that we won't have OOM errors with ++ anymore in sbt 1.0.

jvican · 2017-07-03T14:01:33Z

project/Header.scala

@@ -1,27 +1,27 @@
-import sbt._
-import Keys._
+// import sbt._


Can you open an issue to keep track of this? We will need to enable it after RC is out and sbt-header cross-compiles.

jvican · 2017-07-03T14:02:36Z

project/Scripted.scala

 import sbt._
 import Keys._
 import Def.Initialize
+import sbt.internal.inc.ScalaInstance


Why are these required? Seem they are unnecessary. I wonder if this is a slip.

We should consider moving that out of internal.

But why are these necessary? Why are the imports the only changes in this file?

Because the name has moved, from the sbt package to sbt.internal.inc.

jvican

LGTM. All comments addressed.

Add support for atLeastOne method on a generator

eed3si9n force-pushed the wip/sbt1beta2 branch from 76d4412 to 6a6965b Compare June 28, 2017 04:50

eed3si9n force-pushed the wip/sbt1beta2 branch 2 times, most recently from 85070d5 to 152208f Compare June 30, 2017 07:27

jvican reviewed Jun 30, 2017

View reviewed changes

zinc/src/main/scala/sbt/internal/inc/ZincUtil.scala Outdated

}

}

def constantBridgeProvider(

Copy link

Member

jvican Jun 30, 2017

Choose a reason for hiding this comment

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

I think this is not passing formatting.

eed3si9n added 2 commits June 30, 2017 03:58

Bump to sbt 1.0.0-M6

eea9135

eed3si9n force-pushed the wip/sbt1beta2 branch from 152208f to eea9135 Compare June 30, 2017 08:02

eed3si9n changed the title ~~[wip] Bump to sbt 1.0.0-M6~~ Bump to sbt 1.0.0-M6 Jun 30, 2017

sbt deleted a comment from typesafe-tools Jun 30, 2017

eed3si9n mentioned this pull request Jun 30, 2017

Remove JDK-version dependent code out of the compiler bridge #327

Merged

jvican mentioned this pull request Jun 30, 2017

Migrate to sbt 1.0.0-M6 #316

Closed

eed3si9n requested review from Duhemm, dwijnand and jvican July 2, 2017 00:11

eed3si9n mentioned this pull request Jul 2, 2017

Bump io, util, lm, and Contraband to the latest #331

Merged

dwijnand previously approved these changes Jul 2, 2017

View reviewed changes

jvican requested changes Jul 3, 2017

View reviewed changes

eed3si9n added 2 commits July 3, 2017 14:53

Fix ScalaFmt wiring

d7f75ab

Add more comments per review

0cca946

eed3si9n dismissed dwijnand’s stale review via 0cca946 July 3, 2017 19:02

eed3si9n mentioned this pull request Jul 3, 2017

Bring back sbt-header #332

Closed

jvican approved these changes Jul 3, 2017

View reviewed changes

jvican merged commit 9290758 into sbt:1.0 Jul 3, 2017

cunei pushed a commit to cunei/zinc that referenced this pull request Oct 25, 2017

Merge pull request sbt#323 from ravwojdyla/at_least_one

f0237a1

Add support for atLeastOne method on a generator

Conversation

eed3si9n commented Jun 28, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

eed3si9n commented Jun 28, 2017

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

typesafe-tools commented Jun 30, 2017

Uh oh!

jvican left a comment

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!

jvican Jul 3, 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!

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!

jvican left a comment

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

eed3si9n commented Jun 28, 2017 •

edited

Loading

jvican Jul 3, 2017 •

edited

Loading