From 1de2900a67847a6718642062acf522b990af6aa2 Mon Sep 17 00:00:00 2001 From: Grzegorz Kossakowski Date: Wed, 4 Dec 2013 17:45:45 +0100 Subject: [PATCH] Add Logger and IncOptions as Incremental class constructor args Both Logger and IncOptions instances were passed around Incremental class implementation unmodified. Given the fact that entire implementation of the class uses exactly the same values for those types it makes sense to extract them as constructor arguments so they are accessible everywhere. This helps reducing signatures of other methods to more essential parameters that are more specific to given method. --- .../src/main/scala/sbt/inc/Incremental.scala | 80 +++++++++---------- 1 file changed, 40 insertions(+), 40 deletions(-) diff --git a/compile/inc/src/main/scala/sbt/inc/Incremental.scala b/compile/inc/src/main/scala/sbt/inc/Incremental.scala index 518e16538..a55021c87 100644 --- a/compile/inc/src/main/scala/sbt/inc/Incremental.scala +++ b/compile/inc/src/main/scala/sbt/inc/Incremental.scala @@ -21,17 +21,17 @@ object Incremental log: Logger, options: IncOptions)(implicit equivS: Equiv[Stamp]): (Boolean, Analysis) = { - val incremental = new Incremental - val initialChanges = incremental.changedInitial(entry, sources, previous, current, forEntry, options, log) + val incremental = new Incremental(log, options) + val initialChanges = incremental.changedInitial(entry, sources, previous, current, forEntry) val binaryChanges = new DependencyChanges { val modifiedBinaries = initialChanges.binaryDeps.toArray val modifiedClasses = initialChanges.external.allModified.toArray def isEmpty = modifiedBinaries.isEmpty && modifiedClasses.isEmpty } - val initialInv = incremental.invalidateInitial(previous.relations, initialChanges, log) + val initialInv = incremental.invalidateInitial(previous.relations, initialChanges) log.debug("All initially invalidated sources: " + initialInv + "\n") val analysis = manageClassfiles(options) { classfileManager => - incremental.cycle(initialInv, sources, binaryChanges, previous, doCompile, classfileManager, 1, log, options) + incremental.cycle(initialInv, sources, binaryChanges, previous, doCompile, classfileManager, 1) } (!initialInv.isEmpty, analysis) } @@ -62,7 +62,7 @@ object Incremental } -private class Incremental { +private class Incremental(log: Logger, options: IncOptions) { val incDebugProp = "xsbt.inc.debug" private def incDebug(options: IncOptions): Boolean = options.relationsDebug || java.lang.Boolean.getBoolean(incDebugProp) @@ -75,14 +75,14 @@ private class Incremental { // TODO: the Analysis for the last successful compilation should get returned + Boolean indicating success // TODO: full external name changes, scopeInvalidations @tailrec final def cycle(invalidatedRaw: Set[File], allSources: Set[File], binaryChanges: DependencyChanges, previous: Analysis, - doCompile: (Set[File], DependencyChanges) => Analysis, classfileManager: ClassfileManager, cycleNum: Int, log: Logger, options: IncOptions): Analysis = + doCompile: (Set[File], DependencyChanges) => Analysis, classfileManager: ClassfileManager, cycleNum: Int): Analysis = if(invalidatedRaw.isEmpty) previous else { def debug(s: => String) = if (incDebug(options)) log.debug(s) else () val withPackageObjects = invalidatedRaw ++ invalidatedPackageObjects(invalidatedRaw, previous.relations) - val invalidated = expand(withPackageObjects, allSources, log, options) + val invalidated = expand(withPackageObjects, allSources) val pruned = Incremental.prune(invalidated, previous, classfileManager) debug("********* Pruned: \n" + pruned.relations + "\n*********") @@ -92,18 +92,18 @@ private class 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 _, log, options) + val incChanges = changedIncremental(invalidated, previous.apis.internalAPI _, merged.apis.internalAPI _) debug("\nChanges:\n" + incChanges) val transitiveStep = options.transitiveStep - val incInv = invalidateIncremental(merged.relations, merged.apis, incChanges, invalidated, cycleNum >= transitiveStep, log) - cycle(incInv, allSources, emptyChanges, merged, doCompile, classfileManager, cycleNum+1, log, options) + val incInv = invalidateIncremental(merged.relations, merged.apis, incChanges, invalidated, cycleNum >= transitiveStep) + cycle(incInv, allSources, emptyChanges, merged, doCompile, classfileManager, cycleNum+1) } private[this] def emptyChanges: DependencyChanges = new DependencyChanges { val modifiedBinaries = new Array[File](0) val modifiedClasses = new Array[String](0) def isEmpty = true } - private[this] def expand(invalidated: Set[File], all: Set[File], log: Logger, options: IncOptions): Set[File] = { + private[this] def expand(invalidated: Set[File], all: Set[File]): Set[File] = { val recompileAllFraction = options.recompileAllFraction if(invalidated.size > all.size * recompileAllFraction) { log.debug("Recompiling all " + all.size + " sources: invalidated sources (" + invalidated.size + ") exceeded " + (recompileAllFraction*100.0) + "% of all sources") @@ -123,7 +123,7 @@ private class Incremental { * NOTE: This method creates a new APIDiff instance on every invocation. */ private def logApiChanges[T](apiChanges: Iterable[APIChange[T]], oldAPIMapping: T => Source, - newAPIMapping: T => Source, log: Logger, options: IncOptions): Unit = { + newAPIMapping: T => Source): Unit = { val contextSize = options.apiDiffContextSize try { val apiDiff = new APIDiff @@ -154,19 +154,19 @@ private class Incremental { * 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, log: Logger, options: IncOptions): APIChanges[T] = + def changedIncremental[T](lastSources: collection.Set[T], oldAPI: T => Source, newAPI: T => Source): APIChanges[T] = { val oldApis = lastSources.toSeq map oldAPI val newApis = lastSources.toSeq map newAPI - val apiChanges = (lastSources, oldApis, newApis).zipped.flatMap { (src, oldApi, newApi) => sameSource(src, oldApi, newApi, log, options) } + val apiChanges = (lastSources, oldApis, newApis).zipped.flatMap { (src, oldApi, newApi) => sameSource(src, oldApi, newApi) } if (Incremental.apiDebug(options) && apiChanges.nonEmpty) { - logApiChanges(apiChanges, oldAPI, newAPI, log, options) + logApiChanges(apiChanges, oldAPI, newAPI) } new APIChanges(apiChanges) } - def sameSource[T](src: T, a: Source, b: Source, log: Logger, options: IncOptions): Option[APIChange[T]] = { + def sameSource[T](src: T, a: Source, b: Source): Option[APIChange[T]] = { // Clients of a modified source file (ie, one that doesn't satisfy `shortcutSameSource`) containing macros must be recompiled. val hasMacro = a.hasMacro || b.hasMacro if (shortcutSameSource(a, b)) { @@ -174,11 +174,11 @@ private class Incremental { } else { if (hasMacro && options.recompileOnMacroDef) { Some(APIChangeDueToMacroDefinition(src)) - } else sameAPI(src, a, b, log) + } else sameAPI(src, a, b) } } - def sameAPI[T](src: T, a: Source, b: Source, log: Logger): Option[SourceAPIChange[T]] = { + def sameAPI[T](src: T, a: Source, b: Source): Option[SourceAPIChange[T]] = { if (SameAPI(a,b)) None else { @@ -193,15 +193,15 @@ private class Incremental { } def changedInitial(entry: String => Option[File], sources: Set[File], previousAnalysis: Analysis, current: ReadStamps, - forEntry: File => Option[Analysis], options: IncOptions, log: Logger)(implicit equivS: Equiv[Stamp]): InitialChanges = + forEntry: File => Option[Analysis])(implicit equivS: Equiv[Stamp]): InitialChanges = { val previous = previousAnalysis.stamps val previousAPIs = previousAnalysis.apis 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), log, options) + val binaryDepChanges = previous.allBinaries.filter( externalBinaryModified(entry, forEntry, previous, current)).toSet + val extChanges = changedIncremental(previousAPIs.allExternals, previousAPIs.externalAPI _, currentExternalAPI(entry, forEntry)) InitialChanges(srcChanges, removedProducts, binaryDepChanges, extChanges ) } @@ -215,14 +215,14 @@ private class Incremental { val (changed, unmodified) = inBoth.partition(existingModified) } - def invalidateIncremental(previous: Relations, apis: APIs, changes: APIChanges[File], recompiledSources: Set[File], transitive: Boolean, log: Logger): Set[File] = + def invalidateIncremental(previous: Relations, apis: APIs, changes: APIChanges[File], recompiledSources: Set[File], transitive: Boolean): Set[File] = { val dependsOnSrc = previous.usesInternalSrc _ val propagated = if(transitive) - transitiveDependencies(dependsOnSrc, changes.allModified.toSet, log) + transitiveDependencies(dependsOnSrc, changes.allModified.toSet) else - invalidateIntermediate(previous, changes, log) + invalidateIntermediate(previous, changes) val dups = invalidateDuplicates(previous) if(dups.nonEmpty) @@ -243,23 +243,23 @@ private class Incremental { /** Returns the transitive source dependencies of `initial`. * Because the intermediate steps do not pull in cycles, this result includes the initial files * if they are part of a cycle containing newly invalidated files . */ - def transitiveDependencies(dependsOnSrc: File => Set[File], initial: Set[File], log: Logger): Set[File] = + def transitiveDependencies(dependsOnSrc: File => Set[File], initial: Set[File]): Set[File] = { - val transitiveWithInitial = transitiveDeps(initial, log)(dependsOnSrc) - val transitivePartial = includeInitialCond(initial, transitiveWithInitial, dependsOnSrc, log) + val transitiveWithInitial = transitiveDeps(initial)(dependsOnSrc) + val transitivePartial = includeInitialCond(initial, transitiveWithInitial, dependsOnSrc) log.debug("Final step, transitive dependencies:\n\t" + transitivePartial) transitivePartial } /** Invalidates sources based on initially detected 'changes' to the sources, products, and dependencies.*/ - def invalidateInitial(previous: Relations, changes: InitialChanges, log: Logger): Set[File] = + def invalidateInitial(previous: Relations, changes: InitialChanges): Set[File] = { val srcChanges = changes.internalSrc val srcDirect = srcChanges.removed ++ srcChanges.removed.flatMap(previous.usesInternalSrc) ++ srcChanges.added ++ srcChanges.changed val byProduct = changes.removedProducts.flatMap(previous.produced) val byBinaryDep = changes.binaryDeps.flatMap(previous.usesBinary) val externalModifiedSources = changes.external.allModified.toSet - val byExtSrcDep = invalidateByExternal(previous, externalModifiedSources, log) //changes.external.modified.flatMap(previous.usesExternal) // ++ scopeInvalidations + val byExtSrcDep = invalidateByExternal(previous, externalModifiedSources) //changes.external.modified.flatMap(previous.usesExternal) // ++ scopeInvalidations checkAbsolute(srcChanges.added.toList) log.debug( "\nInitial source changes: \n\tremoved:" + srcChanges.removed + "\n\tadded: " + srcChanges.added + "\n\tmodified: " + srcChanges.changed + @@ -290,14 +290,14 @@ private class Incremental { } /** Sources invalidated by `external` sources in other projects according to the previous `relations`. */ - def invalidateByExternal(relations: Relations, external: Set[String], log: Logger): Set[File] = + def invalidateByExternal(relations: Relations, external: Set[String]): Set[File] = { // Propagate public inheritance dependencies transitively. // This differs from normal because we need the initial crossing from externals to sources in this project. val externalInheritedR = relations.publicInherited.external val byExternalInherited = external flatMap externalInheritedR.reverse val internalInheritedR = relations.publicInherited.internal - val transitiveInherited = transitiveDeps(byExternalInherited, log)(internalInheritedR.reverse _) + val transitiveInherited = transitiveDeps(byExternalInherited)(internalInheritedR.reverse _) // Get the direct dependencies of all sources transitively invalidated by inheritance val directA = transitiveInherited flatMap relations.direct.internal.reverse @@ -306,38 +306,38 @@ private class Incremental { transitiveInherited ++ directA ++ directB } /** Intermediate invalidation step: steps after the initial invalidation, but before the final transitive invalidation. */ - def invalidateIntermediate(relations: Relations, changes: APIChanges[File], log: Logger): Set[File] = + def invalidateIntermediate(relations: Relations, changes: APIChanges[File]): Set[File] = { def reverse(r: Relations.Source) = r.internal.reverse _ - invalidateSources(reverse(relations.direct), reverse(relations.publicInherited), changes, log) + invalidateSources(reverse(relations.direct), reverse(relations.publicInherited), changes) } /** Invalidates inheritance dependencies, transitively. Then, invalidates direct dependencies. Finally, excludes initial dependencies not * included in a cycle with newly invalidated sources. */ - private[this] def invalidateSources(directDeps: File => Set[File], publicInherited: File => Set[File], changes: APIChanges[File], log: Logger): Set[File] = + private[this] def invalidateSources(directDeps: File => Set[File], publicInherited: File => Set[File], changes: APIChanges[File]): Set[File] = { val initial = changes.allModified.toSet log.debug("Invalidating by inheritance (transitively)...") - val transitiveInherited = transitiveDeps(initial, log)(publicInherited) + val transitiveInherited = transitiveDeps(initial)(publicInherited) log.debug("Invalidated by transitive public inheritance: " + transitiveInherited) val direct = transitiveInherited flatMap directDeps log.debug("Invalidated by direct dependency: " + direct) val all = transitiveInherited ++ direct - includeInitialCond(initial, all, f => directDeps(f) ++ publicInherited(f), log) + includeInitialCond(initial, all, f => directDeps(f) ++ publicInherited(f)) } /** Conditionally include initial sources that are dependencies of newly invalidated sources. ** Initial sources included in this step can be because of a cycle, but not always. */ - private[this] def includeInitialCond(initial: Set[File], currentInvalidations: Set[File], allDeps: File => Set[File], log: Logger): Set[File] = + private[this] def includeInitialCond(initial: Set[File], currentInvalidations: Set[File], allDeps: File => Set[File]): Set[File] = { val newInv = currentInvalidations -- initial log.debug("New invalidations:\n\t" + newInv) - val transitiveOfNew = transitiveDeps(newInv, log)(allDeps) + val transitiveOfNew = transitiveDeps(newInv)(allDeps) val initialDependsOnNew = transitiveOfNew & initial log.debug("Previously invalidated, but (transitively) depend on new invalidations:\n\t" + initialDependsOnNew) newInv ++ initialDependsOnNew } - def externalBinaryModified(entry: String => Option[File], analysis: File => Option[Analysis], previous: Stamps, current: ReadStamps, log: Logger)(implicit equivS: Equiv[Stamp]): File => Boolean = + def externalBinaryModified(entry: String => Option[File], analysis: File => Option[Analysis], previous: Stamps, current: ReadStamps)(implicit equivS: Equiv[Stamp]): File => Boolean = dependsOn => { def inv(reason: String): Boolean = { @@ -389,7 +389,7 @@ private class Incremental { def orEmpty(o: Option[Source]): Source = o getOrElse APIs.emptySource def orTrue(o: Option[Boolean]): Boolean = o getOrElse true - private[this] def transitiveDeps[T](nodes: Iterable[T], log: Logger)(dependencies: T => Iterable[T]): Set[T] = + private[this] def transitiveDeps[T](nodes: Iterable[T])(dependencies: T => Iterable[T]): Set[T] = { val xs = new collection.mutable.HashSet[T] def all(from: T, tos: Iterable[T]): Unit = tos.foreach(to => visit(from, to))