From 819257189b3bad515b8f39ddc2a71f489e812845 Mon Sep 17 00:00:00 2001 From: Heather Miller Date: Wed, 8 Oct 2014 12:38:46 -0700 Subject: SI-6502 Reenables loading jars into the running REPL (regression in 2.10) Fixes SI-6502, reenables loading jars into the running REPL (regression in 2.10). This PR allows adding a jar to the compile and runtime classpaths without resetting the REPL state (crucial for Spark SPARK-3257). This follows the lead taken by @som-snytt in PR #3986, which differentiates two jar-loading behaviors (muddled by cp): - adding jars and replaying REPL expressions (using replay) - adding jars without resetting the REPL (deprecated cp, introduced require) This PR implements require (left unimplemented in #3986) This PR is a simplification of a similar approach taken by @gkossakowski in #3884. In this attempt, we check first to make sure that a jar is only added if it only contains new classes/traits/objects, otherwise we emit an error. This differs from the old invalidation approach which also tracked deleted classpath entries. --- src/compiler/scala/tools/nsc/Global.scala | 143 ++++++++++++++++++++- .../scala/tools/nsc/backend/JavaPlatform.scala | 2 +- src/repl/scala/tools/nsc/interpreter/ILoop.scala | 68 +++++++++- src/repl/scala/tools/nsc/interpreter/IMain.scala | 59 ++++++++- 4 files changed, 262 insertions(+), 10 deletions(-) (limited to 'src') diff --git a/src/compiler/scala/tools/nsc/Global.scala b/src/compiler/scala/tools/nsc/Global.scala index 9cc9712b44..0a7fd1c3e1 100644 --- a/src/compiler/scala/tools/nsc/Global.scala +++ b/src/compiler/scala/tools/nsc/Global.scala @@ -13,7 +13,7 @@ import scala.compat.Platform.currentTime import scala.collection.{ mutable, immutable } import io.{ SourceReader, AbstractFile, Path } import reporters.{ Reporter, ConsoleReporter } -import util.{ ClassPath, StatisticsInfo, returning, stackTraceString } +import util.{ ClassPath, MergedClassPath, StatisticsInfo, returning, stackTraceString } import scala.reflect.ClassTag import scala.reflect.internal.util.{ OffsetPosition, SourceFile, NoSourceFile, BatchSourceFile, ScriptSourceFile } import scala.reflect.internal.pickling.{ PickleBuffer, PickleFormat } @@ -841,6 +841,147 @@ class Global(var currentSettings: Settings, var reporter: Reporter) } reverse } + // ------------ Invalidations --------------------------------- + + /** Is given package class a system package class that cannot be invalidated? + */ + private def isSystemPackageClass(pkg: Symbol) = + pkg == RootClass || + pkg == definitions.ScalaPackageClass || { + val pkgname = pkg.fullName + (pkgname startsWith "scala.") && !(pkgname startsWith "scala.tools") + } + + /** Invalidates packages that contain classes defined in a classpath entry, and + * rescans that entry. + * + * First, the classpath entry referred to by one of the `paths` is rescanned, + * so that any new files or changes in subpackages are picked up. + * Second, any packages for which one of the following conditions is met is invalidated: + * - the classpath entry contained during the last compilation run now contains classfiles + * that represent a member in the package; + * - the classpath entry now contains classfiles that represent a member in the package; + * - the set of subpackages has changed. + * + * The invalidated packages are reset in their entirety; all member classes and member packages + * are re-accessed using the new classpath. + * + * System packages that the compiler needs to access as part of standard definitions + * are not invalidated. A system package is: + * Any package rooted in "scala", with the exception of packages rooted in "scala.tools". + * + * @param paths Fully-qualified names that refer to directories or jar files that are + * entries on the classpath. + * + * @return A pair consisting of + * - a list of invalidated packages + * - a list of of packages that should have been invalidated but were not because + * they are system packages. + */ + def invalidateClassPathEntries(paths: String*): (List[ClassSymbol], List[ClassSymbol]) = { + val invalidated, failed = new mutable.ListBuffer[ClassSymbol] + classPath match { + case cp: MergedClassPath[_] => + def assoc(path: String): List[(PlatformClassPath, PlatformClassPath)] = { + val dir = AbstractFile.getDirectory(path) + val canonical = dir.canonicalPath + def matchesCanonical(e: ClassPath[_]) = e.origin match { + case Some(opath) => + AbstractFile.getDirectory(opath).canonicalPath == canonical + case None => + false + } + cp.entries find matchesCanonical match { + case Some(oldEntry) => + List(oldEntry -> cp.context.newClassPath(dir)) + case None => + error(s"Error adding entry to classpath. During invalidation, no entry named $path in classpath $classPath") + List() + } + } + val subst = Map(paths flatMap assoc: _*) + if (subst.nonEmpty) { + platform updateClassPath subst + informProgress(s"classpath updated on entries [${subst.keys mkString ","}]") + def mkClassPath(elems: Iterable[PlatformClassPath]): PlatformClassPath = + if (elems.size == 1) elems.head + else new MergedClassPath(elems, classPath.context) + val oldEntries = mkClassPath(subst.keys) + val newEntries = mkClassPath(subst.values) + mergeNewEntries(newEntries, RootClass, Some(classPath), Some(oldEntries), invalidated, failed) + } + } + def show(msg: String, syms: scala.collection.Traversable[Symbol]) = + if (syms.nonEmpty) + informProgress(s"$msg: ${syms map (_.fullName) mkString ","}") + show("invalidated packages", invalidated) + show("could not invalidate system packages", failed) + (invalidated.toList, failed.toList) + } + + /** Merges new classpath entries into the symbol table + * + * @param newEntries The new classpath entries + * @param root The root symbol to be resynced (a package class) + * @param allEntries Optionally, the corresponding package in the complete current classpath + * @param oldEntries Optionally, the corresponding package in the old classpath entries + * @param invalidated A listbuffer collecting the invalidated package classes + * @param failed A listbuffer collecting system package classes which could not be invalidated + * + * The merging strategy is determined by the absence or presence of classes and packages. + * + * If either oldEntries or newEntries contains classes, root is invalidated provided that a corresponding package + * exists in allEntries. Otherwise it is removed. + * Otherwise, the action is determined by the following matrix, with columns: + * + * old sym action + * + + recurse into all child packages of newEntries + * - + invalidate root + * - - create and enter root + * + * Here, old means classpath, and sym means symboltable. + is presence of an entry in its column, - is absence. + */ + private def mergeNewEntries(newEntries: PlatformClassPath, root: ClassSymbol, + allEntries: OptClassPath, oldEntries: OptClassPath, + invalidated: mutable.ListBuffer[ClassSymbol], failed: mutable.ListBuffer[ClassSymbol]) { + ifDebug(informProgress(s"syncing $root, $oldEntries -> $newEntries")) + + val getName: ClassPath[AbstractFile] => String = (_.name) + def hasClasses(cp: OptClassPath) = cp.isDefined && cp.get.classes.nonEmpty + def invalidateOrRemove(root: ClassSymbol) = { + allEntries match { + case Some(cp) => root setInfo new loaders.PackageLoader(cp) + case None => root.owner.info.decls unlink root.sourceModule + } + invalidated += root + } + def subPackage(cp: PlatformClassPath, name: String): OptClassPath = + cp.packages find (cp1 => getName(cp1) == name) + + val classesFound = hasClasses(oldEntries) || newEntries.classes.nonEmpty + if (classesFound && !isSystemPackageClass(root)) { + invalidateOrRemove(root) + } else { + if (classesFound) { + if (root.isRoot) invalidateOrRemove(EmptyPackageClass) + else failed += root + } + if (!oldEntries.isDefined) invalidateOrRemove(root) + else + for (pstr <- newEntries.packages.map(getName)) { + val pname = newTermName(pstr) + val pkg = (root.info decl pname) orElse { + // package does not exist in symbol table, create symbol to track it + assert(!subPackage(oldEntries.get, pstr).isDefined) + loaders.enterPackage(root, pstr, new loaders.PackageLoader(allEntries.get)) + } + mergeNewEntries(subPackage(newEntries, pstr).get, pkg.moduleClass.asClass, + subPackage(allEntries.get, pstr), subPackage(oldEntries.get, pstr), + invalidated, failed) + } + } + } + // ----------- Runs --------------------------------------- private var curRun: Run = null diff --git a/src/compiler/scala/tools/nsc/backend/JavaPlatform.scala b/src/compiler/scala/tools/nsc/backend/JavaPlatform.scala index 7236bf70d5..4877bd9b80 100644 --- a/src/compiler/scala/tools/nsc/backend/JavaPlatform.scala +++ b/src/compiler/scala/tools/nsc/backend/JavaPlatform.scala @@ -16,7 +16,7 @@ trait JavaPlatform extends Platform { import global._ import definitions._ - private var currentClassPath: Option[MergedClassPath[AbstractFile]] = None + private[nsc] var currentClassPath: Option[MergedClassPath[AbstractFile]] = None def classPath: ClassPath[AbstractFile] = { if (currentClassPath.isEmpty) currentClassPath = Some(new PathResolver(settings).result) diff --git a/src/repl/scala/tools/nsc/interpreter/ILoop.scala b/src/repl/scala/tools/nsc/interpreter/ILoop.scala index 6e18682494..9cbb6ae574 100644 --- a/src/repl/scala/tools/nsc/interpreter/ILoop.scala +++ b/src/repl/scala/tools/nsc/interpreter/ILoop.scala @@ -221,7 +221,7 @@ class ILoop(in0: Option[BufferedReader], protected val out: JPrintWriter) nullary("power", "enable power user mode", powerCmd), nullary("quit", "exit the interpreter", () => Result(keepRunning = false, None)), cmd("replay", "[options]", "reset the repl and replay all previous commands", replayCommand), - //cmd("require", "", "add a jar or directory to the classpath", require), // TODO + cmd("require", "", "add a jar or directory to the classpath", require), cmd("reset", "[options]", "reset the repl to its initial state, forgetting all session entries", resetCommand), cmd("save", "", "save replayable session to a file", saveCommand), shCommand, @@ -612,13 +612,73 @@ class ILoop(in0: Option[BufferedReader], protected val out: JPrintWriter) val f = File(arg).normalize if (f.exists) { addedClasspath = ClassPath.join(addedClasspath, f.path) - val totalClasspath = ClassPath.join(settings.classpath.value, addedClasspath) - echo("Added '%s'. Your new classpath is:\n\"%s\"".format(f.path, totalClasspath)) - replay() + intp.addUrlsToClassPath(f.toURI.toURL) + echo("Added '%s'. Your new classpath is:\n\"%s\"".format(f.path, intp.global.classPath.asClasspathString)) } else echo("The path '" + f + "' doesn't seem to exist.") } + /** Adds jar file to the current classpath. Jar will only be added if it + * does not contain classes that already exist on the current classpath. + * + * Importantly, `require` adds jars to the classpath ''without'' resetting + * the state of the interpreter. This is in contrast to `replay` which can + * be used to add jars to the classpath and which creates a new instance of + * the interpreter and replays all interpreter expressions. + */ + def require(arg: String): Unit = { + class InfoClassLoader extends java.lang.ClassLoader { + def classOf(arr: Array[Byte]): Class[_] = + super.defineClass(null, arr, 0, arr.length) + } + + def readFully(is: InputStream): Array[Byte] = { + val dis = new java.io.DataInputStream(is) + val len = dis.available() + val arr = Array.ofDim[Byte](len) + dis.readFully(arr) + dis.close() + arr + } + + val f = File(arg).normalize + + if (f.isDirectory) { + echo("Adding directories to the classpath is not supported. Add a jar instead.") + return + } + val jarFile = new java.util.jar.JarFile(arg) + val entries = jarFile.entries + val cloader = new InfoClassLoader + var exists = false + + while (entries.hasMoreElements()) { + val entry = entries.nextElement() + // skip directories and manifests + if (!entry.isDirectory() && !entry.getName().endsWith("MF")) { + // for each entry get InputStream + val is = jarFile.getInputStream(entry) + // read InputStream into Array[Byte] + val arr = readFully(is) + val clazz = cloader.classOf(arr) + try { + Class.forName(clazz.getName(), false, intp.classLoader) + exists = true + } catch { + case _: ClassNotFoundException => /* do nothing */ + } + } + } + + if (f.exists && !exists) { + addedClasspath = ClassPath.join(addedClasspath, f.path) + intp.addUrlsToClassPath(f.toURI.toURL) + echo("Added '%s'. Your new classpath is:\n\"%s\"".format(f.path, intp.global.classPath.asClasspathString)) + } else if (exists) { + echo("The path '" + f + "' cannot be loaded, because existing classpath entries conflict.") + } else echo("The path '" + f + "' doesn't seem to exist.") + } + def powerCmd(): Result = { if (isReplPower) "Already in power mode." else enablePowerMode(isDuringInit = false) diff --git a/src/repl/scala/tools/nsc/interpreter/IMain.scala b/src/repl/scala/tools/nsc/interpreter/IMain.scala index 3f4922a602..4f035bae7a 100644 --- a/src/repl/scala/tools/nsc/interpreter/IMain.scala +++ b/src/repl/scala/tools/nsc/interpreter/IMain.scala @@ -18,9 +18,13 @@ import scala.reflect.internal.util.{ BatchSourceFile, SourceFile } import scala.tools.util.PathResolver import scala.tools.nsc.io.AbstractFile import scala.tools.nsc.typechecker.{ TypeStrings, StructuredTypeStrings } -import scala.tools.nsc.util.{ ScalaClassLoader, stringFromReader, stringFromWriter, StackTraceOps } +import scala.tools.nsc.util.{ ScalaClassLoader, stringFromReader, stringFromWriter, StackTraceOps, ClassPath, MergedClassPath } +import ScalaClassLoader.URLClassLoader import scala.tools.nsc.util.Exceptional.unwrap +import scala.tools.nsc.backend.JavaPlatform import javax.script.{AbstractScriptEngine, Bindings, ScriptContext, ScriptEngine, ScriptEngineFactory, ScriptException, CompiledScript, Compilable} +import java.net.URL +import java.io.File /** An interpreter for Scala code. * @@ -82,6 +86,9 @@ class IMain(@BeanProperty val factory: ScriptEngineFactory, initialSettings: Set private var _classLoader: util.AbstractFileClassLoader = null // active classloader private val _compiler: ReplGlobal = newCompiler(settings, reporter) // our private compiler + private trait ExposeAddUrl extends URLClassLoader { def addNewUrl(url: URL) = this.addURL(url) } + private var _runtimeClassLoader: URLClassLoader with ExposeAddUrl = null // wrapper exposing addURL + def compilerClasspath: Seq[java.net.URL] = ( if (isInitializeComplete) global.classPath.asURLs else new PathResolver(settings).result.asURLs // the compiler's classpath @@ -237,6 +244,50 @@ class IMain(@BeanProperty val factory: ScriptEngineFactory, initialSettings: Set new Global(settings, reporter) with ReplGlobal { override def toString: String = "" } } + /** + * Adds all specified jars to the compile and runtime classpaths. + * + * @note Currently only supports jars, not directories. + * @param urls The list of items to add to the compile and runtime classpaths. + */ + def addUrlsToClassPath(urls: URL*): Unit = { + new Run // force some initialization + urls.foreach(_runtimeClassLoader.addNewUrl) // Add jars to runtime classloader + extendCompilerClassPath(urls: _*) // Add jars to compile-time classpath + } + + /** Extend classpath of global.platform and force `global` to rescan updated packages. */ + protected def extendCompilerClassPath(urls: URL*): Unit = { + val newClassPath = mergeUrlsIntoClassPath(global.platform, urls: _*) + global.platform.currentClassPath = Some(newClassPath) + // Reload all specified jars into the current compiler instance (global) + global.invalidateClassPathEntries(urls.map(_.getPath): _*) + } + + /** Merge classpath of `platform` and `urls` into merged classpath */ + protected def mergeUrlsIntoClassPath(platform: JavaPlatform, urls: URL*): MergedClassPath[AbstractFile] = { + // Collect our new jars/directories and add them to the existing set of classpaths + val prevEntries = platform.classPath match { + case mcp: MergedClassPath[AbstractFile] => mcp.entries + case cp: ClassPath[AbstractFile] => List(cp) + } + val allEntries = (prevEntries ++ + urls.map(url => platform.classPath.context.newClassPath( + if (url.getProtocol == "file") { + val f = new File(url.getPath) + if (f.isDirectory) io.AbstractFile.getDirectory(f) + else io.AbstractFile.getFile(f) + } else { + io.AbstractFile.getURL(url) + } + ) + ) + ).distinct + + // Combine all of our classpaths (old and new) into one merged classpath + new MergedClassPath(allEntries, platform.classPath.context) + } + /** Parent classloader. Overridable. */ protected def parentClassLoader: ClassLoader = settings.explicitParentLoader.getOrElse( this.getClass.getClassLoader() ) @@ -329,9 +380,9 @@ class IMain(@BeanProperty val factory: ScriptEngineFactory, initialSettings: Set } } private def makeClassLoader(): util.AbstractFileClassLoader = - new TranslatingClassLoader(parentClassLoader match { - case null => ScalaClassLoader fromURLs compilerClasspath - case p => new ScalaClassLoader.URLClassLoader(compilerClasspath, p) + new TranslatingClassLoader({ + _runtimeClassLoader = new URLClassLoader(compilerClasspath, parentClassLoader) with ExposeAddUrl + _runtimeClassLoader }) // Set the current Java "context" class loader to this interpreter's class loader -- cgit v1.2.3 From a84abd088df0105c542aa3192de5a634f3ceda35 Mon Sep 17 00:00:00 2001 From: Heather Miller Date: Wed, 15 Oct 2014 16:46:21 -0700 Subject: SI-6502 Addressing review comments --- src/compiler/scala/tools/nsc/Global.scala | 21 ++++++------------ src/repl/scala/tools/nsc/interpreter/ILoop.scala | 28 ++++++++++++++++-------- 2 files changed, 26 insertions(+), 23 deletions(-) (limited to 'src') diff --git a/src/compiler/scala/tools/nsc/Global.scala b/src/compiler/scala/tools/nsc/Global.scala index 0a7fd1c3e1..658aa66fd9 100644 --- a/src/compiler/scala/tools/nsc/Global.scala +++ b/src/compiler/scala/tools/nsc/Global.scala @@ -845,12 +845,8 @@ class Global(var currentSettings: Settings, var reporter: Reporter) /** Is given package class a system package class that cannot be invalidated? */ - private def isSystemPackageClass(pkg: Symbol) = - pkg == RootClass || - pkg == definitions.ScalaPackageClass || { - val pkgname = pkg.fullName - (pkgname startsWith "scala.") && !(pkgname startsWith "scala.tools") - } + private def isSystemPackageClass(pkg: Symbol) = + pkg == RootClass || (pkg.hasTransOwner(definitions.ScalaPackageClass) && !pkg.hasTransOwner(this.rootMirror.staticPackage("scala.tools").moduleClass.asClass)) /** Invalidates packages that contain classes defined in a classpath entry, and * rescans that entry. @@ -872,13 +868,11 @@ class Global(var currentSettings: Settings, var reporter: Reporter) * * @param paths Fully-qualified names that refer to directories or jar files that are * entries on the classpath. - * - * @return A pair consisting of - * - a list of invalidated packages - * - a list of of packages that should have been invalidated but were not because - * they are system packages. */ - def invalidateClassPathEntries(paths: String*): (List[ClassSymbol], List[ClassSymbol]) = { + def invalidateClassPathEntries(paths: String*): Unit = { + implicit object ClassPathOrdering extends Ordering[PlatformClassPath] { + def compare(a:PlatformClassPath, b:PlatformClassPath) = a.asClasspathString compare b.asClasspathString + } val invalidated, failed = new mutable.ListBuffer[ClassSymbol] classPath match { case cp: MergedClassPath[_] => @@ -899,7 +893,7 @@ class Global(var currentSettings: Settings, var reporter: Reporter) List() } } - val subst = Map(paths flatMap assoc: _*) + val subst = immutable.TreeMap(paths flatMap assoc: _*) if (subst.nonEmpty) { platform updateClassPath subst informProgress(s"classpath updated on entries [${subst.keys mkString ","}]") @@ -916,7 +910,6 @@ class Global(var currentSettings: Settings, var reporter: Reporter) informProgress(s"$msg: ${syms map (_.fullName) mkString ","}") show("invalidated packages", invalidated) show("could not invalidate system packages", failed) - (invalidated.toList, failed.toList) } /** Merges new classpath entries into the symbol table diff --git a/src/repl/scala/tools/nsc/interpreter/ILoop.scala b/src/repl/scala/tools/nsc/interpreter/ILoop.scala index 9cbb6ae574..c35de9a424 100644 --- a/src/repl/scala/tools/nsc/interpreter/ILoop.scala +++ b/src/repl/scala/tools/nsc/interpreter/ILoop.scala @@ -19,6 +19,7 @@ import scala.reflect.internal.util.{ BatchSourceFile, ScalaClassLoader } import ScalaClassLoader._ import scala.reflect.io.{ File, Directory } import scala.tools.util._ +import io.AbstractFile import scala.collection.generic.Clearable import scala.concurrent.{ ExecutionContext, Await, Future, future } import ExecutionContext.Implicits._ @@ -221,7 +222,7 @@ class ILoop(in0: Option[BufferedReader], protected val out: JPrintWriter) nullary("power", "enable power user mode", powerCmd), nullary("quit", "exit the interpreter", () => Result(keepRunning = false, None)), cmd("replay", "[options]", "reset the repl and replay all previous commands", replayCommand), - cmd("require", "", "add a jar or directory to the classpath", require), + cmd("require", "", "add a jar to the classpath", require), cmd("reset", "[options]", "reset the repl to its initial state, forgetting all session entries", resetCommand), cmd("save", "", "save replayable session to a file", saveCommand), shCommand, @@ -613,7 +614,8 @@ class ILoop(in0: Option[BufferedReader], protected val out: JPrintWriter) if (f.exists) { addedClasspath = ClassPath.join(addedClasspath, f.path) intp.addUrlsToClassPath(f.toURI.toURL) - echo("Added '%s'. Your new classpath is:\n\"%s\"".format(f.path, intp.global.classPath.asClasspathString)) + echo("Added '%s' to classpath.".format(f.path, intp.global.classPath.asClasspathString)) + repldbg("Added '%s'. Your new classpath is:\n\"%s\"".format(f.path, intp.global.classPath.asClasspathString)) } else echo("The path '" + f + "' doesn't seem to exist.") } @@ -647,21 +649,28 @@ class ILoop(in0: Option[BufferedReader], protected val out: JPrintWriter) echo("Adding directories to the classpath is not supported. Add a jar instead.") return } - val jarFile = new java.util.jar.JarFile(arg) - val entries = jarFile.entries + + val jarFile = AbstractFile.getDirectory(new java.io.File(arg)) + + def flatten(f: AbstractFile): Iterator[AbstractFile] = + if (f.isClassContainer) f.iterator.flatMap(flatten) + else Iterator(f) + + val entries = flatten(jarFile) val cloader = new InfoClassLoader var exists = false - while (entries.hasMoreElements()) { - val entry = entries.nextElement() + while (entries.hasNext) { + val entry = entries.next() // skip directories and manifests - if (!entry.isDirectory() && !entry.getName().endsWith("MF")) { + if (!entry.isDirectory && entry.name.endsWith(".class")) { // for each entry get InputStream - val is = jarFile.getInputStream(entry) + val is = entry.input // read InputStream into Array[Byte] val arr = readFully(is) val clazz = cloader.classOf(arr) try { + // pass initialize = false because we don't want to execute class initializers: Class.forName(clazz.getName(), false, intp.classLoader) exists = true } catch { @@ -673,7 +682,8 @@ class ILoop(in0: Option[BufferedReader], protected val out: JPrintWriter) if (f.exists && !exists) { addedClasspath = ClassPath.join(addedClasspath, f.path) intp.addUrlsToClassPath(f.toURI.toURL) - echo("Added '%s'. Your new classpath is:\n\"%s\"".format(f.path, intp.global.classPath.asClasspathString)) + echo("Added '%s' to classpath.".format(f.path, intp.global.classPath.asClasspathString)) + repldbg("Added '%s'. Your new classpath is:\n\"%s\"".format(f.path, intp.global.classPath.asClasspathString)) } else if (exists) { echo("The path '" + f + "' cannot be loaded, because existing classpath entries conflict.") } else echo("The path '" + f + "' doesn't seem to exist.") -- cgit v1.2.3 From f65c43094141169c76c189ad97bfca32f4508284 Mon Sep 17 00:00:00 2001 From: Heather Miller Date: Wed, 15 Oct 2014 17:03:02 -0700 Subject: SI-6502 Addresses @som-snytt's feedback --- src/repl/scala/tools/nsc/interpreter/IMain.scala | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) (limited to 'src') diff --git a/src/repl/scala/tools/nsc/interpreter/IMain.scala b/src/repl/scala/tools/nsc/interpreter/IMain.scala index 4f035bae7a..01b46541cd 100644 --- a/src/repl/scala/tools/nsc/interpreter/IMain.scala +++ b/src/repl/scala/tools/nsc/interpreter/IMain.scala @@ -86,8 +86,7 @@ class IMain(@BeanProperty val factory: ScriptEngineFactory, initialSettings: Set private var _classLoader: util.AbstractFileClassLoader = null // active classloader private val _compiler: ReplGlobal = newCompiler(settings, reporter) // our private compiler - private trait ExposeAddUrl extends URLClassLoader { def addNewUrl(url: URL) = this.addURL(url) } - private var _runtimeClassLoader: URLClassLoader with ExposeAddUrl = null // wrapper exposing addURL + private var _runtimeClassLoader: URLClassLoader = null // wrapper exposing addURL def compilerClasspath: Seq[java.net.URL] = ( if (isInitializeComplete) global.classPath.asURLs @@ -252,7 +251,7 @@ class IMain(@BeanProperty val factory: ScriptEngineFactory, initialSettings: Set */ def addUrlsToClassPath(urls: URL*): Unit = { new Run // force some initialization - urls.foreach(_runtimeClassLoader.addNewUrl) // Add jars to runtime classloader + urls.foreach(_runtimeClassLoader.addURL) // Add jars to runtime classloader extendCompilerClassPath(urls: _*) // Add jars to compile-time classpath } @@ -381,7 +380,7 @@ class IMain(@BeanProperty val factory: ScriptEngineFactory, initialSettings: Set } private def makeClassLoader(): util.AbstractFileClassLoader = new TranslatingClassLoader({ - _runtimeClassLoader = new URLClassLoader(compilerClasspath, parentClassLoader) with ExposeAddUrl + _runtimeClassLoader = new URLClassLoader(compilerClasspath, parentClassLoader) _runtimeClassLoader }) -- cgit v1.2.3 From 04ee526ca7f9d8f786b4b3e5a8d7209ceaa698ba Mon Sep 17 00:00:00 2001 From: Heather Miller Date: Thu, 16 Oct 2014 14:22:17 -0700 Subject: SI-6502 Addresses comments by @som-snytt --- src/repl/scala/tools/nsc/interpreter/ILoop.scala | 12 +++--------- 1 file changed, 3 insertions(+), 9 deletions(-) (limited to 'src') diff --git a/src/repl/scala/tools/nsc/interpreter/ILoop.scala b/src/repl/scala/tools/nsc/interpreter/ILoop.scala index c35de9a424..209e3f96bd 100644 --- a/src/repl/scala/tools/nsc/interpreter/ILoop.scala +++ b/src/repl/scala/tools/nsc/interpreter/ILoop.scala @@ -652,8 +652,8 @@ class ILoop(in0: Option[BufferedReader], protected val out: JPrintWriter) val jarFile = AbstractFile.getDirectory(new java.io.File(arg)) - def flatten(f: AbstractFile): Iterator[AbstractFile] = - if (f.isClassContainer) f.iterator.flatMap(flatten) + def flatten(f: AbstractFile): Iterator[AbstractFile] = + if (f.isClassContainer) f.iterator.flatMap(flatten) else Iterator(f) val entries = flatten(jarFile) @@ -669,13 +669,7 @@ class ILoop(in0: Option[BufferedReader], protected val out: JPrintWriter) // read InputStream into Array[Byte] val arr = readFully(is) val clazz = cloader.classOf(arr) - try { - // pass initialize = false because we don't want to execute class initializers: - Class.forName(clazz.getName(), false, intp.classLoader) - exists = true - } catch { - case _: ClassNotFoundException => /* do nothing */ - } + if ((intp.classLoader tryToLoadClass clazz.getName).isDefined) exists = true } } -- cgit v1.2.3 From be3eb58d0c49139538e69b377834991510447b36 Mon Sep 17 00:00:00 2001 From: Heather Miller Date: Fri, 24 Oct 2014 14:18:38 -0700 Subject: Addresses review comments --- src/compiler/scala/tools/nsc/Global.scala | 2 +- src/repl/scala/tools/nsc/interpreter/ILoop.scala | 35 ++++++------------------ 2 files changed, 9 insertions(+), 28 deletions(-) (limited to 'src') diff --git a/src/compiler/scala/tools/nsc/Global.scala b/src/compiler/scala/tools/nsc/Global.scala index 658aa66fd9..1635b2c404 100644 --- a/src/compiler/scala/tools/nsc/Global.scala +++ b/src/compiler/scala/tools/nsc/Global.scala @@ -845,7 +845,7 @@ class Global(var currentSettings: Settings, var reporter: Reporter) /** Is given package class a system package class that cannot be invalidated? */ - private def isSystemPackageClass(pkg: Symbol) = + private def isSystemPackageClass(pkg: Symbol) = pkg == RootClass || (pkg.hasTransOwner(definitions.ScalaPackageClass) && !pkg.hasTransOwner(this.rootMirror.staticPackage("scala.tools").moduleClass.asClass)) /** Invalidates packages that contain classes defined in a classpath entry, and diff --git a/src/repl/scala/tools/nsc/interpreter/ILoop.scala b/src/repl/scala/tools/nsc/interpreter/ILoop.scala index 209e3f96bd..fd2b2dbb5e 100644 --- a/src/repl/scala/tools/nsc/interpreter/ILoop.scala +++ b/src/repl/scala/tools/nsc/interpreter/ILoop.scala @@ -634,15 +634,6 @@ class ILoop(in0: Option[BufferedReader], protected val out: JPrintWriter) super.defineClass(null, arr, 0, arr.length) } - def readFully(is: InputStream): Array[Byte] = { - val dis = new java.io.DataInputStream(is) - val len = dis.available() - val arr = Array.ofDim[Byte](len) - dis.readFully(arr) - dis.close() - arr - } - val f = File(arg).normalize if (f.isDirectory) { @@ -658,29 +649,19 @@ class ILoop(in0: Option[BufferedReader], protected val out: JPrintWriter) val entries = flatten(jarFile) val cloader = new InfoClassLoader - var exists = false - - while (entries.hasNext) { - val entry = entries.next() - // skip directories and manifests - if (!entry.isDirectory && entry.name.endsWith(".class")) { - // for each entry get InputStream - val is = entry.input - // read InputStream into Array[Byte] - val arr = readFully(is) - val clazz = cloader.classOf(arr) - if ((intp.classLoader tryToLoadClass clazz.getName).isDefined) exists = true - } - } - if (f.exists && !exists) { + def classNameOf(classFile: AbstractFile): String = cloader.classOf(classFile.toByteArray).getName + def alreadyDefined(clsName: String) = intp.classLoader.tryToLoadClass(clsName).isDefined + val exists = entries.filter(_.hasExtension("class")).map(classNameOf).exists(alreadyDefined) + + if (!f.exists) echo(s"The path '$f' doesn't seem to exist.") + else if (exists) echo(s"The path '$f' cannot be loaded, because existing classpath entries conflict.") // TODO tell me which one + else { addedClasspath = ClassPath.join(addedClasspath, f.path) intp.addUrlsToClassPath(f.toURI.toURL) echo("Added '%s' to classpath.".format(f.path, intp.global.classPath.asClasspathString)) repldbg("Added '%s'. Your new classpath is:\n\"%s\"".format(f.path, intp.global.classPath.asClasspathString)) - } else if (exists) { - echo("The path '" + f + "' cannot be loaded, because existing classpath entries conflict.") - } else echo("The path '" + f + "' doesn't seem to exist.") + } } def powerCmd(): Result = { -- cgit v1.2.3 From 9e56c7a6fe73bce5962a3b121615c6a5bc1023d2 Mon Sep 17 00:00:00 2001 From: Heather Miller Date: Wed, 5 Nov 2014 11:53:20 -0800 Subject: SI-6502 Moving methods concerned with the state of Global from IMain to Global --- src/compiler/scala/tools/nsc/Global.scala | 35 ++++++++++++++++++++++++ src/repl/scala/tools/nsc/interpreter/IMain.scala | 34 +---------------------- 2 files changed, 36 insertions(+), 33 deletions(-) (limited to 'src') diff --git a/src/compiler/scala/tools/nsc/Global.scala b/src/compiler/scala/tools/nsc/Global.scala index 1635b2c404..3fb1bd10f0 100644 --- a/src/compiler/scala/tools/nsc/Global.scala +++ b/src/compiler/scala/tools/nsc/Global.scala @@ -8,6 +8,7 @@ package tools package nsc import java.io.{ File, FileOutputStream, PrintWriter, IOException, FileNotFoundException } +import java.net.URL import java.nio.charset.{ Charset, CharsetDecoder, IllegalCharsetNameException, UnsupportedCharsetException } import scala.compat.Platform.currentTime import scala.collection.{ mutable, immutable } @@ -841,6 +842,40 @@ class Global(var currentSettings: Settings, var reporter: Reporter) } reverse } + // ------------ REPL utilities --------------------------------- + + /** Extend classpath of `platform` and rescan updated packages. */ + def extendCompilerClassPath(urls: URL*): Unit = { + val newClassPath = mergeUrlsIntoClassPath(platform, urls: _*) + platform.currentClassPath = Some(newClassPath) + // Reload all specified jars into this compiler instance + invalidateClassPathEntries(urls.map(_.getPath): _*) + } + + /** Merge classpath of `platform` and `urls` into merged classpath */ + def mergeUrlsIntoClassPath(platform: JavaPlatform, urls: URL*): MergedClassPath[AbstractFile] = { + // Collect our new jars/directories and add them to the existing set of classpaths + val prevEntries = platform.classPath match { + case mcp: MergedClassPath[AbstractFile] => mcp.entries + case cp: ClassPath[AbstractFile] => List(cp) + } + val allEntries = (prevEntries ++ + urls.map(url => platform.classPath.context.newClassPath( + if (url.getProtocol == "file") { + val f = new File(url.getPath) + if (f.isDirectory) io.AbstractFile.getDirectory(f) + else io.AbstractFile.getFile(f) + } else { + io.AbstractFile.getURL(url) + } + ) + ) + ).distinct + + // Combine all of our classpaths (old and new) into one merged classpath + new MergedClassPath(allEntries, platform.classPath.context) + } + // ------------ Invalidations --------------------------------- /** Is given package class a system package class that cannot be invalidated? diff --git a/src/repl/scala/tools/nsc/interpreter/IMain.scala b/src/repl/scala/tools/nsc/interpreter/IMain.scala index 01b46541cd..b990e401ec 100644 --- a/src/repl/scala/tools/nsc/interpreter/IMain.scala +++ b/src/repl/scala/tools/nsc/interpreter/IMain.scala @@ -252,39 +252,7 @@ class IMain(@BeanProperty val factory: ScriptEngineFactory, initialSettings: Set def addUrlsToClassPath(urls: URL*): Unit = { new Run // force some initialization urls.foreach(_runtimeClassLoader.addURL) // Add jars to runtime classloader - extendCompilerClassPath(urls: _*) // Add jars to compile-time classpath - } - - /** Extend classpath of global.platform and force `global` to rescan updated packages. */ - protected def extendCompilerClassPath(urls: URL*): Unit = { - val newClassPath = mergeUrlsIntoClassPath(global.platform, urls: _*) - global.platform.currentClassPath = Some(newClassPath) - // Reload all specified jars into the current compiler instance (global) - global.invalidateClassPathEntries(urls.map(_.getPath): _*) - } - - /** Merge classpath of `platform` and `urls` into merged classpath */ - protected def mergeUrlsIntoClassPath(platform: JavaPlatform, urls: URL*): MergedClassPath[AbstractFile] = { - // Collect our new jars/directories and add them to the existing set of classpaths - val prevEntries = platform.classPath match { - case mcp: MergedClassPath[AbstractFile] => mcp.entries - case cp: ClassPath[AbstractFile] => List(cp) - } - val allEntries = (prevEntries ++ - urls.map(url => platform.classPath.context.newClassPath( - if (url.getProtocol == "file") { - val f = new File(url.getPath) - if (f.isDirectory) io.AbstractFile.getDirectory(f) - else io.AbstractFile.getFile(f) - } else { - io.AbstractFile.getURL(url) - } - ) - ) - ).distinct - - // Combine all of our classpaths (old and new) into one merged classpath - new MergedClassPath(allEntries, platform.classPath.context) + global.extendCompilerClassPath(urls: _*) // Add jars to compile-time classpath } /** Parent classloader. Overridable. */ -- cgit v1.2.3 From 24a2ef9728c403b81ffb36f2da241cff35defd78 Mon Sep 17 00:00:00 2001 From: Heather Miller Date: Mon, 10 Nov 2014 14:43:39 +0100 Subject: SI-6502 Refactorings suggested by review - Moves mergeUrlsIntoClassPath from Global into ClassPath - Revises and documents AbstractFile.getURL --- src/compiler/scala/tools/nsc/Global.scala | 26 +---------------------- src/compiler/scala/tools/nsc/util/ClassPath.scala | 19 ++++++++++++++++- src/reflect/scala/reflect/io/AbstractFile.scala | 16 ++++++++------ 3 files changed, 28 insertions(+), 33 deletions(-) (limited to 'src') diff --git a/src/compiler/scala/tools/nsc/Global.scala b/src/compiler/scala/tools/nsc/Global.scala index 3fb1bd10f0..430424d0f8 100644 --- a/src/compiler/scala/tools/nsc/Global.scala +++ b/src/compiler/scala/tools/nsc/Global.scala @@ -846,36 +846,12 @@ class Global(var currentSettings: Settings, var reporter: Reporter) /** Extend classpath of `platform` and rescan updated packages. */ def extendCompilerClassPath(urls: URL*): Unit = { - val newClassPath = mergeUrlsIntoClassPath(platform, urls: _*) + val newClassPath = platform.classPath.mergeUrlsIntoClassPath(urls: _*) platform.currentClassPath = Some(newClassPath) // Reload all specified jars into this compiler instance invalidateClassPathEntries(urls.map(_.getPath): _*) } - /** Merge classpath of `platform` and `urls` into merged classpath */ - def mergeUrlsIntoClassPath(platform: JavaPlatform, urls: URL*): MergedClassPath[AbstractFile] = { - // Collect our new jars/directories and add them to the existing set of classpaths - val prevEntries = platform.classPath match { - case mcp: MergedClassPath[AbstractFile] => mcp.entries - case cp: ClassPath[AbstractFile] => List(cp) - } - val allEntries = (prevEntries ++ - urls.map(url => platform.classPath.context.newClassPath( - if (url.getProtocol == "file") { - val f = new File(url.getPath) - if (f.isDirectory) io.AbstractFile.getDirectory(f) - else io.AbstractFile.getFile(f) - } else { - io.AbstractFile.getURL(url) - } - ) - ) - ).distinct - - // Combine all of our classpaths (old and new) into one merged classpath - new MergedClassPath(allEntries, platform.classPath.context) - } - // ------------ Invalidations --------------------------------- /** Is given package class a system package class that cannot be invalidated? diff --git a/src/compiler/scala/tools/nsc/util/ClassPath.scala b/src/compiler/scala/tools/nsc/util/ClassPath.scala index e89f08ec6b..e78dee5eee 100644 --- a/src/compiler/scala/tools/nsc/util/ClassPath.scala +++ b/src/compiler/scala/tools/nsc/util/ClassPath.scala @@ -197,6 +197,23 @@ abstract class ClassPath[T] { def packages: IndexedSeq[ClassPath[T]] def sourcepaths: IndexedSeq[AbstractFile] + /** The entries this classpath is composed of. In class `ClassPath` it's just the singleton list containing `this`. + * Subclasses such as `MergedClassPath` typically return lists with more elements. + */ + def entries: IndexedSeq[ClassPath[T]] = IndexedSeq(this) + + /** Merge classpath of `platform` and `urls` into merged classpath */ + def mergeUrlsIntoClassPath(urls: URL*): MergedClassPath[T] = { + // Collect our new jars/directories and add them to the existing set of classpaths + val allEntries = + (entries ++ + urls.map(url => context.newClassPath(io.AbstractFile.getURL(url))) + ).distinct + + // Combine all of our classpaths (old and new) into one merged classpath + new MergedClassPath(allEntries, context) + } + /** * Represents classes which can be loaded with a ClassfileLoader and/or SourcefileLoader. */ @@ -322,7 +339,7 @@ extends MergedClassPath[T](original.entries map (e => subst getOrElse (e, e)), o * A classpath unifying multiple class- and sourcepath entries. */ class MergedClassPath[T]( - val entries: IndexedSeq[ClassPath[T]], + override val entries: IndexedSeq[ClassPath[T]], val context: ClassPathContext[T]) extends ClassPath[T] { def this(entries: TraversableOnce[ClassPath[T]], context: ClassPathContext[T]) = diff --git a/src/reflect/scala/reflect/io/AbstractFile.scala b/src/reflect/scala/reflect/io/AbstractFile.scala index ac1159b2ac..bcefcc471f 100644 --- a/src/reflect/scala/reflect/io/AbstractFile.scala +++ b/src/reflect/scala/reflect/io/AbstractFile.scala @@ -48,14 +48,16 @@ object AbstractFile { else null /** - * If the specified URL exists and is a readable zip or jar archive, - * returns an abstract directory backed by it. Otherwise, returns - * `null`. + * If the specified URL exists and is a regular file or a directory, returns an + * abstract regular file or an abstract directory, respectively, backed by it. + * Otherwise, returns `null`. */ - def getURL(url: URL): AbstractFile = { - if (url == null || !Path.isExtensionJarOrZip(url.getPath)) null - else ZipArchive fromURL url - } + def getURL(url: URL): AbstractFile = + if (url.getProtocol == "file") { + val f = new java.io.File(url.getPath) + if (f.isDirectory) getDirectory(f) + else getFile(f) + } else null def getResources(url: URL): AbstractFile = ZipArchive fromManifestURL url } -- cgit v1.2.3