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

[SPARK-44113][BUILD][INFRA][DOCS] Drop support for Scala 2.12 #43008

Closed
wants to merge 10 commits into from

Conversation

LuciferYang
Copy link
Contributor

@LuciferYang LuciferYang commented Sep 20, 2023

What changes were proposed in this pull request?

The main purpose of this PR is to remove support for Scala 2.12 in Apache Spark 4.0, the specific work includes:

  1. Run dev/change-scala-version.sh 2.13 to change the Scala version in all pom.xml files to 2.13
  2. Cleaned up the parent pom.xml configuration related to Scala 2.12 and set the Scala 2.13 configuration as the default
  3. Cleaned up the Scala 2.12 configuration in SparkBuild.scala
  4. Cleaned up the support for Scala 2.12 in python/run-tests.py and dev/run-tests.py
  5. Updated the sections related to Scala 2.12 in docs/building-spark.md, docs/index.md,docs/spark-connect-overview.md,docs/storage-openstack-swift.md,docs/_config.yml to Scala 2.13
  6. Updated the parts related to Scala 2.12 in dev/test-dependencies.sh, dev/scalafmt, dev/mima,dev/lint-scala to Scala 2.13
  7. Removed the support for Scala 2.12 in dev/change-scala-version.sh and cleaned up its invocation in Spark code
  8. Updated dev/deps/spark-deps-hadoop-3-hive-2.3 and LICENSE-binary
  9. Removed task scala-213 from build_and_test.yml because the daily test of other branches will not run this task, and the master branch already uses Scala 2.13 by default.
  10. Replaced Scala 2.12 with Scala 2.13 in the name of the following .yml files: build_ansi.yml, build_coverage.yml, build_java11.yml,build_java17.yml, build_java21.yml,build_maven.yml, and build_rockdb_as_ui_backend.yml
  11. Removed the support for Scala 2.12 in benchmark.yml
  12. Moved files from src/scala-2.13/ directory to src/main/scala and deleted all files in the src/scala-2.12/ directory.
  13. Comment out the code related to multiple Scala versions in load-spark-env.cmd and load-spark-env.sh.
  14. Clean up redundant build-helper-maven-plugin configurations from core/pom.xml, repl/pom.xml, sql/api/pom.xml, sql/catalyst/pom.xml, sql/core/pom.xml and connector/connect/common/pom.xml

Why are the changes needed?

The minimum supported Scala version for Apache Spark 4.0 is Scala 2.13.

Does this PR introduce any user-facing change?

Yes, Apache will no longer support Scala 2.12

How was this patch tested?

Pass GitHub Actions

Was this patch authored or co-authored using generative AI tooling?

No

@LuciferYang LuciferYang marked this pull request as draft September 20, 2023 06:27
@LuciferYang LuciferYang changed the title [SPARK-44113][BUILD] Drop Scala 2.12 Support [WIP][SPARK-44113][BUILD] Drop Scala 2.12 Support Sep 20, 2023
@LuciferYang LuciferYang changed the title [WIP][SPARK-44113][BUILD] Drop Scala 2.12 Support [WIP][SPARK-44113][BUILD] Drop support for Scala 2.12 Sep 20, 2023
@LuciferYang LuciferYang changed the title [WIP][SPARK-44113][BUILD] Drop support for Scala 2.12 [WIP][SPARK-44113][BUILD][INFRA][DOCS] Drop support for Scala 2.12 Sep 20, 2023
@@ -19,7 +19,7 @@

set -e

VALID_VERSIONS=( 2.12 2.13 )
VALID_VERSIONS=( 2.13 )
Copy link
Contributor Author

Choose a reason for hiding this comment

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

No further modifications were made to this file, just made Scala-2.12 an invalid option. We can refactor this script in the future when we need to support multiple Scala versions again.

@LuciferYang LuciferYang changed the title [WIP][SPARK-44113][BUILD][INFRA][DOCS] Drop support for Scala 2.12 [SPARK-44113][BUILD][INFRA][DOCS] Drop support for Scala 2.12 Sep 20, 2023
@LuciferYang LuciferYang marked this pull request as ready for review September 20, 2023 08:19
@LuciferYang
Copy link
Contributor Author

LuciferYang commented Sep 20, 2023

cc @dongjoon-hyun FYI

Do we need to further split this PR ?

If you think this PR is too bloated, I can split
12. Moved files from src/scala-2.13/ directory to src/main/scala and deleted all files in the src/scala-2.12/ directory.
13. Comment out the code related to multiple Scala versions in load-spark-env.cmd and load-spark-env.sh.
14. Clean up redundant build-helper-maven-plugin configurations from core/pom.xml, repl/pom.xml, sql/api/pom.xml, sql/catalyst/pom.xml, sql/core/pom.xml and connector/connect/common/pom.xml

into separate prs.

@dongjoon-hyun
Copy link
Member

I'm good with this single PR. Could you resolve the conflict?

@LuciferYang
Copy link
Contributor Author

I'm good with this single PR. Could you resolve the conflict?

done ~

@dongjoon-hyun
Copy link
Member

cc @HyukjinKwon , @srowen

Copy link
Member

@dongjoon-hyun dongjoon-hyun left a comment

Choose a reason for hiding this comment

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

+1, LGTM (from my side)

Copy link
Member

@srowen srowen left a comment

Choose a reason for hiding this comment

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

Yeah I think this is OK if tests pass. We can next look at making minor code changes that were hard or not possible while supporting 2.13, on the way to looking at supporting Scala 3.

@dongjoon-hyun
Copy link
Member

Could you re-trigger the failed pipeline, @LuciferYang ?

@LuciferYang
Copy link
Contributor Author

LuciferYang commented Sep 22, 2023

Could you re-trigger the failed pipeline, @LuciferYang ?

GA passed: https://github.com/apache/spark/runs/17002787095

Comment on lines +135 to +136
jline/3.22.0//jline-3.22.0.jar
jna/5.13.0//jna-5.13.0.jar
Copy link
Member

Choose a reason for hiding this comment

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

why do they become runtime deps now?

And since both jline 2 and 3 are present at the classpath, is it safe?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, I saw this dependency before, It is a cascading dependency of Scala 2.13. I think it is safe because the package name is different

jline2 is jline

image

jline3 is org.jline

image

Copy link
Member

Choose a reason for hiding this comment

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

it's fine then, thanks for checking

docs/building-spark.md Outdated Show resolved Hide resolved
@dongjoon-hyun
Copy link
Member

Merged to master. Thank you, @LuciferYang and all.

@@ -26,8 +26,8 @@
curr_dir = pwd
cd("..")

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 "Running 'build/sbt -Pscala-2.13 -Pkinesis-asl clean compile unidoc' from " + pwd + "; this may take a few minutes..."
Copy link
Member

Choose a reason for hiding this comment

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

QQ, why do we need scala profile here?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

When we execute dev/change-scala-version.sh 2.13, it simultaneously modifies both copy_api_dirs.rb and mima files, adding these -Pscala-2.13. In this PR, I didn't clean them up. It seems we could add a follow-up to clear these unnecessary -Pscala-2.13.

dongjoon-hyun pushed a commit that referenced this pull request Sep 22, 2023
### What changes were proposed in this pull request?

This PR is a followup of #43008 that removes the leftover scheduled GitHub Actions build for Scala 2.13 scheduled build.

### Why are the changes needed?

After dropping Scala 2.12, the default build is exactly same as the scheduled job for Scala 2.13 now.

### Does this PR introduce _any_ user-facing change?

No, dev-only.

### How was this patch tested?

Will monitor the scheduled builds.

### Was this patch authored or co-authored using generative AI tooling?

No.

Closes #43047 from HyukjinKwon/SPARK-44113-folliwup.

Authored-by: Hyukjin Kwon <[email protected]>
Signed-off-by: Dongjoon Hyun <[email protected]>
@LuciferYang
Copy link
Contributor Author

Thanks all ~

dongjoon-hyun pushed a commit that referenced this pull request Sep 22, 2023
…nts, and scala-2.13 profile usage

### What changes were proposed in this pull request?

This PR is a followup of #43008 that cleanups Scala version specific comments, and `scala-2.13` profile usage.

See also #43008 (comment)

### Why are the changes needed?

To remove unnecessary profile specifications

### Does this PR introduce _any_ user-facing change?

No.

### How was this patch tested?

CI in this PR should test them out. For README.md, I manually checked via GitHub.

### Was this patch authored or co-authored using generative AI tooling?

No.

Closes #43049 from HyukjinKwon/SPARK-44113-followup-2.

Authored-by: Hyukjin Kwon <[email protected]>
Signed-off-by: Dongjoon Hyun <[email protected]>
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