Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[gradle] exclude scala-reflect #13894

Merged
merged 1 commit into from Oct 25, 2023

Conversation

danking
Copy link
Collaborator

@danking danking commented Oct 24, 2023

CHANGELOG: Fixes #13837 in which Hail could break a Spark installation if the Hail JAR appears on the classpath before the Scala JARs.

We and several dependencies of ours are exposing an old version of scala-reflect (rude of us and them). If our JAR appears on the classpath before the scala JARs and the sub-versions of Scala differ, Spark is likely to have incompatible bytecode.

See #13837 .

In this PR, this command,

make shadowJar && \
    jar tf build/libs/hail-all-spark.jar | grep scala/reflect | wc -l && \
    jar tf build/libs/hail-all-spark.jar | grep 'MutableSettings'

prints:

    0

On main (5d7dc2cab7) it prints:

    1417
scala/reflect/internal/settings/MutableSettings$.class
scala/reflect/internal/settings/MutableSettings$SettingValue.class
scala/reflect/internal/settings/MutableSettings.class

A bit more details follow for the curious.

./gradlew dependencies shows these packages as depending on scala-reflect:

+--- org.scalanlp:breeze-natives_2.12:1.1
|    +--- org.scala-lang:scala-library:2.12.10 -> 2.12.17
|    +--- org.scalanlp:breeze_2.12:1.1 -> 1.2
|    |    +--- org.scala-lang:scala-library:2.12.10 -> 2.12.17
|    |    +--- org.scalanlp:breeze-macros_2.12:1.2
|    |    |    +--- org.scala-lang:scala-library:2.12.10 -> 2.12.17
|    |    |    \--- org.scala-lang:scala-reflect:2.12.10 -> 2.12.15
...
+--- org.elasticsearch:elasticsearch-spark-30_2.12:8.4.3
|    +--- org.scala-lang:scala-library:2.12.8 -> 2.12.17
|    +--- org.scala-lang:scala-reflect:2.12.8 -> 2.12.15 (*)
|    \--- org.apache.spark:spark-core_2.12:3.2.1 -> 3.3.0
...
|         +--- org.scala-lang:scala-reflect:2.12.15 (*)
...
+--- org.scala-lang:scala-reflect:2.12.15 (*)
...
+--- org.apache.spark:spark-sql_2.12:3.3.0
...
|    +--- org.apache.spark:spark-catalyst_2.12:3.3.0
|    |    +--- org.scala-lang:scala-reflect:2.12.15 (*)

This root (the fourth to last package above):

+--- org.scala-lang:scala-reflect:2.12.15 (*)

is coming from this line in build.gradle:

    shadow 'org.scala-lang:scala-reflect:' + scalaVersion

I think shadow means "needed to compile but do not include in shadow JAR", so I do not know if we can trust ./gradlew dependencies to tell us why any particular class file is in our shadow JAR.

CHANGELOG: Fixes hail-is#13837 in which Hail could break a Spark installation if the Hail JAR appears on the classpath before the Scala JARs.

Several dependencies of ours *and us* are exposing an old version of scala-reflect (rude of us and
them). If our JAR appears on the classpath before the scala JARs and the sub-versions of Scala
differ, Spark is likely to have incompatible bytecode.

See hail-is#13837 .

In this PR, this command,

```
make shadowJar && \
    jar tf build/libs/hail-all-spark.jar | grep scala/reflect | wc -l && \
    jar tf build/libs/hail-all-spark.jar | grep 'MutableSettings'
```

prints:

```
    0

```

On main (`5d7dc2cab7`) it prints:

```
    1417
scala/reflect/internal/settings/MutableSettings$.class
scala/reflect/internal/settings/MutableSettings$SettingValue.class
scala/reflect/internal/settings/MutableSettings.class
```

---

A bit more details follow for the curious.

`./gradlew dependencies` shows these packages as depending on `scala-reflect`:

```
+--- org.scalanlp:breeze-natives_2.12:1.1
|    +--- org.scala-lang:scala-library:2.12.10 -> 2.12.17
|    +--- org.scalanlp:breeze_2.12:1.1 -> 1.2
|    |    +--- org.scala-lang:scala-library:2.12.10 -> 2.12.17
|    |    +--- org.scalanlp:breeze-macros_2.12:1.2
|    |    |    +--- org.scala-lang:scala-library:2.12.10 -> 2.12.17
|    |    |    \--- org.scala-lang:scala-reflect:2.12.10 -> 2.12.15
...
+--- org.elasticsearch:elasticsearch-spark-30_2.12:8.4.3
|    +--- org.scala-lang:scala-library:2.12.8 -> 2.12.17
|    +--- org.scala-lang:scala-reflect:2.12.8 -> 2.12.15 (*)
|    \--- org.apache.spark:spark-core_2.12:3.2.1 -> 3.3.0
...
|         +--- org.scala-lang:scala-reflect:2.12.15 (*)
...
+--- org.scala-lang:scala-reflect:2.12.15 (*)
...
+--- org.apache.spark:spark-sql_2.12:3.3.0
...
|    +--- org.apache.spark:spark-catalyst_2.12:3.3.0
|    |    +--- org.scala-lang:scala-reflect:2.12.15 (*)
```

This root (the fourth to last package above):

```
+--- org.scala-lang:scala-reflect:2.12.15 (*)
```

is coming from this line in build.gradle:

```gradle
    shadow 'org.scala-lang:scala-reflect:' + scalaVersion
```

I think `shadow` means "needed to compile but do not include in shadow JAR", so I do not know if we
can trust `./gradlew dependencies` to tell us why any particular class file is in our shadow JAR.
@danking
Copy link
Collaborator Author

danking commented Oct 25, 2023

bump; I'd like to include this in 0.2.125

@danking danking merged commit 0402aad into hail-is:main Oct 25, 2023
8 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

hail v0.2.124 on AWS - java error
4 participants