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

[WIP] SBT build support #5721

Closed
wants to merge 3 commits into from
Closed

Conversation

davidyuan1223
Copy link
Contributor

@davidyuan1223 davidyuan1223 commented Nov 17, 2023

🔍 Description

Issue References 🔗

Describe Your Solution 🔧

Currently maven build is slowly, we need a faster tool to build, so we can support sbt in project, it's quickly than maven

Types of changes 🔖

  • Bugfix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to change)

Test Plan 🧪

Behavior Without This Pull Request ⚰️

Behavior With This Pull Request 🎉

Related Unit Tests


Checklists

📝 Author Self Checklist

  • My code follows the style guidelines of this project
  • I have performed a self-review
  • I have commented my code, particularly in hard-to-understand areas
  • I have made corresponding changes to the documentation
  • My changes generate no new warnings
  • I have added tests that prove my fix is effective or that my feature works
  • New and existing unit tests pass locally with my changes
  • This patch was not authored or co-authored using Generative Tooling

📝 Committer Pre-Merge Checklist

  • Pull request title is okay.
  • No license issues.
  • Milestone correctly set?
  • Test coverage is ok
  • Assignees are selected.
  • Minimum number of approvals
  • No changes are requested

Be nice. Be informative.

@cxzl25 cxzl25 changed the title [#5396] SBT build support [KYUUBI #5396] SBT build support Nov 17, 2023
@davidyuan1223
Copy link
Contributor Author

The main error in package is the scala plugin's args in kyuubi parent pom

<arg>-P:silencer:globalFilters=.*deprecated.*</arg>
<arg>-P:silencer:globalFilters=.*Could not find any member to link for.*</arg>
<arg>-P:silencer:globalFilters=.*undefined in comment for class.*</arg>

The sbt error info is if we not override the plugin in pom

[error] bad option: -P:silencer:globalFilters=.*deprecated.*
[error] bad option: -P:silencer:globalFilters=.*Could not find any member to link for.*
[error] bad option: -P:silencer:globalFilters=.*undefined in comment for class.*
[error] bad option: -P:silencer:globalFilters=.*deprecated.*
[error] bad option: -P:silencer:globalFilters=.*Could not find any member to link for.*
[error] bad option: -P:silencer:globalFilters=.*undefined in comment for class.*

can we remove them?
@pan3793 @cfmcgrady

# When creating new tests for Spark SQL Hive, the HADOOP_CLASSPATH must contain the hive jars so
# that we can run Hive to generate the golden answer. This is not required for normal development
# or testing.
if [ -n "$HIVE_HOME" ]; then
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

consider copying this file from celeborn projects, we did some explores and optimization before

@pan3793
Copy link
Member

pan3793 commented Nov 17, 2023

remove silencer could break the Maven build. cc @cxzl25

@bowenliang123
Copy link
Contributor

Cool. What's the scope of supported modules? Could you add some examples in description for clarification, for either building all modules or part of them?

@bowenliang123 bowenliang123 marked this pull request as draft November 20, 2023 02:24
@davidyuan1223
Copy link
Contributor Author

davidyuan1223 commented Nov 20, 2023

Cool. What's the scope of supported modules? Could you add some examples in description for clarification, for either building all modules or part of them?

currently, the main prolem is sbt package can not support some arguments, #5721 (comment) so i can not give the module scope, there hava many module can not package if their pom not override the scala plugin.

i'm not sure remove those arguments will break the maven build, or overwrite the scala-plugin in the module's pom, could you give me some suggest? @cxzl25

@davidyuan1223
Copy link
Contributor Author

davidyuan1223 commented Nov 20, 2023

https://github.com/ghik/silencer/tree/master
find the root cause, silencer arg is a plugin, need added in sbt‘s KyuubiBuild, I need some time to rebuild a basic KyuubiBuild similar to SparkBuild

@davidyuan1223 davidyuan1223 changed the title [KYUUBI #5396] SBT build support [KYUUBI #5396][WIP] SBT build support Nov 20, 2023
@cfmcgrady
Copy link
Contributor

find the root cause, silencer arg is a plugin, need added in sbt‘s KyuubiBuild,

you are right.

it works after applying the following patch.

diff --git a/project/Build.scala b/project/Build.scala
index 6f728747b..bc20ebcf8 100644
--- a/project/Build.scala
+++ b/project/Build.scala
@@ -25,6 +25,8 @@ import sbtprotoc.ProtocPlugin.autoImport._
 import sbtassembly.AssemblyPlugin.autoImport._
 
 import scala.util.Properties
+import sbt.librarymanagement.{ VersionNumber, SemanticSelector }
+
 
 
 object KyuubiBuild extends PomBuild {
@@ -48,6 +50,7 @@ object KyuubiBuild extends PomBuild {
 
   val kyuubiCommon = ProjectRef(buildLocation, "kyuubi-common")
   val kyuubiServer = ProjectRef(buildLocation, "kyuubi-server")
+  val kyuubiUtilScala = ProjectRef(buildLocation, "kyuubi-util-scala")
 
 
   val spark3Client = ProjectRef(buildLocation, "spark-3-shaded")
@@ -56,13 +59,76 @@ object KyuubiBuild extends PomBuild {
   client, server, ha, _*
   ) = Seq(
     "kyuubi-rest-client", "kyuubi-servers", "worker"
-  ).map(ProjectRef(buildLocation, _)).++(Seq(kyuubiCommon, kyuubiServer, spark3Client))
+  ).map(ProjectRef(buildLocation, _)).++(Seq(kyuubiCommon, kyuubiUtilScala, kyuubiServer, spark3Client))
 
 
-  lazy val sharedSettings = Seq(
+  lazy val sharedSettings = compilerWarningSettings ++ Seq(
     SbtPomKeys.profiles.:=(profiles)
   )
 
+  val silencerVersion = "1.7.13"
+  lazy val compilerWarningSettings: Seq[sbt.Def.Setting[_]] = Seq(
+    libraryDependencies ++= {
+      if (VersionNumber(scalaVersion.value).matchesSemVer(SemanticSelector("<2.13.2"))) {
+        Seq(
+          compilerPlugin("com.github.ghik" % "silencer-plugin" % silencerVersion cross CrossVersion.full),
+          "com.github.ghik" % "silencer-lib" % silencerVersion % Provided cross CrossVersion.full
+        )
+      } else {
+        Seq.empty
+      }
+    },
+    (Compile / scalacOptions) ++= {
+      if (VersionNumber(scalaVersion.value).matchesSemVer(SemanticSelector("<2.13.2"))) {
+        Seq(
+          "-Xfatal-warnings",
+          "-deprecation",
+          "-Ywarn-unused-import",
+          "-P:silencer:globalFilters=.*deprecated.*" //regex to catch deprecation warnings and suppress them
+        )
+      } else {
+        Seq(
+          // replace -Xfatal-warnings with fine-grained configuration, since 2.13.2
+          // verbose warning on deprecation, error on all others
+          // see `scalac -Wconf:help` for details
+          "-Wconf:cat=deprecation:wv,any:e",
+          // 2.13-specific warning hits to be muted (as narrowly as possible) and addressed separately
+          "-Wunused:imports",
+          "-Wconf:cat=lint-multiarg-infix:wv",
+          "-Wconf:cat=other-nullary-override:wv",
+          "-Wconf:cat=other-match-analysis&site=org.apache.spark.sql.catalyst.catalog.SessionCatalog.lookupFunction.catalogFunction:wv",
+          "-Wconf:cat=other-pure-statement&site=org.apache.spark.streaming.util.FileBasedWriteAheadLog.readAll.readFile:wv",
+          "-Wconf:cat=other-pure-statement&site=org.apache.spark.scheduler.OutputCommitCoordinatorSuite.<local OutputCommitCoordinatorSuite>.futureAction:wv",
+          "-Wconf:cat=other-pure-statement&site=org.apache.spark.sql.streaming.sources.StreamingDataSourceV2Suite.testPositiveCase.\\$anonfun:wv",
+          // SPARK-33775 Suppress compilation warnings that contain the following contents.
+          // TODO(SPARK-33805): Undo the corresponding deprecated usage suppression rule after
+          //  fixed.
+          "-Wconf:msg=^(?=.*?method|value|type|object|trait|inheritance)(?=.*?deprecated)(?=.*?since 2.13).+$:s",
+          "-Wconf:msg=^(?=.*?Widening conversion from)(?=.*?is deprecated because it loses precision).+$:s",
+          "-Wconf:msg=Auto-application to \\`\\(\\)\\` is deprecated:s",
+          "-Wconf:msg=method with a single empty parameter list overrides method without any parameter list:s",
+          "-Wconf:msg=method without a parameter list overrides a method with a single empty one:s",
+          // SPARK-35574 Prevent the recurrence of compilation warnings related to `procedure syntax is deprecated`
+          "-Wconf:cat=deprecation&msg=procedure syntax is deprecated:e",
+          // SPARK-35496 Upgrade Scala to 2.13.7 and suppress:
+          // 1. `The outer reference in this type test cannot be checked at run time`
+          // 2. `the type test for pattern TypeA cannot be checked at runtime because it
+          //    has type parameters eliminated by erasure`
+          // 3. `abstract type TypeA in type pattern Seq[TypeA] (the underlying of
+          //    Seq[TypeA]) is unchecked since it is eliminated by erasure`
+          // 4. `fruitless type test: a value of TypeA cannot also be a TypeB`
+          "-Wconf:cat=unchecked&msg=outer reference:s",
+          "-Wconf:cat=unchecked&msg=eliminated by erasure:s",
+          "-Wconf:msg=^(?=.*?a value of type)(?=.*?cannot also be).+$:s",
+          // TODO(SPARK-43850): Remove the following suppression rules and remove `import scala.language.higherKinds`
+          // from the corresponding files when Scala 2.12 is no longer supported.
+          "-Wconf:cat=unused-imports&src=org\\/apache\\/spark\\/graphx\\/impl\\/VertexPartitionBase.scala:s",
+          "-Wconf:cat=unused-imports&src=org\\/apache\\/spark\\/graphx\\/impl\\/VertexPartitionBaseOps.scala:s"
+        )
+      }
+    }
+  )
+
   val projectsMap: Map[String, Seq[Setting[_]]] = Map.empty
 
   def enable(settings: Seq[Setting[_]])(projectRef: ProjectRef): Any = {

@codecov-commenter
Copy link

Codecov Report

All modified and coverable lines are covered by tests ✅

Comparison is base (765cdaa) 61.33% compared to head (50db01c) 61.35%.
Report is 22 commits behind head on master.

❗ Current head 50db01c differs from pull request most recent head 76ef642. Consider uploading reports for the commit 76ef642 to get more accurate results

Additional details and impacted files
@@             Coverage Diff              @@
##             master    #5721      +/-   ##
============================================
+ Coverage     61.33%   61.35%   +0.01%     
  Complexity       23       23              
============================================
  Files           607      607              
  Lines         35817    35820       +3     
  Branches       4912     4912              
============================================
+ Hits          21967    21976       +9     
+ Misses        11466    11463       -3     
+ Partials       2384     2381       -3     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@davidyuan1223
Copy link
Contributor Author

davidyuan1223 commented Nov 23, 2023

@cfmcgrady hello, could you give me some adive, i have added the property sbt.project.name for every project's pom, but the sbt can not resolve all of them, i only can get those projects

sbt:kyuubi-parent> projects
[info] In file:/Users/fuyuanyuan/Programs/code/github/kyuubi/
[info] 	   integration-tests
[info] 	 * kyuubi
[info] 	   kyuubi-assembly
[info] 	   kyuubi-chat-engine
[info] 	   kyuubi-codecov
[info] 	   kyuubi-common
[info] 	   kyuubi-ctl
[info] 	   kyuubi-download
[info] 	   kyuubi-events
[info] 	   kyuubi-extension-spark-jdbc-dialect
[info] 	   kyuubi-flink-it
[info] 	   kyuubi-flink-sql-engine
[info] 	   kyuubi-ha
[info] 	   kyuubi-hive-beeline
[info] 	   kyuubi-hive-it
[info] 	   kyuubi-hive-jdbc
[info] 	   kyuubi-hive-jdbc-shaded
[info] 	   kyuubi-hive-sql-engine
[info] 	   kyuubi-jdbc-engine
[info] 	   kyuubi-jdbc-it
[info] 	   kyuubi-metrics
[info] 	   kyuubi-rest-client
[info] 	   kyuubi-server
[info] 	   kyuubi-server-plugin
[info] 	   kyuubi-spark-authz
[info] 	   kyuubi-spark-authz-shaded
[info] 	   kyuubi-spark-connector-common
[info] 	   kyuubi-spark-connector-tpcds
[info] 	   kyuubi-spark-connector-tpch
[info] 	   kyuubi-spark-lineage
[info] 	   kyuubi-spark-sql-engine
[info] 	   kyuubi-trino-it
[info] 	   kyuubi-trino-sql-engine
[info] 	   kyuubi-util
[info] 	   kyuubi-util-scala
[info] 	   kyuubi-zookeeper
[info] 	   kyuubi-zookeeper-it

@davidyuan1223 davidyuan1223 changed the title [KYUUBI #5396][WIP] SBT build support [WIP] SBT build support Nov 26, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants