Skip to content

[SPARK-45272][BUILD][INFRA][DOCS] Remove Scala version specific comments, and scala-2.13 profile usage#43049

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.

Sign up for GitHub

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

Closed
HyukjinKwon wants to merge 2 commits into from
Closed

[SPARK-45272][BUILD][INFRA][DOCS] Remove Scala version specific comments, and scala-2.13 profile usage #43049

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
970a097
profile cleanup
HyukjinKwon Sep 22, 2023
88ba14b
Comment out scala-2.13 instead of removing them
HyukjinKwon Sep 22, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 0 additions & 2 deletions connector/avro/pom.xml
View file
Open in desktop
Original file line numberDiff line numberDiff line change
Expand Up@@ -70,12 +70,10 @@
<groupId>org.apache.spark</groupId>
<artifactId>spark-tags_${scala.binary.version}</artifactId>
</dependency>
<!-- #if scala-2.13 -->

@LuciferYangLuciferYangSep 22, 2023•
edited
Loading

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Are we sure about deleting these comments? I was hesitant before, because I didn't know if we would need to use a similar approach to dynamically change dependencies when supporting Scala 3, so I took a conservative approach and kept them.

@HyukjinKwonHyukjinKwonSep 22, 2023

Copy link
Copy Markdown
MemberAuthor

Choose a reason for hiding this comment

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

Do we need to change scala-parallel-collections for Scala 3 upgrade? I would think we'd need it for other libraries. I am not removing the mechanism.

@LuciferYangLuciferYangSep 22, 2023

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

It seems that for Scala 3 the scala.binary.version is always 3? Then it looks like we can delete this comment, the corresponding dependency for Scala 3 is

<dependency>
<groupId>org.scala-lang.modules</groupId>
<artifactId>scala-parallel-collections_3</artifactId>
<version>1.0.4</version>
</dependency>

It seems that this dependency no longer needs to be dynamically changed.

<dependency>
<groupId>org.scala-lang.modules</groupId>
<artifactId>scala-parallel-collections_${scala.binary.version}</artifactId>
</dependency>
<!-- #endif scala-2.13 -->
<dependency>
<groupId>org.tukaani</groupId>
<artifactId>xz</artifactId>
Expand Down
2 changes: 0 additions & 2 deletions connector/connect/server/pom.xml
View file
Open in desktop
Original file line numberDiff line numberDiff line change
Expand Up@@ -152,12 +152,10 @@
</exclusion>
</exclusions>
</dependency>
<!-- #if scala-2.13 -->
<dependency>
<groupId>org.scala-lang.modules</groupId>
<artifactId>scala-parallel-collections_${scala.binary.version}</artifactId>
</dependency>
<!-- #endif scala-2.13 -->
<dependency>
<groupId>com.google.guava</groupId>
<artifactId>guava</artifactId>
Expand Down
2 changes: 0 additions & 2 deletions connector/kafka-0-10-sql/pom.xml
View file
Open in desktop
Original file line numberDiff line numberDiff line change
Expand Up@@ -74,12 +74,10 @@
<type>test-jar</type>
<scope>test</scope>
</dependency>
<!-- #if scala-2.13 -->
<dependency>
<groupId>org.scala-lang.modules</groupId>
<artifactId>scala-parallel-collections_${scala.binary.version}</artifactId>
</dependency>
<!-- #endif scala-2.13 -->
<dependency>
<groupId>org.apache.kafka</groupId>
<artifactId>kafka-clients</artifactId>
Expand Down
2 changes: 0 additions & 2 deletions connector/kafka-0-10/pom.xml
View file
Open in desktop
Original file line numberDiff line numberDiff line change
Expand Up@@ -59,12 +59,10 @@
<type>test-jar</type>
<scope>test</scope>
</dependency>
<!-- #if scala-2.13 -->
<dependency>
<groupId>org.scala-lang.modules</groupId>
<artifactId>scala-parallel-collections_${scala.binary.version}</artifactId>
</dependency>
<!-- #endif scala-2.13 -->
<dependency>
<groupId>org.apache.kafka</groupId>
<artifactId>kafka-clients</artifactId>
Expand Down
2 changes: 0 additions & 2 deletions connector/protobuf/pom.xml
View file
Open in desktop
Original file line numberDiff line numberDiff line change
Expand Up@@ -70,12 +70,10 @@
<groupId>org.apache.spark</groupId>
<artifactId>spark-tags_${scala.binary.version}</artifactId>
</dependency>
<!-- #if scala-2.13 -->
<dependency>
<groupId>org.scala-lang.modules</groupId>
<artifactId>scala-parallel-collections_${scala.binary.version}</artifactId>
</dependency>
<!-- #endif scala-2.13 -->
<dependency>
<groupId>com.google.protobuf</groupId>
<artifactId>protobuf-java</artifactId>
Expand Down
2 changes: 0 additions & 2 deletions core/pom.xml
View file
Open in desktop
Original file line numberDiff line numberDiff line change
Expand Up@@ -35,12 +35,10 @@
</properties>

<dependencies>
<!-- #if scala-2.13 -->
<dependency>
<groupId>org.scala-lang.modules</groupId>
<artifactId>scala-parallel-collections_${scala.binary.version}</artifactId>
</dependency>
<!-- #endif scala-2.13 -->
<dependency>
<groupId>org.apache.avro</groupId>
<artifactId>avro</artifactId>
Expand Down
8 changes: 4 additions & 4 deletions dev/mima
View file
Open in desktop
Original file line numberDiff line numberDiff line change
Expand Up@@ -24,9 +24,9 @@ set -e
FWDIR="$(cd "`dirname "$0"`"/..; pwd)"
cd "$FWDIR"

SPARK_PROFILES=${1:-"-Pscala-2.13 -Pmesos -Pkubernetes -Pyarn -Pspark-ganglia-lgpl -Pkinesis-asl -Phive-thriftserver -Phive"}
TOOLS_CLASSPATH="$(build/sbt -Pscala-2.13 -DcopyDependencies=false "export tools/fullClasspath" | grep jar | tail -n1)"
OLD_DEPS_CLASSPATH="$(build/sbt -Pscala-2.13 -DcopyDependencies=false $SPARK_PROFILES "export oldDeps/fullClasspath" | grep jar | tail -n1)"

@dongjoon-hyundongjoon-hyunSep 22, 2023•
edited
Loading

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

This is good. What I conceded is only pom.xml file changes. Oh, sorry. I missed that this is also directly related.

@LuciferYangLuciferYangSep 22, 2023•
edited
Loading

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

While cleaning this up, I suggest removing 2.13 from VALID_VERSIONS in the change-scala-version.sh script too, otherwise if we execute dev/change-scala-version.sh 2.13, these files will generate some unnecessary git diff.

spark/dev/change-scala-version.sh

Line 22 in 5f643ee

VALID_VERSIONS=( 2.13 )

@HyukjinKwonHyukjinKwonSep 22, 2023

Copy link
Copy Markdown
MemberAuthor

Choose a reason for hiding this comment

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

I commented dev/change-scala-version.sh usage out in the README.md :-).

SPARK_PROFILES=${1:-"-Pmesos -Pkubernetes -Pyarn -Pspark-ganglia-lgpl -Pkinesis-asl -Phive-thriftserver -Phive"}
TOOLS_CLASSPATH="$(build/sbt -DcopyDependencies=false "export tools/fullClasspath" | grep jar | tail -n1)"
OLD_DEPS_CLASSPATH="$(build/sbt -DcopyDependencies=false $SPARK_PROFILES "export oldDeps/fullClasspath" | grep jar | tail -n1)"

rm -f .generated-mima*

Expand All@@ -42,7 +42,7 @@ $JAVA_CMD \
-cp "$TOOLS_CLASSPATH:$OLD_DEPS_CLASSPATH" \
org.apache.spark.tools.GenerateMIMAIgnore

echo -e "q\n" | build/sbt -Pscala-2.13 -mem 5632 -DcopyDependencies=false "$@" mimaReportBinaryIssues | grep -v -e "info.*Resolving"
echo -e "q\n" | build/sbt -mem 5632 -DcopyDependencies=false "$@" mimaReportBinaryIssues | grep -v -e "info.*Resolving"
ret_val=$?

if [ $ret_val != 0 ]; then
Expand Down
12 changes: 6 additions & 6 deletions docs/_plugins/copy_api_dirs.rb
View file
Open in desktop
Original file line numberDiff line numberDiff line change
Expand Up@@ -26,8 +26,8 @@
curr_dir = pwd
cd("..")

puts "Running 'build/sbt -Pscala-2.13 -Pkinesis-asl clean compile unidoc' from " + pwd + "; this may take a few minutes..."
system("build/sbt -Pscala-2.13 -Pkinesis-asl clean compile unidoc") || raise("Unidoc generation failed")
puts "Running 'build/sbt -Pkinesis-asl clean compile unidoc' from " + pwd + "; this may take a few minutes..."
system("build/sbt -Pkinesis-asl clean compile unidoc") || raise("Unidoc generation failed")

puts "Moving back into docs dir."
cd("docs")
Expand DownExpand Up@@ -119,8 +119,8 @@
puts "Moving to project root and building API docs."
cd("..")

puts "Running 'build/sbt -Pscala-2.13 clean package -Phive' from " + pwd + "; this may take a few minutes..."
system("build/sbt -Pscala-2.13 clean package -Phive") || raise("PySpark doc generation failed")
puts "Running 'build/sbt clean package -Phive' from " + pwd + "; this may take a few minutes..."
system("build/sbt clean package -Phive") || raise("PySpark doc generation failed")

puts "Moving back into docs dir."
cd("docs")
Expand DownExpand Up@@ -165,8 +165,8 @@
puts "Moving to project root and building API docs."
cd("..")

puts "Running 'build/sbt -Pscala-2.13 clean package -Phive' from " + pwd + "; this may take a few minutes..."
system("build/sbt -Pscala-2.13 clean package -Phive") || raise("SQL doc generation failed")
puts "Running 'build/sbt clean package -Phive' from " + pwd + "; this may take a few minutes..."
system("build/sbt clean package -Phive") || raise("SQL doc generation failed")

puts "Moving back into docs dir."
cd("docs")
Expand Down
2 changes: 2 additions & 0 deletions docs/building-spark.md
View file
Open in desktop
Original file line numberDiff line numberDiff line change
Expand Up@@ -261,6 +261,7 @@ or

./build/sbt docker-integration-tests/test

<!---
## Change Scala Version

When other versions of Scala like 2.13 are supported, it will be possible to build for that version.
Expand All@@ -275,6 +276,7 @@ Enable the profile (e.g. 2.13):

# For sbt
./build/sbt -Pscala-2.13 compile
-->

## Running Jenkins tests with GitHub Enterprise

Expand Down
2 changes: 0 additions & 2 deletions mllib/pom.xml
View file
Open in desktop
Original file line numberDiff line numberDiff line change
Expand Up@@ -91,12 +91,10 @@
<type>test-jar</type>
<scope>test</scope>
</dependency>
<!-- #if scala-2.13 -->
<dependency>
<groupId>org.scala-lang.modules</groupId>
<artifactId>scala-parallel-collections_${scala.binary.version}</artifactId>
</dependency>
<!-- #endif scala-2.13 -->
<dependency>
<groupId>org.scalanlp</groupId>
<artifactId>breeze_${scala.binary.version}</artifactId>
Expand Down
12 changes: 6 additions & 6 deletions pom.xml
View file
Open in desktop
Original file line numberDiff line numberDiff line change
Expand Up@@ -440,13 +440,11 @@
<version>${project.version}</version>
<type>test-jar</type>
</dependency>
<!-- #if scala-2.13 -->
<dependency>
<groupId>org.scala-lang.modules</groupId>
<artifactId>scala-parallel-collections_${scala.binary.version}</artifactId>
<version>1.0.4</version>
</dependency>
<!-- #endif scala-2.13 -->
<dependency>
<groupId>com.twitter</groupId>
<artifactId>chill_${scala.binary.version}</artifactId>
Expand DownExpand Up@@ -3648,13 +3646,14 @@
</properties>
</profile>

<!--
SPARK-34774 Add this property to ensure change-scala-version.sh can replace the public `scala.version`
property correctly.
-->

@HyukjinKwonHyukjinKwonSep 22, 2023

Copy link
Copy Markdown
MemberAuthor

Choose a reason for hiding this comment

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

Nesting a comment doesn't work .. so I took it out.

<!--
<profile>
<id>scala-2.13</id>
Comment thread
HyukjinKwon marked this conversation as resolved.
<properties>
<!--
SPARK-34774 Add this property to ensure change-scala-version.sh can replace the public `scala.version`
property correctly.
-->
<scala.version>2.13.11</scala.version>
</properties>
<build>
Expand All@@ -3664,6 +3663,7 @@
</pluginManagement>
</build>
</profile>
-->

<!--
This is a profile to enable the use of the ASF snapshot and staging repositories
Expand Down
2 changes: 0 additions & 2 deletions sql/catalyst/pom.xml
View file
Open in desktop
Original file line numberDiff line numberDiff line change
Expand Up@@ -83,12 +83,10 @@
<artifactId>spark-sketch_${scala.binary.version}</artifactId>
<version>${project.version}</version>
</dependency>
<!-- #if scala-2.13 -->
<dependency>
<groupId>org.scala-lang.modules</groupId>
<artifactId>scala-parallel-collections_${scala.binary.version}</artifactId>
</dependency>
<!-- #endif scala-2.13 -->
<dependency>
<groupId>org.scalacheck</groupId>
<artifactId>scalacheck_${scala.binary.version}</artifactId>
Expand Down
2 changes: 0 additions & 2 deletions sql/core/pom.xml
View file
Open in desktop
Original file line numberDiff line numberDiff line change
Expand Up@@ -89,12 +89,10 @@
<scope>test</scope>
</dependency>

<!-- #if scala-2.13 -->
<dependency>
<groupId>org.scala-lang.modules</groupId>
<artifactId>scala-parallel-collections_${scala.binary.version}</artifactId>
</dependency>
<!-- #endif scala-2.13 -->
<dependency>
<groupId>org.apache.orc</groupId>
<artifactId>orc-core</artifactId>
Expand Down
2 changes: 0 additions & 2 deletions sql/hive-thriftserver/pom.xml
View file
Open in desktop
Original file line numberDiff line numberDiff line change
Expand Up@@ -61,12 +61,10 @@
<type>test-jar</type>
<scope>test</scope>
</dependency>
<!-- #if scala-2.13 -->
<dependency>
<groupId>org.scala-lang.modules</groupId>
<artifactId>scala-parallel-collections_${scala.binary.version}</artifactId>
</dependency>
<!-- #endif scala-2.13 -->
<dependency>
<groupId>com.google.guava</groupId>
<artifactId>guava</artifactId>
Expand Down
2 changes: 0 additions & 2 deletions sql/hive/pom.xml
View file
Open in desktop
Original file line numberDiff line numberDiff line change
Expand Up@@ -79,12 +79,10 @@
<type>test-jar</type>
<scope>test</scope>
</dependency>
<!-- #if scala-2.13 -->
<dependency>
<groupId>org.scala-lang.modules</groupId>
<artifactId>scala-parallel-collections_${scala.binary.version}</artifactId>
</dependency>
<!-- #endif scala-2.13 -->
<dependency>
<groupId>${hive.group}</groupId>
<artifactId>hive-common</artifactId>
Expand Down
2 changes: 0 additions & 2 deletions streaming/pom.xml
View file
Open in desktop
Original file line numberDiff line numberDiff line change
Expand Up@@ -50,12 +50,10 @@
<groupId>org.apache.spark</groupId>
<artifactId>spark-tags_${scala.binary.version}</artifactId>
</dependency>
<!-- #if scala-2.13 -->
<dependency>
<groupId>org.scala-lang.modules</groupId>
<artifactId>scala-parallel-collections_${scala.binary.version}</artifactId>
</dependency>
<!-- #endif scala-2.13 -->

<!--
This spark-tags test-dep is needed even though it isn't used in this module, otherwise testing-cmds that exclude
Expand Down