Remove resource layer for AllDependencyJars strategy

Changed resources were causing the dependency layer to be invalidated on
resource changes in turbo mode because the resource layer was in between
the scala library layer. This commit reworks the layers for the
AllDependencyJars strategy so that the top layer is able to load _all_
of the resources during a test run.

The resource layer was added to address the problem that dependencies
may need to be able to load resources from the project classpath but
wouldn't be able to do so if the dependencies were in a separate layer
from the rest of the classpath. The resource layer was a classloader
that could load any resource on the full classpath but couldn't load any
classes. When I added the resource layer, I was thinking that when
resources changed, the resource class loader needed to be invalidated.
Resources, however, are different from classes in that the same
ClassLoader can find the same resources in a different place because
getResource and getResourceAsStream just return locations but do not
actually do any loading.

Taking advantage of this, I add a proxy classloader for finding
resource locations to ReverseLookupClassLoader. We can reset the
classpath of the resource loader in
ReverseLookupClassLoaderHolder.checkout. This allows us to see the new
versions of the resources without invalidating the dependency layer.

main/src/main/java/sbt/internal/ResourceLoader.java

@@ -1,23 +0,0 @@
-/*
- * sbt
- * Copyright 2011 - 2018, Lightbend, Inc.
- * Copyright 2008 - 2010, Mark Harrah
- * Licensed under Apache License 2.0 (see LICENSE)
- */
-
-package sbt.internal;
-
-import java.io.File;
-import scala.collection.immutable.Map;
-import scala.collection.Seq;
-
-final class ResourceLoader extends ResourceLoaderImpl {
-  ResourceLoader(
-      final Seq<File> classpath, final ClassLoader parent, final Map<String, String> resources) {
-    super(classpath, parent, resources);
-  }
-
-  static {
-    ClassLoader.registerAsParallelCapable();
-  }
-}

main/src/main/scala/sbt/internal/ClassLoaders.scala

@@ -35,12 +35,10 @@ private[sbt] object ClassLoaders {
       if (si.isManagedVersion) rawCP
       else si.libraryJars.map(j => j -> IO.getModifiedTimeOrZero(j)).toSeq ++ rawCP
     val exclude = dependencyJars(exportedProducts).value.toSet ++ si.libraryJars
-    val resourceCP = modifiedTimes((outputFileStamps in resources).value)
     buildLayers(
       strategy = classLoaderLayeringStrategy.value,
       si = si,
       fullCP = fullCP,
-      resourceCP = resourceCP,
       allDependencies = dependencyJars(dependencyClasspath).value.filterNot(exclude),
       cache = extendedClassLoaderCache.value,
       resources = ClasspathUtilities.createClasspathResources(fullCP.map(_._1), si),
@@ -55,7 +53,6 @@ private[sbt] object ClassLoaders {
     val s = streams.value
     val opts = forkOptions.value
     val options = javaOptions.value
-    val resourceCP = modifiedTimes((outputFileStamps in resources).value)
     if (fork.value) {
       s.log.debug(s"javaOptions: $options")
       Def.task(new ForkRun(opts))
@@ -85,7 +82,6 @@ private[sbt] object ClassLoaders {
               strategy = classLoaderLayeringStrategy.value: @sbtUnchecked,
               si = instance,
               fullCP = classpath.map(f => f -> IO.getModifiedTimeOrZero(f)),
-              resourceCP = resourceCP,
               allDependencies = transformedDependencies,
               cache = extendedClassLoaderCache.value: @sbtUnchecked,
               resources = ClasspathUtilities.createClasspathResources(classpath, instance),
@@ -118,7 +114,6 @@ private[sbt] object ClassLoaders {
       strategy: ClassLoaderLayeringStrategy,
       si: ScalaInstance,
       fullCP: Seq[(File, Long)],
-      resourceCP: Seq[(File, Long)],
       allDependencies: Seq[File],
       cache: ClassLoaderCache,
       resources: Map[String, String],
@@ -156,34 +151,28 @@ private[sbt] object ClassLoaders {
           }
           .getOrElse(scalaLibraryLayer)
 
-        // layer 2 (resources)
+        // layer 2 (optional if in the test config and the runtime layer is not shared)
-        val resourceLayer =
+        val dependencyLayer: ClassLoader =
-          if (layerDependencies)
-            getResourceLayer(cpFiles, resourceCP, scalaReflectLayer, cache, resources)
-          else scalaReflectLayer
-
-        // layer 3 (optional if in the test config and the runtime layer is not shared)
-        val dependencyLayer =
           if (layerDependencies && allDependencies.nonEmpty) {
             cache(
               allDependencies.toList.map(f => f -> IO.getModifiedTimeOrZero(f)),
-              resourceLayer,
+              scalaReflectLayer,
-              () => new ReverseLookupClassLoaderHolder(allDependencies, resourceLayer)
+              () => new ReverseLookupClassLoaderHolder(allDependencies, scalaReflectLayer)
             )
-          } else resourceLayer
+          } else scalaReflectLayer
 
         val scalaJarNames = (si.libraryJars ++ scalaReflectJar).map(_.getName).toSet
-        // layer 4
+        // layer 3
         val filteredSet =
           if (layerDependencies) allDependencies.toSet ++ si.libraryJars ++ scalaReflectJar
           else Set(si.libraryJars ++ scalaReflectJar: _*)
         val dynamicClasspath = cpFiles.filterNot(f => filteredSet(f) || scalaJarNames(f.getName))
         dependencyLayer match {
           case dl: ReverseLookupClassLoaderHolder =>
-            dl.checkout(dynamicClasspath, tmp)
+            dl.checkout(cpFiles, tmp)
           case cl =>
             cl.getParent match {
-              case dl: ReverseLookupClassLoaderHolder => dl.checkout(dynamicClasspath, tmp)
+              case dl: ReverseLookupClassLoaderHolder => dl.checkout(cpFiles, tmp)
               case _                                  => new LayeredClassLoader(dynamicClasspath, cl, tmp)
             }
         }
@@ -194,24 +183,6 @@ private[sbt] object ClassLoaders {
       key: sbt.TaskKey[Seq[Attributed[File]]]
   ): Def.Initialize[Task[Seq[File]]] = Def.task(data(key.value).filter(_.getName.endsWith(".jar")))
 
-  // Creates a one or two layered classloader for the provided classpaths depending on whether
-  // or not the classpath contains any snapshots. If it does, the snapshots are placed in a layer
-  // above the regular jar layer. This allows the snapshot layer to be invalidated without
-  // invalidating the regular jar layer. If the classpath is empty, it just returns the parent
-  // loader.
-  private def getResourceLayer(
-      classpath: Seq[File],
-      resources: Seq[(File, Long)],
-      parent: ClassLoader,
-      cache: ClassLoaderCache,
-      resourceMap: Map[String, String]
-  ): ClassLoader = {
-    if (resources.nonEmpty) {
-      val mkLoader = () => new ResourceLoader(classpath, parent, resourceMap)
-      cache(resources.toList, parent, mkLoader)
-    } else parent
-  }
-
   // helper methods
   private def flatLoader(classpath: Seq[File], parent: ClassLoader): ClassLoader =
     new FlatLoader(classpath.map(_.toURI.toURL).toArray, parent)

main/src/main/scala/sbt/internal/LayeredClassLoaders.scala

@@ -8,7 +8,7 @@
 package sbt.internal
 
 import java.io.File
-import java.net.URLClassLoader
+import java.net.{ URL, URLClassLoader }
 import java.util.concurrent.ConcurrentHashMap
 import java.util.concurrent.atomic.{ AtomicBoolean, AtomicReference }
 
@@ -69,7 +69,7 @@ private[internal] final class ReverseLookupClassLoaderHolder(
    *
    * @return a ClassLoader
    */
-  def checkout(dependencyClasspath: Seq[File], tempDir: File): ClassLoader = {
+  def checkout(fullClasspath: Seq[File], tempDir: File): ClassLoader = {
     if (closed.get()) {
       val msg = "Tried to extract class loader from closed ReverseLookupClassLoaderHolder. " +
         "Try running the `clearCaches` command and re-trying."
@@ -79,8 +79,8 @@ private[internal] final class ReverseLookupClassLoaderHolder(
       case null => new ReverseLookupClassLoader
       case c    => c
     }
-    reverseLookupClassLoader.setTempDir(tempDir)
+    reverseLookupClassLoader.setup(tempDir, fullClasspath)
-    new BottomClassLoader(dependencyClasspath, reverseLookupClassLoader, tempDir)
+    new BottomClassLoader(fullClasspath, reverseLookupClassLoader, tempDir)
   }
 
   private def checkin(reverseLookupClassLoader: ReverseLookupClassLoader): Unit = {
@@ -149,6 +149,19 @@ private[internal] final class ReverseLookupClassLoaderHolder(
     private[this] val classLoadingLock = new ClassLoadingLock
     def isDirty: Boolean = dirty.get()
     def setDescendant(classLoader: BottomClassLoader): Unit = directDescendant.set(classLoader)
+    private[this] val resourceLoader = new AtomicReference[ResourceLoader](null)
+    private class ResourceLoader(cp: Seq[File])
+        extends URLClassLoader(cp.map(_.toURI.toURL).toArray, parent) {
+      def lookup(name: String): URL = findResource(name)
+    }
+    private[ReverseLookupClassLoaderHolder] def setup(tmpDir: File, cp: Seq[File]): Unit = {
+      setTempDir(tmpDir)
+      resourceLoader.set(new ResourceLoader(cp))
+    }
+    override def findResource(name: String): URL = resourceLoader.get() match {
+      case null => null
+      case l    => l.lookup(name)
+    }
     def loadClass(name: String, resolve: Boolean, reverseLookup: Boolean): Class[_] = {
       classLoadingLock.withLock(name) {
         try super.loadClass(name, resolve)