square / gradle-dependencies-sorter

A CLI app and Gradle plugin to sort the dependencies in your Gradle build scripts
Apache License 2.0
261 stars 12 forks source link

[Question] Is icu4j dependency required on the runtime classpath? #14

Closed SimonMarquis closed 1 year ago

SimonMarquis commented 1 year ago

The packaged app is currently ~19MiB, and most of this size comes from IBM ICU resources. I was wondering if these resources are really needed at runtime or if they should only be there at compile time.

The dependency comes from: :sort → :grammar → org.antlr:antlr4:4.10.1 → com.ibm.icu:icu4j:69.1

And here is the complete dependency graph for the runtimeClasspath configuration:

------------------------------------------------------------
Project ':app'
------------------------------------------------------------

runtimeClasspath - Runtime classpath of compilation 'main' (target  (jvm)).
+--- org.jetbrains.kotlin:kotlin-stdlib-jdk8:1.7.20
|    +--- org.jetbrains.kotlin:kotlin-stdlib:1.7.20
|    |    +--- org.jetbrains.kotlin:kotlin-stdlib-common:1.7.20
|    |    \--- org.jetbrains:annotations:13.0
|    \--- org.jetbrains.kotlin:kotlin-stdlib-jdk7:1.7.20
|         \--- org.jetbrains.kotlin:kotlin-stdlib:1.7.20 (*)
+--- org.jetbrains.kotlin:kotlin-bom:1.7.20
|    +--- org.jetbrains.kotlin:kotlin-stdlib-jdk8:1.7.20 (c)
|    +--- org.jetbrains.kotlin:kotlin-stdlib:1.7.20 (c)
|    +--- org.jetbrains.kotlin:kotlin-stdlib-jdk7:1.7.20 (c)
|    \--- org.jetbrains.kotlin:kotlin-stdlib-common:1.7.20 (c)
+--- project :sort
|    +--- project :grammar
|    |    +--- org.antlr:antlr4:4.10.1
|    |    |    +--- org.antlr:antlr4-runtime:4.10.1
|    |    |    +--- org.antlr:antlr-runtime:3.5.3
|    |    |    +--- org.antlr:ST4:4.3.3
|    |    |    |    \--- org.antlr:antlr-runtime:3.5.2 -> 3.5.3
|    |    |    +--- org.abego.treelayout:org.abego.treelayout.core:1.0.3
|    |    |    +--- org.glassfish:javax.json:1.0.4
|    |    |    \--- com.ibm.icu:icu4j:69.1
|    |    +--- org.antlr:antlr4-runtime:4.10.1
|    |    +--- org.jetbrains.kotlin:kotlin-stdlib-jdk8:1.7.20 (*)
|    |    \--- org.jetbrains.kotlin:kotlin-bom:1.7.20 (*)
|    +--- org.jetbrains.kotlin:kotlin-stdlib-jdk8:1.7.20 (*)
|    \--- org.jetbrains.kotlin:kotlin-bom:1.7.20 (*)
+--- info.picocli:picocli:4.6.2
\--- org.slf4j:slf4j-simple:2.0.0-alpha7
     \--- org.slf4j:slf4j-api:2.0.0-alpha7
autonomousapps commented 1 year ago

Thanks for the issue. This is the result of assuming that the antlr project correctly declares its dependencies. In general, dependencies required at compile time are also required at runtime.

PRs that result in a smaller binary that still works are welcome.

SimonMarquis commented 1 year ago

It seems like simply removing the transitive dependency keeps the build in good shape. Although it might fail to decode non-standard chars like emojis and other special unicode characters 🤷

The new dependency graph is

------------------------------------------------------------
Project ':app'
------------------------------------------------------------

runtimeClasspath - Runtime classpath of compilation 'main' (target  (jvm)).
+--- org.jetbrains.kotlin:kotlin-stdlib-jdk8:1.8.10
|    +--- org.jetbrains.kotlin:kotlin-stdlib:1.8.10
|    |    +--- org.jetbrains.kotlin:kotlin-stdlib-common:1.8.10
|    |    \--- org.jetbrains:annotations:13.0
|    \--- org.jetbrains.kotlin:kotlin-stdlib-jdk7:1.8.10
|         \--- org.jetbrains.kotlin:kotlin-stdlib:1.8.10 (*)
+--- org.jetbrains.kotlin:kotlin-bom:1.8.10
|    +--- org.jetbrains.kotlin:kotlin-stdlib-jdk8:1.8.10 (c)
|    +--- org.jetbrains.kotlin:kotlin-stdlib:1.8.10 (c)
|    +--- org.jetbrains.kotlin:kotlin-stdlib-jdk7:1.8.10 (c)
|    \--- org.jetbrains.kotlin:kotlin-stdlib-common:1.8.10 (c)
+--- project :sort
|    +--- project :grammar
|    |    +--- org.antlr:antlr4:4.10.1
|    |    |    +--- org.antlr:antlr4-runtime:4.10.1
|    |    |    +--- org.antlr:antlr-runtime:3.5.3
|    |    |    +--- org.antlr:ST4:4.3.3
|    |    |    |    \--- org.antlr:antlr-runtime:3.5.2 -> 3.5.3
|    |    |    +--- org.abego.treelayout:org.abego.treelayout.core:1.0.3
|    |    |    \--- org.glassfish:javax.json:1.0.4
|    |    +--- org.jetbrains.kotlin:kotlin-stdlib-jdk8:1.8.10 (*)
|    |    +--- org.jetbrains.kotlin:kotlin-bom:1.8.10 (*)
|    |    \--- org.antlr:antlr4-runtime:4.10.1
|    +--- org.jetbrains.kotlin:kotlin-stdlib-jdk8:1.8.10 (*)
|    \--- org.jetbrains.kotlin:kotlin-bom:1.8.10 (*)
+--- info.picocli:picocli:4.6.2
\--- org.slf4j:slf4j-simple:2.0.5
     \--- org.slf4j:slf4j-api:2.0.5
-\--- project :sort
-     \--- project :grammar
-          \--- org.antlr:antlr4:4.10.1
-               \--- com.ibm.icu:icu4j:69.1

Which leads to this change in file size

-    18M Jul 22 11:09 sort-gradle-dependencies-app/0.4-SNAPSHOT/sort-gradle-dependencies-app-0.4-SNAPSHOT-all.jar
+   5.6M Jul 22 11:10 sort-gradle-dependencies-app/0.5-SNAPSHOT/sort-gradle-dependencies-app-0.5-SNAPSHOT-all.jar
autonomousapps commented 1 year ago

I'm not really thrilled about just accepting potential runtime failures for otherwise valid text. Can we talk about the justification for this?

SimonMarquis commented 1 year ago

icu4j was added in antlr to expose unicode categories as tokens (similar to regex categories). For example EMOJI = '\p{Emoji}'. And I don't think we should "support this" specifically. Even if we want to support some specific characters, we probably don't really need their "category name". Maybe the culprit is antlr's Gradle plugin which incorrectly adds the "tools" dependencies to the wrong classpath 🤷

FWIW: In the PR, they also talked about the size of this dependency.

autonomousapps commented 1 year ago

Ok, thanks for finding that PR! I'll respond further on the PR.