Cross build for sbt 1.0 #179

jonas · 2017-09-15T17:29:45Z

Bumps dependencies and fixes warnings when compiling with sbt 1.0.

References #150

jonas · 2017-09-15T17:41:04Z

build.sbt

@@ -88,9 +87,7 @@ val `scalajs-bundler` =
        commitNextVersion,
        pushChanges,
        releaseStepTask(GhPagesKeys.pushSite in manual)
-      ),
-      scriptedLaunchOpts += "-Dplugin.version=" + version.value,
-      scriptedBufferLog := false


Not sure why the top-level project contains settings for the scripted plugins, so I've removed them.

If I remember correctly that was needed to run the scripted command from the root project. But maybe that’s not needed anymore with your runScripted task? Also, what would be the canonical way to run scripted tests for contributors, now?

Completely missed this comment. I will update CONTRIBUTING.md to mention the new runScripted task. A future improvement could be to run the supported scripted tests as part of the normal test target

test in Test := Def.sequential( test in Test, runScriptedTask ).value,

Updated CONTRIBUTING.md to mention runScripted.

jonas · 2017-09-15T18:14:05Z

I'm seeing a lot of library dependencies warnings, should the sbt-scalajs version be updated in the scripted tests?

[info] [warn] There may be incompatibilities among your library dependencies.
[info] [warn] Here are some of the libraries that were evicted:
[info] [warn] 	* org.scala-js:sbt-scalajs:0.6.18 -> 0.6.19
[info] [warn] Run 'evicted' to see detailed eviction warnings

sjrd · 2017-09-15T18:22:57Z

See https://contributors.scala-lang.org/t/a-guide-on-binary-compatibility-need-your-input/1089/19?u=sjrd

jonas · 2017-09-15T18:30:07Z

@sjrd Not sure I understand. Are you saying that we should keep the sbt-scalajs plugin version as is?

sjrd · 2017-09-15T18:32:22Z

Yes, basically. Or at least that the reported warning is not a concern and should not itself be the cause to upgrade.

jonas · 2017-09-15T21:02:08Z

Currently working on getting the scripted tests to run for sbt 1.0. I'll need some guidance from the maintainers on how to approach this. Some tests only support sbt 0.13 (for example those depending on the play sbt-plugin), other tests we want to run for sbt 1.0 needs to be upgraded to use Scala.js 0.6.19 (unless we remove the explicit declaration and rely on the version the scalajs-bundler plugin adds). In addition, we need to remove the project/build.properties files from scripted tests that should run for sbt 1.0.

Also, do I understand correctly that we want to keep the sbt 0.13 version of the plugin on Scala.js 0.6.18?

I'll try to push a WIP branch later tonight.

jonas · 2017-09-16T00:45:54Z

build.sbt

+  def args = tests.map(Path.relativeTo(base)).flatten.mkString(" ")
+
+  if (tests.nonEmpty)
+    Def.task(scripted.toTask(" " + args).value)


This is a super generic way to filter scripted tests based on @xuwei-k's amazing work on sbt/sbt-site#107. If necessary we can simplify it to filter only the last directory, e.g. under sbt/sbt-{,web}-scalajs-bundle and include some defaults such as include tests matching *sbt-${sbtVersion}*. Let me know what you think/

Based on @julienrf 's comment we can make this filtering based on the content of project/build.properties. If the file is missing, run for all sbt versions else parse the binary version and run it if it matches sbtBinaryVersion in pluginCrossBuild.

julienrf · 2017-09-16T15:56:11Z

sbt-scalajs-bundler/src/main/scala-sbt-0.13/scalajsbundler/Compat.scala

+  implicit class ProcessLoggerOps(val self: ProcessLogger) extends AnyVal {
+    def out(msg: String) = self.info(msg)
+  }
+}


Why not use scala.sys.Process everywhere instead of introducing this abstraction over scala.sys.Process and sbt.Process?

I will give it a try.

Done in 8f2e318

julienrf · 2017-09-16T15:59:10Z

...scalajs-bundler/src/sbt-test/sbt-scalajs-bundler/additonalNpmConfig/project/build.properties

@@ -1 +0,0 @@
-sbt.version=0.13.15


Why remove that? I think keeping it (and setting it to 1.0.1) makes it possible to load the project whatever sbt version is installed.

If we keep it, it will force the test to only work with that specific sbt version. Most tests I assume we want to run for both versions of sbt. Those we want to fix to a specific sbt version will have to be configured so the CI can filter them out. I've already added a mechanism to do this as pointed out elsewhere.

OK I see! I didn’t notice that the tests were executed with both sbt 1.0 and 0.13. This is great actually :) We should not have to introduce a mechanism testing whether a test should be run with sbt 1.0 or 0.13 only.

The mechanism is needed for sbt-web-scalajs-bundler/play until the Play sbt plugin has been published for sbt 1.0 (playframework/playframework#7261). However, based on @sjrd's concern, we should probably also add tests that target the minimum version of sbt 0.13 and sbt 1.0 that scalajs-bundler supports to ensure documentation about those requirements stays up to date.

julienrf · 2017-09-16T19:32:00Z

sbt-scalajs-bundler/src/main/scala/scalajsbundler/sbtplugin/ScalaJSBundlerPlugin.scala

      if (!installDir.exists()) {
        log.info(s"Installing jsdom in ${installDir.absolutePath}")
        IO.createDirectory(installDir)
-        install(installDir, useYarn.value, log)(s"jsdom@$jsdomVersion")
+        install(installDir, usingYarn, log)(s"jsdom@$jsdomVersion")


I wish sbt/sbt#3299 could be fixed soon so that we don’t have to introduce these redundant identifiers (ie usingYarn instead of useYarn.value)

4b2d2f9 updates to newly released sbt 1.0.2 and allows to remove these workarounds.

julienrf · 2017-09-17T06:19:17Z

build.sbt

+def runScriptedTask = Def.taskDyn {
+  val sbtBinVersion = (sbtBinaryVersion in pluginCrossBuild).value
+  val base = sbtTestDirectory.value
+  val testDirectoryFinder = base * AllPassFilter * AllPassFilter filter { _.isDirectory }


Why does that find test directories?

It works similar to the shell glob .../sbt-test/*/*

julienrf

This is great, thanks! Could you squash the commits into a single one before we merge?

jonas · 2017-09-18T00:26:51Z

OK, squashed and added tests for minimum versions of sbt.

julienrf · 2017-09-19T13:06:26Z

appveyor.yml

@@ -7,8 +7,8 @@ install:
 build_script:
  - sbt clean compile
 test_script:
-  - sbt test scripted
+  - sbt test runScripted


Do you know if it’s possible to have the same matrix thing as we have with travis?

Done in the latest version. Take note of the comment about special handling of ^. It looks related with appveyor/ci#1500.

- Bump Scala.js to 0.6.19 which supports sbt 1.0.0-RC2+. - Fix error from the DSL checker when compiling with sbt 1.0.2. - Use scala.sys.process in both sbt versions instead of the sbt.Process API which has been removed in sbt 1.0. - Remove project/build.properties from scripted tests that should run for both sbt 0.13 and 1.0 and provide a new `runScripted` task to conditionally run tests supported by the configured sbt version. - Add tests for minimum required sbt versions: 0.13.9 and 1.0.0-RC2. - Configure build matrices in AppVeyor and Travis with sbt build versions.

Allows to more easily filter both sbt 0.13 and sbt 1.0 tests. Based on scalacenter/scalajs-bundler#179

julienrf · 2017-09-21T10:02:47Z

This is great work, thanks a lot Jonas!

jonas mentioned this pull request Sep 15, 2017

Publish for SBT 1.0.0 #150

Closed

jonas commented Sep 15, 2017

View reviewed changes

jonas changed the title ~~Prepare support for sbt 1.0~~ Cross build plugins for sbt 1.0 Sep 15, 2017

jonas force-pushed the prepare-for-sbt-1.0 branch from 1b716f8 to 2522d6d Compare September 15, 2017 18:57

jonas commented Sep 16, 2017

View reviewed changes

jonas force-pushed the prepare-for-sbt-1.0 branch from df12481 to 5ddcabb Compare September 16, 2017 03:03

julienrf reviewed Sep 16, 2017

View reviewed changes

julienrf reviewed Sep 17, 2017

View reviewed changes

julienrf approved these changes Sep 17, 2017

View reviewed changes

jonas force-pushed the prepare-for-sbt-1.0 branch from d4c9552 to 7b326d5 Compare September 18, 2017 00:25

jonas changed the title ~~Cross build plugins for sbt 1.0~~ Cross build for sbt 1.0 Sep 18, 2017

jonas force-pushed the prepare-for-sbt-1.0 branch 3 times, most recently from 7edf2cc to 2cfe6f0 Compare September 18, 2017 02:14

jonas mentioned this pull request Sep 19, 2017

Cross build paradox for sbt 0.13 and 1.0 lightbend/paradox#147

Merged

julienrf reviewed Sep 19, 2017

View reviewed changes

jonas force-pushed the prepare-for-sbt-1.0 branch from 2cfe6f0 to 9c8d490 Compare September 19, 2017 16:17

jonas added a commit to sbt/sbt-site that referenced this pull request Sep 20, 2017

Filter scripted tests based on project/build.properties

33fda84

Allows to more easily filter both sbt 0.13 and sbt 1.0 tests. Based on scalacenter/scalajs-bundler#179

julienrf merged commit 919b74b into scalacenter:master Sep 21, 2017

jonas deleted the prepare-for-sbt-1.0 branch September 21, 2017 11:41

sjrd mentioned this pull request Oct 2, 2017

Dependency unresolved 0.8.0 #182

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Cross build for sbt 1.0 #179

Cross build for sbt 1.0 #179

jonas commented Sep 15, 2017

jonas Sep 15, 2017

julienrf Sep 17, 2017 •

edited

Loading

jonas Sep 19, 2017 •

edited

Loading

jonas Sep 19, 2017

jonas commented Sep 15, 2017 •

edited

Loading

sjrd commented Sep 15, 2017

jonas commented Sep 15, 2017 •

edited

Loading

sjrd commented Sep 15, 2017 •

edited

Loading

jonas commented Sep 15, 2017 •

edited

Loading

jonas Sep 16, 2017

jonas Sep 16, 2017

julienrf Sep 16, 2017

jonas Sep 16, 2017

jonas Sep 17, 2017

julienrf Sep 16, 2017

jonas Sep 16, 2017

julienrf Sep 16, 2017

jonas Sep 17, 2017 •

edited

Loading

julienrf Sep 16, 2017

jonas Sep 17, 2017

julienrf Sep 17, 2017

jonas Sep 17, 2017

julienrf left a comment

jonas commented Sep 18, 2017

julienrf Sep 19, 2017

jonas Sep 19, 2017

julienrf commented Sep 21, 2017

		@@ -1 +0,0 @@
		sbt.version=0.13.15

Cross build for sbt 1.0 #179

Cross build for sbt 1.0 #179

Conversation

jonas commented Sep 15, 2017

Choose a reason for hiding this comment

julienrf Sep 17, 2017 • edited Loading

Choose a reason for hiding this comment

jonas Sep 19, 2017 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jonas commented Sep 15, 2017 • edited Loading

sjrd commented Sep 15, 2017

jonas commented Sep 15, 2017 • edited Loading

sjrd commented Sep 15, 2017 • edited Loading

jonas commented Sep 15, 2017 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jonas Sep 17, 2017 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

julienrf left a comment

Choose a reason for hiding this comment

jonas commented Sep 18, 2017

Choose a reason for hiding this comment

Choose a reason for hiding this comment

julienrf commented Sep 21, 2017

julienrf Sep 17, 2017 •

edited

Loading

jonas Sep 19, 2017 •

edited

Loading

jonas commented Sep 15, 2017 •

edited

Loading

jonas commented Sep 15, 2017 •

edited

Loading

sjrd commented Sep 15, 2017 •

edited

Loading

jonas commented Sep 15, 2017 •

edited

Loading

jonas Sep 17, 2017 •

edited

Loading