Skip to content

move to sbt 1.3, fix build on JDK 13+#8525

Merged
lrytz merged 1 commit intoscala:2.13.xfrom
SethTisue:sbt-1.3
Jan 21, 2020
Merged

move to sbt 1.3, fix build on JDK 13+#8525
lrytz merged 1 commit intoscala:2.13.xfrom
SethTisue:sbt-1.3

Conversation

@SethTisue
Copy link
Member

@SethTisue SethTisue commented Nov 5, 2019

the sbt 1.3 upgrade is good for us to have, regardless of JDK version. but it does also allow building on JDK 13

as for the JDK 13+ specific part, we might still backslide since we aren't testing it in CI, but regardless, this is progress

this came up over at sbt/sbt#5093 (comment); a prospective contributor was trying to build Scala on JDK 13

@scala-jenkins scala-jenkins added this to the 2.13.2 milestone Nov 5, 2019
@SethTisue SethTisue added the internal not resulting in user-visible changes (build changes, tests, internal cleanups) label Nov 5, 2019
@SethTisue
Copy link
Member Author

SethTisue commented Nov 5, 2019

Travis is failing with:

$ sdk install java $(sdk list java | grep -o "$ADOPTOPENJDK\.[0-9\.]*hs-adpt" | head -1)
No output has been received in the last 10m0s, this potentially indicates a stalled build or something wrong with the build itself.

which is unrelated to these changes, so I think this should be mergeable as long as Jenkins likes it

/cc @eed3si9n

@SethTisue
Copy link
Member Author

scala.ipr.SAMPLE probably isn't right after this change, but I don't know what the procedure is for updating it, @lrytz it looks like you were the last one to regenerate that file, perhaps you could contribute that here...?

@SethTisue
Copy link
Member Author

@eed3si9n what do you make of this, from https://scala-ci.typesafe.com/job/scala-2.13.x-validate-main/10185/consoleFull ?

[error] java.io.IOException: Access to URL https://scala-ci.typesafe.com/artifactory/scala-pr-validation-snapshots;build.timestamp=1572979320077/org/scala-lang/scala-compiler/2.13.2-bin-f644970-SNAPSHOT/scala-compiler-2.13.2-bin-f644970-SNAPSHOT.pom was refused by the server: Forbidden

@albertnetymk
Copy link

Thank you for pushing this; I am counting on you to get this resolved.

@lrytz
Copy link
Member

lrytz commented Nov 6, 2019

scala.ipr.SAMPLE

The dependency versions in this file don't need to be up to date, running sbt intellij picks the current versions from the build and puts them in the generated files, this works when generating them for the first time, or to update them. You can use intellijToSample, but that usually generates a lot of noop-noise-changes.

@eed3si9n
Copy link
Member

eed3si9n commented Nov 6, 2019

@SethTisue

$ sdk install java $(sdk list java | grep -o "$ADOPTOPENJDK\.[0-9\.]*hs-adpt" | head -1)
No output has been received in the last 10m0s, this potentially indicates a stalled build or  something wrong with the build itself.

I think SDKMAN is getting stuck. Here's my attempt at this issue - #8528

@eed3si9n
Copy link
Member

eed3si9n commented Nov 6, 2019

@SethTisue Ivy does some weird things when it encounters a SNAPSHOT dependency and tries to figure out the publish date (to compare or invalidate the SNAPSHOT versions). One of them is screen scraping, so maybe it somehow is doing that? Did versioning somehow change?

@SethTisue SethTisue force-pushed the sbt-1.3 branch 2 times, most recently from ed056e4 to 1a05738 Compare December 19, 2019 03:25
@SethTisue
Copy link
Member Author

@eed3si9n whatever it was, it doesn't seem to be happening anymore

@dwijnand
Copy link
Member

o_O

[error] Saved log of reflect/mimaReportBinaryIssues:
[error]  * static method <clinit>()Unit in class java.lang.Object is inaccessible in other version, it must be public.
[error]    filter with: ProblemFilters.exclude[InaccessibleMethodProblem]("java.lang.Object.<clinit>")
[error]  * static method <clinit>()Unit in class java.lang.Object is inaccessible in other version, it must be public.
[error]    filter with: ProblemFilters.exclude[InaccessibleMethodProblem]("java.lang.Object.<clinit>")
[error]  * static method <clinit>()Unit in class java.lang.Object is inaccessible in other version, it must be public.
[error]    filter with: ProblemFilters.exclude[InaccessibleMethodProblem]("java.lang.Object.<clinit>")

@SethTisue
Copy link
Member Author

@dwijnand I'll just filter that out I guess

@SethTisue
Copy link
Member Author

Jenkins likes it now

Travis-CI fails in the Dotty portion (which the Jenkins build lacks entirely) with:

[info] Compiling 622 Scala sources and 148 Java sources to /home/travis/build/scala/scala/build/quick/classes/library ...
[error] Could not find package scalaShadowing from compiler core libraries.
[error] Make sure the compiler core libraries are on the classpath.
[error]    

I'll try rebasing on top of #8604 and see if that helps

@SethTisue
Copy link
Member Author

SethTisue commented Dec 19, 2019

I still get the scalaShadowing error even with the newer Dotty version. it's reproducible locally with sbt -Dscala.build.compileWithDotty=true library/compile which is what .travis.yml does

UPDATE: I upgraded sbt-dotty from 0.3.3 to 0.3.4, but it didn't help.

@anatoliykmetyuk any idea what's going on here? I see there is some fiddly stuff around this in DottySupport.scala, can I ask you to update that?

@anatoliykmetyuk
Copy link
Contributor

The error is emitted by Dotty compiler when it tries to locate the symbol for scalaShadowing but fails. It then infers that the entire Dotty library is not present on dotc classpath. I don't think DottySupport has anything to do with this: it merely modifies the files targeted by the compiler, not the compiler settings such as classpath. The job of bringing the Dotty library to the compiler classpath belongs to the Dotty SBT plugin. I'm not sure what is going on here, maybe some weird interaction between the newer SBT, the plugin and the build setup. WDYT @smarter?

@smarter
Copy link
Member

smarter commented Jan 8, 2020

@SethTisue Here's an extra commit on top of your branch that fixes the issue: smarter@1886950

@smarter
Copy link
Member

smarter commented Jan 8, 2020

... turns out I can push to that branch myself, so I did :).

smarter added a commit to dotty-staging/dotty that referenced this pull request Jan 9, 2020
smarter added a commit to dotty-staging/dotty that referenced this pull request Jan 9, 2020
smarter added a commit to dotty-staging/dotty that referenced this pull request Jan 10, 2020
smarter added a commit to dotty-staging/dotty that referenced this pull request Jan 13, 2020
smarter added a commit to dotty-staging/dotty that referenced this pull request Jan 14, 2020
smarter added a commit to dotty-staging/dotty that referenced this pull request Jan 15, 2020
@SethTisue
Copy link
Member Author

CI liked it on 1.3.6; I added one more commit to go to 1.3.7. if CI likes that too, let's squash and merge

in order to also allow building on JDK 13+, we also disambiguate some
overloads and remove a no-longer-necesary scalaVersion override (at
the build definition level)

note that to use current sbt, we must also use current mima
and exclude some spurious issues it reports

this commit also includes some Dotty changes, contributed by
Guillaume Martres. he describes them as follows:

Fix stdlib compilation with dotty when using sbt 1.3

When we compile the standard library with dotty, we actually compile the
scala-library and dotty-library sources together in one go since we
can't in general reuse a dotty-library compiled with an old
scala-library. There were two problems with the way this was done that
were somehow masked so far (likely because of classpath pollution, but I
don't know what changed in sbt 1.3 exactly):

- Compiling the stdlib requires having all sources on the -sourcepath,
  but the sources from dotty-library itself were missing.
- the files in scalaShadowing/ in dotty-library do need to be compiled,
  the comment above the exclusion was wrong.

Additionally, I also removed the other exclusions done to files in
dotty-library because they're no longer necessary.

Co-authored-by: Guillaume Martres <smarter@ubuntu.com>
@SethTisue
Copy link
Member Author

@retronym review/merge?

@retronym
Copy link
Member

retronym commented Jan 21, 2020

I diffed the snapshot build of this PR vs that of the base commit.

jardiff $(coursier fetch --keep-optional -q -p -r https://scala-ci.typesafe.com/artifactory/scala-pr-validation-snapshots -r https://scala-ci.typesafe.com/artifactory/scala-integration org.scala-lang:scala-compiler:2.13.2-bin-23f0ead-SNAPSHOT) $(coursier fetch --keep-optional -q -p -r https://scala-ci.typesafe.com/artifactory/scala-pr-validation-snapshots -r https://scala-ci.typesafe.com/artifactory/scala-integration org.scala-lang:scala-compiler:2.13.2-bin-20a1e68-SNAPSHOT)

There are a few bytecode diffs that I don't understand. Looks like the behaviour of the optmizing backned is slightly different in the two runs. No idea if this is related to the SBT upgrade or if its a latent instability that we aren't catching. /cc @lrytz


Oh I guess that's just the difference between a fully bootstrapped build (done on the merge commit) and a quick (done on this PR).


No, I think its a genuine instablity in the 2.13.x optimizer. The first thing I'd try would be:

diff --git a/src/compiler/scala/tools/nsc/backend/jvm/opt/InlinerHeuristics.scala b/src/compiler/scala/tools/nsc/backend/jvm/opt/InlinerHeuristics.scala
index 78d7c473bf..e7218cad98 100644
--- a/src/compiler/scala/tools/nsc/backend/jvm/opt/InlinerHeuristics.scala
+++ b/src/compiler/scala/tools/nsc/backend/jvm/opt/InlinerHeuristics.scala
@@ -134,7 +134,7 @@ abstract class InlinerHeuristics extends PerRunInit {
     var size = method.instructions.size
     val res = mutable.ListBuffer.empty[InlineRequest]
     def include(kind: InlineReason, limit: Int): Unit = {
-      var rs = byReason.getOrElse(kind, Nil)
+      var rs = byReason.getOrElse(kind, Nil).sorted(inliner.inlineRequestOrdering)
       while (rs.nonEmpty && size < limit) {
         val r = rs.head
         rs = rs.tail

@lrytz lrytz merged commit f0e9c40 into scala:2.13.x Jan 21, 2020
@smarter
Copy link
Member

smarter commented Jan 21, 2020

This broke scalac from sbt (need more unit tests!):

> scalac -Xprint:typer try/id.scala
[info] running (fork) scala.tools.nsc.Main -usejavacp -Xprint:typer try/id.scala
error: java.lang.StringIndexOutOfBoundsException: String index out of range: -1
        at java.lang.String.charAt(String.java:658)
        at scala.reflect.io.ZipArchive$.splitPath(ZipArchive.scala:61)
        at scala.reflect.io.ZipArchive$.scala$reflect$io$ZipArchive$$dirName(ZipArchive.scala:58)
        at scala.reflect.io.ZipArchive.getDir(ZipArchive.scala:137)
        at scala.reflect.io.FileZipArchive.root$lzycompute(ZipArchive.scala:204)
        at scala.reflect.io.FileZipArchive.root(ZipArchive.scala:189)
        at scala.reflect.io.FileZipArchive.allDirs$lzycompute(ZipArchive.scala:225)
        at scala.reflect.io.FileZipArchive.allDirs(ZipArchive.scala:225)
        at scala.tools.nsc.classpath.ZipArchiveFileLookup.findDirEntry(ZipArchiveFileLookup.scala:79)
        at scala.tools.nsc.classpath.ZipArchiveFileLookup.list(ZipArchiveFileLookup.scala:66)
        at scala.tools.nsc.classpath.ZipArchiveFileLookup.list$(ZipArchiveFileLookup.scala:65)
        at scala.tools.nsc.classpath.ZipAndJarClassPathFactory$ZipArchiveClassPath.list(ZipAndJarFileLookupFactory.scala:57)
        at scala.tools.nsc.classpath.AggregateClassPath.$anonfun$list$3(AggregateClassPath.scala:92)
        at scala.collection.IterableOnceOps.foreach(IterableOnce.scala:553)
        at scala.collection.IterableOnceOps.foreach$(IterableOnce.scala:551)
        at scala.collection.AbstractIterable.foreach(Iterable.scala:920)
        at scala.tools.nsc.classpath.AggregateClassPath.list(AggregateClassPath.scala:88)
        at scala.tools.nsc.util.ClassPath.list(ClassPath.scala:36)
        at scala.tools.nsc.util.ClassPath.list$(ClassPath.scala:36)
        at scala.tools.nsc.classpath.AggregateClassPath.list(AggregateClassPath.scala:31)
        at scala.tools.nsc.symtab.SymbolLoaders$PackageLoader.doComplete(SymbolLoaders.scala:277)
        at scala.tools.nsc.symtab.SymbolLoaders$SymbolLoader.$anonfun$complete$2(SymbolLoaders.scala:229)
        at scala.runtime.java8.JFunction0$mcV$sp.apply(JFunction0$mcV$sp.scala:18)
        at scala.reflect.internal.SymbolTable.informingProgress(SymbolTable.scala:85)
        at scala.tools.nsc.symtab.SymbolLoaders$SymbolLoader.complete(SymbolLoaders.scala:227)
        at scala.reflect.internal.Symbols$Symbol.completeInfo(Symbols.scala:1541)
        at scala.reflect.internal.Symbols$Symbol.info(Symbols.scala:1513)
        at scala.reflect.internal.Mirrors$RootsBase.init(Mirrors.scala:262)
        at scala.tools.nsc.Global.rootMirror$lzycompute(Global.scala:73)
        at scala.tools.nsc.Global.rootMirror(Global.scala:71)
        at scala.tools.nsc.Global.rootMirror(Global.scala:43)
        at scala.reflect.internal.Definitions$DefinitionsClass.ObjectClass$lzycompute(Definitions.scala:286)
        at scala.reflect.internal.Definitions$DefinitionsClass.ObjectClass(Definitions.scala:286)
        at scala.reflect.internal.Definitions$DefinitionsClass.init(Definitions.scala:1582)
        at scala.tools.nsc.Global$Run.<init>(Global.scala:1205)
        at scala.tools.nsc.Driver.doCompile(Driver.scala:47)
        at scala.tools.nsc.MainClass.doCompile(Main.scala:30)
        at scala.tools.nsc.Driver.process(Driver.scala:68)
        at scala.tools.nsc.Driver.main(Driver.scala:82)
        at scala.tools.nsc.Main.main(Main.scala)

@dwijnand
Copy link
Member

Upgrading the version of sbt used to build scalac broke in sbt's usage of scalac? I can't breathe.

@eed3si9n
Copy link
Member

Not sure why or how but there's a ZipEntry with blank name.

diff --git a/src/reflect/scala/reflect/io/ZipArchive.scala b/src/reflect/scala/reflect/io/ZipArchive.scala
index 985bde4cba..5af4bd6bed 100644
--- a/src/reflect/scala/reflect/io/ZipArchive.scala
+++ b/src/reflect/scala/reflect/io/ZipArchive.scala
@@ -124,6 +124,8 @@ abstract class ZipArchive(override val file: JFile, release: Option[String]) ext
   }

   protected def getDir(dirs: java.util.Map[String, DirEntry], entry: ZipEntry): DirEntry = {
+    println(s"""entry: "$entry" """)
+
     def ensureDir(path: String): DirEntry =
       dirs.get(path) match {
         case null =>
sbt:root> scalac -Xprint:typer try/id.scala
[info] Compiling 1 Scala source to /Users/eed3si9n/work/scala-modules/scala/build/quick/classes/reflect ...
[info] running (fork) scala.tools.nsc.Main -usejavacp -Xprint:typer try/id.scala
entry: "META-INF/MANIFEST.MF"
entry: "META-INF/mailcap.default"
....
entry: "scala/reflect/macros/compiler/Validators.class"
entry: "templates/tool-windows.tmpl"
entry: "templates/tool-unix.tmpl"
entry: "rootdoc.txt"
entry: ""
error: java.lang.StringIndexOutOfBoundsException: String index out of range: -1
	at java.lang.String.charAt(String.java:658)
	at scala.reflect.io.ZipArchive$.splitPath(ZipArchive.scala:61)
	at scala.reflect.io.ZipArchive$.scala$reflect$io$ZipArchive$$dirName(ZipArchive.scala:58)
	at scala.reflect.io.ZipArchive.getDir(ZipArchive.scala:139)
	at scala.reflect.io.FileZipArchive.root$lzycompute(ZipArchive.scala:206)
	at scala.reflect.io.FileZipArchive.root(ZipArchive.scala:191)

@dwijnand
Copy link
Member

I think this broke intellij too:

> intellij
[info] Updating
https://repo1.maven.org/maven2/org/scala-lang/modules/scala-asm/7.0.0-scala-1/scala-asm-7.0.0-scala-1.pom
  100.0% [##########] 1.5 KiB (40.4 KiB / s)
[info] Resolved  dependencies
[info] Fetching artifacts of
[info] Fetched artifacts of
[info] Updating
https://repo1.maven.org/maven2/org/openjdk/jol/jol-core/0.6/jol-core-0.6.pom
  100.0% [##########] 4.8 KiB (48.0 KiB / s)
https://repo1.maven.org/maven2/org/openjdk/jol/jol-parent/0.6/jol-parent-0.6.pom
  100.0% [##########] 2.6 KiB (25.1 KiB / s)
[info] Resolved  dependencies
[info] Fetching artifacts of
[info] Fetched artifacts of
[info] Updating
https://repo1.maven.org/maven2/org/openjdk/jol/jol-core/0.9/jol-core-0.9.pom
  100.0% [##########] 4.6 KiB (268.7 KiB / s)
https://repo1.maven.org/maven2/org/openjdk/jol/jol-parent/0.9/jol-parent-0.9.pom
  100.0% [##########] 4.4 KiB (182.3 KiB / s)
[info] Resolved  dependencies
^C
[warn] Canceling execution...
^C
[warn] Canceling execution...
^C
[warn] Canceling execution...
^C
[warn] Canceling execution...
^C
[warn] Canceling execution...
^C
[warn] Canceling execution...
^C

@dwijnand
Copy link
Member

dwijnand commented Jan 21, 2020

This broke scalac from sbt (need more unit tests!):

Oh, did you mean the scalac custom command? I just reproduced this with its scala sibling.

@retronym
Copy link
Member

sbt:root> show compiler/packageBin::mappings
[info] * (/Users/jz/code/scala-sbt-bump-fix/build/quick/classes/compiler,)  !!!! THIS IS NEW AND UNWANTED !!!
[info] * (/Users/jz/code/scala-sbt-bump-fix/build/quick/classes/compiler/scala-buildcharacter.properties,scala-buildcharacter.properties)
[info] * (/Users/jz/code/scala-sbt-bump-fix/build/quick/classes/compiler/META-INF,META-INF)
[info] * (/Users/jz/code/scala-sbt-bump-fix/build/quick/classes/compiler/META-INF/services,META-INF/services)
[info] * (/Users/jz/code/scala-sbt-bump-fix/build/quick/classes/compiler/META-INF/services/javax.script.ScriptEngineFactory,META-INF/services/javax.script.ScriptEngineFactory)
[info] * (/Users/jz/code/scala-sbt-bump-fix/build/quick/classes/compiler/compiler.properties,compiler.properties)

@retronym
Copy link
Member

This comes from a behaviour change in Path.allSubpaths.

Before:

sbt:root> eval Path.allSubpaths(file("/Users/jz/code/scala-sbt-bump-fix/build/quick/classes/compiler"))
[info] ans: Traversable[(java.io.File, String)] = ArrayBuffer((/Users/jz/code/scala-sbt-bump-fix/build/quick/classes/compiler/scala-buildcharacter.properties,scala-buildcharacter.properties), (/Users/jz/code/scala-sbt-bump-fix/build/quick/classes/compiler/META-INF,META-INF), (

After:

sbt:root> eval Path.allSubpaths(file("/Users/jz/code/scala-sbt-bump-fix/build/quick/classes/compiler"))
sbt:root> eval Path.allSubpaths(file("/Users/jz/code/scala-sbt-bump-fix/build/quick/classes/compiler"))
[info] ans: Traversable[(java.io.File, String)] = ArrayBuffer((/Users/jz/code/scala-sbt-bump-fix/build/quick/classes/compiler,), (/Users/jz/code/scala-sbt-bump-fix/build/quick/classes/compiler/scala-buildcharacter.properties,scala-buildcharacter.properties), (/Users/jz/code/scala-sbt-bump-fix/build/quick/classes/compiler/META-INF,META-INF), ...

@retronym
Copy link
Member

retronym commented Jan 21, 2020

/cc @eatkins

@eatkins
Copy link
Contributor

eatkins commented Jan 21, 2020

Whoops. sbt/io#285.

@retronym
Copy link
Member

retronym commented Jan 21, 2020

I think this broke intellij too:

@dwijnand The problem is that the supershell buffers our "Are you sure [Y/N]" output line that we print to console in the intellij task. You can still type y<enter> to make it work. @eed3si9n is there a supported way for tasks to interact with the user like that or should we do that in a a command?

I just found interactionService.value.confirm (and tried it in retronym@4938b01), but the prompt is still swallowed by supershell.

@retronym
Copy link
Member

Workaround in #8652

@eed3si9n
Copy link
Member

@retronym @eatkins has a comprehensive fix for console and supershell interference - sbt/sbt#5319

@SethTisue SethTisue deleted the sbt-1.3 branch January 22, 2020 02:35
eed3si9n pushed a commit to eed3si9n/io that referenced this pull request Jan 27, 2020
It was reported in scala/scala#8525 that sbt
does not correctly exclude the base directory when calling
Path.allSubpaths. This ended up causing jar files to be created with an
empty entry. The problem was in an incorrect equality check because the
compiler did not flag for us that
`PathFinder(base).globRecursive(filter).get()` returned `Seq[File]` but
that we were filtering the results by comparing a `File` to a `Path`.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

internal not resulting in user-visible changes (build changes, tests, internal cleanups)

Projects

None yet

Development

Successfully merging this pull request may close these issues.

10 participants