From b6cdd60cf8f3b2dec832d51c11ed0213485211de Mon Sep 17 00:00:00 2001 From: Ethan Atkins Date: Sat, 25 May 2019 18:44:58 -0700 Subject: [PATCH] Simplify layering strategies The ShareRuntimeDependenciesLayerWithTestDependencies strategy doesn't really work with resources, so it makes sense to get rid of it. Without the share layer, there is no point in having separate RuntimeDependencies and TestDependencies layers so I consolidated them to Dependencies. If we really care about binary/source compatibility for the 1.3.0-RCx series, I can restore the traits and objects and set them private[sbt]. I think it was kind of a bug that they existed at all given the issue with resources so it makes sense to just remove them. --- .../sbt/ClassLoaderLayeringStrategy.scala | 32 ++--------- main/src/main/scala/sbt/Defaults.scala | 20 ++----- .../scala/sbt/internal/ClassLoaders.scala | 55 ++++--------------- .../classloader-cache/akka-actor-system/test | 2 +- .../classloader-cache/library-mismatch/test | 12 ---- .../resources/libraries/foo/build.sbt | 4 +- .../classloader-cache/runtime-layers/test | 16 +----- .../sbt-test/classloader-cache/scalatest/test | 6 +- 8 files changed, 26 insertions(+), 121 deletions(-) diff --git a/main/src/main/scala/sbt/ClassLoaderLayeringStrategy.scala b/main/src/main/scala/sbt/ClassLoaderLayeringStrategy.scala index b32eb44fd..483900728 100644 --- a/main/src/main/scala/sbt/ClassLoaderLayeringStrategy.scala +++ b/main/src/main/scala/sbt/ClassLoaderLayeringStrategy.scala @@ -89,36 +89,12 @@ object ClassLoaderLayeringStrategy { case object ScalaLibrary extends ScalaLibrary /** - * Add a layer on top of the ScalaLibrary layer for the runtime jar dependencies. + * Add a layer on top of the ScalaLibrary layer for all of the task jar dependencies. */ - sealed trait RuntimeDependencies extends ScalaLibrary + sealed trait AllLibraryJars extends ScalaLibrary /** - * Add a layer on top of the ScalaLibrary layer for the runtime jar dependencies. + * Add a layer on top of the ScalaLibrary layer for all of the jar dependencies. */ - case object RuntimeDependencies extends ScalaLibrary with RuntimeDependencies - - /** - * Add a layer on top of the ScalaLibrary layer for the test jar dependencies. - */ - sealed trait TestDependencies extends ScalaLibrary - - /** - * Add a layer on top of the ScalaInstance layer for the test jar dependencies. - */ - case object TestDependencies extends ScalaLibrary with TestDependencies - - /** - * Add the TestDependencies layer on top of the RuntimeDependencies layer on top of the - * ScalaLibrary layer. This differs from TestDependencies in that it will not reload the - * runtime classpath. The drawback to using this is that if the test dependencies evict - * classes provided in the runtime layer, then tests can fail. In order for sharing the runtime - * layer to work, it is necessary to set [[Keys.bgCopyClasspath]] to false. Otherwise the - * runtime and test classpaths are completely different. - */ - case object ShareRuntimeDependenciesLayerWithTestDependencies - extends ScalaLibrary - with RuntimeDependencies - with TestDependencies - + object AllLibraryJars extends AllLibraryJars } diff --git a/main/src/main/scala/sbt/Defaults.scala b/main/src/main/scala/sbt/Defaults.scala index 4f83c85f3..def9c4045 100755 --- a/main/src/main/scala/sbt/Defaults.scala +++ b/main/src/main/scala/sbt/Defaults.scala @@ -168,8 +168,7 @@ object Defaults extends BuildCommon { private[sbt] lazy val globalJvmCore: Seq[Setting[_]] = Seq( compilerCache := state.value get Keys.stateCompilerCache getOrElse CompilerCache.fresh, - classLoaderLayeringStrategy :== ClassLoaderLayeringStrategy.RuntimeDependencies, - classLoaderLayeringStrategy in Test :== ClassLoaderLayeringStrategy.TestDependencies, + classLoaderLayeringStrategy :== ClassLoaderLayeringStrategy.AllLibraryJars, sourcesInBase :== true, autoAPIMappings := false, apiMappings := Map.empty, @@ -1059,7 +1058,7 @@ object Defaults extends BuildCommon { cp, forkedParallelExecution = false, javaOptions = Nil, - strategy = ClassLoaderLayeringStrategy.TestDependencies, + strategy = ClassLoaderLayeringStrategy.AllLibraryJars, projectId = "", ) } @@ -1082,7 +1081,7 @@ object Defaults extends BuildCommon { cp, forkedParallelExecution, javaOptions = Nil, - strategy = ClassLoaderLayeringStrategy.TestDependencies, + strategy = ClassLoaderLayeringStrategy.AllLibraryJars, projectId = "", ) } @@ -1863,18 +1862,7 @@ object Defaults extends BuildCommon { Classpaths.compilerPluginConfig ++ deprecationSettings lazy val compileSettings: Seq[Setting[_]] = - configSettings ++ (mainBgRunMainTask +: mainBgRunTask) ++ - Classpaths.addUnmanagedLibrary ++ - Vector( - bgCopyClasspath in bgRun := { - val old = (bgCopyClasspath in bgRun).value - old && (Test / classLoaderLayeringStrategy).value != ClassLoaderLayeringStrategy.ShareRuntimeDependenciesLayerWithTestDependencies - }, - bgCopyClasspath in bgRunMain := { - val old = (bgCopyClasspath in bgRunMain).value - old && (Test / classLoaderLayeringStrategy).value != ClassLoaderLayeringStrategy.ShareRuntimeDependenciesLayerWithTestDependencies - }, - ) + configSettings ++ (mainBgRunMainTask +: mainBgRunTask) ++ Classpaths.addUnmanagedLibrary lazy val testSettings: Seq[Setting[_]] = configSettings ++ testTasks diff --git a/main/src/main/scala/sbt/internal/ClassLoaders.scala b/main/src/main/scala/sbt/internal/ClassLoaders.scala index 67963eb80..bf7039566 100644 --- a/main/src/main/scala/sbt/internal/ClassLoaders.scala +++ b/main/src/main/scala/sbt/internal/ClassLoaders.scala @@ -13,14 +13,12 @@ import java.net.URLClassLoader import sbt.ClassLoaderLayeringStrategy._ import sbt.Keys._ -import sbt.SlashSyntax0._ import sbt.internal.classpath.ClassLoaderCache import sbt.internal.inc.ScalaInstance import sbt.internal.inc.classpath.ClasspathUtilities import sbt.internal.util.Attributed import sbt.internal.util.Attributed.data import sbt.io.IO -import sbt.librarymanagement.Configurations.Runtime private[sbt] object ClassLoaders { private[this] val interfaceLoader = classOf[sbt.testing.Framework].getClassLoader @@ -36,8 +34,6 @@ private[sbt] object ClassLoaders { strategy = classLoaderLayeringStrategy.value, si = si, fullCP = fullCP, - rawRuntimeDependencies = - dependencyJars(Runtime / dependencyClasspath).value.filterNot(exclude), allDependencies = dependencyJars(dependencyClasspath).value.filterNot(exclude), cache = extendedClassLoaderCache.value, resources = ClasspathUtilities.createClasspathResources(fullCP, si), @@ -72,7 +68,6 @@ private[sbt] object ClassLoaders { s.log.warn(s"$showJavaOptions will be ignored, $showFork is set to false") } val exclude = dependencyJars(exportedProducts).value.toSet ++ instance.allJars - val runtimeDeps = dependencyJars(Runtime / dependencyClasspath).value.filterNot(exclude) val allDeps = dependencyJars(dependencyClasspath).value.filterNot(exclude) val newLoader = (classpath: Seq[File]) => { @@ -80,7 +75,6 @@ private[sbt] object ClassLoaders { strategy = classLoaderLayeringStrategy.value: @sbtUnchecked, si = instance, fullCP = classpath, - rawRuntimeDependencies = runtimeDeps, allDependencies = allDeps, cache = extendedClassLoaderCache.value: @sbtUnchecked, resources = ClasspathUtilities.createClasspathResources(classpath, instance), @@ -103,10 +97,9 @@ private[sbt] object ClassLoaders { * Create a layered classloader. There are up to five layers: * 1) the scala instance class loader * 2) the resource layer - * 3) the runtime dependencies - * 4) the test dependencies - * 5) the rest of the classpath - * The first two layers may be optionally cached to reduce memory usage and improve + * 3) the dependency jars + * 4) the rest of the classpath + * The first three layers may be optionally cached to reduce memory usage and improve * start up latency. Because there may be mutually incompatible libraries in the runtime * and test dependencies, it's important to be able to configure which layers are used. */ @@ -114,36 +107,20 @@ private[sbt] object ClassLoaders { strategy: ClassLoaderLayeringStrategy, si: ScalaInstance, fullCP: Seq[File], - rawRuntimeDependencies: Seq[File], allDependencies: Seq[File], cache: ClassLoaderCache, resources: Map[String, String], tmp: File, scope: Scope ): ClassLoader = { - val isTest = scope.config.toOption.map(_.name) == Option("test") val raw = strategy match { case Flat => flatLoader(fullCP, interfaceLoader) case _ => - val (layerDependencies, layerTestDependencies) = strategy match { - case ShareRuntimeDependenciesLayerWithTestDependencies if isTest => (true, true) - case ScalaLibrary => (false, false) - case RuntimeDependencies => (true, false) - case TestDependencies if isTest => (false, true) - case badStrategy => - val msg = s"Layering strategy $badStrategy is not valid for the classloader in " + - s"$scope. Valid options are: ClassLoaderLayeringStrategy.{ " + - "Flat, ScalaInstance, RuntimeDependencies }" - throw new IllegalArgumentException(msg) + val layerDependencies = strategy match { + case _: AllLibraryJars => true + case _ => false } val allDependenciesSet = allDependencies.toSet - // The raw declarations are to avoid having to make a dynamic task. The - // allDependencies and allTestDependencies create a mutually exclusive list of jar - // dependencies for layers 2 and 3. Note that in the Runtime or Compile configs, it - // should always be the case that allTestDependencies == Nil - val allTestDependencies = if (layerTestDependencies) allDependenciesSet else Set.empty[File] - val allRuntimeDependencies = (if (layerDependencies) rawRuntimeDependencies else Nil).toSet - val scalaLibraryLayer = layer(si.libraryJar :: Nil, interfaceLoader, cache, resources, tmp) // layer 2 (resources) @@ -152,24 +129,16 @@ private[sbt] object ClassLoaders { else scalaLibraryLayer // layer 3 (optional if in the test config and the runtime layer is not shared) - val runtimeDependencySet = allDependenciesSet intersect allRuntimeDependencies - val runtimeDependencies = rawRuntimeDependencies.filter(runtimeDependencySet) - lazy val runtimeLayer = - if (layerDependencies) - layer(runtimeDependencies, resourceLayer, cache, resources, tmp) + val dependencyLayer = + if (layerDependencies) layer(allDependencies, resourceLayer, cache, resources, tmp) else resourceLayer - // layer 4 (optional if testDependencies are empty) - val testDependencySet = allTestDependencies diff runtimeDependencySet - val testDependencies = allDependencies.filter(testDependencySet) - val testLayer = layer(testDependencies, runtimeLayer, cache, resources, tmp) - - // layer 5 + // layer 4 val dynamicClasspath = - fullCP.filterNot(testDependencySet ++ runtimeDependencies + si.libraryJar) + fullCP.filterNot(allDependenciesSet + si.libraryJar) if (dynamicClasspath.nonEmpty) - new LayeredClassLoader(dynamicClasspath, testLayer, resources, tmp) - else testLayer + new LayeredClassLoader(dynamicClasspath, dependencyLayer, resources, tmp) + else dependencyLayer } ClasspathUtilities.filterByClasspath(fullCP, raw) } diff --git a/sbt/src/sbt-test/classloader-cache/akka-actor-system/test b/sbt/src/sbt-test/classloader-cache/akka-actor-system/test index abcb2b542..b14110376 100644 --- a/sbt/src/sbt-test/classloader-cache/akka-actor-system/test +++ b/sbt/src/sbt-test/classloader-cache/akka-actor-system/test @@ -1,4 +1,4 @@ -> set Test / classLoaderLayeringStrategy := ClassLoaderLayeringStrategy.ShareRuntimeDependenciesLayerWithTestDependencies +> run > run diff --git a/sbt/src/sbt-test/classloader-cache/library-mismatch/test b/sbt/src/sbt-test/classloader-cache/library-mismatch/test index abdaa10fa..d737a4d50 100644 --- a/sbt/src/sbt-test/classloader-cache/library-mismatch/test +++ b/sbt/src/sbt-test/classloader-cache/library-mismatch/test @@ -1,15 +1,3 @@ -> set Test / classLoaderLayeringStrategy := ClassLoaderLayeringStrategy.ShareRuntimeDependenciesLayerWithTestDependencies - -> run - -# This fails because the runtime layer includes an old version of the foo-lib library that doesn't -# have the sbt.foo.Foo.y method defined. -> test - -> set Test / classLoaderLayeringStrategy := ClassLoaderLayeringStrategy.TestDependencies - > run > test - -> test diff --git a/sbt/src/sbt-test/classloader-cache/resources/libraries/foo/build.sbt b/sbt/src/sbt-test/classloader-cache/resources/libraries/foo/build.sbt index b73c67d27..860c958ab 100644 --- a/sbt/src/sbt-test/classloader-cache/resources/libraries/foo/build.sbt +++ b/sbt/src/sbt-test/classloader-cache/resources/libraries/foo/build.sbt @@ -6,6 +6,6 @@ publishTo := Some(Resolver.file("test-resolver", file("").getCanonicalFile / "iv version := "0.1.0" -classLoaderLayeringStrategy := ClassLoaderLayeringStrategy.Dependencies +classLoaderLayeringStrategy := ClassLoaderLayeringStrategy.AllLibraryJars -Test / classLoaderLayeringStrategy := ClassLoaderLayeringStrategy.Dependencies +Test / classLoaderLayeringStrategy := ClassLoaderLayeringStrategy.AllLibraryJars diff --git a/sbt/src/sbt-test/classloader-cache/runtime-layers/test b/sbt/src/sbt-test/classloader-cache/runtime-layers/test index 8ba3debaa..c6234883b 100644 --- a/sbt/src/sbt-test/classloader-cache/runtime-layers/test +++ b/sbt/src/sbt-test/classloader-cache/runtime-layers/test @@ -8,14 +8,10 @@ > run -> set Compile / classLoaderLayeringStrategy := ClassLoaderLayeringStrategy.RuntimeDependencies +> set Compile / classLoaderLayeringStrategy := ClassLoaderLayeringStrategy.AllLibraryJars > run -> set Compile / classLoaderLayeringStrategy := ClassLoaderLayeringStrategy.TestDependencies - --> run - > set Test / classLoaderLayeringStrategy := ClassLoaderLayeringStrategy.Flat > Test / runMain sbt.scripted.TestAkkaTest @@ -24,14 +20,6 @@ > Test / runMain sbt.scripted.TestAkkaTest -> set Test / classLoaderLayeringStrategy := ClassLoaderLayeringStrategy.RuntimeDependencies - -> Test / runMain sbt.scripted.TestAkkaTest - -> set Test / classLoaderLayeringStrategy := ClassLoaderLayeringStrategy.ShareRuntimeDependenciesLayerWithTestDependencies - -> Test / runMain sbt.scripted.TestAkkaTest - -> set Test / classLoaderLayeringStrategy := ClassLoaderLayeringStrategy.TestDependencies +> set Test / classLoaderLayeringStrategy := ClassLoaderLayeringStrategy.AllLibraryJars > Test / runMain sbt.scripted.TestAkkaTest diff --git a/sbt/src/sbt-test/classloader-cache/scalatest/test b/sbt/src/sbt-test/classloader-cache/scalatest/test index 2fb0d98cd..9b34bf4a6 100644 --- a/sbt/src/sbt-test/classloader-cache/scalatest/test +++ b/sbt/src/sbt-test/classloader-cache/scalatest/test @@ -1,9 +1,5 @@ > test -> set Test / classLoaderLayeringStrategy := ClassLoaderLayeringStrategy.ShareRuntimeDependenciesLayerWithTestDependencies - -> test - > set Test / classLoaderLayeringStrategy := ClassLoaderLayeringStrategy.Flat > test @@ -12,7 +8,7 @@ > test -> set Test / classLoaderLayeringStrategy := ClassLoaderLayeringStrategy.TestDependencies +> set Test / classLoaderLayeringStrategy := ClassLoaderLayeringStrategy.AllLibraryJars $ copy-file changes/bad.scala src/test/scala/sbt/ScalatestTest.scala