Skip to content

[SPARK-50807][BUILD] Upgrade Scala to 2.13.16 #49478

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

Closed
wants to merge 1 commit into from

Conversation

dongjoon-hyun
Copy link
Member

@dongjoon-hyun dongjoon-hyun commented Jan 14, 2025

What changes were proposed in this pull request?

This PR aims to Upgrade Scala to 2.13.16.

For now, this is a draft because the following plugins are not ready. So, we are using this PR to track the readiness.

Why are the changes needed?

To deliver the latest Scala 2.13.x version to Apache Spark 4.0.0.

Does this PR introduce any user-facing change?

No because Apache Spark 4 is not released yet.

How was this patch tested?

Pass the CIs.

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

No.

@github-actions github-actions bot added the BUILD label Jan 14, 2025
@dongjoon-hyun
Copy link
Member Author

dongjoon-hyun commented Jan 14, 2025

FYI, @LuciferYang and @panbingkun

@dongjoon-hyun
Copy link
Member Author

genjavadoc is ready and this is blocked by Ammonite currently.

@dongjoon-hyun
Copy link
Member Author

For the record, the root cause and workaround is on the way in Scala and Ammonite community.

@dongjoon-hyun dongjoon-hyun force-pushed the SPARK-50807 branch 2 times, most recently from 7d58376 to 6108d0d Compare January 29, 2025 02:39
@dongjoon-hyun
Copy link
Member Author

Currently, I'm testing with newly published ammonite dev version.

-    <ammonite.version>3.0.0</ammonite.version>
+    <ammonite.version>dev-0-5b8ba21</ammonite.version>

pom.xml Outdated
@@ -230,7 +230,7 @@
and ./python/packaging/connect/setup.py too.
-->
<arrow.version>18.1.0</arrow.version>
<ammonite.version>3.0.0</ammonite.version>
<ammonite.version>dev-0-5b8ba21</ammonite.version>
Copy link
Member Author

Choose a reason for hiding this comment

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

We need to give our feedback after testing this.

@github-actions github-actions bot added the CORE label Jan 29, 2025
dongjoon-hyun added a commit that referenced this pull request Jan 29, 2025
…lements` for array comparison

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

This PR aims to fix `CryptoStreamUtilsSuite` to use `sameElements` for array comparison.

### Why are the changes needed?

Since the existing assertion is invalid due to `String != Array[Byte]` comparison, it causes a compilation error from Scala 2.13.16.

https://github.com/apache/spark/blob/6bbfa2dad8c70b94ca52eb7cddde5ec68efbe0b1/core/src/test/scala/org/apache/spark/security/CryptoStreamUtilsSuite.scala#L115

- #49478

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

No, this is a test-only fix.

### How was this patch tested?

Pass the CIs.

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

No.

Closes #49727 from dongjoon-hyun/SPARK-51033.

Authored-by: Dongjoon Hyun <[email protected]>
Signed-off-by: Dongjoon Hyun <[email protected]>
dongjoon-hyun added a commit that referenced this pull request Jan 29, 2025
…lements` for array comparison

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

This PR aims to fix `CryptoStreamUtilsSuite` to use `sameElements` for array comparison.

### Why are the changes needed?

Since the existing assertion is invalid due to `String != Array[Byte]` comparison, it causes a compilation error from Scala 2.13.16.

https://github.com/apache/spark/blob/6bbfa2dad8c70b94ca52eb7cddde5ec68efbe0b1/core/src/test/scala/org/apache/spark/security/CryptoStreamUtilsSuite.scala#L115

- #49478

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

No, this is a test-only fix.

### How was this patch tested?

Pass the CIs.

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

No.

Closes #49727 from dongjoon-hyun/SPARK-51033.

Authored-by: Dongjoon Hyun <[email protected]>
Signed-off-by: Dongjoon Hyun <[email protected]>
(cherry picked from commit 5cce6e9)
Signed-off-by: Dongjoon Hyun <[email protected]>
dongjoon-hyun added a commit that referenced this pull request Jan 29, 2025
…lements` for array comparison

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

This PR aims to fix `CryptoStreamUtilsSuite` to use `sameElements` for array comparison.

### Why are the changes needed?

Since the existing assertion is invalid due to `String != Array[Byte]` comparison, it causes a compilation error from Scala 2.13.16.

https://github.com/apache/spark/blob/6bbfa2dad8c70b94ca52eb7cddde5ec68efbe0b1/core/src/test/scala/org/apache/spark/security/CryptoStreamUtilsSuite.scala#L115

- #49478

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

No, this is a test-only fix.

### How was this patch tested?

Pass the CIs.

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

No.

Closes #49727 from dongjoon-hyun/SPARK-51033.

Authored-by: Dongjoon Hyun <[email protected]>
Signed-off-by: Dongjoon Hyun <[email protected]>
(cherry picked from commit 5cce6e9)
Signed-off-by: Dongjoon Hyun <[email protected]>
@github-actions github-actions bot added DOCS and removed CORE labels Jan 29, 2025
@dongjoon-hyun dongjoon-hyun marked this pull request as ready for review January 30, 2025 03:53
@dongjoon-hyun
Copy link
Member Author

Finally, this PR is ready for review. Could you review this PR when you have some time, @LuciferYang ?

Copy link
Contributor

@LuciferYang LuciferYang left a comment

Choose a reason for hiding this comment

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

LGTM

LuciferYang pushed a commit that referenced this pull request Jan 30, 2025
### What changes were proposed in this pull request?

This PR aims to Upgrade Scala to 2.13.16.

For now, this is a draft because the following plugins are not ready. So, we are using this PR to track the readiness.
- [x] lightbend/genjavadoc#375
- [x] com-lihaoyi/Ammonite#1597
    - scala/scala#10868 (comment)

### Why are the changes needed?

To deliver the latest Scala 2.13.x version to Apache Spark 4.0.0.
- https://github.com/scala/scala/releases/tag/v2.13.16

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

No because Apache Spark 4 is not released yet.

### How was this patch tested?

Pass the CIs.

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

No.

Closes #49478 from dongjoon-hyun/SPARK-50807.

Authored-by: Dongjoon Hyun <[email protected]>
Signed-off-by: yangjie01 <[email protected]>
(cherry picked from commit 585f434)
Signed-off-by: yangjie01 <[email protected]>
@LuciferYang
Copy link
Contributor

Merged into master and branch-4.0. Thanks @dongjoon-hyun

@dongjoon-hyun
Copy link
Member Author

Thank you, @LuciferYang !

dongjoon-hyun added a commit that referenced this pull request Jan 31, 2025
…Scala 2.13.16

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

This PR aims to regenerate benchmark results of `master` after upgrading to Scala 2.13.16.
- #49478

### Why are the changes needed?

To check a regression again. Currently, there is only one known benchmark suite failure.
- #49411 (comment)

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

No, this updates only test results.

### How was this patch tested?

Manual review.

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

No.

Closes #49744 from dongjoon-hyun/bm_41.

Authored-by: Dongjoon Hyun <[email protected]>
Signed-off-by: Dongjoon Hyun <[email protected]>
dongjoon-hyun added a commit that referenced this pull request Jan 31, 2025
…g to Scala 2.13.16

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

This PR aims to regenerate benchmark results of `branch-4.0` after upgrading to Scala 2.13.16.
- #49478

### Why are the changes needed?

To check a regression again
- #49411 (comment)

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

No, this updates only test results.

### How was this patch tested?

Manual review.

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

No.

Closes #49741 from dongjoon-hyun/bm_40.

Authored-by: Dongjoon Hyun <[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.

2 participants