From 025eae9103774a3133a06e940b9fbb027d5bc9f3 Mon Sep 17 00:00:00 2001 From: Grzegorz Kossakowski Date: Tue, 25 Jun 2013 00:04:39 -0700 Subject: [PATCH] Log API diffs using ShowAPI and java-diff-utils library. Implement displaying API changes by using textual representation of an API (ShowAPI) and good, old textual diff algorithm. We are using java-diff-utils library that is distributed under Apache 2.0 license. Notice that we have only soft dependency on java-diff-utils. It means that we'll try to lookup java-diff-utils class through reflection and fail gracefully if none is found on the classpath. This way sbt is not getting any new dependency. If user needs to debug api diffs then it's matter of starting sbt with `-Dsbt.extraClasspath=path/to/diffutils.jar` option passed to sbt launcher. --- .../inc/src/main/scala/sbt/inc/APIDiff.scala | 67 +++++++++++++++++++ .../src/main/scala/sbt/inc/Incremental.scala | 39 +++++++++-- 2 files changed, 102 insertions(+), 4 deletions(-) create mode 100644 compile/inc/src/main/scala/sbt/inc/APIDiff.scala diff --git a/compile/inc/src/main/scala/sbt/inc/APIDiff.scala b/compile/inc/src/main/scala/sbt/inc/APIDiff.scala new file mode 100644 index 000000000..3f79faa00 --- /dev/null +++ b/compile/inc/src/main/scala/sbt/inc/APIDiff.scala @@ -0,0 +1,67 @@ +package sbt.inc + +import xsbti.api.SourceAPI +import xsbt.api.ShowAPI +import xsbt.api.DefaultShowAPI._ +import java.lang.reflect.Method +import java.util.{List => JList} + +/** + * A class which computes diffs (unified diffs) between two textual representations of an API. + * + * Internally, it uses java-diff-utils library but it calls it through reflection so there's + * no hard dependency on java-diff-utils. + * + * The reflective lookup of java-diff-utils library is performed in the constructor. Exceptions + * thrown by reflection are passed as-is to the caller of the constructor. + * + * @throws ClassNotFoundException if difflib.DiffUtils class cannot be located + * @throws LinkageError + * @throws ExceptionInInitializerError + */ +class APIDiff { + + import APIDiff._ + + private val diffUtilsClass = Class.forName(diffUtilsClassName) + // method signature: diff(List, List) + private val diffMethod: Method = + diffUtilsClass.getMethod(diffMethodName, classOf[JList[_]], classOf[JList[_]]) + + private val generateUnifiedDiffMethod: Method = { + val patchClass = Class.forName(patchClassName) + // method signature: generateUnifiedDiff(String, String, List, Patch, int) + diffUtilsClass.getMethod(generateUnifiedDiffMethodName, classOf[String], + classOf[String], classOf[JList[String]], patchClass, classOf[Int]) + } + + /** + * Generates an unified diff between textual representations of `api1` and `api2`. + */ + def generateApiDiff(fileName: String, api1: SourceAPI, api2: SourceAPI, contextSize: Int): String = { + val api1Str = ShowAPI.show(api1) + val api2Str = ShowAPI.show(api2) + generateApiDiff(fileName, api1Str, api2Str, contextSize) + } + + private def generateApiDiff(fileName: String, f1: String, f2: String, contextSize: Int): String = { + assert((diffMethod != null) && (generateUnifiedDiffMethod != null), "APIDiff isn't properly initialized.") + import scala.collection.JavaConverters._ + def asJavaList[T](it: Iterator[T]): java.util.List[T] = it.toSeq.asJava + val f1Lines = asJavaList(f1.lines) + val f2Lines = asJavaList(f2.lines) + //val diff = DiffUtils.diff(f1Lines, f2Lines) + val diff /*: Patch*/ = diffMethod.invoke(null, f1Lines, f2Lines) + val unifiedPatch: JList[String] = generateUnifiedDiffMethod.invoke(null, fileName, fileName, f1Lines, diff, + (contextSize: java.lang.Integer)).asInstanceOf[JList[String]] + unifiedPatch.asScala.mkString("\n") + } + +} + +object APIDiff { + private val diffUtilsClassName = "difflib.DiffUtils" + private val patchClassName = "difflib.Patch" + private val diffMethodName = "diff" + private val generateUnifiedDiffMethodName = "generateUnifiedDiff" +} diff --git a/compile/inc/src/main/scala/sbt/inc/Incremental.scala b/compile/inc/src/main/scala/sbt/inc/Incremental.scala index 88289a426..fa39550ab 100644 --- a/compile/inc/src/main/scala/sbt/inc/Incremental.scala +++ b/compile/inc/src/main/scala/sbt/inc/Incremental.scala @@ -76,7 +76,7 @@ object Incremental val merged = pruned ++ fresh//.copy(relations = pruned.relations ++ fresh.relations, apis = pruned.apis ++ fresh.apis) debug("********* Merged: \n" + merged.relations + "\n*********") - val incChanges = changedIncremental(invalidated, previous.apis.internalAPI _, merged.apis.internalAPI _, options) + val incChanges = changedIncremental(invalidated, previous.apis.internalAPI _, merged.apis.internalAPI _, log, options) debug("\nChanges:\n" + incChanges) val transitiveStep = options.transitiveStep val incInv = invalidateIncremental(merged.relations, incChanges, invalidated, cycleNum >= transitiveStep, log) @@ -101,17 +101,48 @@ object Incremental private[this] def invalidatedPackageObjects(invalidated: Set[File], relations: Relations): Set[File] = invalidated flatMap relations.publicInherited.internal.reverse filter { _.getName == "package.scala" } + /** + * Logs API changes using debug-level logging. The API are obtained using the APIDiff class. + * + * NOTE: This method creates a new APIDiff instance on every invocation. + */ + private def logApiChanges[T](changes: (collection.Set[T], Seq[Source], Seq[Source]), log: Logger, + options: IncOptions): Unit = { + val contextSize = 5 + try { + val apiDiff = new APIDiff + changes.zipped foreach { + case (src, oldApi, newApi) => + val apiUnifiedPatch = apiDiff.generateApiDiff(src.toString, oldApi.api, newApi.api, contextSize) + log.debug("Detected a change in a public API:\n" + apiUnifiedPatch) + } + } catch { + case e: ClassNotFoundException => + log.error("You have api debugging enabled but DiffUtils library cannot be found on sbt's classpath") + case e: LinkageError => + log.error("Encoutared linkage error while trying to load DiffUtils library.") + log.trace(e) + case e: Exception => + log.error("An exception has been thrown while trying to dump an api diff.") + log.trace(e) + } + } + /** * Accepts the sources that were recompiled during the last step and functions * providing the API before and after the last step. The functions should return * an empty API if the file did not/does not exist. */ - def changedIncremental[T](lastSources: collection.Set[T], oldAPI: T => Source, newAPI: T => Source, options: IncOptions): APIChanges[T] = + def changedIncremental[T](lastSources: collection.Set[T], oldAPI: T => Source, newAPI: T => Source, log: Logger, options: IncOptions): APIChanges[T] = { val oldApis = lastSources.toSeq map oldAPI val newApis = lastSources.toSeq map newAPI val changes = (lastSources, oldApis, newApis).zipped.filter { (src, oldApi, newApi) => !sameSource(oldApi, newApi) } + if (apiDebug(options) && changes.zipped.nonEmpty) { + logApiChanges(changes, log, options) + } + val changedNames = TopLevel.nameChanges(changes._3, changes._2 ) val modifiedAPIs = changes._1.toSet @@ -138,7 +169,7 @@ object Incremental val srcChanges = changes(previous.allInternalSources.toSet, sources, f => !equivS.equiv( previous.internalSource(f), current.internalSource(f) ) ) val removedProducts = previous.allProducts.filter( p => !equivS.equiv( previous.product(p), current.product(p) ) ).toSet val binaryDepChanges = previous.allBinaries.filter( externalBinaryModified(entry, forEntry, previous, current, log)).toSet - val extChanges = changedIncremental(previousAPIs.allExternals, previousAPIs.externalAPI _, currentExternalAPI(entry, forEntry), options) + val extChanges = changedIncremental(previousAPIs.allExternals, previousAPIs.externalAPI _, currentExternalAPI(entry, forEntry), log, options) InitialChanges(srcChanges, removedProducts, binaryDepChanges, extChanges ) } @@ -229,7 +260,7 @@ object Incremental /** Intermediate invalidation step: steps after the initial invalidation, but before the final transitive invalidation. */ def invalidateIntermediate(relations: Relations, modified: Set[File], log: Logger): Set[File] = { - def reverse(r: Relations.Source) = r.internal.reverse _ + def reverse(r: Relations.Source) = r.internal.reverse _ invalidateSources(reverse(relations.direct), reverse(relations.publicInherited), modified, log) } /** Invalidates inheritance dependencies, transitively. Then, invalidates direct dependencies. Finally, excludes initial dependencies not