Restore class files after an unsuccessful compilation.

This is useful when an error occurs in a later incremental step that
requires a fix in the originally changed files.

CC @gkossakowski

compile/inc/src/main/scala/sbt/inc/ClassfileManager.scala

@@ -0,0 +1,65 @@
+package sbt.inc
+
+	import sbt.IO
+	import java.io.File
+	import collection.mutable
+
+/** During an incremental compilation run, a ClassfileManager deletes class files and is notified of generated class files.
+* A ClassfileManager can be used only once.*/
+trait ClassfileManager
+{
+	/** Called once per compilation step with the class files to delete prior to that step's compilation.
+	* The files in `classes` must not exist if this method returns normally.
+	* Any empty ancestor directories of deleted files must not exist either.*/
+	def delete(classes: Iterable[File]): Unit
+
+	/** Called once per compilation step with the class files generated during that step.*/	
+	def generated(classes: Iterable[File]): Unit
+
+	/** Called once at the end of the whole compilation run, with `success` indicating whether compilation succeeded (true) or not (false).*/
+	def complete(success: Boolean): Unit
+}
+
+object ClassfileManager
+{
+	/** Constructs a minimal ClassfileManager implementation that immediately deletes class files when requested. */
+	val deleteImmediately: () => ClassfileManager = () => new ClassfileManager
+	{
+		def delete(classes: Iterable[File]): Unit = IO.deleteFilesEmptyDirs(classes)
+		def generated(classes: Iterable[File]) {}
+		def complete(success: Boolean) {}
+	}
+	/** When compilation fails, this ClassfileManager restores class files to the way they were before compilation.*/
+	def transactional(tempDir0: File): () => ClassfileManager = () => new ClassfileManager
+	{
+		val tempDir = tempDir0.getCanonicalFile
+		IO.delete(tempDir)
+		IO.createDirectory(tempDir)
+
+		private[this] val generatedClasses = new mutable.HashSet[File]
+		private[this] val movedClasses = new mutable.HashMap[File, File]
+	
+		def delete(classes: Iterable[File])
+		{
+			for(c <- classes) if(c.exists && !movedClasses.contains(c) && !generatedClasses(c))
+				movedClasses.put(c, move(c))
+			IO.deleteFilesEmptyDirs(classes)
+		}
+		def generated(classes: Iterable[File]): Unit = generatedClasses ++= classes
+		def complete(success: Boolean)
+		{
+			if(!success) {
+				IO.deleteFilesEmptyDirs(generatedClasses)
+				for( (orig, tmp) <- movedClasses ) IO.move(tmp, orig)
+			}
+			IO.delete(tempDir)
+		}
+
+		def move(c: File): File =
+		{
+			val target = File.createTempFile("sbt", ".class", tempDir)
+			IO.move(c, target)
+			target
+		}
+	}
+}

compile/inc/src/main/scala/sbt/inc/IncOptions.scala

@@ -1,37 +1,40 @@
 package sbt.inc
 
+	import java.io.File
+
 /**
- * Case class that represents all configuration options for incremental compiler.
- *
- * Those are options that configure incremental compiler itself and not underlying
- * Java/Scala compiler.
+ * Represents all configuration options for the incremental compiler itself and
+ * not the underlying Java/Scala compiler.
  */
-case class IncOptions(
-    /** After which step include whole transitive closure of invalidated source files. */
-    val transitiveStep: Int,
-    /**
-     * What's the fraction of invalidated source files when we switch to recompiling
-     * all files and giving up incremental compilation altogether. That's useful in
-     * cases when probability that we end up recompiling most of source files but
-     * in multiple steps is high. Multi-step incremental recompilation is slower
-     * than recompiling everything in one step.
-     */
-    val recompileAllFraction: Double,
-    /** Print very detail information about relations (like dependencies between source files). */
-    val relationsDebug: Boolean,
-    /**
-     * Enable tools for debugging API changes. At the moment that option is unused but in the
-     * future it will enable for example:
-     *   - disabling API hashing and API minimization (potentially very memory consuming)
-     *   - dumping textual API representation into files
-     */
-    val apiDebug: Boolean,
-    /**
-     * The directory where we dump textual representation of APIs. This method might be called
-     * only if apiDebug returns true. This is unused option at the moment as the needed functionality
-     * is not implemented yet.
-     */
-    val apiDumpDirectory: Option[java.io.File])
+final case class IncOptions(
+	/** After which step include whole transitive closure of invalidated source files. */
+	transitiveStep: Int,
+	/**
+	* What's the fraction of invalidated source files when we switch to recompiling
+	* all files and giving up incremental compilation altogether. That's useful in
+	* cases when probability that we end up recompiling most of source files but
+	* in multiple steps is high. Multi-step incremental recompilation is slower
+	* than recompiling everything in one step.
+	*/
+	recompileAllFraction: Double,
+	/** Print very detailed information about relations, such as dependencies between source files. */
+	relationsDebug: Boolean,
+	/**
+	* Enable tools for debugging API changes. At the moment this option is unused but in the
+	* future it will enable for example:
+	*   - disabling API hashing and API minimization (potentially very memory consuming)
+	*   - dumping textual API representation into files
+	*/
+	apiDebug: Boolean,
+	/**
+	* The directory where we dump textual representation of APIs. This method might be called
+	* only if apiDebug returns true. This is unused option at the moment as the needed functionality
+	* is not implemented yet.
+	*/
+	apiDumpDirectory: Option[java.io.File],
+	/** Creates a new ClassfileManager that will handle class file deletion and addition during a single incremental compilation run. */
+	newClassfileManager: () => ClassfileManager
+)
 
 object IncOptions {
 	val Default = IncOptions(
@@ -39,7 +42,10 @@ object IncOptions {
 		recompileAllFraction = 0.5,
 		relationsDebug = false,
 		apiDebug = false,
-		apiDumpDirectory = None)
+		apiDumpDirectory = None,
+		newClassfileManager = ClassfileManager.deleteImmediately
+	)
+	def defaultTransactional(tempDir: File): IncOptions = Default.copy(newClassfileManager = ClassfileManager.transactional(tempDir))
 
 	val transitiveStepKey       = "transitiveStep"
 	val recompileAllFractionKey = "recompileAllFraction"
@@ -72,7 +78,7 @@ object IncOptions {
 			else None
 		}
 
-		IncOptions(getTransitiveStep, getRecompileAllFraction, getRelationsDebug, getApiDebug, getApiDumpDirectory)
+		IncOptions(getTransitiveStep, getRecompileAllFraction, getRelationsDebug, getApiDebug, getApiDumpDirectory, ClassfileManager.deleteImmediately)
 	}
 
 	def toStringMap(o: IncOptions): java.util.Map[String, String] = {

compile/inc/src/main/scala/sbt/inc/Incremental.scala

@@ -13,13 +13,13 @@ import java.io.File
 object Incremental
 {
 	def compile(sources: Set[File],
-	    entry: String => Option[File],
-	    previous: Analysis,
-	    current: ReadStamps,
-	    forEntry: File => Option[Analysis],
-	    doCompile: (Set[File], DependencyChanges) => Analysis,
-	    log: Logger,
-	    options: IncOptions)(implicit equivS: Equiv[Stamp]): (Boolean, Analysis) =
+		entry: String => Option[File],
+		previous: Analysis,
+		current: ReadStamps,
+		forEntry: File => Option[Analysis],
+		doCompile: (Set[File], DependencyChanges) => Analysis,
+		log: Logger,
+		options: IncOptions)(implicit equivS: Equiv[Stamp]): (Boolean, Analysis) =
 	{
 		val initialChanges = changedInitial(entry, sources, previous, current, forEntry, options)
 		val binaryChanges = new DependencyChanges {
@@ -29,18 +29,32 @@ object Incremental
 		}
 		val initialInv = invalidateInitial(previous.relations, initialChanges, log)
 		log.debug("Initially invalidated: " + initialInv)
-		val analysis = cycle(initialInv, sources, binaryChanges, previous, doCompile, 1, log, options)
+		val analysis = manageClassfiles(options) { classfileManager =>
+			cycle(initialInv, sources, binaryChanges, previous, doCompile, classfileManager, 1, log, options)
+		}
 		(!initialInv.isEmpty, analysis)
 	}
 
+	private[this] def manageClassfiles[T](options: IncOptions)(run: ClassfileManager => T): T =
+	{
+		val classfileManager = options.newClassfileManager()
+		val result = try run(classfileManager) catch { case e: Exception =>
+			classfileManager.complete(success = false)
+			throw e
+		}
+		classfileManager.complete(success = true)
+		result
+	}
+
 	val incDebugProp = "xsbt.inc.debug"
 	private def incDebug(options: IncOptions): Boolean = options.relationsDebug || java.lang.Boolean.getBoolean(incDebugProp)
 	val apiDebugProp = "xsbt.api.debug"
 	def apiDebug(options: IncOptions): Boolean =  options.apiDebug || java.lang.Boolean.getBoolean(apiDebugProp)
+
 	// TODO: the Analysis for the last successful compilation should get returned + Boolean indicating success
 	// TODO: full external name changes, scopeInvalidations
-	def cycle(invalidatedRaw: Set[File], allSources: Set[File], binaryChanges: DependencyChanges, previous: Analysis,
-	    doCompile: (Set[File], DependencyChanges) => Analysis, cycleNum: Int, log: Logger, options: IncOptions): Analysis =
+	@tailrec 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 =
 		if(invalidatedRaw.isEmpty)
 			previous
 		else
@@ -48,17 +62,20 @@ object Incremental
 			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 pruned = prune(invalidated, previous)
+			val pruned = prune(invalidated, previous, classfileManager)
 			debug("********* Pruned: \n" + pruned.relations + "\n*********")
+
 			val fresh = doCompile(invalidated, binaryChanges)
+			classfileManager.generated(fresh.relations.allProducts)
 			debug("********* Fresh: \n" + fresh.relations + "\n*********")
 			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)
 			debug("Changes:\n" + incChanges)
 			val transitiveStep = options.transitiveStep
 			val incInv = invalidateIncremental(merged.relations, incChanges, invalidated, cycleNum >= transitiveStep, log)
-			cycle(incInv, allSources, emptyChanges, merged, doCompile, cycleNum+1, log, options)
+			cycle(incInv, allSources, emptyChanges, merged, doCompile, classfileManager, cycleNum+1, log, options)
 		}
 	private[this] def emptyChanges: DependencyChanges = new DependencyChanges {
 		val modifiedBinaries = new Array[File](0)
@@ -227,8 +244,11 @@ object Incremental
 	}
 
 	def prune(invalidatedSrcs: Set[File], previous: Analysis): Analysis =
+		prune(invalidatedSrcs, previous, ClassfileManager.deleteImmediately())
+
+	def prune(invalidatedSrcs: Set[File], previous: Analysis, classfileManager: ClassfileManager): Analysis =
 	{
-		IO.deleteFilesEmptyDirs( invalidatedSrcs.flatMap(previous.relations.products) )
+		classfileManager.delete( invalidatedSrcs.flatMap(previous.relations.products) )
 		previous -- invalidatedSrcs
 	}
 

main/src/main/scala/sbt/Defaults.scala

@@ -200,7 +200,7 @@ object Defaults extends BuildCommon
 		compilersSetting,
 		javacOptions in GlobalScope :== Nil,
 		scalacOptions in GlobalScope :== Nil,
-		incOptions in GlobalScope :== sbt.inc.IncOptions.Default,
+		incOptions in GlobalScope := sbt.inc.IncOptions.defaultTransactional(crossTarget.value.getParentFile / "classes.bak"),
 		scalaInstance <<= scalaInstanceTask,
 		scalaVersion in GlobalScope := appConfiguration.value.provider.scalaProvider.version,
 		scalaBinaryVersion in GlobalScope := binaryScalaVersion(scalaVersion.value),

sbt/src/sbt-test/source-dependencies/restore-classes/build.sbt

@@ -0,0 +1,12 @@
+import complete.DefaultParsers._
+
+crossTarget in Compile := target.value
+
+val checkIterations = inputKey[Unit]("Verifies the accumlated number of iterations of incremental compilation.")
+
+checkIterations := {
+        val expected: Int = (Space ~> NatBasic).parsed
+        val actual: Int = (compile in Compile).value.compilations.allCompilations.size
+        assert(expected == actual, s"Expected $expected compilations, got $actual")
+}
+

sbt/src/sbt-test/source-dependencies/restore-classes/changes/A1.scala

@@ -0,0 +1,3 @@
+object A {
+  val x = 3
+}

sbt/src/sbt-test/source-dependencies/restore-classes/changes/A2.scala

@@ -0,0 +1,5 @@
+object A {
+  val x = "a"
+}
+
+class C 

sbt/src/sbt-test/source-dependencies/restore-classes/changes/B.scala

@@ -0,0 +1,4 @@
+object B {
+  val y: Int = A.x
+}
+

sbt/src/sbt-test/source-dependencies/restore-classes/test

@@ -0,0 +1,23 @@
+$ copy-file changes/A1.scala A.scala
+$ copy-file changes/B.scala B.scala
+# B depends on A
+# 1 iteration
+> compile
+
+$ copy-file changes/A2.scala A.scala
+
+# will successfully compile A.scala in the first step but fail to compile B.scala in the second
+# because type of A.x changed.  The original classes should be restored after this failure.
+# 2 iterations, but none are recorded in the Analysis
+-> compile
+
+# the class file for C should be deleted:
+#  it was only added by A2, but compilation hasn't succeeded yet
+$ absent target/classes/C.class
+
+
+$ copy-file changes/A1.scala A.scala
+# if the classes were correctly restored, another compilation shouldn't be necessary
+> compile
+# so, there should only be the original 1 iteration recorded in the Analysis
+> checkIterations 1