upgrade build from sbt 0.13 -> sbt 1 [ci: last-only] #6256
Conversation
|
@dwijnand @eed3si9n private[this] def setup(name: String)(f: Seq[String] => Seq[Setting[_]]) = Command.args(name, name) { case (state, seq) =>
// `Project.extract(state).append(f(seq) ++ resetLogLevels, state)` would be simpler, but it
// takes the project's initial state and discards all changes that were made in the sbt console.
val session = Project.session(state)
val extracted = Project.extract(state)
val settings = f(seq) ++ resetLogLevels
val appendSettings = Load.transformSettings(Load.projectScope(extracted.currentRef), extracted.currentRef.build, extracted.rootProject, settings)
val newStructure = Load.reapply(session.mergeSettings ++ appendSettings, extracted.structure)(extracted.showKey)
Project.setProject(session, newStructure, state)
} |
|
There is no migration path, outside of requesting an API be made public. In this case I think what we should do is fix |
|
sbt 1.1.1 is out and includes sbt/sbt#3865 |
|
eh. I meant 1.1.1. we'll fix it when we squash later |
SethTisue
changed the title
|
thanks Dale for taking this over! I've squashed/rebased and made you author of the resulting commit |
There was a problem hiding this comment.
LGTM, except possibly the removal of antStyle — team (@retronym?), I think I might recall that actually still being used sometimes...? if so, maybe we could give it a better name.
also Dale, you commented out -Xlint in project/plugins.sbt, would leaving it enabled present some special difficulty...?
antStyle was torn out of the incremental compiler, I'm not sure when (or what commit).
it could stay but it would need to be tweaked, for instance make it not tell you about every import that sbt injects into build.sbt but that build doesn't use. stuff like that. I took the easy way out, but I can be more diligent if you'd like. |
|
|
| // | ||
| // https://github.com/scala/scala-dev/issues/100 | ||
| def silenceScalaBinaryVersionWarning = ivyConfiguration := { | ||
| ivyConfiguration.value match { |
There was a problem hiding this comment.
The warnings we're trying to suppress are still emitted by SBT 1.1, so we'll have to figure out how to port this, or otherwise have a plan to make the warning suppressable in SBT itself.
sbt:root> test:compile
[warn] Binary version (2.13.0-pre-SNAPSHOT) for dependency org.scala-lang#scala-library;2.13.0-pre-SNAPSHOT
[warn] in org.scala-lang#scala-reflect;2.13.0-pre-SNAPSHOT differs from Scala binary version in project (2.13.0-M3).
[warn] Binary version (2.13.0-pre-SNAPSHOT) for dependency org.scala-lang#scala-library;2.13.0-pre-SNAPSHOT
[warn] in org.scala-lang#scala-compiler;2.13.0-pre-SNAPSHOT differs from Scala binary version in project (2.13.0-M3).
[warn] Binary version (2.13.0-pre-SNAPSHOT) for dependency org.scala-lang#scala-reflect;2.13.0-pre-SNAPSHOT
[warn] in org.scala-lang#scala-compiler;2.13.0-pre-SNAPSHOT differs from Scala binary version in project (2.13.0-M3).
[warn] Binary version (2.13.0-pre-SNAPSHOT) for dependency org.scala-lang#scala-compiler;2.13.0-pre-SNAPSHOT
[warn] in org.scala-lang#scalap;2.13.0-pre-SNAPSHOT differs from Scala binary version in project (2.13.0-M3).
[warn] Binary version (2.13.0-pre-SNAPSHOT) for dependency org.scala-lang#scala-compiler;2.13.0-pre-SNAPSHOT
[warn] in org.scala-lang#scala-compiler-interactive;2.13.0-pre-SNAPSHOT differs from Scala binary version in project (2.13.0-M3).
[warn] Binary version (2.13.0-pre-SNAPSHOT) for dependency org.scala-lang#scala-compiler;2.13.0-pre-SNAPSHOT
[warn] in org.scala-lang#repl;2.13.0-pre-SNAPSHOT differs from Scala binary version in project (2.13.0-M3).
[warn] Binary version (2.13.0-pre-SNAPSHOT) for dependency org.scala-lang#scala-compiler;2.13.0-pre-SNAPSHOT
[warn] in org.scala-lang#scala-compiler-doc;2.13.0-pre-SNAPSHOT differs from Scala binary version in project (2.13.0-M3).
[info] Running scala.tools.docutil.ManMaker fsc, scala, scalac, scaladoc, scalap /Users/jz/code/scala-sbt-1/target/scala-dist/resource_managed/main/doc/tools /Users/jz/code/scala-sbt-1/target/scala-dist/resource_managed/main/genman
[info] Compiling 526 Scala sources and 120 Java sources to /Users/jz/code/scala-sbt-1/build/quick/classes/library ...
|
We can act on a new incremental compiler warning by deleting the file that contains only comments: |
|
I took care of removing |
|
To measure, I use a package io.github.retronym
import sbt._
import Keys._
object CleanClasses extends AutoPlugin {
override def trigger = allRequirements
override def requires = sbt.plugins.JvmPlugin
val cleanClasses = taskKey[Unit]("clean the classes directory")
override lazy val projectSettings = List(Compile, Test).flatMap(c => inConfig(c)(Seq(
cleanClasses := IO.delete(classDirectory.value)
)))
}
Then run the following repeatedly in both the 2.13.x branch (using SBT 0.13) and this PR: Zinc uses a finer granularity for its metadata tracking now (class based vs file based), which might be relevant. I previously benchmarked SBT 1.x as being about 8% slower than 0.13 for clean builds, the experience in the Scala build seems even worse than that. /cc @jvican |
|
I repeated the compilation/clean cycle a few times, the attached FlightRecorder to collect a profile. During that time, this error was issued:
Or maybe there are multiple UPDATE I actually got the same error on 0.13: UPDATE 2 Lines 196 to 199 in 7dd6daf |
|
Back when it was first introduced, I noted that class-based dependency tracking introduced a big overhead for scalac : sbt/sbt#1104 (comment), this is probably still the case. At the time, this overhead didn't seem to affect dotty, I can't test dotty with sbt 1 right now but we'll probably be done with porting dotty to sbt 1 sometimes in the next century or two (current progress at scala/scala3#3872, but this has been ongoing for over a year). once this is done I'll try to run some comparison benchmarks too. |
In dotty we do: baseDirectory in (Compile, run) := baseDirectory.value / "..",
baseDirectory in Test := baseDirectory.value / ".."and haven't run into problem yet (and I don't see anything suspicious in the root target directory) |
Looks like we can suppress the check with |
|
|
|
I profiled the warm, clean compile. I noticed something unusual with busy-waiting from a LMAX disruptor thread involved in async logging, details in sbt/util#68 The new logging implementation also seems to spend an inordinate amount of time in Scala reflection to evaluate type tags to add to log messages, I've raised https://github.com/sbt/util/issues/151 with an initial diagnosis. Of time taken to compile, the profile suggests around 40% is spent in Zinc's API phase.
|
|
Rebased. |
|
Hmm, while running under |
|
The artifact name From private[sbt] def getDefaultBridgeModule(scalaVersion: String): ModuleID = {
def compilerBridgeId(scalaVersion: String) = {
scalaVersion match {
case sc if (sc startsWith "2.10.") => "compiler-bridge_2.10"
case sc if (sc startsWith "2.11.") => "compiler-bridge_2.11"
case sc if (sc startsWith "2.12.") => "compiler-bridge_2.12"
case "2.13.0-M1" => "compiler-bridge_2.12"
case sc if (sc startsWith "2.13.0-pre-") => "compiler-bridge_2.13.0-M2"
case sc if (sc startsWith "2.13.0-M") => "compiler-bridge_2.13.0-M2"
case sc if (sc startsWith "2.13.0-RC") => "compiler-bridge_2.13.0-M2"
case _ => "compiler-bridge_2.13"
}
}
|
|
Thanks, that fixed it. My local.sbt now has: |
|
I'll redo the stability check on the current build and sbt 1.2.3. If I don't get any discrepancies, I assume this PR is ready to go? |
|
Looks like we're good re: stability. Transcript of my due diligence: |
|
Rebased |
1. Adapt to renaming of ivyScala 2. Be less intrusive with customization of baseDirectory In our attempt to make forked project take on the working directory of the root project, rather than of their module, we ended up causing a clash in the directory used by the incremental compiler to backup classfiles, which uses a value relative to that base. The symptom was an intermitten error such as: ``` [error] ## Exception when compiling 0 sources to /Users/jz/code/scala-sbt-1/target/partest-extras/test-classes [error] Could not create directory /Users/jz/code/scala-sbt-1/target/scala-2.13.0-M3/classes.bak ``` This commit adjusts ForkOptions instead to more directly achieve our goal. 3. Heed deprecation warning in ScalaInstance tweak 4. Disable finding sources files in project base Something has changed in SBT 1.x, and our attempt to clear the sources for scalacheck/compile: weren't successful: ``` sbt:root> show scalacheck / Compile / unmanagedSources [info] * /Users/jz/code/scala/test/scalacheck/array-new.scala [info] * /Users/jz/code/scala/test/scalacheck/array-old.scala [info] * /Users/jz/code/scala/test/scalacheck/CheckCollections.scala [info] * /Users/jz/code/scala/test/scalacheck/CheckEither.scala ``` What changed? 4502348, earlier in this PR, removed our override of `baseDirectory`. So sub-projects now have their own sub-directory as the base (which is the standard SBT behaviour). If these include source files directly, as is the case with `./test/scalacheck`, these would be included by virtue of the long standing SBT behaviour to let you skip the hassle of source directories for toy projects. This commit disables that behaviour with `sourcesInBase := false` 5. Fixup OSGi test suite after baseDirectory change 6. Let SBT add api.url to the POM Since sbt/sbt#2262, we don't need to do this. And attempting to do it results in duplicate properties in the POM file, which doesn't pass POM validation by artifactory. 7. Use the recommended, and working, `sbt -warn` In favour of `--warn`, which is broken in SBT 1.x. Early commands are ones that are run before project load, in this case, `-warn` used to mean `-` (schedule early) and `warn` (the command to change log level)`. `sbt-extras` translates its `-w` option into `--warn`, or passes through `--warn` if provided. The official SBT launcher passes through `--warn`. `sbt -warn` still works in 1.x, but `sbt --warn` doesn't give the non-zero error code after a failure. ``` for sbt in 0.13.17 1.1.4; do for opt in "--warn" "-warn"; do (set -x; java -jar /home/jenkins/.sbt/launchers/0.13.17/sbt-launch.jar -Dsbt.version=$sbt $opt 'eval ???'; echo $?;) done; done + java -jar /home/jenkins/.sbt/launchers/0.13.17/sbt-launch.jar -Dsbt.version=0.13.17 --warn 'eval ???' scala.NotImplementedError: an implementation is missing at scala.Predef$.$qmark$qmark$qmark(Predef.scala:252) ... [error] scala.NotImplementedError: an implementation is missing [error] Use 'last' for the full log. + echo 1 1 + java -jar /home/jenkins/.sbt/launchers/0.13.17/sbt-launch.jar -Dsbt.version=0.13.17 -warn 'eval ???' scala.NotImplementedError: an implementation is missing at scala.Predef$.$qmark$qmark$qmark(Predef.scala:252) ... [error] scala.NotImplementedError: an implementation is missing [error] Use 'last' for the full log. + echo 1 1 + java -jar /home/jenkins/.sbt/launchers/0.13.17/sbt-launch.jar -Dsbt.version=1.1.4 --warn 'eval ???' [warn] sbt version mismatch, current: 1.1.4, in build.properties: "0.13.17", use 'reboot' to use the new value. [info] Loading project definition from /tmp/scratch/project [info] Updating ProjectRef(uri("file:/tmp/scratch/project/"), "scratch-build")... [info] Done updating. [info] Set current project to scratch (in build file:/tmp/scratch/) [warn] The `-` command is deprecated in favor of `onFailure` and will be removed in a later version [error] scala.NotImplementedError: an implementation is missing [error] at scala.Predef$.$qmark$qmark$qmark(Predef.scala:284) .. [error] scala.NotImplementedError: an implementation is missing [error] Use 'last' for the full log. + echo 0 0 + java -jar /home/jenkins/.sbt/launchers/0.13.17/sbt-launch.jar -Dsbt.version=1.1.4 -warn 'eval ???' + echo 1 1 ``` This commits changes our scripts to use the recommended `-warn`. However, I'd say that SBT 1.x should either reject `--warn` outright or be compatible with the old behaviour.
|
Rebased once more, with date ordering for github and more squashiness. |
|
discussed at team meeting today — nobody had objections to merging this as soon as CI is green. |
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
|
Looks green to me. |
|
ahhhhhhhhhh that makes my day. |
| trait ScalaOsgiHelper { | ||
|
|
||
| private def allBundleFiles = { | ||
| println(new File(".").getAbsolutePath) |
| @@ -0,0 +1 @@ | |||
| sbt.version=1.1.4 | |||
There was a problem hiding this comment.
nbd, but we should bump this one to 1.2.3 too
9 participants
wippity wip wip. let's get the ball rolling, at leastUPDATE: this is nearing completion.