[SPARK-50891][BUILD] Remove the explicit dependency on Guava from plugins.sbt
#49550
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
LuciferYang
commented
Jan 17, 2025
| @@ -21,9 +21,6 @@ addSbtPlugin("software.purpledragon" % "sbt-checkstyle-plugin" % "4.0.1") | |||
| // please check pom.xml in the root of the source tree too. | |||
| libraryDependencies += "com.puppycrawl.tools" % "checkstyle" % "10.20.2" | |||
|
|
|||
| // checkstyle uses guava 33.3.1-jre. | |||
| libraryDependencies += "com.google.guava" % "guava" % "33.3.1-jre" | |||
There was a problem hiding this comment.
@HyukjinKwon , do you still remember the reason for adding an explicit dependency on Guava in #21399? I found that even without configuring it, the execution of dev/sbt-checkstyle meets the expectations.
There was a problem hiding this comment.
However, I found that before this pr, the format of the check results had already changed to something similar to
Checkstyle failed at following occurrences:
[error] java.lang.RuntimeException: Severity of checkstyle errors exceeds project limit
[error] at scala.sys.package$.error(package.scala:30)
[error] at com.etsy.sbt.checkstyle.Checkstyle$.checkstyle(Checkstyle.scala:86)
[error] at com.etsy.sbt.checkstyle.CheckstylePlugin$autoImport$.$anonfun$checkstyleTask$1(CheckstylePlugin.scala:57)
[error] at com.etsy.sbt.checkstyle.CheckstylePlugin$autoImport$.$anonfun$checkstyleTask$1$adapted(CheckstylePlugin.scala:49)
[error] at scala.Function1.$anonfun$compose$1(Function1.scala:49)
[error] at sbt.internal.util.$tilde$greater.$anonfun$$u2219$1(TypeFunctions.scala:63)
[error] at sbt.std.Transform$$anon$4.work(Transform.scala:69)
[error] at sbt.Execute.$anonfun$submit$2(Execute.scala:283)
[error] at sbt.internal.util.ErrorHandling$.wideConvert(ErrorHandling.scala:24)
[error] at sbt.Execute.work(Execute.scala:292)
[error] at sbt.Execute.$anonfun$submit$1(Execute.scala:283)
[error] at sbt.ConcurrentRestrictions$$anon$4.$anonfun$submitValid$1(ConcurrentRestrictions.scala:265)
[error] at sbt.CompletionService$$anon$2.call(CompletionService.scala:65)
[error] at java.base/java.util.concurrent.FutureTask.run(FutureTask.java:264)
[error] at java.base/java.util.concurrent.Executors$RunnableAdapter.call(Executors.java:539)
[error] at java.base/java.util.concurrent.FutureTask.run(FutureTask.java:264)
[error] at java.base/java.util.concurrent.ThreadPoolExecutor.runWorker(ThreadPoolExecutor.java:1136)
[error] at java.base/java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:635)
[error] at java.base/java.lang.Thread.run(Thread.java:840)
[error] (core / checkstyle) Severity of checkstyle errors exceeds project limit
, rather than
There was a problem hiding this comment.
I do remember. It was done at 4a14dc0, and scalas scheck style failed because of Guava version mismatch (SBT specifically). Since the Guava version in plugins.sbt here doesn't affect the main code, we can just set whatever version we want.
There was a problem hiding this comment.
It doesn't affect anything in the main code, so if it works better, we can remove, or upgrade, etc. It's like if it works, then LGTM
There was a problem hiding this comment.
Okay, let me confirm again. If we can remove it, we'll have one less place to modify when upgrading Guava.
HyukjinKwon
approved these changes
Jan 20, 2025
LuciferYang
changed the title
Guava from plugins.sbtGuava from plugins.sbt
|
Merged to master and branch-4.0. |
HyukjinKwon
pushed a commit
that referenced
this pull request
Jan 20, 2025
…plugins.sbt` This pr aims to remove the explicit dependency on `Guava` from `plugins.sbt` to minimize the number of modifications needed when upgrading the Guava version. Reduce the configuration items for defining the Guava version. No - Pass GitHub Actions - Manual check `dev/sbt-checkstyle`, It can work properly. For example: https://github.com/apache/spark/blob/1c4cfcb0277d673a3f53b49e74ab74452118da91/core/src/main/java/org/apache/spark/util/collection/unsafe/sort/UnsafeInMemorySorter.java#L217-L220 Remove the line 218 from `UnsafeInMemorySorter.java`( `// checkstyle.off: RegexpSinglelineJava` )and then execute `dev/sbt-checkstyle`. **Before** ``` Checkstyle failed at following occurrences: [error] java.lang.RuntimeException: Severity of checkstyle errors exceeds project limit [error] at scala.sys.package$.error(package.scala:30) [error] at com.etsy.sbt.checkstyle.Checkstyle$.checkstyle(Checkstyle.scala:86) [error] at com.etsy.sbt.checkstyle.CheckstylePlugin$autoImport$.$anonfun$checkstyleTask$1(CheckstylePlugin.scala:57) [error] at com.etsy.sbt.checkstyle.CheckstylePlugin$autoImport$.$anonfun$checkstyleTask$1$adapted(CheckstylePlugin.scala:49) [error] at scala.Function1.$anonfun$compose$1(Function1.scala:49) [error] at sbt.internal.util.$tilde$greater.$anonfun$$u2219$1(TypeFunctions.scala:63) [error] at sbt.std.Transform$$anon$4.work(Transform.scala:69) [error] at sbt.Execute.$anonfun$submit$2(Execute.scala:283) [error] at sbt.internal.util.ErrorHandling$.wideConvert(ErrorHandling.scala:24) [error] at sbt.Execute.work(Execute.scala:292) [error] at sbt.Execute.$anonfun$submit$1(Execute.scala:283) [error] at sbt.ConcurrentRestrictions$$anon$4.$anonfun$submitValid$1(ConcurrentRestrictions.scala:265) [error] at sbt.CompletionService$$anon$2.call(CompletionService.scala:65) [error] at java.base/java.util.concurrent.FutureTask.run(FutureTask.java:264) [error] at java.base/java.util.concurrent.Executors$RunnableAdapter.call(Executors.java:539) [error] at java.base/java.util.concurrent.FutureTask.run(FutureTask.java:264) [error] at java.base/java.util.concurrent.ThreadPoolExecutor.runWorker(ThreadPoolExecutor.java:1136) [error] at java.base/java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:635) [error] at java.base/java.lang.Thread.run(Thread.java:840) [error] (core / checkstyle) Severity of checkstyle errors exceeds project limit ``` **After** ``` Checkstyle failed at following occurrences: [error] java.lang.RuntimeException: Severity of checkstyle errors exceeds project limit [error] at scala.sys.package$.error(package.scala:30) [error] at com.etsy.sbt.checkstyle.Checkstyle$.checkstyle(Checkstyle.scala:86) [error] at com.etsy.sbt.checkstyle.CheckstylePlugin$autoImport$.$anonfun$checkstyleTask$1(CheckstylePlugin.scala:57) [error] at com.etsy.sbt.checkstyle.CheckstylePlugin$autoImport$.$anonfun$checkstyleTask$1$adapted(CheckstylePlugin.scala:49) [error] at scala.Function1.$anonfun$compose$1(Function1.scala:49) [error] at sbt.internal.util.$tilde$greater.$anonfun$$u2219$1(TypeFunctions.scala:63) [error] at sbt.std.Transform$$anon$4.work(Transform.scala:69) [error] at sbt.Execute.$anonfun$submit$2(Execute.scala:283) [error] at sbt.internal.util.ErrorHandling$.wideConvert(ErrorHandling.scala:24) [error] at sbt.Execute.work(Execute.scala:292) [error] at sbt.Execute.$anonfun$submit$1(Execute.scala:283) [error] at sbt.ConcurrentRestrictions$$anon$4.$anonfun$submitValid$1(ConcurrentRestrictions.scala:265) [error] at sbt.CompletionService$$anon$2.call(CompletionService.scala:65) [error] at java.base/java.util.concurrent.FutureTask.run(FutureTask.java:264) [error] at java.base/java.util.concurrent.Executors$RunnableAdapter.call(Executors.java:539) [error] at java.base/java.util.concurrent.FutureTask.run(FutureTask.java:264) [error] at java.base/java.util.concurrent.ThreadPoolExecutor.runWorker(ThreadPoolExecutor.java:1136) [error] at java.base/java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:635) [error] at java.base/java.lang.Thread.run(Thread.java:840) [error] (core / checkstyle) Severity of checkstyle errors exceeds project limit ``` The displayed check results show no difference. No Closes #49550 from LuciferYang/remove-guava-from-plugins. Authored-by: yangjie01 <[email protected]> Signed-off-by: Hyukjin Kwon <[email protected]> (cherry picked from commit 4874b6e) Signed-off-by: Hyukjin Kwon <[email protected]>
|
Thanks @HyukjinKwon |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
What changes were proposed in this pull request?
This pr aims to remove the explicit dependency on
Guavafromplugins.sbtto minimize the number of modifications needed when upgrading the Guava version.Why are the changes needed?
Reduce the configuration items for defining the Guava version.
Does this PR introduce any user-facing change?
No
How was this patch tested?
dev/sbt-checkstyle, It can work properly.For example:
spark/core/src/main/java/org/apache/spark/util/collection/unsafe/sort/UnsafeInMemorySorter.java
Lines 217 to 220 in 1c4cfcb
Remove the line 218 from
UnsafeInMemorySorter.java(// checkstyle.off: RegexpSinglelineJava)and then executedev/sbt-checkstyle.Before
After
The displayed check results show no difference.
Was this patch authored or co-authored using generative AI tooling?
No