Skip to content
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

Keep track of java dependencies in JDK versions > 8 #609

Open
jvican opened this issue Oct 18, 2018 · 5 comments
Open

Keep track of java dependencies in JDK versions > 8 #609

jvican opened this issue Oct 18, 2018 · 5 comments

Comments

@jvican
Copy link
Member

jvican commented Oct 18, 2018

In a JDK version greater than 8, zinc will not correctly register dependencies on Java code coming from the JDK distribution (standard library) because the associated file will be none of the following types:

def processExternalDependency(binaryClassName: String, at: AbstractFile) = {
at match {
case zipEntry: ZipArchive#Entry =>
// The dependency comes from a JAR
for {
zip <- zipEntry.underlyingSource
jarFile <- Option(zip.file)
if !jarFile.isDirectory // workaround for JDK9 and Scala 2.10/2.11, see https://github.com/sbt/sbt/pull/3701
} binaryDependency(jarFile, binaryClassName)
case pf: PlainFile =>
// The dependency comes from a class file
binaryDependency(pf.file, binaryClassName)
case _ =>
reporter.error(
NoPosition,
s"Internal error: ${binaryClassName} comes from unknown origin ${at}"
)
}

Which means we would emit an error, and fail compilation. The associated file of a Java class coming from the JDK distribution now uses the JRT file system, which only 2.12.{6,7} and 2.13.0-M5 understand.

This is arguably not such a big problem because java dependencies are always there and never change. So I believe that this won't affect the correctness of the incremental compiler. However, as is now in latest master, it will emit errors on JDK versions > 8 and fail compilation.

I propose we remove the emission of the errors for any class coming from the java standard library. We'll see how we can do this in the 2.12.x series. A complication is that the compiler bridge source works at the version series level rather than at a specific Scala version, which means we cannot compile the bridge with specific Scala code for versions < 2.12.6 and > 2.12.7.

@smarter
Copy link
Contributor

smarter commented Oct 18, 2018

This is arguably not such a big problem because java dependencies are always there and never change.

This is true for now, but in the future the Scala compiler will support using Java and Scala libraries defined in the modulepath instead of the classpath, and everything in the modulepath will be in the jrt filesystem and represented by scalac using PlainNioFile. What is needed is a way to get the path of the module from a PlainNioFile, I haven't checked but it's possible PlainNioFile#container already does the right thing.

@jvican
Copy link
Member Author

jvican commented Oct 18, 2018

Yes, good point, I was only referring to this as a temporary solution for our current versions of Scala. We'll probably need to rethink this for zinc support in 2.13 and Scala 3.

@smarter
Copy link
Contributor

smarter commented Oct 19, 2018

@retronym What's the proper way to go from a PlainNioFile to the corresponding file in the real file system if one exists? Is calling .container good enough ?

@retronym
Copy link
Member

retronym commented Oct 22, 2018

Looks like we don't have a good story there.

Using https://github.com/retronym/scala/tree/review/jdk11

$ ./build/quick/bin/scala -nobootcp -Yjpms -modulepath $(coursier fetch -q -p commons-io:commons-io:2.6) -addmodules:org.apache.commons.io -addreads:org.apache.commons.io=ALL-UNNAMED

scala> def pathOfNioPath(f: scala.reflect.io.AbstractFile) = { val fld = f.getClass.getDeclaredField("nioPath"); fld.setAccessible(true); fld.get(f).asInstanceOf[java.nio.file.Path] }
pathOfNioPath: (f: scala.reflect.io.AbstractFile)java.nio.file.Path

scala> val assocFile = rootMirror.getRequiredClass("org.apache.commons.io.IOUtils")
assocFile: $r.intp.global.ClassSymbol = class IOUtils

scala> val assocFile = rootMirror.getRequiredClass("org.apache.commons.io.IOUtils").initialize.associatedFile
assocFile: scala.reflect.io.AbstractFile = /org/apache/commons/io/IOUtils.class

scala> val assocNioPath = pathOfNioPath(assocFile)
assocNioPath: java.nio.file.Path = /org/apache/commons/io/IOUtils.class

scala> assocNioPath.to
toAbsolutePath   toFile   toRealPath   toString   toUri

scala> assocNioPath.toUri
res60: java.net.URI = jar:file:///Users/jz/.coursier/cache/v1/https/repo1.maven.org/maven2/commons-io/commons-io/2.6/commons-io-2.6.jar!/org/apache/commons/io/IOUtils.class

scala> assocNioPath.getFileSystem.provider
res70: java.nio.file.spi.FileSystemProvider = jdk.nio.zipfs.ZipFileSystemProvider@5140c751

scala> assocNioPath.getFileSystem.provider.getScheme
res74: String = jar


scala> assocNioPath.getFileSystem.getFileStores.iterator.next.name
res83: String = /Users/jz/.coursier/cache/v1/https/repo1.maven.org/maven2/commons-io/commons-io/2.6/commons-io-2.6.jar/

We should change scalac to:

  • make PlainNioFile non-private and give you a way to get out the underlying nio.file.Path
  • Implement PlainNioFile.underlyingSource with the logic above to recover the JAR.

Added these to scala/scala#7218

@eed3si9n eed3si9n added the area/jdk_x jdk 9/10/11 label Feb 18, 2019
retronym added a commit to retronym/scala that referenced this issue Feb 22, 2019
```
$ ./build/quick/bin/scala
Welcome to Scala 2.13.0-20190221-140355-c0a7682 (OpenJDK 64-Bit Server VM, Java 11).
Type in expressions for evaluation. Or try :help.

scala> :power

scala> val path = symbolOf[java.lang.String].associatedFile.asInstanceOf[scala.reflect.io.PlainNioFile].underlyingSource
path: Option[scala.reflect.io.AbstractFile] = Some(/Users/jz/.jabba/jdk/openjdk@1.11.0/Contents/Home/jmods/java.base.jmod)

scala> .get.exists
res0: Boolean = true
```

```
$ ./build/quick/bin/scala -release 8
Welcome to Scala 2.13.0-20190221-140355-c0a7682 (OpenJDK 64-Bit Server VM, Java 11).
Type in expressions for evaluation. Or try :help.

scala> :power

scala> val path = symbolOf[java.lang.String].associatedFile.asInstanceOf[scala.reflect.io.PlainNioFile]
path: scala.reflect.io.PlainNioFile = /8/java/lang/String.sig

scala> .exists
res2: Boolean = true
```

References sbt/zinc#609
retronym added a commit to retronym/scala that referenced this issue Feb 22, 2019
```
$ ./build/quick/bin/scala
Welcome to Scala 2.13.0-20190221-140355-c0a7682 (OpenJDK 64-Bit Server VM, Java 11).
Type in expressions for evaluation. Or try :help.

scala> :power

scala> val path = symbolOf[java.lang.String].associatedFile
path: scala.reflect.io.AbstractFile = /modules/java.base/java/lang/String.class

scala> .underlyingSource
res0: Option[scala.reflect.io.AbstractFile] = Some(/Users/jz/.jabba/jdk/openjdk@1.11.0/Contents/Home/jmods/java.base.jmod)

scala> .get.exists
res1: Boolean = true
res0: Boolean = true
```

```
$ ./build/quick/bin/scala -release 8
Welcome to Scala 2.13.0-20190221-140355-c0a7682 (OpenJDK 64-Bit Server VM, Java 11).
Type in expressions for evaluation. Or try :help.

scala> :power

scala> val path = symbolOf[java.lang.String].associatedFile
path: scala.reflect.io.AbstractFile = /8/java/lang/String.sig

scala> .underlyingSource
res3: Option[scala.reflect.io.AbstractFile] = Some(/Users/jz/.jabba/jdk/openjdk@1.11.0/Contents/Home/lib/ct.sym)

scala> .get.exists
res4: Boolean = true
```

References sbt/zinc#609
retronym added a commit to retronym/scala that referenced this issue Feb 22, 2019
```
$ ./build/quick/bin/scala
Welcome to Scala 2.13.0-20190221-140355-c0a7682 (OpenJDK 64-Bit Server VM, Java 11).
Type in expressions for evaluation. Or try :help.

scala> :power

scala> val path = symbolOf[java.lang.String].associatedFile
path: scala.reflect.io.AbstractFile = /modules/java.base/java/lang/String.class

scala> .underlyingSource
res0: Option[scala.reflect.io.AbstractFile] = Some(/Users/jz/.jabba/jdk/openjdk@1.11.0/Contents/Home/jmods/java.base.jmod)

scala> .get.exists
res1: Boolean = true
res0: Boolean = true
```

```
$ ./build/quick/bin/scala -release 8
Welcome to Scala 2.13.0-20190221-140355-c0a7682 (OpenJDK 64-Bit Server VM, Java 11).
Type in expressions for evaluation. Or try :help.

scala> :power

scala> val path = symbolOf[java.lang.String].associatedFile
path: scala.reflect.io.AbstractFile = /8/java/lang/String.sig

scala> .underlyingSource
res3: Option[scala.reflect.io.AbstractFile] = Some(/Users/jz/.jabba/jdk/openjdk@1.11.0/Contents/Home/lib/ct.sym)

scala> .get.exists
res4: Boolean = true
```

References sbt/zinc#609
@radeusgd
Copy link

radeusgd commented Jun 23, 2020

We have a similar issue where zinc is throwing exceptions when analyzing dependencies.

We have a use-case where we use sbt/zinc to compile a mixed Scala and Java project that depends on a Java module (GraalVM Truffle API) which is closed by default (to prevent insecure introspection) but it ships with an alternative open version that is to be used for compilation.
Unfortunately, when discovering the dependencies (

def load(tpe: String, errMsg: => Option[String]): Option[Class[_]] = {
) zinc is using the default ClassLoader which sees the closed module that is on the default JVM classpath. This makes it unable to open some of the classfiles and track their dependencies. But moreover, for some it opens them successfully but only later it fails with IllegallAccessError. As this fail happens after the load call (when manipulating the loaded class to find its inheritance dependencies) and it is not expected, the exception aborts compilation.

Our current fix to it is to override the module seen by the JVM running sbt and zinc by adding --upgrade-module-path=the-open-version-of-truffle-api.jar to .jvmopts. This is however problematic, because this setting has to be applied at startup and we have to download the open JAR first, so we need to do a full reboot of sbt when updating the dependencies.

Any chance that a future version will have some more sophisticated support of overriding the modules or maybe just overriding these security exceptions altogether? If javac succeeded, then compilation should not be terminated because the dependency analysis process is unable to read some class files due to insufficient permissions. Intuitively it seems that the compiler should have all the permissions enabled by default.

@dwijnand dwijnand changed the title Keep track of java dependencies in jDK versions > 8 Keep track of java dependencies in JDK versions > 8 Jul 10, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

5 participants