Skip to content

Commit

Permalink
[SPARK-47770][INFRA] Fix GenerateMIMAIgnore.isPackagePrivateModule
Browse files Browse the repository at this point in the history
…to return `false` instead of failing

### What changes were proposed in this pull request?

This PR aims to fix `GenerateMIMAIgnore.isPackagePrivateModule` to work correctly.

For example, `Metadata` is a case class inside package private `DefaultParamsReader` class. Currently, MIMA fails at this class analysis.

https://github.com/apache/spark/blob/f8e652e88320528a70e605a6a3cf986725e153a5/mllib/src/main/scala/org/apache/spark/ml/util/ReadWrite.scala#L474-L485

The root cause is `isPackagePrivateModule` fails due to `scala.ScalaReflectionException`. We can simply make `isPackagePrivateModule` return `false`  instead of failing.
```
Error instrumenting class:org.apache.spark.ml.util.DefaultParamsReader$Metadata
Exception in thread "main" scala.ScalaReflectionException: type Serializable is not a class
	at scala.reflect.api.Symbols$SymbolApi.asClass(Symbols.scala:284)
	at scala.reflect.api.Symbols$SymbolApi.asClass$(Symbols.scala:284)
	at scala.reflect.internal.Symbols$SymbolContextApiImpl.asClass(Symbols.scala:99)
	at scala.reflect.runtime.JavaMirrors$JavaMirror.classToScala1(JavaMirrors.scala:1085)
	at scala.reflect.runtime.JavaMirrors$JavaMirror.$anonfun$classToScala$1(JavaMirrors.scala:1040)
	at scala.reflect.runtime.JavaMirrors$JavaMirror.$anonfun$toScala$1(JavaMirrors.scala:150)
	at scala.reflect.runtime.TwoWayCaches$TwoWayCache.toScala(TwoWayCaches.scala:50)
	at scala.reflect.runtime.JavaMirrors$JavaMirror.toScala(JavaMirrors.scala:148)
	at scala.reflect.runtime.JavaMirrors$JavaMirror.classToScala(JavaMirrors.scala:1040)
	at scala.reflect.runtime.JavaMirrors$JavaMirror.typeToScala(JavaMirrors.scala:1148)
	at scala.reflect.runtime.JavaMirrors$JavaMirror$FromJavaClassCompleter.$anonfun$completeRest$2(JavaMirrors.scala:816)
	at scala.reflect.runtime.JavaMirrors$JavaMirror$FromJavaClassCompleter.$anonfun$completeRest$1(JavaMirrors.scala:816)
	at scala.reflect.runtime.JavaMirrors$JavaMirror$FromJavaClassCompleter.completeRest(JavaMirrors.scala:810)
	at scala.reflect.runtime.JavaMirrors$JavaMirror$FromJavaClassCompleter.complete(JavaMirrors.scala:806)
	at scala.reflect.internal.Symbols$Symbol.completeInfo(Symbols.scala:1575)
	at scala.reflect.internal.Symbols$Symbol.info(Symbols.scala:1538)
	at scala.reflect.runtime.SynchronizedSymbols$SynchronizedSymbol$$anon$13.scala$reflect$runtime$SynchronizedSymbols$SynchronizedSymbol$$super$info(SynchronizedSymbols.scala:221)
	at scala.reflect.runtime.SynchronizedSymbols$SynchronizedSymbol.info(SynchronizedSymbols.scala:158)
	at scala.reflect.runtime.SynchronizedSymbols$SynchronizedSymbol.info$(SynchronizedSymbols.scala:158)
	at scala.reflect.runtime.SynchronizedSymbols$SynchronizedSymbol$$anon$13.info(SynchronizedSymbols.scala:221)
	at scala.reflect.internal.Symbols$Symbol.initialize(Symbols.scala:1733)
	at scala.reflect.runtime.SynchronizedSymbols$SynchronizedSymbol.privateWithin(SynchronizedSymbols.scala:109)
	at scala.reflect.runtime.SynchronizedSymbols$SynchronizedSymbol.privateWithin$(SynchronizedSymbols.scala:107)
	at scala.reflect.runtime.SynchronizedSymbols$SynchronizedSymbol$$anon$13.privateWithin(SynchronizedSymbols.scala:221)
	at scala.reflect.runtime.SynchronizedSymbols$SynchronizedSymbol$$anon$13.privateWithin(SynchronizedSymbols.scala:221)
	at org.apache.spark.tools.GenerateMIMAIgnore$.isPackagePrivateModule(GenerateMIMAIgnore.scala:48)
	at org.apache.spark.tools.GenerateMIMAIgnore$.$anonfun$privateWithin$1(GenerateMIMAIgnore.scala:67)
	at scala.collection.immutable.List.foreach(List.scala:334)
	at org.apache.spark.tools.GenerateMIMAIgnore$.privateWithin(GenerateMIMAIgnore.scala:61)
	at org.apache.spark.tools.GenerateMIMAIgnore$.main(GenerateMIMAIgnore.scala:125)
	at org.apache.spark.tools.GenerateMIMAIgnore.main(GenerateMIMAIgnore.scala)
```

### Why are the changes needed?

**BEFORE**
```
$ dev/mima | grep org.apache.spark.ml.util.DefaultParamsReader
Using SPARK_LOCAL_IP=localhost
Using SPARK_LOCAL_IP=localhost
Error instrumenting class:org.apache.spark.ml.util.DefaultParamsReader$Metadata$
Error instrumenting class:org.apache.spark.ml.util.DefaultParamsReader$Metadata
Using SPARK_LOCAL_IP=localhost

# I checked the following before deleing `.generated-mima-class-excludes `
$ cat .generated-mima-class-excludes | grep org.apache.spark.ml.util.DefaultParamsReader
org.apache.spark.ml.util.DefaultParamsReader$
org.apache.spark.ml.util.DefaultParamsReader#
org.apache.spark.ml.util.DefaultParamsReader
```

**AFTER**
```
$ dev/mima | grep org.apache.spark.ml.util.DefaultParamsReader
Using SPARK_LOCAL_IP=localhost
Using SPARK_LOCAL_IP=localhost
[WARN] Unable to detect inner functions for class:org.apache.spark.ml.util.DefaultParamsReader.Metadata
[WARN] Unable to detect inner functions for class:org.apache.spark.ml.util.DefaultParamsReader.Metadata
Using SPARK_LOCAL_IP=localhost

# I checked the following before deleting `.generated-mima-class-excludes `.
$ cat .generated-mima-class-excludes | grep org.apache.spark.ml.util.DefaultParamsReader
org.apache.spark.ml.util.DefaultParamsReader$Metadata$
org.apache.spark.ml.util.DefaultParamsReader$
org.apache.spark.ml.util.DefaultParamsReader#Metadata#
org.apache.spark.ml.util.DefaultParamsReader#
org.apache.spark.ml.util.DefaultParamsReader$Metadata
org.apache.spark.ml.util.DefaultParamsReader#Metadata
org.apache.spark.ml.util.DefaultParamsReader
```

### Does this PR introduce _any_ user-facing change?

No.

### How was this patch tested?

Manual tests.

### Was this patch authored or co-authored using generative AI tooling?

No.

Closes #45938 from dongjoon-hyun/SPARK-47770.

Authored-by: Dongjoon Hyun <[email protected]>
Signed-off-by: yangjie01 <[email protected]>
  • Loading branch information
dongjoon-hyun authored and LuciferYang committed Apr 9, 2024
1 parent fea1994 commit 08c4963
Showing 1 changed file with 8 additions and 1 deletion.
Original file line number Diff line number Diff line change
Expand Up @@ -44,8 +44,15 @@ object GenerateMIMAIgnore {
private def isPackagePrivate(sym: unv.Symbol) =
!sym.privateWithin.fullName.startsWith("<none>")

private def isPackagePrivateModule(moduleSymbol: unv.ModuleSymbol) =
private def isPackagePrivateModule(moduleSymbol: unv.ModuleSymbol) = try {
!moduleSymbol.privateWithin.fullName.startsWith("<none>")
} catch {
case e: Throwable =>
// scalastyle:off println
println("[WARN] Unable to check module:" + moduleSymbol)
// scalastyle:on println
false
}

/**
* For every class checks via scala reflection if the class itself or contained members
Expand Down

0 comments on commit 08c4963

Please sign in to comment.