Restrict library and reflect to compact1 profile #6164

adriaanm · 2017-11-06T19:21:43Z

The scala library (core and reflect) now depends only on the compact1 profile.

jdeps -s -profile /Users/adriaan/.ivy2/local/org.scala-lang/scala-{library,reflect}/2.13.0-bin-local-07ff7ac/jars/scala-*.jar                                                                                                               
scala-library.jar -> java.base (compact1)
scala-reflect.jar -> java.base (compact1)
scala-reflect.jar -> /Users/adriaan/.ivy2/local/org.scala-lang/scala-library/2.13.0-bin-local-07ff7ac/jars/scala-library.jar

See scala/bug#10559

The `@remote` annotation will be removed in 2.13. It was deprecated in 2.12, and will be gone entirely in 2.13.0-M4, but first we must release a milestone that no longer refers to it (so that the compiler can bootstrap without a definition for `@remote`). This cuts tie 1 of 3 to the Java profiles beyond compact1.

The `BeanInfo` generation was deprecated in 2.12 and will be removed in 2.13. First, we cut ties from the compiler in M3, so that we can remove the classes `scala.beans.{BeanInfo, ScalaBeanInfo}` entirely in M4, along with `scala.beans.{BeanInfoSkip, BeanDescription, BeanDisplayName}`, which were part of the deprecated feature. (Note: for good measure, we should deprecate all of them in 2.12.5, instead of just the entry point). This cuts tie 2 of 3 to the Java profiles beyond compact1.

scala-{library,reflect} now depend only on the compact1 profile.

lrytz

cc @nilskp for the removal of BeanInfo, since you recently voiced your interest in that feature (https://users.scala-lang.org/t/beaninfo-alternative/1301). I guess the same functionality could be implemented with a compiler plugin.

@adriaanm how did you identify implementations that use classes outside the compact1 profile? How can we prevent new references from sneaking in?

lrytz · 2017-11-10T09:15:55Z

src/library/scala/sys/process/package.scala

-    def javaVmArguments: List[String] = {
-      import scala.collection.JavaConverters._
-
-      java.lang.management.ManagementFactory.getRuntimeMXBean.getInputArguments.asScala.toList


Since this method is not deprecated in 2.12, we could do a reflective implementation

scala> import scala.collection.JavaConverters._ import scala.collection.JavaConverters._ scala> val mxb = Class.forName("java.lang.management.ManagementFactory").getMethod("getRuntimeMXBean").invoke(null) mxb: Object = sun.management.RuntimeImpl@4447c594 scala> Class.forName("java.lang.management.RuntimeMXBean").getMethod("getInputArguments").invoke(mxb).asInstanceOf[java.util.List[String]].asScala.toList res19: List[String] = List(-Xmx1536m, -Xms1536m, -Xss5m, -XX:+UseCompressedOops, -XX:+UseParallelGC, -Xbootclasspath/a:/Users/luc/scala/scala-2.12.3/lib/jline-2.14.4.jar:/Users/luc/scala/scala-2.12.3/lib/scala-compiler.jar:/Users/luc/scala/scala-2.12.3/lib/scala-library.jar:/Users/luc/scala/scala-2.12.3/lib/scala-parser-combinators_2.12-1.0.6.jar:/Users/luc/scala/scala-2.12.3/lib/scala-reflect.jar:/Users/luc/scala/scala-2.12.3/lib/scala-swing_2.12-2.0.0.jar:/Users/luc/scala/scala-2.12.3/lib/scala-xml_2.12-1.0.6.jar:/Users/luc/scala/scala-2.12.3/lib/scalap-2.12.3.jar, -Dscala.boot.class.path=/Users/luc/scala/scala-2.12.3/lib/jline-2.14.4.jar:/Users/luc/scala/scala-2.12.3/lib/scala-compiler.jar:/Users/luc/scala/scala-2.12.3/lib/scala-library.jar:/Users/luc/scala/...

This would fail with java.lang.ClassNotFoundException if the classes are not available, which is probably comparable to what currently happens.

We can deprecate the method at the same time.

I see you're deprecating it for 2.12.5; I guess either way is fine.

lrytz · 2017-11-10T09:21:30Z

Ah, jdeps!

dwijnand · 2017-11-10T10:54:02Z

How can we prevent new references from sneaking in?

👍 jdeps should be used to enforce nothing sneaks in (I don't mean necessarily in this PR).

adriaanm · 2017-11-13T21:05:26Z

I can't figure out how to add a test to our build that checks that jdeps -s -P build/pack/lib/scala-{library,reflect}.jar | grep -v build/pack | perl -pe 's/.*$(.*)$$/$1/' | sort | uniq outputs (only) compact1. @szeiger, could you take a stab at that please?

So far, I have something like:

lazy val jdeps = taskKey[Unit]("Run jdeps to check dependencies")
lazy val restrictDeps = project
  .dependsOn(library, reflect)
  .settings(commonSettings)
  .settings(disableDocs)
  .settings(disablePublishing)
  .settings(
    Keys.test in Test := (Keys.test in Test).dependsOn(jdeps).value,
    jdeps := {
      import Process._
      val profilePart = ".*\\((.*)\\)$".r
      val jdepsOut = s"jdeps -s -P ${(packageBin in library in Compile).value.getPath} ${(packageBin in reflect in Compile).value.getPath}".lines_!
      val profiles = jdepsOut.flatMap(line => line.split(" -> ", 2) match { case Array(_, profilePart(profile)) => Some(profile) case s => None }).toSet
      if (profiles != Set("compact1")) streams.value.log.error("Detected dependency outside of compact1:\n"+jdepsOut.mkString("\n")) // TODO how to report error?
    }
  )

Not sure how to run this as a test and report the error

dwijnand · 2017-11-14T12:36:59Z

Task failure is reported by throwing an exception.

szeiger · 2017-11-14T16:36:59Z

I suggest the following:

    jdeps := {
      val profilePart = ".*\\((.*)\\)$".r
      val jdepsOut = Process("jdeps", Seq("-s", "-P", (packageBin in library in Compile).value.getPath, (packageBin in reflect in Compile).value.getPath)).lines_!
      val profiles = jdepsOut.flatMap(line => line.split(" -> ", 2) match { case Array(_, profilePart(profile)) => Some(profile) case s => None }).toSet
      if (profiles != Set("compact1"))
        throw new RuntimeException("Detected dependency outside of compact1:\n"+jdepsOut.mkString("\n"))
    },

Errors are reported as Exceptions. I've also changed the Process call to avoid command line parsing. The other version would fail with spaces in the base directory. IMHO this is bad API design in sbt: doing the wrong thing is easier than doing the right thing.

I don't see any reason for the special restrictDeps project. I simply put it into root for testing. I'd rather call this kind of "extra" test from testAll instead of running it with the standard test command.

@dwijnand Does sbt have any code for localizing javac that could be co-opted for jdeps or do you simply call whatever javac is on the path?

dwijnand · 2017-11-14T18:56:02Z

By default it uses what it finds on the PATH, with support for a custom javaHome setting. But I wouldn't bother getting fancy until you need it.

This mirror similar changes done by Adriaan Moors in scala#6164

This was already partially done in 31f9681. This mirror similar changes done by Adriaan Moors in scala#6164

As verify by running `jdep -s -P` on the dotty-compiler jar. See scala/bug#10559 for why this is useful and scala/scala#6164 for how this was implemented in scalac.

adriaanm · 2017-12-13T22:57:07Z

I won't have time to implement the test for this. Tracking as scala/scala-dev#460

See scala/scala#6164 Ref scala/scala-dev#461 (still need to release partest & upgrade)

See scala#6164 Ref scala/scala-dev#461 (still need to release partest & upgrade)

Enforcement of scala#6164 Tested manually by reverting the sys.process.javaVmArguments change 07ff7ac and then running testJDepsImpl: > testJDeps [info] Compiling 1 Scala source to /d/scala/build/quick/classes/library... [info] Packaging /d/scala/build/pack/lib/scala-library.jar ... [info] Packaging /d/scala/build/pack/lib/scala-reflect.jar ... [info] Done packaging. [info] Done packaging. [trace] Stack trace suppressed: run last root/*:testJDeps for the full output. [error] (root/*:testJDeps) Detected dependency outside of compact1: [error] scala-library.jar -> /Library/Java/JavaVirtualMachines/jdk1.8.0_144.jdk/Contents/Home/jre/lib/rt.jar (compact3) [error] scala-reflect.jar -> /Library/Java/JavaVirtualMachines/jdk1.8.0_144.jdk/Contents/Home/jre/lib/rt.jar (compact1) [error] scala-reflect.jar -> /d/scala/build/pack/lib/scala-library.jar [error] Total time: 4 s, completed 14-Jun-2018 14:20:02 Fixes scala/scala-dev#437 Fixes scala/bug#10559

scala-jenkins added this to the 2.13.0-M3 milestone Nov 6, 2017

nicolasstucki mentioned this pull request Nov 6, 2017

Meta-issue: Port Backend improvements scala/scala3#3113

Closed

10 tasks

adriaanm force-pushed the modularize-drop-remote branch from 6047d9b to e8d03d9 Compare November 6, 2017 22:36

adriaanm added 3 commits November 6, 2017 15:30

Drop scala.sys.process.javaVmArguments,

07ff7ac

scala-{library,reflect} now depend only on the compact1 profile.

adriaanm changed the title ~~Remove compiler dependency on @remote~~ Restrict library and reflect to compact1 profile Nov 6, 2017

adriaanm force-pushed the modularize-drop-remote branch from e8d03d9 to 07ff7ac Compare November 6, 2017 23:43

adriaanm requested a review from lrytz November 6, 2017 23:43

This was referenced Nov 6, 2017

Reduce dependencies from Full JRE to compact1 profile scala/bug#10559

Closed

Library modularisation scala/scala-dev#323

Closed

Deprecations to prepare adherence to compact1 profile #6165

Merged

lrytz reviewed Nov 10, 2017

View reviewed changes

smarter added a commit to dotty-staging/scala that referenced this pull request Dec 7, 2017

Remove compiler dependency on @BeanInfo

d3111e3

This mirror similar changes done by Adriaan Moors in scala#6164

smarter added a commit to dotty-staging/scala that referenced this pull request Dec 7, 2017

Remove compiler dependency on @remote

819fdc2

This was already partially done in 31f9681. This mirror similar changes done by Adriaan Moors in scala#6164

smarter mentioned this pull request Dec 7, 2017

Restrict dotty-compiler to the jvm compact1 profile scala/scala3#3640

Merged

adriaanm mentioned this pull request Dec 13, 2017

add jdeps test to enforce library is within compact1 profile scala/scala-dev#460

Closed

adriaanm merged commit 2f3791c into scala:2.13.x Dec 13, 2017

SethTisue added the release-notes worth highlighting in next release notes label Dec 13, 2017

dwijnand added a commit to dwijnand/scala-partest that referenced this pull request Dec 14, 2017

Avoid scala.sys.process.javaVmArguments

4772268

See scala/scala#6164 Ref scala/scala-dev#461 (still need to release partest & upgrade)

dwijnand mentioned this pull request Dec 14, 2017

Avoid scala.sys.process.javaVmArguments scala/scala-partest#92

Merged

lrytz pushed a commit to lrytz/scala-partest that referenced this pull request May 9, 2018

Avoid scala.sys.process.javaVmArguments

436b5d1

See scala/scala#6164 Ref scala/scala-dev#461 (still need to release partest & upgrade)

lrytz pushed a commit to lrytz/scala-partest that referenced this pull request May 9, 2018

Avoid scala.sys.process.javaVmArguments

25cb18c

See scala/scala#6164 Ref scala/scala-dev#461 (still need to release partest & upgrade)

lrytz pushed a commit to lrytz/scala that referenced this pull request May 9, 2018

Avoid scala.sys.process.javaVmArguments

aa644a2

See scala#6164 Ref scala/scala-dev#461 (still need to release partest & upgrade)

lrytz pushed a commit to lrytz/scala that referenced this pull request May 9, 2018

Avoid scala.sys.process.javaVmArguments

734b2a2

See scala#6164 Ref scala/scala-dev#461 (still need to release partest & upgrade)

adriaanm mentioned this pull request Jun 14, 2018

Prevent scala-{library,reflect} from depending on JVM code outside of compact1 profile scala/scala-dev#437

Closed

dwijnand mentioned this pull request Jun 14, 2018

Enforce library/reflect restriction to compat1 profile #6800

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Restrict library and reflect to compact1 profile #6164

Restrict library and reflect to compact1 profile #6164

Uh oh!

adriaanm commented Nov 6, 2017 •

edited

Loading

Uh oh!

lrytz left a comment

Uh oh!

lrytz Nov 10, 2017 •

edited

Loading

Uh oh!

lrytz Nov 10, 2017

Uh oh!

lrytz commented Nov 10, 2017

Uh oh!

dwijnand commented Nov 10, 2017

Uh oh!

adriaanm commented Nov 13, 2017 •

edited

Loading

Uh oh!

dwijnand commented Nov 14, 2017

Uh oh!

szeiger commented Nov 14, 2017

Uh oh!

dwijnand commented Nov 14, 2017

Uh oh!

adriaanm commented Dec 13, 2017

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

Restrict library and reflect to compact1 profile #6164

Restrict library and reflect to compact1 profile #6164

Uh oh!

Conversation

adriaanm commented Nov 6, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

lrytz left a comment

Choose a reason for hiding this comment

Uh oh!

lrytz Nov 10, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

lrytz Nov 10, 2017

Choose a reason for hiding this comment

Uh oh!

lrytz commented Nov 10, 2017

Uh oh!

dwijnand commented Nov 10, 2017

Uh oh!

adriaanm commented Nov 13, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

dwijnand commented Nov 14, 2017

Uh oh!

szeiger commented Nov 14, 2017

Uh oh!

dwijnand commented Nov 14, 2017

Uh oh!

adriaanm commented Dec 13, 2017

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

adriaanm commented Nov 6, 2017 •

edited

Loading

lrytz Nov 10, 2017 •

edited

Loading

adriaanm commented Nov 13, 2017 •

edited

Loading