[backport] Adapt to changes in Java 9 #6113

retronym · 2017-10-05T23:56:48Z

Backports of a few changes in Scala 2.12.3 that remove some impediments to running on Java 9.

We used to rely on `cls.getPackage == null` for `cls` defined in the empty package. Under Java 9, we actually get the empty package back from that call. This commit ensures we use the one true empty package symbol on either Java 8 or 9. (cherry picked from commit b81bc77)

sjrd · 2017-10-06T06:16:37Z

If there's going to be a 2.10.7 version for the sake of Java 9 support, could 72076e5 also be ported please? It is necessary for parallel collections not to crash on Java 9. This caused issues in Scala.js, which forced us to plain disable parallelism in the Scala.js optimizer on Java 9.

dwijnand · 2017-10-06T07:02:20Z

👍 on what Sébastien said, #5276 was also mentioned in playframework/playframework#7879.

[edit: Oh, it has been cherry picked into this PR]

sjrd · 2017-10-06T09:25:21Z

Thanks! 🙇‍♂️

adriaanm · 2017-10-14T00:30:35Z

Oh version numbers!

  [partest] java.lang.NumberFormatException: Not a version: 1.71
  [partest] 
  [partest] 	at scala.util.PropertiesTrait$class.isJavaAtLeast(Properties.scala:184)
  [partest] 	at scala.util.Properties$.isJavaAtLeast(Properties.scala:16)
  [partest] 	at Test$delayedInit$body.apply(t7265.scala:12)
  [partest] testing: [...]/files/run/t7265.scala                                  [FAILED]

dwijnand · 2017-10-14T06:42:59Z

I don't mean to throw more weight on the backport wagon, but I hit this exact thing yesterday when doing some JDK 9 related work in a 2.11 cross-built codebase. And had to resort to some variant of isJavaAtLeast. (sbt/librarymanagement#173). So backporting that fix too would be ace..

The utility method compares javaSpecVersion, which has the form "1.8" previously and "9" going forward. The method accepts "1.n" for n < 9. More correctly, the string argument should be a single number. Supports JEP-223. (cherry picked from commit 72076e5) Also cherry-picks subsequent bug fixes and test improvements from 06f8b62 Backport accumulated improvements, tests for isJavaAtLeast

retronym · 2017-10-16T11:10:55Z

I've backported the accumulated fixes and tests for isJavaAtLeast. I've also added a commit to use javax.tools (the API for javac) as a way to support the jrt:// file system directly, which will mean the SBT no longer needs to export rt.jar.

SethTisue · 2017-10-16T23:45:35Z

trying another Jenkins validate-main run with the new commits: https://scala-ci.typesafe.com/view/scala-2.11.x/job/scala-2.11.x-validate-main/3157/consoleFull (as Adriaan realized, although Scabot ignores the 2.10.x branch, you can manually trigger a 2.10.x run of validate-main using the 2.11.x job)

SethTisue · 2017-10-17T00:28:08Z

publish-core succeeded (2.10.6-05cdf51-SNAPSHOT was published)

but,

     [echo] Checking backward binary compatibility for scala-reflect (against 2.10.0)
     [mima] Found 2 binary incompatibiities (60 were filtered out)
     [mima] ======================================================
     [mima]  * method underlyingSource()scala.Some in class
     [mima]    scala.reflect.io.ZipArchive#Entry has now a different result type; was:
     [mima]    scala.Some, is now: scala.Option
     [mima]  * method underlyingSource()scala.Some in class scala.reflect.io.ZipArchive
     [mima]    has now a different result type; was: scala.Some, is now: scala.Option
     [mima] Generated filter config definition
     [mima] ==================================
     [mima] 
     [mima]     filter {
     [mima]         problems=[
     [mima]             {
     [mima]                 matchName="scala.reflect.io.ZipArchive#Entry.underlyingSource"
     [mima]                 problemName=IncompatibleResultTypeProblem
     [mima]             },
     [mima]             {
     [mima]                 matchName="scala.reflect.io.ZipArchive.underlyingSource"
     [mima]                 problemName=IncompatibleResultTypeProblem
     [mima]             }
     [mima]         ]
     [mima]     }

SethTisue · 2017-10-17T00:37:17Z

@retronym since this PR is what we're testing as possible-future-2.10.7, I'd suggest rebasing onto current 2.10.x in order to get the #6128 and #6129 changes

lrytz · 2017-10-18T05:35:55Z

which asm version was in there before? i could not easily find out looking at the git history.

retronym · 2017-10-25T06:55:09Z

@lrytz The 2.10.x version at least predates f9053e5 which upgrades to ASM 4.1. I'm pretty sure that it is 4.0.

In Scala 2.13.x, we've supported this by using the NIO jrt:// virtual filesystem. We can't (easily) ship code in older Scala distributions that require Java 7+, but we can fall back to use of `javax.tools`, which offers programatic access to the Java compiler's classpath implementation. This is incompatible with the performance trick used in the runner scripts that places the Scala compiler on the JVM boot classpath to avoid the cost of classfile verification. The boot classpath can only see the base platform on Java 9, which puts `javax.tools` out of access. But, we can use `scalac -nobootcp` to make things work! This obviates the workaround used in SBT which exports rt.jar from JDK9 and passes to scalac as a custom boot classpath.

retronym · 2017-10-26T01:26:08Z

Disk space problem on jenkins-worker-behemoth-2?

Caused by: hudson.plugins.git.GitException: Command "git config remote.scala.url https://github.com/scala/scala.git" returned status code 4:
stdout: 
stderr: error: failed to write new configuration file /home/jenkins/workspace/scala-2.11.x-validate-publish-core/.git/config.lock

SethTisue · 2017-10-26T02:24:01Z

Disk space problem

yeah scala/community-build#617

The Scala classfile and java source parsers make Java annotation classes (which are actually interfaces at the classfile level) look like Scala annotation classes: - the INTERFACE / ABSTRACT flags are not added - scala.annotation.Annotation is added as superclass - scala.annotation.ClassfileAnnotation is added as interface This makes type-checking @annot uniform, whether it is defined in Java or Scala. This is a hack that leads to various bugs (SI-9393, SI-9400). Instead the type-checking should be special-cased for Java annotations. This commit fixes SI-9393 and a part of SI-9400, but it's still easy to generate invalid classfiles. Restores the assertions that were disabled in scala#4621. I'd like to leave these assertions in: they are valuable and helped uncovering the issue being fixed here. A new flag JAVA_ANNOTATION is introduced for Java annotation ClassSymbols, similar to the existing ENUM flag. When building ClassBTypes for Java annotations, the flags, superclass and interfaces are recovered to represent the situation in the classfile. The test for SI-9393 is extended to test both the classfile and the java source parser. Partial cherry pick of 77b8b6a

@6

Without this, you get: ``` (java_use 9; qscalac -nobootcp $(f "class C { java.util.List.of(this, this)}" ) && qscala -nobootcp -e 'new C') java.lang.VerifyError: Illegal type at constant pool entry 18 in class C Exception Details: Location: C.<init>()V @6: invokestatic Reason: Constant pool index 18 is invalid Bytecode: 0000000: 2ab7 000c 2a2a b800 1257 b1 ``` See https://bugs.openjdk.java.net/browse/JDK-8037385

retronym · 2017-10-27T00:07:55Z

++ ant '-Dmaven.version.suffix="-2ae75e9-SNAPSHOT"' -Dremote.snapshot.repository=https://scala-ci.typesafe.com/artifactory/scala-pr-validation-snapshots/ -Drepository.credentials.id=pr-scala -Dremote.release.repository=noyoudont deploy-core.snapshot
Buildfile: /home/jenkins/workspace/scala-2.11.x-validate-publish-core/dists/maven/2.10.6-2ae75e9-SNAPSHOT/build.xml

init.properties:
     [echo] Using server[pr-scala] for maven repository credentials.
     [echo]        Please make sure that your ~/.m2/settings.xml has the needed username/password for this server id
     [echo]      

init.maven:

deploy.remote.init:

deploy-core.snapshot:
     [copy] Copying 1 file to /home/jenkins/workspace/scala-2.11.x-validate-publish-core/dists/maven/2.10.6-2ae75e9-SNAPSHOT/scala-library

BUILD FAILED
/home/jenkins/workspace/scala-2.11.x-validate-publish-core/dists/maven/2.10.6-2ae75e9-SNAPSHOT/build.xml:288: The following error occurred while executing this line:
/home/jenkins/workspace/scala-2.11.x-validate-publish-core/dists/maven/2.10.6-2ae75e9-SNAPSHOT/build.xml:184: The following error occurred while executing this line:
/home/jenkins/workspace/scala-2.11.x-validate-publish-core/dists/maven/2.10.6-2ae75e9-SNAPSHOT/build.xml:134: settingsFile does not exist: /home/jenkins/.m2/settings.xml

😕

retronym · 2017-10-27T01:43:39Z

Restored that file, I'm guessing it was accidentally deleted in a manual purge of ~/.m2/repository

lrytz · 2017-10-27T12:15:55Z

LGTM, a bit of squashing. ASM update looks good too, I went through asm changes in 2.12.x. Curious, why did you backport the fix for scala/bug#9393?

lrytz · 2017-10-27T12:18:34Z

Actually, why did you move the asm sources from src/asm to src/asm/java in c3d8750, was that intended?

retronym · 2017-10-30T01:34:36Z

Curious, why did you backport the fix for scala/bug#9393?

I adapted GenASM to set the itf parameter (newly added in ASM 5.2) which controls whether an InterfaceMethodRef or MethodRef is used. This logic is based in part of isJavaInterface, which gave a false negative for annotations. Two run tests in test suite that called methods on runtime retained annotations failed with a verify error.

was that intended?

Nope, that was a mistake. I'll move them back.

I'll also do a jardiff after a bootstrap to doublecheck things are okay.

retronym · 2017-10-30T03:07:05Z

Only one diff, but one that I can't immediately explain.

diff --git a/scala/collection/parallel/ParIterableLike.class.asm b/scala/collection/parallel/ParIterableLike.class.asm
index e7ebdef..0c753e0 100644
--- a/scala/collection/parallel/ParIterableLike.class.asm
+++ b/scala/collection/parallel/ParIterableLike.class.asm
@@ -157,9 +157,9 @@
   // access flags 0x1
   public INNERCLASS scala/collection/parallel/ParIterableLike$CreateScanTree scala/collection/parallel/ParIterableLike CreateScanTree
   // access flags 0x401
-  public abstract INNERCLASS scala/collection/parallel/ParIterableLike$Accessor$class scala/collection/parallel/ParIterableLike Accessor$class
-  // access flags 0x401
   public abstract INNERCLASS scala/collection/parallel/ParIterableLike$ScanTree$class scala/collection/parallel/ParIterableLike ScanTree$class
+  // access flags 0x401
+  public abstract INNERCLASS scala/collection/parallel/ParIterableLike$Accessor$class scala/collection/parallel/ParIterableLike Accessor$class
   // access flags 0x1
   public INNERCLASS scala/collection/parallel/ParIterableLike$$anonfun$min$1 null null
   // access flags 0x1
diff --git a/scala/tools/nsc/symtab/SymbolTrackers$SymbolTracker$Node$.class.asm b/scala/tools/nsc/symtab/SymbolTrackers$SymbolTracker$Node$.class.asm
index 878393a..7d6f2af 100644
--- a/scala/tools/nsc/symtab/SymbolTrackers$SymbolTracker$Node$.class.asm
+++ b/scala/tools/nsc/symtab/SymbolTrackers$SymbolTracker$Node$.class.asm
@@ -187,9 +187,7 @@
     INVOKEVIRTUAL scala/collection/immutable/Set$.canBuildFrom ()Lscala/collection/generic/CanBuildFrom;
     INVOKEINTERFACE scala/collection/SetLike.map (Lscala/Function1;Lscala/collection/generic/CanBuildFrom;)Ljava/lang/Object;
     CHECKCAST scala/collection/GenTraversableOnce
-    GETSTATIC scala/collection/immutable/Set$.MODULE$ : Lscala/collection/immutable/Set$;
-    INVOKEVIRTUAL scala/collection/immutable/Set$.canBuildFrom ()Lscala/collection/generic/CanBuildFrom;
-    INVOKEINTERFACE scala/collection/immutable/Set.$plus$plus (Lscala/collection/GenTraversableOnce;Lscala/collection/generic/CanBuildFrom;)Ljava/lang/Object;
+    INVOKEINTERFACE scala/collection/immutable/Set.$plus$plus (Lscala/collection/GenTraversableOnce;)Lscala/collection/Set;
     CHECKCAST scala/collection/immutable/Set
     ALOAD 0
     GETFIELD scala/tools/nsc/symtab/SymbolTrackers$SymbolTracker$Node$.$outer : Lscala/tools/nsc/symtab/SymbolTrackers$SymbolTracker;

Produced with:

% rm build.properties
% ant -Dhas.java7=true locker.unlock all.clean pack.done
% cp -R build/pack /tmp/bootstrapped
% ant -Dhas.java7=true -Dlocker.skip=1 locker.unlock all.clean pack.done
% cp -R build/pack /tmp/skip
% jardiff /tmp/skip/lib/scala-compiler.jar /tmp/bootstrapped/lib/scala-compiler.jar

retronym · 2017-10-30T05:14:30Z

Okay, I see now that I was in fact testing differences in using 2.10.3 (starr.version) and the tip of this PR in code generation. Something seems to have changed overload resolution between 2.10.3 and 2.10.4, which is unrelated to this PR.

lrytz

just a bit of squashing left to do, looks all good otherwise.

retronym · 2017-10-30T22:32:05Z

I left this unsquashed for now to make it a bit easier to create the forward merge to 2.11.x, #6151. I've got that PR green now, so I could rebase/squash this and turn the 2.11.x version into a single commit.

adriaanm · 2017-10-31T00:06:43Z

+1 I think it's fine to leave the history in place for expediency as well as future archeology

xuwei-k · 2017-11-10T02:11:18Z

😢 scala/bug#10603

Ref scala/scala#6113

retronym · 2018-03-19T00:33:47Z

FTR, I've had a report of this exception when using Java 9, SBT 0.13.17, which uses the new code here to compile build.sbt etc.

java.lang.NullPointerException
        at scala.reflect.io.JavaToolsPlatformArchive.iterator(ZipArchive.scala:242)
        at scala.collection.IterableLike$class.foreach(IterableLike.scala:72)
        at scala.reflect.io.AbstractFile.foreach(AbstractFile.scala:92)
        at scala.tools.nsc.util.DirectoryClassPath.traverse(ClassPath.scala:308)
        at scala.tools.nsc.util.DirectoryClassPath.x$16$lzycompute(ClassPath.scala:317)
        at scala.tools.nsc.util.DirectoryClassPath.x$16(ClassPath.scala:317)
        at scala.tools.nsc.util.DirectoryClassPath.packages$lzycompute(ClassPath.scala:317)
        at scala.tools.nsc.util.DirectoryClassPath.packages(ClassPath.scala:317)
        at scala.tools.nsc.util.DirectoryClassPath.packages(ClassPath.scala:297)
        at scala.tools.nsc.util.MergedClassPath$$anonfun$packages$1.apply(ClassPat

I believe this is due to using a JRE rather than JDK, which gives a null from getSystemJavaCompiler in:

val fileManager = ToolProvider.getSystemJavaCompiler.getStandardFileManager(new DiagnosticCollector, java.util.Locale.getDefault, Charset.defaultCharset)

retronym force-pushed the backport/reflection branch from 2cf4a6a to e43d48c Compare October 5, 2017 23:59

SethTisue changed the title ~~[backport] Fix runtime refletion of empty package members under Java 9.~~ [backport] Fix runtime reflection of empty package members under Java 9. Oct 6, 2017

adriaanm added this to the 2.10.7 milestone Oct 6, 2017

retronym mentioned this pull request Oct 6, 2017

Runtime reflection of classes in the empty package broken under Java 9 scala/scala-dev#304

Closed

SethTisue mentioned this pull request Oct 6, 2017

Support building on JDK 9 scala-js/scala-js#3147

Closed

retronym changed the title ~~[backport] Fix runtime reflection of empty package members under Java 9.~~ [backport] Adapt to changes in Java 9 Oct 11, 2017

adriaanm mentioned this pull request Oct 11, 2017

Release Scala 2.10.7 scala/scala-dev#434

Closed

9 tasks

lrytz approved these changes Oct 13, 2017

View reviewed changes

retronym force-pushed the backport/reflection branch from 3817b40 to 05cdf51 Compare October 16, 2017 11:09

retronym force-pushed the backport/reflection branch 3 times, most recently from 46ba07e to 91db09e Compare October 25, 2017 23:36

retronym force-pushed the backport/reflection branch from 91db09e to 2f6be1b Compare October 26, 2017 00:30

retronym force-pushed the backport/reflection branch from 6bf893e to ded1024 Compare October 26, 2017 06:58

retronym added 6 commits October 27, 2017 06:12

Update to scala-asm v5.2.0-scala-2

c3d8750

Support -target:jvm1.8, needed to call static methods on Java interfaces

69b79ba

Limit use of java file manager to JDK 9

efbd9d5

Add MiMa whitelist entries

7e1a4b9

retronym force-pushed the backport/reflection branch from ded1024 to 7e1a4b9 Compare October 26, 2017 20:13

Fixup cherrypick of test case

2ae75e9

scala-jenkins added the reviewed label Oct 27, 2017

Move ASM sources back to the original location

4b84876

lrytz approved these changes Oct 30, 2017

View reviewed changes

retronym merged commit 89e57bc into scala:2.10.x Nov 1, 2017

SethTisue mentioned this pull request Nov 7, 2017

merge 2.10.x to 2.11.x [ci: last-only] #6167

Merged

eed3si9n added a commit to eed3si9n/sbt that referenced this pull request Jan 3, 2018

Bump Scala for JDK 9 support

d06e63d

Ref scala/scala#6113

kpbochenek mentioned this pull request Mar 28, 2020

Metals does not work with Java 9 scalameta/metals#1037

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[backport] Adapt to changes in Java 9 #6113

[backport] Adapt to changes in Java 9 #6113

retronym commented Oct 5, 2017 •

edited

Loading

sjrd commented Oct 6, 2017

dwijnand commented Oct 6, 2017 •

edited

Loading

sjrd commented Oct 6, 2017

adriaanm commented Oct 14, 2017

dwijnand commented Oct 14, 2017

retronym commented Oct 16, 2017

SethTisue commented Oct 16, 2017 •

edited

Loading

SethTisue commented Oct 17, 2017

SethTisue commented Oct 17, 2017

lrytz commented Oct 18, 2017

retronym commented Oct 25, 2017 •

edited

Loading

retronym commented Oct 26, 2017

SethTisue commented Oct 26, 2017

retronym commented Oct 27, 2017 •

edited

Loading

retronym commented Oct 27, 2017 •

edited

Loading

lrytz commented Oct 27, 2017

lrytz commented Oct 27, 2017

retronym commented Oct 30, 2017

retronym commented Oct 30, 2017 •

edited

Loading

retronym commented Oct 30, 2017 •

edited

Loading

lrytz left a comment

retronym commented Oct 30, 2017

adriaanm commented Oct 31, 2017

xuwei-k commented Nov 10, 2017

retronym commented Mar 19, 2018

[backport] Adapt to changes in Java 9 #6113

[backport] Adapt to changes in Java 9 #6113

Conversation

retronym commented Oct 5, 2017 • edited Loading

sjrd commented Oct 6, 2017

dwijnand commented Oct 6, 2017 • edited Loading

sjrd commented Oct 6, 2017

adriaanm commented Oct 14, 2017

dwijnand commented Oct 14, 2017

retronym commented Oct 16, 2017

SethTisue commented Oct 16, 2017 • edited Loading

SethTisue commented Oct 17, 2017

SethTisue commented Oct 17, 2017

lrytz commented Oct 18, 2017

retronym commented Oct 25, 2017 • edited Loading

retronym commented Oct 26, 2017

SethTisue commented Oct 26, 2017

retronym commented Oct 27, 2017 • edited Loading

retronym commented Oct 27, 2017 • edited Loading

lrytz commented Oct 27, 2017

lrytz commented Oct 27, 2017

retronym commented Oct 30, 2017

retronym commented Oct 30, 2017 • edited Loading

retronym commented Oct 30, 2017 • edited Loading

lrytz left a comment

Choose a reason for hiding this comment

retronym commented Oct 30, 2017

adriaanm commented Oct 31, 2017

xuwei-k commented Nov 10, 2017

retronym commented Mar 19, 2018

retronym commented Oct 5, 2017 •

edited

Loading

dwijnand commented Oct 6, 2017 •

edited

Loading

SethTisue commented Oct 16, 2017 •

edited

Loading

retronym commented Oct 25, 2017 •

edited

Loading

retronym commented Oct 27, 2017 •

edited

Loading

retronym commented Oct 27, 2017 •

edited

Loading

retronym commented Oct 30, 2017 •

edited

Loading

retronym commented Oct 30, 2017 •

edited

Loading