From 541bd4a6a4fdcc31683d5d17b2e91d50cedea886 Mon Sep 17 00:00:00 2001 From: Martin Duhem Date: Wed, 28 Oct 2015 15:23:45 +0100 Subject: [PATCH 1/5] Fix for sbt/sbt#1933 sbt was reporting warning abouts inconsistent versions of dependencies even if these dependencies didn't have the same configuration (as in `provided` vs `compile`). This commit fixes this problem by comparing the dependencies by organization, artifact name and configuration. Fixes sbt/sbt#1933 --- ivy/src/main/scala/sbt/Ivy.scala | 8 ++++++-- ivy/src/test/scala/InconsistentDuplicateSpec.scala | 5 +++++ 2 files changed, 11 insertions(+), 2 deletions(-) diff --git a/ivy/src/main/scala/sbt/Ivy.scala b/ivy/src/main/scala/sbt/Ivy.scala index b8540fd68..ef9b695e9 100644 --- a/ivy/src/main/scala/sbt/Ivy.scala +++ b/ivy/src/main/scala/sbt/Ivy.scala @@ -536,14 +536,18 @@ private[sbt] object IvySbt { { import IvyRetrieve.toModuleID val dds = moduleID.getDependencies - inconsistentDuplicateWarning(dds map { dd => toModuleID(dd.getDependencyRevisionId) }) + val deps = dds flatMap { dd => + val module = toModuleID(dd.getDependencyRevisionId) + dd.getModuleConfigurations map (c => module.copy(configurations = Some(c))) + } + inconsistentDuplicateWarning(deps) } def inconsistentDuplicateWarning(dependencies: Seq[ModuleID]): List[String] = { val warningHeader = "Multiple dependencies with the same organization/name but different versions. To avoid conflict, pick one version:" val out: mutable.ListBuffer[String] = mutable.ListBuffer() - (dependencies groupBy { dep => (dep.organization, dep.name) }) foreach { + (dependencies groupBy { dep => (dep.organization, dep.name, dep.configurations) }) foreach { case (k, vs) if vs.size > 1 => val v0 = vs.head (vs find { _.revision != v0.revision }) foreach { v => diff --git a/ivy/src/test/scala/InconsistentDuplicateSpec.scala b/ivy/src/test/scala/InconsistentDuplicateSpec.scala index b9f4a05ef..34a5a441f 100644 --- a/ivy/src/test/scala/InconsistentDuplicateSpec.scala +++ b/ivy/src/test/scala/InconsistentDuplicateSpec.scala @@ -9,6 +9,7 @@ class InconsistentDuplicateSpec extends Specification { Duplicate with different version should be warned $warn1 + not be warned if in different configurations $nodupe2 Duplicate with same version should not be warned $nodupe1 @@ -25,4 +26,8 @@ class InconsistentDuplicateSpec extends Specification { def nodupe1 = IvySbt.inconsistentDuplicateWarning(Seq(akkaActor230Test, akkaActor230)) must_== Nil + + def nodupe2 = + IvySbt.inconsistentDuplicateWarning(Seq(akkaActor214, akkaActor230Test)) must_== Nil + } From 26e15c2dd43910b4f52d697370e00da0678054a5 Mon Sep 17 00:00:00 2001 From: Martin Duhem Date: Wed, 28 Oct 2015 16:05:45 +0100 Subject: [PATCH 2/5] Add notes for #2258 --- notes/0.13.10/fix-duplicate-warnings.markdown | 13 +++++++++++++ 1 file changed, 13 insertions(+) create mode 100644 notes/0.13.10/fix-duplicate-warnings.markdown diff --git a/notes/0.13.10/fix-duplicate-warnings.markdown b/notes/0.13.10/fix-duplicate-warnings.markdown new file mode 100644 index 000000000..a8e9c8bed --- /dev/null +++ b/notes/0.13.10/fix-duplicate-warnings.markdown @@ -0,0 +1,13 @@ + + [@Duhemm]: http://github.com/Duhemm + [1933]: https://github.com/sbt/sbt/issues/1933 + [2258]: https://github.com/sbt/sbt/pull/2258 + +### Fixes with compatibility implications + +### Improvements + +### Bug fixes + +- Fixes [#1933][1933] duplicate warnings for artifacts in distinct configurations [#2258][2258] by + [@Duhemm][@Duhemm] From 9fbbaab58e246bce7b21117e079206519c386b57 Mon Sep 17 00:00:00 2001 From: Stu Hood Date: Tue, 10 Nov 2015 14:39:44 -0800 Subject: [PATCH 3/5] Add a test that we`re able to parse inner classes. --- .../api/src/main/scala/sbt/ClassToAPI.scala | 6 +-- .../main/scala/sbt/classfile/ClassFile.scala | 1 - .../src/main/scala/sbt/classfile/Parser.scala | 17 +++++--- .../sbt/classfile/ParserSpecification.scala | 24 +++++++++++ util/io/src/main/scala/sbt/IO.scala | 42 ++++++++++--------- .../src/test/scala/sbt/IOSpecification.scala | 1 + 6 files changed, 61 insertions(+), 30 deletions(-) create mode 100644 util/classfile/src/test/scala/sbt/classfile/ParserSpecification.scala diff --git a/compile/api/src/main/scala/sbt/ClassToAPI.scala b/compile/api/src/main/scala/sbt/ClassToAPI.scala index 8511d3599..9589865ec 100644 --- a/compile/api/src/main/scala/sbt/ClassToAPI.scala +++ b/compile/api/src/main/scala/sbt/ClassToAPI.scala @@ -89,10 +89,8 @@ object ClassToAPI { } /** TODO: over time, ClassToAPI should switch the majority of access to the classfile parser */ - private[this] def classFileForClass(c: Class[_]): ClassFile = { - val file = new java.io.File(IO.classLocationFile(c), s"${c.getName.replace('.', '/')}.class") - classfile.Parser.apply(file) - } + private[this] def classFileForClass(c: Class[_]): ClassFile = + classfile.Parser.apply(IO.classfileLocation(c)) private[this] def lzyS[T <: AnyRef](t: T): xsbti.api.Lazy[T] = lzy(t) def lzy[T <: AnyRef](t: => T): xsbti.api.Lazy[T] = xsbti.SafeLazy(t) diff --git a/util/classfile/src/main/scala/sbt/classfile/ClassFile.scala b/util/classfile/src/main/scala/sbt/classfile/ClassFile.scala index 3f1a0d915..e78b9cfd2 100644 --- a/util/classfile/src/main/scala/sbt/classfile/ClassFile.scala +++ b/util/classfile/src/main/scala/sbt/classfile/ClassFile.scala @@ -10,7 +10,6 @@ import java.io.File private[sbt] trait ClassFile { val majorVersion: Int val minorVersion: Int - val fileName: String val className: String val superClassName: String val interfaceNames: Array[String] diff --git a/util/classfile/src/main/scala/sbt/classfile/Parser.scala b/util/classfile/src/main/scala/sbt/classfile/Parser.scala index 16c5bf656..06a5e70e4 100644 --- a/util/classfile/src/main/scala/sbt/classfile/Parser.scala +++ b/util/classfile/src/main/scala/sbt/classfile/Parser.scala @@ -4,6 +4,7 @@ package sbt package classfile +import java.net.URL import java.io.{ DataInputStream, File, InputStream } import scala.annotation.switch @@ -15,15 +16,19 @@ import scala.annotation.switch import Constants._ private[sbt] object Parser { - def apply(file: File): ClassFile = Using.fileInputStream(file)(parse(file.getAbsolutePath)).right.get - private def parse(fileName: String)(is: InputStream): Either[String, ClassFile] = Right(parseImpl(fileName, is)) - private def parseImpl(filename: String, is: InputStream): ClassFile = + def apply(file: File): ClassFile = + Using.fileInputStream(file)(parse(file.toString)).right.get + + def apply(url: URL): ClassFile = + Using.urlInputStream(url)(parse(url.toString)).right.get + + private def parse(readableName: String)(is: InputStream): Either[String, ClassFile] = Right(parseImpl(readableName, is)) + private def parseImpl(readableName: String, is: InputStream): ClassFile = { val in = new DataInputStream(is) - new ClassFile { - assume(in.readInt() == JavaMagic, "Invalid class file: " + fileName) + assume(in.readInt() == JavaMagic, "Invalid class file: " + readableName) - val fileName = filename + new ClassFile { val minorVersion: Int = in.readUnsignedShort() val majorVersion: Int = in.readUnsignedShort() diff --git a/util/classfile/src/test/scala/sbt/classfile/ParserSpecification.scala b/util/classfile/src/test/scala/sbt/classfile/ParserSpecification.scala new file mode 100644 index 000000000..aa1cb743e --- /dev/null +++ b/util/classfile/src/test/scala/sbt/classfile/ParserSpecification.scala @@ -0,0 +1,24 @@ +package sbt +package classfile + +import util.Try + +import org.scalacheck._ +import Prop._ + +object ParserSpecification extends Properties("Parser") { + property("able to parse all relevant classes") = + Prop.forAll(classes) { (c: Class[_]) => + Parser(IO.classfileLocation(c)) ne null + } + + implicit def classes: Gen[Class[_]] = + Gen.oneOf( + this.getClass, + classOf[java.lang.Integer], + classOf[java.util.AbstractMap.SimpleEntry[String, String]], + classOf[String], + classOf[Thread], + classOf[Properties] + ) +} diff --git a/util/io/src/main/scala/sbt/IO.scala b/util/io/src/main/scala/sbt/IO.scala index 0fd5f52ad..179faa7e2 100644 --- a/util/io/src/main/scala/sbt/IO.scala +++ b/util/io/src/main/scala/sbt/IO.scala @@ -36,39 +36,43 @@ object IO { val utf8 = Charset.forName("UTF-8") /** - * Returns a URL for the directory or jar containing the the class file `cl`. + * Returns a URL for the classfile containing the given class * If the location cannot be determined, an error is generated. */ - def classLocation(cl: Class[_]): URL = { - val codeSource = cl.getProtectionDomain.getCodeSource - if (codeSource ne null) { - codeSource.getLocation - } else { - // NB: This assumes that classes without code sources are System classes, and thus located in - // jars. It assumes that `urlAsFile` will truncate to the containing jar file. - val clsfile = s"${cl.getName.replace('.', '/')}.class" - Option(ClassLoader.getSystemClassLoader.getResource(clsfile)) - .flatMap { - urlAsFile - }.getOrElse { - sys.error("No class location for " + cl) - }.toURI.toURL + def classfileLocation(cl: Class[_]): URL = { + val clsfile = s"${cl.getName.replace('.', '/')}.class" + try { + Stream(Option(cl.getClassLoader), Some(ClassLoader.getSystemClassLoader)).flatten.flatMap { classLoader => + Option(classLoader.getResource(clsfile)) + }.headOption.getOrElse { + sys.error("No class location for " + cl) + } + } catch { + case e => + e.printStackTrace() + throw e } } /** - * Returns the directory or jar file containing the the class file `cl`. - * If the location cannot be determined or it is not a file, an error is generated. + * Returns the directory or jar file containing the class file `cl`. + * If the location cannot be determined or if it is not a file, an error is generated. * Note that Java standard library classes typically do not have a location associated with them. */ - def classLocationFile(cl: Class[_]): File = toFile(classLocation(cl)) + def classLocationFile(cl: Class[_]): File = { + val classURL = + Option(cl.getProtectionDomain.getCodeSource).getOrElse { + sys.error("No class location for " + cl) + }.getLocation + toFile(classURL) + } /** * Returns a URL for the directory or jar containing the class file for type `T` (as determined by an implicit Manifest). * If the location cannot be determined, an error is generated. * Note that Java standard library classes typically do not have a location associated with them. */ - def classLocation[T](implicit mf: SManifest[T]): URL = classLocation(mf.runtimeClass) + def classfileLocation[T](implicit mf: SManifest[T]): URL = classfileLocation(mf.runtimeClass) /** * Returns the directory or jar file containing the the class file for type `T` (as determined by an implicit Manifest). diff --git a/util/io/src/test/scala/sbt/IOSpecification.scala b/util/io/src/test/scala/sbt/IOSpecification.scala index 882f147c5..e932334b6 100644 --- a/util/io/src/test/scala/sbt/IOSpecification.scala +++ b/util/io/src/test/scala/sbt/IOSpecification.scala @@ -18,6 +18,7 @@ object IOSpecification extends Properties("IO") { Gen.oneOf( this.getClass, classOf[java.lang.Integer], + classOf[java.util.AbstractMap.SimpleEntry[String, String]], classOf[String], classOf[Thread], classOf[Properties] From 4a4a3ca94ed1f369132a058222902fe229e0370a Mon Sep 17 00:00:00 2001 From: Martin Duhem Date: Wed, 11 Nov 2015 14:21:54 +0100 Subject: [PATCH 4/5] Report failed compilation if errors are registered. In some cases, the local java compiler may report compilation errors, but succeed in compiling the source files (for instance, if there are encoding problems in the source). However, the command line javac will report a failed compilation on the same input. To have the local java compiler behave like the forked java compiler, we now report the compilation as failed if error messages have been registered during compilation. Fixes sbt/sbt#2228 --- .../main/scala/sbt/compiler/javac/DiagnosticsReporter.scala | 5 +++++ compile/src/main/scala/sbt/compiler/javac/LocalJava.scala | 6 +++++- 2 files changed, 10 insertions(+), 1 deletion(-) diff --git a/compile/src/main/scala/sbt/compiler/javac/DiagnosticsReporter.scala b/compile/src/main/scala/sbt/compiler/javac/DiagnosticsReporter.scala index bb1b144e6..40958d36f 100644 --- a/compile/src/main/scala/sbt/compiler/javac/DiagnosticsReporter.scala +++ b/compile/src/main/scala/sbt/compiler/javac/DiagnosticsReporter.scala @@ -14,6 +14,10 @@ import javax.tools.Diagnostic.NOPOS final class DiagnosticsReporter(reporter: Reporter) extends DiagnosticListener[JavaFileObject] { val END_OF_LINE_MATCHER = "(\r\n)|[\r]|[\n]" val EOL = System.getProperty("line.separator") + + private[this] var errorEncountered = false + def hasErrors: Boolean = errorEncountered + private def fixedDiagnosticMessage(d: Diagnostic[_ <: JavaFileObject]): String = { def getRawMessage = d.getMessage(null) def fixWarnOrErrorMessage = { @@ -110,6 +114,7 @@ final class DiagnosticsReporter(reporter: Reporter) extends DiagnosticListener[J if (sourceUri.isDefined) s"${sourceUri.get}:${if (line.isDefined) line.get else -1}" else "" } + if (severity == Severity.Error) errorEncountered = true reporter.log(pos, msg, severity) } } diff --git a/compile/src/main/scala/sbt/compiler/javac/LocalJava.scala b/compile/src/main/scala/sbt/compiler/javac/LocalJava.scala index 81f4e1293..9b6811151 100644 --- a/compile/src/main/scala/sbt/compiler/javac/LocalJava.scala +++ b/compile/src/main/scala/sbt/compiler/javac/LocalJava.scala @@ -66,6 +66,10 @@ final class LocalJavaCompiler(compiler: javax.tools.JavaCompiler) extends JavaCo val diagnostics = new DiagnosticsReporter(reporter) val fileManager = compiler.getStandardFileManager(diagnostics, null, null) val jfiles = fileManager.getJavaFileObjectsFromFiles(sources.asJava) - compiler.getTask(logWriter, fileManager, diagnostics, options.asJava, null, jfiles).call() + val success = compiler.getTask(logWriter, fileManager, diagnostics, options.asJava, null, jfiles).call() + // The local compiler may report a successful compilation even though there are errors (e.g. encoding problems in the + // source files). In a similar situation, command line javac reports a failed compilation. To have the local java compiler + // stick to javac's behavior, we report a failed compilation if there have been errors. + success && !diagnostics.hasErrors } } \ No newline at end of file From e19ca7bf852229987a831f22a2495215a289ead7 Mon Sep 17 00:00:00 2001 From: Martin Duhem Date: Wed, 11 Nov 2015 14:30:05 +0100 Subject: [PATCH 5/5] Notes for sbt/sbt#2271 --- notes/0.13.10/local-java-report-errors.md | 12 ++++++++++++ 1 file changed, 12 insertions(+) create mode 100644 notes/0.13.10/local-java-report-errors.md diff --git a/notes/0.13.10/local-java-report-errors.md b/notes/0.13.10/local-java-report-errors.md new file mode 100644 index 000000000..8c36fb710 --- /dev/null +++ b/notes/0.13.10/local-java-report-errors.md @@ -0,0 +1,12 @@ + + [@Duhemm]: http://github.com/Duhemm + [2228]: https://github.com/sbt/sbt/issues/2228 + [2271]: https://github.com/sbt/sbt/pull/2271 + +### Fixes with compatibility implications + +### Improvements + +### Bug fixes + +- Report java compilation as failed if the local java compiler reported compilation errors (issue [#2228][2228]) [#2271][2271] by [@Duhemm][@Duhemm]