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-34774][BUILD] Ensure change-scala-version.sh update scala.version in parent POM correctly #31865

Closed
wants to merge 2 commits into from

Conversation

LuciferYang
Copy link
Contributor

@LuciferYang LuciferYang commented Mar 17, 2021

What changes were proposed in this pull request?

After SPARK-34507, execute change-scala-version.sh script will update scala.version in parent pom, but if we execute the following commands in order:

dev/change-scala-version.sh 2.13
dev/change-scala-version.sh 2.12
git status

there will generate git diff as follow:

diff --git a/pom.xml b/pom.xml
index ddc4ce2f68..f43d8c8f78 100644
--- a/pom.xml
+++ b/pom.xml
@@ -162,7 +162,7 @@
     <commons.math3.version>3.4.1</commons.math3.version>
     <!-- managed up from 3.2.1 for SPARK-11652 -->
     <commons.collections.version>3.2.2</commons.collections.version>
-    <scala.version>2.12.10</scala.version>
+    <scala.version>2.13.5</scala.version>
     <scala.binary.version>2.12</scala.binary.version>
     <scalatest-maven-plugin.version>2.0.0</scalatest-maven-plugin.version>
     <scalafmt.parameters>--test</scalafmt.parameters>

seem 'scala.version' property was not update correctly.

So this pr add an extra 'scala.version' to scala-2.12 profile to ensure change-scala-version.sh can update the public scala.version property correctly.

Why are the changes needed?

Bug fix.

Does this PR introduce any user-facing change?

No

How was this patch tested?

Manual test

Execute the following commands in order:

dev/change-scala-version.sh 2.13
dev/change-scala-version.sh 2.12
git status

Before

diff --git a/pom.xml b/pom.xml
index ddc4ce2f68..f43d8c8f78 100644
--- a/pom.xml
+++ b/pom.xml
@@ -162,7 +162,7 @@
     <commons.math3.version>3.4.1</commons.math3.version>
     <!-- managed up from 3.2.1 for SPARK-11652 -->
     <commons.collections.version>3.2.2</commons.collections.version>
-    <scala.version>2.12.10</scala.version>
+    <scala.version>2.13.5</scala.version>
     <scala.binary.version>2.12</scala.binary.version>
     <scalatest-maven-plugin.version>2.0.0</scalatest-maven-plugin.version>
     <scalafmt.parameters>--test</scalafmt.parameters>

After

No git diff.

@LuciferYang
Copy link
Contributor Author

cc @srowen @HyukjinKwon

@AmplabJenkins
Copy link

Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/136154/

@AmplabJenkins
Copy link

Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder-K8s/40735/

@sarutak
Copy link
Member

sarutak commented Mar 17, 2021

Hmm, this seems to duplicate #31863.

@LuciferYang
Copy link
Contributor Author

LuciferYang commented Mar 17, 2021

Hmm, this seems to duplicate #31863.

@sarutak It seems that we found the same problem almost at the same time :)

But maybe add a scala.version property to the scala-2.12 profile will be able to keep the fix for SPARK-34507, the disadvantage is that this is a duplicate property in parent

@github-actions github-actions bot added the BUILD label Mar 17, 2021
@sarutak
Copy link
Member

sarutak commented Mar 17, 2021

O.K, this solution can be more prominent.

@srowen
Copy link
Member

srowen commented Mar 17, 2021

I'm surprised if it can't be evaluated without being in a profile?

@LuciferYang
Copy link
Contributor Author

LuciferYang commented Mar 17, 2021

I'm surprised if it can't be evaluated without being in a profile?

@srowen Yes, at least the above test method will trigger this problem, but I'm not sure if there's a better way to fix it

@srowen
Copy link
Member

srowen commented Mar 17, 2021

Fix seems fine is so FWIW

@HyukjinKwon
Copy link
Member

I'm okay with this.

@AmplabJenkins
Copy link

Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder-K8s/40778/

@AmplabJenkins
Copy link

Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/136196/

@srowen srowen closed this in 2e836cd Mar 18, 2021
srowen pushed a commit that referenced this pull request Mar 18, 2021
…ion in parent POM correctly

### What changes were proposed in this pull request?
After SPARK-34507,  execute` change-scala-version.sh` script will update `scala.version` in parent pom, but if we execute the following commands in order:

```
dev/change-scala-version.sh 2.13
dev/change-scala-version.sh 2.12
git status
```

there will generate git diff as follow:

```
diff --git a/pom.xml b/pom.xml
index ddc4ce2f68..f43d8c8f78 100644
--- a/pom.xml
+++ b/pom.xml
 -162,7 +162,7
     <commons.math3.version>3.4.1</commons.math3.version>

     <commons.collections.version>3.2.2</commons.collections.version>
-    <scala.version>2.12.10</scala.version>
+    <scala.version>2.13.5</scala.version>
     <scala.binary.version>2.12</scala.binary.version>
     <scalatest-maven-plugin.version>2.0.0</scalatest-maven-plugin.version>
     <scalafmt.parameters>--test</scalafmt.parameters>
```

seem 'scala.version' property was not update correctly.

So this pr add an extra 'scala.version' to scala-2.12 profile to ensure change-scala-version.sh can update the public `scala.version` property correctly.

### Why are the changes needed?
Bug fix.

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

### How was this patch tested?
**Manual test**

Execute the following commands in order:

```
dev/change-scala-version.sh 2.13
dev/change-scala-version.sh 2.12
git status
```

**Before**

```
diff --git a/pom.xml b/pom.xml
index ddc4ce2f68..f43d8c8f78 100644
--- a/pom.xml
+++ b/pom.xml
 -162,7 +162,7
     <commons.math3.version>3.4.1</commons.math3.version>

     <commons.collections.version>3.2.2</commons.collections.version>
-    <scala.version>2.12.10</scala.version>
+    <scala.version>2.13.5</scala.version>
     <scala.binary.version>2.12</scala.binary.version>
     <scalatest-maven-plugin.version>2.0.0</scalatest-maven-plugin.version>
     <scalafmt.parameters>--test</scalafmt.parameters>
```

**After**

No git diff.

Closes #31865 from LuciferYang/SPARK-34774.

Authored-by: yangjie01 <yangjie01@baidu.com>
Signed-off-by: Sean Owen <srowen@gmail.com>
(cherry picked from commit 2e836cd)
Signed-off-by: Sean Owen <srowen@gmail.com>
srowen pushed a commit that referenced this pull request Mar 18, 2021
…ion in parent POM correctly

### What changes were proposed in this pull request?
After SPARK-34507,  execute` change-scala-version.sh` script will update `scala.version` in parent pom, but if we execute the following commands in order:

```
dev/change-scala-version.sh 2.13
dev/change-scala-version.sh 2.12
git status
```

there will generate git diff as follow:

```
diff --git a/pom.xml b/pom.xml
index ddc4ce2f68..f43d8c8f78 100644
--- a/pom.xml
+++ b/pom.xml
 -162,7 +162,7
     <commons.math3.version>3.4.1</commons.math3.version>

     <commons.collections.version>3.2.2</commons.collections.version>
-    <scala.version>2.12.10</scala.version>
+    <scala.version>2.13.5</scala.version>
     <scala.binary.version>2.12</scala.binary.version>
     <scalatest-maven-plugin.version>2.0.0</scalatest-maven-plugin.version>
     <scalafmt.parameters>--test</scalafmt.parameters>
```

seem 'scala.version' property was not update correctly.

So this pr add an extra 'scala.version' to scala-2.12 profile to ensure change-scala-version.sh can update the public `scala.version` property correctly.

### Why are the changes needed?
Bug fix.

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

### How was this patch tested?
**Manual test**

Execute the following commands in order:

```
dev/change-scala-version.sh 2.13
dev/change-scala-version.sh 2.12
git status
```

**Before**

```
diff --git a/pom.xml b/pom.xml
index ddc4ce2f68..f43d8c8f78 100644
--- a/pom.xml
+++ b/pom.xml
 -162,7 +162,7
     <commons.math3.version>3.4.1</commons.math3.version>

     <commons.collections.version>3.2.2</commons.collections.version>
-    <scala.version>2.12.10</scala.version>
+    <scala.version>2.13.5</scala.version>
     <scala.binary.version>2.12</scala.binary.version>
     <scalatest-maven-plugin.version>2.0.0</scalatest-maven-plugin.version>
     <scalafmt.parameters>--test</scalafmt.parameters>
```

**After**

No git diff.

Closes #31865 from LuciferYang/SPARK-34774.

Authored-by: yangjie01 <yangjie01@baidu.com>
Signed-off-by: Sean Owen <srowen@gmail.com>
(cherry picked from commit 2e836cd)
Signed-off-by: Sean Owen <srowen@gmail.com>
@srowen
Copy link
Member

srowen commented Mar 18, 2021

(Jenkins won't test this anyway, note)
Merged to master / 3.1 / 3.0.
@LuciferYang do you want to make a similar change for 2.x? I can do it too.

@LuciferYang
Copy link
Contributor Author

@LuciferYang do you want to make a similar change for 2.x? I can do it too.

@srowen OK, I will do this later ~

@LuciferYang
Copy link
Contributor Author

@LuciferYang do you want to make a similar change for 2.x? I can do it too.

@srowen for branch-2.4: #31893

flyrain pushed a commit to flyrain/spark that referenced this pull request Sep 21, 2021
…ion in parent POM correctly

### What changes were proposed in this pull request?
After SPARK-34507,  execute` change-scala-version.sh` script will update `scala.version` in parent pom, but if we execute the following commands in order:

```
dev/change-scala-version.sh 2.13
dev/change-scala-version.sh 2.12
git status
```

there will generate git diff as follow:

```
diff --git a/pom.xml b/pom.xml
index ddc4ce2f68..f43d8c8f78 100644
--- a/pom.xml
+++ b/pom.xml
 -162,7 +162,7
     <commons.math3.version>3.4.1</commons.math3.version>

     <commons.collections.version>3.2.2</commons.collections.version>
-    <scala.version>2.12.10</scala.version>
+    <scala.version>2.13.5</scala.version>
     <scala.binary.version>2.12</scala.binary.version>
     <scalatest-maven-plugin.version>2.0.0</scalatest-maven-plugin.version>
     <scalafmt.parameters>--test</scalafmt.parameters>
```

seem 'scala.version' property was not update correctly.

So this pr add an extra 'scala.version' to scala-2.12 profile to ensure change-scala-version.sh can update the public `scala.version` property correctly.

### Why are the changes needed?
Bug fix.

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

### How was this patch tested?
**Manual test**

Execute the following commands in order:

```
dev/change-scala-version.sh 2.13
dev/change-scala-version.sh 2.12
git status
```

**Before**

```
diff --git a/pom.xml b/pom.xml
index ddc4ce2f68..f43d8c8f78 100644
--- a/pom.xml
+++ b/pom.xml
 -162,7 +162,7
     <commons.math3.version>3.4.1</commons.math3.version>

     <commons.collections.version>3.2.2</commons.collections.version>
-    <scala.version>2.12.10</scala.version>
+    <scala.version>2.13.5</scala.version>
     <scala.binary.version>2.12</scala.binary.version>
     <scalatest-maven-plugin.version>2.0.0</scalatest-maven-plugin.version>
     <scalafmt.parameters>--test</scalafmt.parameters>
```

**After**

No git diff.

Closes apache#31865 from LuciferYang/SPARK-34774.

Authored-by: yangjie01 <yangjie01@baidu.com>
Signed-off-by: Sean Owen <srowen@gmail.com>
(cherry picked from commit 2e836cd)
Signed-off-by: Sean Owen <srowen@gmail.com>
@LuciferYang LuciferYang deleted the SPARK-34774 branch June 6, 2022 03:47
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
5 participants