-
Notifications
You must be signed in to change notification settings - Fork 450
[CELEBORN-2379] Keep sbt and maven dependency lists in sync and cross-check both in CI #3757
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
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -19,10 +19,6 @@ | |
|
|
||
| set -ex | ||
|
|
||
| # Explicitly set locale in order to make `sort` output consistent across machines. | ||
| # See https://stackoverflow.com/questions/28881 for more details. | ||
| export LC_ALL=C | ||
|
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The way this global environment variable is written may cause some character exceptions to be displayed during Java compilation. Main PR |
||
|
|
||
| PWD=$(cd "$(dirname "$0")"/.. || exit; pwd) | ||
|
|
||
| MVN="${PWD}/build/mvn" | ||
|
|
@@ -52,7 +48,7 @@ function mvn_build_classpath() { | |
| classifier_end_index=index(jar_name, ".jar") - 1; | ||
| classifier=substr(jar_name, classifier_start_index, classifier_end_index - classifier_start_index + 1); | ||
| print artifact_id"/"version"/"classifier"/"jar_name | ||
| }' | grep -v "celeborn" | sort -u >> "${DEP_PR}" | ||
| }' | grep -v "celeborn" | LC_ALL=C sort -u >> "${DEP_PR}" | ||
| } | ||
|
|
||
| function sbt_build_client_classpath() { | ||
|
|
@@ -108,7 +104,7 @@ function sbt_process_classpath() { | |
|
|
||
| result=("${result1[@]}" "${result2[@]}") | ||
|
|
||
| echo "${result[@]}" | tr ' ' '\n' | sort -u >> "${DEP_PR}" | ||
| echo "${result[@]}" | tr ' ' '\n' | LC_ALL=C sort -u >> "${DEP_PR}" | ||
| } | ||
|
|
||
| function check_diff() { | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -688,7 +688,6 @@ object CelebornCommon { | |
| Dependencies.commonsLang3, | ||
| Dependencies.hadoopClientApi, | ||
| Dependencies.hadoopClientRuntime, | ||
| Dependencies.jdkTools, | ||
| Dependencies.leveldbJniAll, | ||
| Dependencies.roaringBitmap, | ||
| Dependencies.scalaReflect, | ||
|
|
@@ -707,6 +706,14 @@ object CelebornCommon { | |
| Dependencies.bouncycastleBcprovJdk18on, | ||
| Dependencies.bouncycastleBcpkixJdk18on | ||
| ) ++ commonUnitTestDependencies, | ||
| // maven-jdk-tools-wrapper (jdkTools) only provides jdk.tools (tools.jar) on | ||
| // JDK 8; on JDK 9+ the tools API is built into the JDK, so the wrapper is a | ||
| // no-op there. Gate it to JDK 8 to match the `JDKTools` plugin | ||
| // (project/JDKTools.scala) and the Maven `jdk-8` profile, and to keep the | ||
| // sbt/maven classpaths in sync (see dev/dependencies.sh). | ||
| libraryDependencies ++= | ||
|
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. If Flink2.3 uses sbt, it will bring additional maven-jdk-tools-wrapper dependency, which is wrong. This issue is fixed here. |
||
| (if (System.getProperty("java.specification.version").startsWith("1.")) | ||
| Seq(Dependencies.jdkTools) else Nil), | ||
|
|
||
| Compile / sourceGenerators += Def.task { | ||
| val file = (Compile / sourceManaged).value / "org" / "apache" / "celeborn" / "package.scala" | ||
|
|
@@ -1004,6 +1011,7 @@ object Spark41 extends SparkClientProjects { | |
| val scalaBinaryVersion = "2.13" | ||
|
|
||
| override val sparkColumnarShuffleVersion: String = "4" | ||
| override val paranamerVersionOverride: Option[String] = Some("2.8.3") | ||
| } | ||
|
|
||
| object Spark42 extends SparkClientProjects { | ||
|
|
@@ -1022,6 +1030,7 @@ object Spark42 extends SparkClientProjects { | |
|
|
||
| override val lz4JavaGroup = "at.yawk.lz4" | ||
| override val sparkColumnarShuffleVersion: String = "4" | ||
| override val paranamerVersionOverride: Option[String] = Some("2.8.3") | ||
| } | ||
|
|
||
| trait SparkClientProjects { | ||
|
|
@@ -1037,6 +1046,8 @@ trait SparkClientProjects { | |
| val sparkVersion: String | ||
| val zstdJniVersion: String | ||
|
|
||
| val paranamerVersionOverride: Option[String] = None | ||
|
|
||
| val includeColumnarShuffle: Boolean = true | ||
|
|
||
| def modules: Seq[Project] = { | ||
|
|
@@ -1082,7 +1093,10 @@ trait SparkClientProjects { | |
| "org.apache.spark" %% "spark-sql" % sparkVersion % "provided", | ||
| Dependencies.javaxServletApi % "test", | ||
| Dependencies.jakartaServletApi % "test" | ||
| ) ++ commonUnitTestDependencies ++ Seq(Dependencies.mockitoInline % "test") | ||
| ) ++ commonUnitTestDependencies ++ Seq(Dependencies.mockitoInline % "test"), | ||
| dependencyOverrides ++= paranamerVersionOverride | ||
| .map(v => Seq("com.thoughtworks.paranamer" % "paranamer" % v)) | ||
| .getOrElse(Seq.empty) | ||
| ) | ||
| } | ||
|
|
||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
jdk11 jdk17 does not use sbt check, so some dependencies fail in sbt, here use matrix to reduce some duplication.