-
Notifications
You must be signed in to change notification settings - Fork 1.1k
Upgrade to Scala 2.13.4, switch CI to JDK 15, tweak CI configuration #10392
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.
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
Upgrade to Scala 2.13.4, switch CI to JDK 15, tweak CI configuration #10392
Conversation
45e55ee
to
9f80cdf
Compare
While you're changing the CI configuration, what do you think about adding a JDK 11 bootstrapped test during the nightly run, so that #10319 will actually catch any regressions? |
I already regenerated the docker image with JDK 15 and 8 but not 11, so it's a bit late for that (and I'm getting tired of staring at YAML), I'm not too worried about #9200 regressing again so I don't think it's critical to get a JDK 11 build in the CI for now. |
Big 👍 on the magic strings in commit messages to tweak the CI run. |
13ead99
to
d221fb5
Compare
401a1ab
to
4bb823f
Compare
Sigh, it looks like |
How about using |
So to turn on/off some checks we'd have to modify the pull request message? That's less convenient than the commit message but I guess we can go with that if there's no alternative. |
@sjrd I tried upgrading to JDK 15 / Scala 2.13.4 / Scala.js 1.3.1 but a JUnit Scala.js test fails, any idea what this is? java.lang.IllegalArgumentException: requirement failed: a member can have a constructor name iff it is in the constructor namespace while compiling /__w/dotty/dotty/tests/sjs-junit/../out/bootstrap/sjsJUnitTests/scala-3.0.0-M2/src_managed/main/BuildInfo.scala, /__w/dotty/dotty/tests/sjs-junit/../out/bootstrap/sjsJUnitTests/scala-js-src-1.3.1/test-suite/js/src/main/scala/org/scalajs/testsuite/jsinterop/NonNativeTypeTestSeparateRun.scala, /__w/dotty/dotty/tests/sjs-junit/../out/bootstrap/sjsJUnitTests/scala-js-src-1.3.1/test-suite/js/src/main/scala/org/scalajs/testsuite/junit/MultiCompilationTest.scala, /__w/dotty/dotty/tests/sjs-junit/../out/bootstrap/sjsJUnitTests/scala-js-src-1.3.1/test-suite/js/src/main/scala/org/scalajs/testsuite/utils/Platform.scala
Error: ## Exception when compiling 4 sources to /__w/dotty/dotty/tests/sjs-junit/../out/bootstrap/sjsJUnitTests/scala-3.0.0-M2/classes
Error: java.lang.IllegalArgumentException: requirement failed: a member can have a constructor name iff it is in the constructor namespace
Error: scala.Predef$.require(Predef.scala:338)
Error: org.scalajs.ir.Trees$MethodDef.<init>(Trees.scala:1073)
Error: org.scalajs.ir.Trees$MethodDef$.apply(Trees.scala:1062)
Error: dotty.tools.backend.sjs.JSCodeGen.genMethodWithCurrentLocalNameScope$$anonfun$1(JSCodeGen.scala:1098)
Error: dotty.tools.backend.sjs.ScopedVar$.withScopedVars(ScopedVar.scala:35)
Error: dotty.tools.backend.sjs.JSCodeGen.genMethodWithCurrentLocalNameScope(JSCodeGen.scala:1120)
Error: dotty.tools.backend.sjs.JSCodeGen.$anonfun$59(JSCodeGen.scala:976)
Error: scala.collection.immutable.List.flatMap(List.scala:293)
Error: dotty.tools.backend.sjs.JSCodeGen.genJSClassCapturesAndConstructor$$anonfun$2(JSCodeGen.scala:976)
Error: dotty.tools.backend.sjs.JSCodeGen.withNewLocalNameScope$$anonfun$1(JSCodeGen.scala:117)
Error: dotty.tools.backend.sjs.ScopedVar$.withScopedVars(ScopedVar.scala:35)
Error: dotty.tools.backend.sjs.JSCodeGen.withNewLocalNameScope(JSCodeGen.scala:118)
Error: dotty.tools.backend.sjs.JSCodeGen.genJSClassCapturesAndConstructor(JSCodeGen.scala:997)
Error: dotty.tools.backend.sjs.JSCodeGen.genNonNativeJSClass(JSCodeGen.scala:525)
Error: dotty.tools.backend.sjs.JSCodeGen.genCompilationUnit$$anonfun$6$$anonfun$1(JSCodeGen.scala:211)
Error: dotty.tools.backend.sjs.ScopedVar$.withScopedVars(ScopedVar.scala:35)
Error: dotty.tools.backend.sjs.JSCodeGen.genCompilationUnit$$anonfun$2(JSCodeGen.scala:221)
Error: dotty.runtime.function.JProcedure1.apply(JProcedure1.java:15)
Error: dotty.runtime.function.JProcedure1.apply(JProcedure1.java:10)
Error: scala.collection.immutable.List.foreach(List.scala:333)
Error: dotty.tools.backend.sjs.JSCodeGen.genCompilationUnit(JSCodeGen.scala:223)
Error: dotty.tools.backend.sjs.JSCodeGen.run(JSCodeGen.scala:152)
Error: dotty.tools.backend.sjs.GenSJSIR.run(GenSJSIR.scala:15)
Error: dotty.tools.dotc.core.Phases$Phase.runOn$$anonfun$1(Phases.scala:296)
Error: scala.collection.immutable.List.map(List.scala:250)
Error: dotty.tools.dotc.core.Phases$Phase.runOn(Phases.scala:297)
Error: dotty.tools.dotc.Run.runPhases$4$$anonfun$4(Run.scala:185)
Error: dotty.runtime.function.JProcedure1.apply(JProcedure1.java:15)
Error: dotty.runtime.function.JProcedure1.apply(JProcedure1.java:10)
Error: scala.collection.ArrayOps$.foreach$extension(ArrayOps.scala:1323)
Error: dotty.tools.dotc.Run.runPhases$5(Run.scala:195)
Error: dotty.tools.dotc.Run.compileUnits$$anonfun$1(Run.scala:203)
Error: dotty.runtime.function.JFunction0$mcV$sp.apply(JFunction0$mcV$sp.java:12)
Error: dotty.tools.dotc.util.Stats$.maybeMonitored(Stats.scala:67)
Error: dotty.tools.dotc.Run.compileUnits(Run.scala:210)
Error: dotty.tools.dotc.Run.compileSources(Run.scala:147)
Error: dotty.tools.dotc.Run.compile(Run.scala:129)
Error: dotty.tools.dotc.Driver.doCompile(Driver.scala:38)
Error: dotty.tools.dotc.Driver.process(Driver.scala:195)
Error: dotty.tools.dotc.Main.process(Main.scala) |
Yeah, you'd have to edit the pull request message or the pull request title and re-sync. Definitely less convenient but a drop in replacement for what you've already built and it's better than what we have now (nothing). IIRC scala/scala uses magic strings in PR titles, such as [ci: last-only], maybe using the title is preferable to the body? |
The changes for Scala.js can be found in #10423. You can either merge that one first, or rebase this PR on top of it, or cherry-pick the commit. Whichever works best for you. |
70ac796
to
bb2cce4
Compare
I ended up going with the PR body since that avoids adding noise to the titles when looking at the list of open PRs. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The conditional logic for the community build jobs needs fixing so that publish_sbt_release
will work, and there is a check file that should be deleted. Otherwise LGTM.
bb2cce4
to
52d5477
Compare
2.13.4 brings JDK 15 support with scala/scala#9292 - tests/run/t8153.scala was removed like it was removed in Scala 2 with scala/scala@8f6e522 - tests/neg/missing-implicit-2.check was updated because the implicitNotFound message was changed in 2.13.4 - tests/run/t6260-delambdafy.scala had to be changed to behave the same under JDK <= 15 and JDK 15.
We want to make sure that everything works on Java 8, so we should be running the full set of bootstrapped tests.
They mostly check a subset of things that can be detected with bootstrapped tests, it's still useful to know that everything works without bootstrapping on master so keep those tests on pushed commits and releases.
With this commit, the CI will look for some magic strings in the body of the PR message (but not in the title of the PR or in individual commit messages) to decide what tests to run, for example if "[test_non_bootstrapped]" appears anywhere in the PR message, the non-bootstrapped tests will be run on top of the normal tests we run for PRs. It's also possible for example to turn off the windows tests with "[skip test_windows]".
I've seen it fail on the Windows CI before.
52d5477
to
07bb6cf
Compare
2.13.4 brings JDK 15 support with scala/scala#9292