From 5b754b45f301a310e9232f3a5270201ebfc16076 Mon Sep 17 00:00:00 2001 From: Prashant Sharma Date: Wed, 11 Jun 2014 10:47:06 -0700 Subject: [SPARK-2069] MIMA false positives Fixes SPARK 2070 and 2071 Author: Prashant Sharma Closes #1021 from ScrapCodes/SPARK-2070/package-private-methods and squashes the following commits: 7979a57 [Prashant Sharma] addressed code review comments 558546d [Prashant Sharma] A little fancy error message. 59275ab [Prashant Sharma] SPARK-2071 Mima ignores classes and its members from previous versions too. 0c4ff2b [Prashant Sharma] SPARK-2070 Ignore methods along with annotated classes. --- .../apache/spark/tools/GenerateMIMAIgnore.scala | 123 +++++++++++---------- 1 file changed, 67 insertions(+), 56 deletions(-) (limited to 'tools/src/main/scala/org') diff --git a/tools/src/main/scala/org/apache/spark/tools/GenerateMIMAIgnore.scala b/tools/src/main/scala/org/apache/spark/tools/GenerateMIMAIgnore.scala index 6a261e19a3..03a73f92b2 100644 --- a/tools/src/main/scala/org/apache/spark/tools/GenerateMIMAIgnore.scala +++ b/tools/src/main/scala/org/apache/spark/tools/GenerateMIMAIgnore.scala @@ -40,74 +40,78 @@ object GenerateMIMAIgnore { private val classLoader = Thread.currentThread().getContextClassLoader private val mirror = runtimeMirror(classLoader) - private def classesPrivateWithin(packageName: String): Set[String] = { + + private def isDeveloperApi(sym: unv.Symbol) = + sym.annotations.exists(_.tpe =:= unv.typeOf[org.apache.spark.annotation.DeveloperApi]) + + private def isExperimental(sym: unv.Symbol) = + sym.annotations.exists(_.tpe =:= unv.typeOf[org.apache.spark.annotation.Experimental]) + + + private def isPackagePrivate(sym: unv.Symbol) = + !sym.privateWithin.fullName.startsWith("") + + private def isPackagePrivateModule(moduleSymbol: unv.ModuleSymbol) = + !moduleSymbol.privateWithin.fullName.startsWith("") + + /** + * For every class checks via scala reflection if the class itself or contained members + * have DeveloperApi or Experimental annotations or they are package private. + * Returns the tuple of such classes and members. + */ + private def privateWithin(packageName: String): (Set[String], Set[String]) = { val classes = getClasses(packageName) val ignoredClasses = mutable.HashSet[String]() + val ignoredMembers = mutable.HashSet[String]() - def isPackagePrivate(className: String) = { + for (className <- classes) { try { - /* Couldn't figure out if it's possible to determine a-priori whether a given symbol - is a module or class. */ - - val privateAsClass = mirror - .classSymbol(Class.forName(className, false, classLoader)) - .privateWithin - .fullName - .startsWith(packageName) - - val privateAsModule = mirror - .staticModule(className) - .privateWithin - .fullName - .startsWith(packageName) - - privateAsClass || privateAsModule - } catch { - case _: Throwable => { - println("Error determining visibility: " + className) - false + val classSymbol = mirror.classSymbol(Class.forName(className, false, classLoader)) + val moduleSymbol = mirror.staticModule(className) // TODO: see if it is necessary. + val directlyPrivateSpark = + isPackagePrivate(classSymbol) || isPackagePrivateModule(moduleSymbol) + val developerApi = isDeveloperApi(classSymbol) + val experimental = isExperimental(classSymbol) + + /* Inner classes defined within a private[spark] class or object are effectively + invisible, so we account for them as package private. */ + lazy val indirectlyPrivateSpark = { + val maybeOuter = className.toString.takeWhile(_ != '$') + if (maybeOuter != className) { + isPackagePrivate(mirror.classSymbol(Class.forName(maybeOuter, false, classLoader))) || + isPackagePrivateModule(mirror.staticModule(maybeOuter)) + } else { + false + } + } + if (directlyPrivateSpark || indirectlyPrivateSpark || developerApi || experimental) { + ignoredClasses += className + } else { + // check if this class has package-private/annotated members. + ignoredMembers ++= getAnnotatedOrPackagePrivateMembers(classSymbol) } - } - } - def isDeveloperApi(className: String) = { - try { - val clazz = mirror.classSymbol(Class.forName(className, false, classLoader)) - clazz.annotations.exists(_.tpe =:= unv.typeOf[org.apache.spark.annotation.DeveloperApi]) } catch { - case _: Throwable => { - println("Error determining Annotations: " + className) - false - } + case _: Throwable => println("Error instrumenting class:" + className) } } + (ignoredClasses.flatMap(c => Seq(c, c.replace("$", "#"))).toSet, ignoredMembers.toSet) + } - for (className <- classes) { - val directlyPrivateSpark = isPackagePrivate(className) - val developerApi = isDeveloperApi(className) - - /* Inner classes defined within a private[spark] class or object are effectively - invisible, so we account for them as package private. */ - val indirectlyPrivateSpark = { - val maybeOuter = className.toString.takeWhile(_ != '$') - if (maybeOuter != className) { - isPackagePrivate(maybeOuter) - } else { - false - } - } - if (directlyPrivateSpark || indirectlyPrivateSpark || developerApi) { - ignoredClasses += className - } - } - ignoredClasses.flatMap(c => Seq(c, c.replace("$", "#"))).toSet + private def getAnnotatedOrPackagePrivateMembers(classSymbol: unv.ClassSymbol) = { + classSymbol.typeSignature.members + .filter(x => isPackagePrivate(x) || isDeveloperApi(x) || isExperimental(x)).map(_.fullName) } def main(args: Array[String]) { - scala.tools.nsc.io.File(".generated-mima-excludes"). - writeAll(classesPrivateWithin("org.apache.spark").mkString("\n")) - println("Created : .generated-mima-excludes in current directory.") + val (privateClasses, privateMembers) = privateWithin("org.apache.spark") + scala.tools.nsc.io.File(".generated-mima-class-excludes"). + writeAll(privateClasses.mkString("\n")) + println("Created : .generated-mima-class-excludes in current directory.") + scala.tools.nsc.io.File(".generated-mima-member-excludes"). + writeAll(privateMembers.mkString("\n")) + println("Created : .generated-mima-member-excludes in current directory.") } @@ -140,10 +144,17 @@ object GenerateMIMAIgnore { * Get all classes in a package from a jar file. */ private def getClassesFromJar(jarPath: String, packageName: String) = { + import scala.collection.mutable val jar = new JarFile(new File(jarPath)) val enums = jar.entries().map(_.getName).filter(_.startsWith(packageName)) - val classes = for (entry <- enums if entry.endsWith(".class")) - yield Class.forName(entry.replace('/', '.').stripSuffix(".class"), false, classLoader) + val classes = mutable.HashSet[Class[_]]() + for (entry <- enums if entry.endsWith(".class")) { + try { + classes += Class.forName(entry.replace('/', '.').stripSuffix(".class"), false, classLoader) + } catch { + case _: Throwable => println("Unable to load:" + entry) + } + } classes } } -- cgit v1.2.3