Skip to content

Commit

Permalink
Fix Output and Compilation API
Browse files Browse the repository at this point in the history
* Remove dangerous use of output groups as fields in compilation. In the
  case of `SingleOutput`, the source directory was invented and
  incorrect (the root). Users of the API processing this file could
  create chaos in people's computers. The new design forces consumers
  of the `Compilation` API to make up their minds and identify whether
  the compilation was used with a single output or several outputs.
  In order to do that, we return `Output` instead of an array of
  `OutputGroup`.
* Rename `outputDirectory` and `sourceDirectory` to be Java friendly.
* Augment `Output` interface with Java-friendly methods. Scala users
  can use pattern matching instead.
  • Loading branch information
jvican committed May 21, 2017
1 parent 1f2d12c commit 761ea0e
Show file tree
Hide file tree
Showing 25 changed files with 122 additions and 220 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -18,9 +18,9 @@ sealed abstract class CallbackGlobal(settings: Settings,

lazy val outputDirs: Iterable[File] = {
output match {
case single: SingleOutput => List(single.outputDirectory)
case single: SingleOutput => List(single.getOutputDirectory)
// Use Stream instead of List because Analyzer maps intensively over the directories
case multi: MultipleOutput => multi.outputGroups.toStream map (_.outputDirectory)
case multi: MultipleOutput => multi.getOutputGroups.toStream map (_.outputDirectory)
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -68,11 +68,11 @@ private final class CachedCompiler0(args: Array[String],
val settings = new Settings(s => initialLog(s))
output match {
case multi: MultipleOutput =>
for (out <- multi.outputGroups)
for (out <- multi.getOutputGroups)
settings.outputDirs
.add(out.sourceDirectory.getAbsolutePath, out.outputDirectory.getAbsolutePath)
case single: SingleOutput =>
settings.outputDirs.setSingleOutput(single.outputDirectory.getAbsolutePath)
settings.outputDirs.setSingleOutput(single.getOutputDirectory.getAbsolutePath)
}

val command = Command(args.toList, settings)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -185,8 +185,8 @@ class ScalaCompilerForUnitTesting {
classpath: String = "."): ZincCompiler = {
val args = Array.empty[String]
object output extends SingleOutput {
def outputDirectory: File = outputDir
override def toString = s"SingleOutput($outputDirectory)"
def getOutputDirectory: File = outputDir
override def toString = s"SingleOutput($getOutputDirectory)"
}
val weakLog = new WeakLog(ConsoleLogger(), ConsoleReporter)
val cachedCompiler = new CachedCompiler0(args, output, weakLog, false)
Expand Down
2 changes: 1 addition & 1 deletion internal/compiler-interface/src/main/contraband/other.json
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,7 @@
"type": "record",
"fields": [
{ "name": "startTime", "type": "long" },
{ "name": "outputs", "type": "xsbti.compile.OutputGroup*" }
{ "name": "output", "type": "xsbti.compile.Output" }
]
},
{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,9 +8,30 @@
package xsbti.compile;

import java.io.File;
import java.util.Optional;

/** Define the interface for several kind of output directories. */
/**
* Represents a mapping of several outputs depending on the source directory.
* <p>
* This option is used only by the Scala compiler.
*/
public interface MultipleOutput extends Output {
/** Return an array of the existent output groups. */
OutputGroup[] outputGroups();
/**
* Return an array of the existent output groups.
* <p>
* Incremental compilation manages the class files in these directories, so
* don't play with them out of the Zinc API. Zinc already takes care of
* deleting classes before every compilation run.
*/
OutputGroup[] getOutputGroups();

@Override
public default Optional<File> getSingleOutput() {
return Optional.empty();
}

@Override
public default Optional<OutputGroup[]> getMultipleOutput() {
return Optional.of(getOutputGroups());
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -7,10 +7,41 @@

package xsbti.compile;

import java.io.File;
import java.io.Serializable;
import java.util.Optional;

/**
* Define an abstract interface that represents the output of the compilation.
*
* <p>
* Inheritors are {@link SingleOutput} with a global output directory and
* {@link MultipleOutput} that specifies the output directory per source file.
* <p>
* These two subclasses exist to satisfy the Scala compiler which accepts both
* single and multiple targets. These targets may depend on the sources to be
* compiled.
* <p>
* Note that Javac does not support multiple output and any attempt to use it
* will result in a runtime exception.
* <p>
* This class is used both as an input to the compiler and as an output of the
* {@link xsbti.compile.CompileAnalysis}.
*/
public interface Output {}
public interface Output extends Serializable {
/**
* Returns the multiple outputs passed or to be passed to the Scala compiler.
* If single output directory is used or Javac will consume this setting,
* it returns {@link java.util.Optional#EMPTY}.
*
* @see xsbti.compile.MultipleOutput
*/
public Optional<OutputGroup[]> getMultipleOutput();

/**
* Returns the single output passed or to be passed to the Scala or Java compiler.
* If multiple outputs are used, it returns {@link java.util.Optional#EMPTY}.
*
* @see xsbti.compile.SingleOutput
*/
public Optional<File> getSingleOutput();
}
Original file line number Diff line number Diff line change
@@ -1,24 +1,27 @@
package xsbti.compile;

import java.io.File;
import java.io.Serializable;

/** Define the interface of a group of outputs. */
public interface OutputGroup {
/**
* Define the interface of a group of outputs.
*/
public interface OutputGroup extends Serializable {
/**
* Return the directory where source files are stored for this group.
*
* <p>
* Note that source directories should uniquely identify the group
* for a certain source file.
*/
public File sourceDirectory();

/**
* Return the directory where class files should be generated.
*
* <p>
* Incremental compilation manages the class files in this directory, so
* don't play with this directory out of the Zinc API. Zinc already takes
* care of deleting classes before every compilation run.
*
* <p>
* This directory must be exclusively used for one set of sources.
*/
public File outputDirectory();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,20 +8,31 @@
package xsbti.compile;

import java.io.File;
import java.util.Optional;

/**
* Represent a single output directory where the Zinc incremental compiler
* will store all the generated class files by Java and Scala sources.
*/
public interface SingleOutput extends Output {
/**
* Return the directory where class files should be generated.
*
* Incremental compilation manages the class files in this directory, so
* don't play with this directory out of the Zinc API. Zinc already takes
* care of deleting classes before every compilation run.
*
* This directory must be exclusively used for one set of sources.
*/
File outputDirectory();
/**
* Return the directory where class files should be generated.
* <p>
* Incremental compilation manages the class files in this directory, so
* don't play with this directory out of the Zinc API. Zinc already takes
* care of deleting classes before every compilation run.
* <p>
* This directory must be exclusively used for one set of sources.
*/
File getOutputDirectory();

@Override
public default Optional<File> getSingleOutput() {
return Optional.of(getOutputDirectory());
}

@Override
public default Optional<OutputGroup[]> getMultipleOutput() {
return Optional.empty();
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -120,8 +120,8 @@ private[xsbt] object ZincBenchmark {
compilationInfo: CompilationInfo
): ZincCompiler = {
object output extends SingleOutput {
def outputDirectory: File = outputDir
override def toString = s"SingleOutput($outputDirectory)"
def getOutputDirectory: File = outputDir
override def toString = s"SingleOutput($getOutputDirectory)"
}
val args = compilationInfo.scalacOptions
val classpath = compilationInfo.classpath
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -25,8 +25,8 @@ object CompileOutput {
* @return An instance of [[SingleOutput]] that stores contents in <code>dir</code>.
*/
def apply(dir: File): Output = new SingleOutput {
def outputDirectory = dir
override def toString = s"SingleOutput($outputDirectory)"
def getOutputDirectory = dir
override def toString = s"SingleOutput($getOutputDirectory)"
}

/**
Expand All @@ -37,14 +37,14 @@ object CompileOutput {
* @return An instance of [[MultipleOutput]].
*/
def apply(groups: (File, File)*): Output = new MultipleOutput {
def outputGroups = groups.toArray map {
def getOutputGroups = groups.toArray map {
case (src, out) =>
new OutputGroup {
def sourceDirectory = src
def outputDirectory = out
override def toString = s"OutputGroup($src -> $out)"
}
}
override def toString = s"MultiOutput($outputGroups)"
override def toString = s"MultiOutput($getOutputGroups)"
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -95,7 +95,7 @@ object JavaCompiler {
* make use of it (e.g. the Eclipse compiler does this via EJC).
* See https://github.com/sbt/zinc/issues/163. */
val target = output match {
case so: SingleOutput => Some(so.outputDirectory)
case so: SingleOutput => Some(so.getOutputDirectory)
case _: MultipleOutput => None
}
// TODO(jvican): Fix the `libraryJar` deprecation.
Expand Down
14 changes: 2 additions & 12 deletions internal/zinc-core/src/main/scala/sbt/internal/inc/Compile.scala
Original file line number Diff line number Diff line change
Expand Up @@ -131,18 +131,8 @@ private final class AnalysisCallback(
options: IncOptions
) extends xsbti.AnalysisCallback {

private[this] val compilation: Compilation = {
val outputSettings = output match {
case single: SingleOutput =>
// TODO: Why are we using the root as the file with all the sources? Smell.
val rootFile = new java.io.File("/")
List(new SimpleOutputGroup(rootFile, single.outputDirectory))
case multi: MultipleOutput =>
multi.outputGroups.toList.map(out =>
new SimpleOutputGroup(out.sourceDirectory, out.outputDirectory))
}
new Compilation(System.currentTimeMillis, outputSettings.toArray)
}
private[this] val compilation: Compilation =
new Compilation(System.currentTimeMillis, output)

override def toString =
(List("Class APIs", "Object APIs", "Binary deps", "Products", "Source deps") zip
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -92,15 +92,15 @@ object MiniSetupUtil {

def equiv(out1: APIOutput, out2: APIOutput) = (out1, out2) match {
case (m1: MultipleOutput, m2: MultipleOutput) =>
(m1.outputGroups.length == m2.outputGroups.length) &&
(m1.outputGroups.sorted zip m2.outputGroups.sorted forall {
(m1.getOutputGroups.length == m2.getOutputGroups.length) &&
(m1.getOutputGroups.sorted zip m2.getOutputGroups.sorted forall {
case (a, b) =>
equivFile
.equiv(a.sourceDirectory, b.sourceDirectory) && equivFile
.equiv(a.outputDirectory, b.outputDirectory)
})
case (s1: SingleOutput, s2: SingleOutput) =>
equivFile.equiv(s1.outputDirectory, s2.outputDirectory)
equivFile.equiv(s1.getOutputDirectory, s2.getOutputDirectory)
case _ =>
false
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -320,11 +320,11 @@ class TextAnalysisFormat(override val mappers: AnalysisMappers)
val (mode, outputAsMap) = setup.output match {
case s: SingleOutput =>
// just to be compatible with multipleOutputMode
val ignored = s.outputDirectory
(singleOutputMode, Map(ignored -> s.outputDirectory))
val ignored = s.getOutputDirectory
(singleOutputMode, Map(ignored -> s.getOutputDirectory))
case m: MultipleOutput =>
(multipleOutputMode,
m.outputGroups.map(x => x.sourceDirectory -> x.outputDirectory).toMap)
m.getOutputGroups.map(x => x.sourceDirectory -> x.outputDirectory).toMap)
}

writeSeq(out)(Headers.outputMode, mode :: Nil, identity[String])
Expand Down Expand Up @@ -365,19 +365,19 @@ class TextAnalysisFormat(override val mappers: AnalysisMappers)
s match {
case `singleOutputMode` =>
new SingleOutput {
val outputDirectory = outputAsMap.values.head
val getOutputDirectory = outputAsMap.values.head
}
case `multipleOutputMode` =>
new MultipleOutput {
val outputGroups: Array[OutputGroup] = outputAsMap.toArray.map {
val getOutputGroups: Array[OutputGroup] = outputAsMap.toArray.map {
case (src: File, out: File) =>
new OutputGroup {
val sourceDirectory = src
val outputDirectory = out
override def toString = s"OutputGroup($src -> $out)"
}
}
override def toString = s"MultipleOuput($outputGroups)"
override def toString = s"MultipleOuput($getOutputGroups)"
}
case str: String => throw new ReadException("Unrecognized output mode: " + str)
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,7 @@ class ExportableCache(val cacheLocation: Path, cleanOutputMode: CleanOutputMode

protected def outputDirFor(setup: MiniSetup): File = setup.output() match {
case single: SingleOutput =>
single.outputDirectory()
single.getOutputDirectory()
case _ => throw new RuntimeException("Only single output is supported")
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@ trait BaseTextAnalysisFormatTest { self: Properties =>

val storeApis = true
val dummyOutput = new xsbti.compile.SingleOutput {
def outputDirectory: java.io.File = new java.io.File("/dummy")
def getOutputDirectory: java.io.File = new java.io.File("/dummy")
}
val commonSetup = new MiniSetup(dummyOutput,
new MiniOptions(Array(), Array(), Array()),
Expand Down Expand Up @@ -115,9 +115,9 @@ trait BaseTextAnalysisFormatTest { self: Properties =>
private def compareOutputs(left: Output, right: Output): Prop = {
(left, right) match {
case (l: SingleOutput, r: SingleOutput) =>
"Single output dir" |: l.outputDirectory() =? r.outputDirectory()
"Single output dir" |: l.getOutputDirectory() =? r.getOutputDirectory()
case (l: MultipleOutput, r: MultipleOutput) =>
"Output group match" |: l.outputGroups() =? r.outputGroups()
"Output group match" |: l.getOutputGroups() =? r.getOutputGroups()
case _ =>
s"Cannot compare $left with $right" |: left =? right
}
Expand Down

This file was deleted.

Loading

0 comments on commit 761ea0e

Please sign in to comment.