From f62124dfb8ec4b834d7e7bf096b4f479d6b607eb Mon Sep 17 00:00:00 2001 From: Seth Tisue Date: Thu, 17 Feb 2022 10:56:43 -0800 Subject: [PATCH 1/6] add helpful commentary about checkRecompilations/checkDependencies --- .../scala/sbt/internal/inc/IncHandler.scala | 6 ++++ .../check-recompilations/test | 36 +++++++++++++++++++ .../class-based-inheritance/test | 3 ++ 3 files changed, 45 insertions(+) diff --git a/internal/zinc-scripted/src/test/scala/sbt/internal/inc/IncHandler.scala b/internal/zinc-scripted/src/test/scala/sbt/internal/inc/IncHandler.scala index 8ccfe31184..8f40d54450 100644 --- a/internal/zinc-scripted/src/test/scala/sbt/internal/inc/IncHandler.scala +++ b/internal/zinc-scripted/src/test/scala/sbt/internal/inc/IncHandler.scala @@ -208,12 +208,18 @@ class IncHandler(directory: Path, cacheDir: Path, scriptedLog: ManagedLogger, co new AnalyzingCompiler(instance, bridgeProvider, classpath, unit, IncHandler.classLoaderCache) } + // hopefully the meaning of the commands can be understood by looking at examples. + // the `check-recompilations` test is a good one for seeing how `checkDependencies` + // and `checkRecompilations` work. lazy val commands: Map[String, IncCommand] = Map( noArgs("compile") { case (p, i) => p.compile(i).map(_ => ()) }, noArgs("clean") { case (p, _) => p.clean() }, onArgs("checkIterations") { case (p, x :: Nil, i) => p.checkNumberOfCompilerIterations(i, x.toInt) }, + // note that this can only tell us the *last* round a class got compiled in. + // it can't tell us *every* round something got compiled in, since only + // still-extant classfiles are available for inspection onArgs("checkRecompilations") { case (p, step :: classNames, i) => p.checkRecompilations(i, step.toInt, classNames) }, diff --git a/zinc/src/sbt-test/source-dependencies/check-recompilations/test b/zinc/src/sbt-test/source-dependencies/check-recompilations/test index 60220ed86e..d5f414e7f3 100644 --- a/zinc/src/sbt-test/source-dependencies/check-recompilations/test +++ b/zinc/src/sbt-test/source-dependencies/check-recompilations/test @@ -1,9 +1,45 @@ +# this was the first checkRecompilations test ever written. it's a good +# one for understanding how checkDependencies and checkRecompilations +# are used. + +# the first compile command causes round 0 to happen + > compile +# every class gets compiled. + +> checkRecompilations 0 A B C D + +# now zinc knows the dependency relations between the classes: +# ┌─┐┌─┐ +# │D││B│ +# └┬┘└┬┘ +# ┌▽┐ │ +# │C│ │ +# └┬┘ │ +# ┌▽──▽┐ +# │A │ +# └────┘ + +> checkDependencies A: +> checkDependencies B: A +> checkDependencies C: A +> checkDependencies D: C + +# next, we swap in changed code + $ copy-file changes/A.scala A.scala +# the second compile command causes rounds 1, 2, and 3 to happen + > compile +# checkRecompilations only knows the *last* round in which something +# got compiled. it doesn't know *all* the rounds when something got +# compiled. so that's why `checkRecompilations 0` is now empty. +# after rounds 1, 2, and 3 have all finished, none of the classfiles +# from round 0 are still extant. + > checkRecompilations 0 > checkRecompilations 1 A > checkRecompilations 2 B C diff --git a/zinc/src/sbt-test/source-dependencies/class-based-inheritance/test b/zinc/src/sbt-test/source-dependencies/class-based-inheritance/test index 56f649bedd..53d5642504 100644 --- a/zinc/src/sbt-test/source-dependencies/class-based-inheritance/test +++ b/zinc/src/sbt-test/source-dependencies/class-based-inheritance/test @@ -8,9 +8,12 @@ # introduces first compile iteration > compile +> checkRecompilations 0 A A2 B C $ copy-file changes/A1.scala src/main/scala/A.scala # second iteration and third iteration (due to invalidation of C) > compile +> checkRecompilations 1 A A.AA A2 +> checkRecompilations 2 C $ copy-file changes/A2.scala src/main/scala/A.scala # fourth iteration > compile From e25bdd4f4337262544341a531b6e5bed9119d9b5 Mon Sep 17 00:00:00 2001 From: Seth Tisue Date: Mon, 14 Feb 2022 16:51:50 -0800 Subject: [PATCH 2/6] silence some warnings --- .../src/main/java/xsbti/compile/MultipleOutput.java | 1 + .../src/main/java/xsbti/compile/SingleOutput.java | 1 + 2 files changed, 2 insertions(+) diff --git a/internal/compiler-interface/src/main/java/xsbti/compile/MultipleOutput.java b/internal/compiler-interface/src/main/java/xsbti/compile/MultipleOutput.java index c51553feb6..10810520eb 100755 --- a/internal/compiler-interface/src/main/java/xsbti/compile/MultipleOutput.java +++ b/internal/compiler-interface/src/main/java/xsbti/compile/MultipleOutput.java @@ -30,6 +30,7 @@ public interface MultipleOutput extends Output { */ public OutputGroup[] getOutputGroups(); + @Deprecated @Override public default Optional getSingleOutput() { return Optional.empty(); diff --git a/internal/compiler-interface/src/main/java/xsbti/compile/SingleOutput.java b/internal/compiler-interface/src/main/java/xsbti/compile/SingleOutput.java index ad4e21187b..8844fae91c 100755 --- a/internal/compiler-interface/src/main/java/xsbti/compile/SingleOutput.java +++ b/internal/compiler-interface/src/main/java/xsbti/compile/SingleOutput.java @@ -51,6 +51,7 @@ public default Path getOutputDirectoryAsPath() { return getOutputDirectory().toPath(); } + @Deprecated @Override public default Optional getSingleOutput() { return Optional.of(getOutputDirectory()); From d890a914345cf4f37fe20d576abe71e6b2a30b21 Mon Sep 17 00:00:00 2001 From: Seth Tisue Date: Mon, 14 Feb 2022 19:53:37 -0800 Subject: [PATCH 3/6] update license notice --- .../src/main/scala/sbt/internal/inc/classfile/Parser.scala | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/internal/zinc-classfile/src/main/scala/sbt/internal/inc/classfile/Parser.scala b/internal/zinc-classfile/src/main/scala/sbt/internal/inc/classfile/Parser.scala index 6d17971889..6e8e68af5c 100644 --- a/internal/zinc-classfile/src/main/scala/sbt/internal/inc/classfile/Parser.scala +++ b/internal/zinc-classfile/src/main/scala/sbt/internal/inc/classfile/Parser.scala @@ -24,8 +24,8 @@ import sbt.internal.io.ErrorHandling import scala.annotation.switch import sbt.io.Using -// Translation of jdepend.framework.ClassFileParser by Mike Clark, Clarkware Consulting, Inc. -// BSD Licensed +// Loosely based on jdepend.framework.ClassFileParser by Mike Clark, Clarkware Consulting, Inc. +// (BSD license at time of initial port; MIT license as of 2022) // // Note that unlike the rest of sbt, some things might be null. From 525ffd82c00890691f3531abab74bed0c682e374 Mon Sep 17 00:00:00 2001 From: Seth Tisue Date: Mon, 14 Feb 2022 19:55:51 -0800 Subject: [PATCH 4/6] fix fragility in JavaCompilerForUnitTesting --- .../internal/inc/classfile/JavaCompilerForUnitTesting.scala | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/internal/zinc-classfile/src/test/scala/sbt/internal/inc/classfile/JavaCompilerForUnitTesting.scala b/internal/zinc-classfile/src/test/scala/sbt/internal/inc/classfile/JavaCompilerForUnitTesting.scala index 9ae7b0e8d6..8172c0ad10 100644 --- a/internal/zinc-classfile/src/test/scala/sbt/internal/inc/classfile/JavaCompilerForUnitTesting.scala +++ b/internal/zinc-classfile/src/test/scala/sbt/internal/inc/classfile/JavaCompilerForUnitTesting.scala @@ -109,7 +109,10 @@ object JavaCompilerForUnitTesting { private val extractParents: Seq[Class[_]] => Set[(String, String)] = { classes => def canonicalNames(p: (Class[_], Class[_])): (String, String) = p._1.getCanonicalName -> p._2.getCanonicalName - val parents = classes.map(c => c -> c.getSuperclass) + val parents = + classes + .map(c => c -> c.getSuperclass) + .filterNot(_._2 == null) // may be null for an interface val parentInterfaces = classes.flatMap(c => c.getInterfaces.map(i => c -> i)) (parents ++ parentInterfaces).map(canonicalNames).toSet } From f436193fb0cad8f5a10cbfba2e4ba62eba2291c4 Mon Sep 17 00:00:00 2001 From: Seth Tisue Date: Wed, 16 Feb 2022 19:30:45 -0800 Subject: [PATCH 5/6] add logging capability to classfile.Parser otherwise debugging scripted tests is hard, since printlns aren't buffered but logging is --- .../scala/sbt/internal/inc/ClassToAPI.scala | 3 ++- .../internal/inc/classfile/JavaAnalyze.scala | 2 +- .../sbt/internal/inc/classfile/Parser.scala | 20 ++++++++++--------- .../JavaCompilerForUnitTesting.scala | 1 + .../inc/classfile/ParserSpecification.scala | 7 ++++++- 5 files changed, 21 insertions(+), 12 deletions(-) diff --git a/internal/zinc-apiinfo/src/main/scala/sbt/internal/inc/ClassToAPI.scala b/internal/zinc-apiinfo/src/main/scala/sbt/internal/inc/ClassToAPI.scala index 16b517812d..31273a88b3 100644 --- a/internal/zinc-apiinfo/src/main/scala/sbt/internal/inc/ClassToAPI.scala +++ b/internal/zinc-apiinfo/src/main/scala/sbt/internal/inc/ClassToAPI.scala @@ -21,6 +21,7 @@ import xsbti.api import xsbti.api.SafeLazyProxy import collection.mutable import sbt.io.IO +import sbt.util.Logger object ClassToAPI { def apply(c: Seq[Class[_]]): Seq[api.ClassLike] = process(c)._1 @@ -219,7 +220,7 @@ object ClassToAPI { /** TODO: over time, ClassToAPI should switch the majority of access to the classfile parser */ private[this] def classFileForClass(c: Class[_]): ClassFile = - classfile.Parser.apply(IO.classfileLocation(c)) + classfile.Parser.apply(IO.classfileLocation(c), Logger.Null) @inline private[this] def lzyS[T <: AnyRef](t: T): xsbti.api.Lazy[T] = SafeLazyProxy.strict(t) @inline final def lzy[T <: AnyRef](t: => T): xsbti.api.Lazy[T] = SafeLazyProxy(t) diff --git a/internal/zinc-classfile/src/main/scala/sbt/internal/inc/classfile/JavaAnalyze.scala b/internal/zinc-classfile/src/main/scala/sbt/internal/inc/classfile/JavaAnalyze.scala index 1e8230e6c7..9da2d56b4c 100644 --- a/internal/zinc-classfile/src/main/scala/sbt/internal/inc/classfile/JavaAnalyze.scala +++ b/internal/zinc-classfile/src/main/scala/sbt/internal/inc/classfile/JavaAnalyze.scala @@ -82,7 +82,7 @@ private[sbt] object JavaAnalyze { // as class->class dependencies that must be mapped back to source->class dependencies using the source+class assignment for { newClass <- newClasses - classFile = Parser(newClass) + classFile = Parser(newClass, log) _ <- classFile.sourceFile orElse guessSourceName(newClass.getFileName.toString) source <- guessSourcePath(sourceMap, classFile, log) binaryClassName = classFile.className diff --git a/internal/zinc-classfile/src/main/scala/sbt/internal/inc/classfile/Parser.scala b/internal/zinc-classfile/src/main/scala/sbt/internal/inc/classfile/Parser.scala index 6e8e68af5c..fe59701b58 100644 --- a/internal/zinc-classfile/src/main/scala/sbt/internal/inc/classfile/Parser.scala +++ b/internal/zinc-classfile/src/main/scala/sbt/internal/inc/classfile/Parser.scala @@ -23,6 +23,7 @@ import sbt.internal.io.ErrorHandling import scala.annotation.switch import sbt.io.Using +import sbt.util.Logger // Loosely based on jdepend.framework.ClassFileParser by Mike Clark, Clarkware Consulting, Inc. // (BSD license at time of initial port; MIT license as of 2022) @@ -32,14 +33,15 @@ import sbt.io.Using import Constants._ private[sbt] object Parser { - def apply(file: Path): ClassFile = - Using.bufferedInputStream(Files.newInputStream(file))(parse(file.toString)).right.get - def apply(file: File): ClassFile = - Using.fileInputStream(file)(parse(file.toString)).right.get + def apply(file: Path, log: Logger): ClassFile = + Using.bufferedInputStream(Files.newInputStream(file))(parse(file.toString, log)).right.get - def apply(url: URL): ClassFile = - usingUrlInputStreamWithoutCaching(url)(parse(url.toString)).right.get + def apply(file: File, log: Logger): ClassFile = + Using.fileInputStream(file)(parse(file.toString, log)).right.get + + def apply(url: URL, log: Logger): ClassFile = + usingUrlInputStreamWithoutCaching(url)(parse(url.toString, log)).right.get // JarURLConnection with caching enabled will never close the jar private val usingUrlInputStreamWithoutCaching = Using.resource((u: URL) => @@ -50,9 +52,9 @@ private[sbt] object Parser { } ) - private def parse(readableName: String)(is: InputStream): Either[String, ClassFile] = - Right(parseImpl(readableName, is)) - private def parseImpl(readableName: String, is: InputStream): ClassFile = { + private def parse(readableName: String, log: Logger)(is: InputStream): Either[String, ClassFile] = + Right(parseImpl(readableName, log, is)) + private def parseImpl(readableName: String, log: Logger, is: InputStream): ClassFile = { val in = new DataInputStream(is) assume(in.readInt() == JavaMagic, "Invalid class file: " + readableName) diff --git a/internal/zinc-classfile/src/test/scala/sbt/internal/inc/classfile/JavaCompilerForUnitTesting.scala b/internal/zinc-classfile/src/test/scala/sbt/internal/inc/classfile/JavaCompilerForUnitTesting.scala index 8172c0ad10..2a70c5536e 100644 --- a/internal/zinc-classfile/src/test/scala/sbt/internal/inc/classfile/JavaCompilerForUnitTesting.scala +++ b/internal/zinc-classfile/src/test/scala/sbt/internal/inc/classfile/JavaCompilerForUnitTesting.scala @@ -81,6 +81,7 @@ object JavaCompilerForUnitTesting { val classloader = new URLClassLoader(Array(classesDir.toURI.toURL)) val logger = ConsoleLogger() + // logger.setLevel(sbt.util.Level.Debug) // we pass extractParents as readAPI. In fact, Analyze expect readAPI to do both things: // - extract api representation out of Class (and saved it via a side effect) diff --git a/internal/zinc-classfile/src/test/scala/sbt/internal/inc/classfile/ParserSpecification.scala b/internal/zinc-classfile/src/test/scala/sbt/internal/inc/classfile/ParserSpecification.scala index b23179034b..009ad7b560 100644 --- a/internal/zinc-classfile/src/test/scala/sbt/internal/inc/classfile/ParserSpecification.scala +++ b/internal/zinc-classfile/src/test/scala/sbt/internal/inc/classfile/ParserSpecification.scala @@ -14,12 +14,17 @@ package internal package inc package classfile +import sbt.internal.util.ConsoleLogger + import org.scalacheck._ import Prop._ object ParserSpecification extends Properties("Parser") { + + val log = ConsoleLogger() + property("able to parse all relevant classes") = Prop.forAll(classes) { (c: Class[_]) => - Parser(sbt.io.IO.classfileLocation(c)) ne null + Parser(sbt.io.IO.classfileLocation(c), log) ne null } implicit def classes: Gen[Class[_]] = From 5c8609315b9809d5b64b80866c473b68d8d12757 Mon Sep 17 00:00:00 2001 From: Seth Tisue Date: Tue, 15 Mar 2022 18:44:46 -0500 Subject: [PATCH 6/6] remove some old Scala 2.8/9 support code --- .../src/main/scala/xsbt/ExtractAPI.scala | 12 +----------- 1 file changed, 1 insertion(+), 11 deletions(-) diff --git a/internal/compiler-bridge/src/main/scala/xsbt/ExtractAPI.scala b/internal/compiler-bridge/src/main/scala/xsbt/ExtractAPI.scala index 4e0bf4cf6d..d18dccb6bb 100644 --- a/internal/compiler-bridge/src/main/scala/xsbt/ExtractAPI.scala +++ b/internal/compiler-bridge/src/main/scala/xsbt/ExtractAPI.scala @@ -340,7 +340,6 @@ class ExtractAPI[GlobalType <: Global]( makeParameter(simpleName(s), tp, tp.typeSymbol, s) } - // paramSym is only for 2.8 and is to determine if the parameter has a default def makeParameter( name: String, tpe: Type, @@ -692,9 +691,7 @@ class ExtractAPI[GlobalType <: Global]( private def tparamID(s: Symbol): String = existentialRenamings.renaming(s) match { case Some(rename) => - // can't use debuglog because it doesn't exist in Scala 2.9.x - if (settings.debug.value) - log("Renaming existential type variable " + s.fullName + " to " + rename) + debuglog(s"Renaming existential type variable ${s.fullName} to $rename") rename case None => s.fullName @@ -832,13 +829,6 @@ class ExtractAPI[GlobalType <: Global]( private def staticAnnotations(annotations: List[AnnotationInfo]): List[AnnotationInfo] = if (annotations == Nil) Nil else { - // compat stub for 2.8/2.9 - class IsStatic(ann: AnnotationInfo) { - def isStatic: Boolean = - ann.atp.typeSymbol isNonBottomSubClass definitions.StaticAnnotationClass - } - implicit def compat(ann: AnnotationInfo): IsStatic = new IsStatic(ann) - // `isStub` for scala/bug#11679: annotations of inherited members may be absent from the compile time // classpath so avoid calling `isNonBottomSubClass` on these stub symbols which would trigger an error. //