You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@spark.apache.org by yaooqinn <gi...@git.apache.org> on 2018/02/11 03:56:22 UTC
[GitHub] spark pull request #20571: [SPARK-23383][Build][Minor]Make a distribution sh...
GitHub user yaooqinn opened a pull request:
https://github.com/apache/spark/pull/20571
[SPARK-23383][Build][Minor]Make a distribution should exit with usage while detecting wrong options
## What changes were proposed in this pull request?
```shell
./dev/make-distribution.sh --name ne-1.0.0-SNAPSHOT xyz --tgz -Phadoop-2.7
+++ dirname ./dev/make-distribution.sh
++ cd ./dev/..
++ pwd
+ SPARK_HOME=/Users/Kent/Documents/spark
+ DISTDIR=/Users/Kent/Documents/spark/dist
+ MAKE_TGZ=false
+ MAKE_PIP=false
+ MAKE_R=false
+ NAME=none
+ MVN=/Users/Kent/Documents/spark/build/mvn
+ (( 5 ))
+ case $1 in
+ NAME=ne-1.0.0-SNAPSHOT
+ shift
+ shift
+ (( 3 ))
+ case $1 in
+ break
+ '[' -z /Users/Kent/.jenv/candidates/java/current ']'
+ '[' -z /Users/Kent/.jenv/candidates/java/current ']'
++ command -v git
+ '[' /usr/local/bin/git ']'
++ git rev-parse --short HEAD
+ GITREV=98ea6a7
+ '[' '!' -z 98ea6a7 ']'
+ GITREVSTRING=' (git revision 98ea6a7)'
+ unset GITREV
++ command -v /Users/Kent/Documents/spark/build/mvn
+ '[' '!' /Users/Kent/Documents/spark/build/mvn ']'
++ /Users/Kent/Documents/spark/build/mvn help:evaluate -Dexpression=project.version xyz --tgz -Phadoop-2.7
++ grep -v INFO
++ tail -n 1
+ VERSION=' -X,--debug Produce execution debug output'
```
It is better to declare the mistakes and exit with usage than `break`
## How was this patch tested?
manually
cc @srowen
You can merge this pull request into a Git repository by running:
$ git pull https://github.com/yaooqinn/spark SPARK-23383
Alternatively you can review and apply these changes as the patch at:
https://github.com/apache/spark/pull/20571.patch
To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:
This closes #20571
----
commit 81c1b2407ceb478d6795438de82ac6afe65024c8
Author: Kent Yao <ya...@...>
Date: 2018-02-11T03:48:30Z
exit with usage
----
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #20571: [SPARK-23383][Build][Minor]Make a distribution should ex...
Posted by srowen <gi...@git.apache.org>.
Github user srowen commented on the issue:
https://github.com/apache/spark/pull/20571
Ping @yaooqinn
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #20571: [SPARK-23383][Build][Minor]Make a distribution should ex...
Posted by yaooqinn <gi...@git.apache.org>.
Github user yaooqinn commented on the issue:
https://github.com/apache/spark/pull/20571
yes， single dash means the coming ones are all maven options，they will be handled later by mvn command. we do not parse each single option in this while loop from then till now.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #20571: [SPARK-23383][Build][Minor]Make a distribution should ex...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/20571
Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/testing-k8s-prb-make-spark-distribution/781/
Test PASSed.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #20571: [SPARK-23383][Build][Minor]Make a distribution should ex...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/20571
Merged build finished. Test PASSed.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #20571: [SPARK-23383][Build][Minor]Make a distribution should ex...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/20571
Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/87540/
Test PASSed.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #20571: [SPARK-23383][Build][Minor]Make a distribution should ex...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/20571
**[Test build #87295 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/87295/testReport)** for PR 20571 at commit [`81c1b24`](https://github.com/apache/spark/commit/81c1b2407ceb478d6795438de82ac6afe65024c8).
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #20571: [SPARK-23383][Build][Minor]Make a distribution should ex...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/20571
Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/testing-k8s-prb-make-spark-distribution/957/
Test PASSed.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request #20571: [SPARK-23383][Build][Minor]Make a distribution sh...
Posted by srowen <gi...@git.apache.org>.
Github user srowen commented on a diff in the pull request:
https://github.com/apache/spark/pull/20571#discussion_r167437130
--- Diff: dev/make-distribution.sh ---
@@ -72,8 +76,15 @@ while (( "$#" )); do
--help)
exit_with_usage
;;
+ --*)
--- End diff --
I think this is fine. It will print usage for every unrecognized arg, but, that's probably OK, as it's rare to have more than one.
I guess the other option is to fail outright to make sure the user knows the command wasn't parsed as given. How about that? I don't feel strongly.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #20571: [SPARK-23383][Build][Minor]Make a distribution should ex...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/20571
Merged build finished. Test PASSed.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #20571: [SPARK-23383][Build][Minor]Make a distribution should ex...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/20571
Merged build finished. Test PASSed.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #20571: [SPARK-23383][Build][Minor]Make a distribution should ex...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/20571
Test FAILed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/87539/
Test FAILed.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #20571: [SPARK-23383][Build][Minor]Make a distribution should ex...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/20571
Merged build finished. Test PASSed.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request #20571: [SPARK-23383][Build][Minor]Make a distribution sh...
Posted by asfgit <gi...@git.apache.org>.
Github user asfgit closed the pull request at:
https://github.com/apache/spark/pull/20571
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request #20571: [SPARK-23383][Build][Minor]Make a distribution sh...
Posted by yaooqinn <gi...@git.apache.org>.
Github user yaooqinn commented on a diff in the pull request:
https://github.com/apache/spark/pull/20571#discussion_r167440294
--- Diff: dev/make-distribution.sh ---
@@ -72,8 +76,15 @@ while (( "$#" )); do
--help)
exit_with_usage
;;
+ --*)
--- End diff --
I guess both options are ok too.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #20571: [SPARK-23383][Build][Minor]Make a distribution should ex...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/20571
Merged build finished. Test PASSed.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #20571: [SPARK-23383][Build][Minor]Make a distribution should ex...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/20571
**[Test build #87313 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/87313/testReport)** for PR 20571 at commit [`4194658`](https://github.com/apache/spark/commit/4194658f014d4283554184770c5f191a314cb65e).
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #20571: [SPARK-23383][Build][Minor]Make a distribution should ex...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/20571
Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/87313/
Test PASSed.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request #20571: [SPARK-23383][Build][Minor]Make a distribution sh...
Posted by felixcheung <gi...@git.apache.org>.
Github user felixcheung commented on a diff in the pull request:
https://github.com/apache/spark/pull/20571#discussion_r168389419
--- Diff: dev/make-distribution.sh ---
@@ -72,9 +76,19 @@ while (( "$#" )); do
--help)
exit_with_usage
;;
- *)
+ --*)
+ echo "Error: $1 is not supported, it will be ignored and not take effect"
+ usage
+ shift
+ continue
--- End diff --
+1 on not continue
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #20571: [SPARK-23383][Build][Minor]Make a distribution should ex...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/20571
**[Test build #87540 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/87540/testReport)** for PR 20571 at commit [`b99c43c`](https://github.com/apache/spark/commit/b99c43c9d89b10ba34ae2bd3e31831abd6cd2513).
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #20571: [SPARK-23383][Build][Minor]Make a distribution should ex...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/20571
Merged build finished. Test PASSed.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #20571: [SPARK-23383][Build][Minor]Make a distribution should ex...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/20571
Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/87295/
Test PASSed.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #20571: [SPARK-23383][Build][Minor]Make a distribution should ex...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/20571
**[Test build #87313 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/87313/testReport)** for PR 20571 at commit [`4194658`](https://github.com/apache/spark/commit/4194658f014d4283554184770c5f191a314cb65e).
* This patch passes all tests.
* This patch merges cleanly.
* This patch adds no public classes.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #20571: [SPARK-23383][Build][Minor]Make a distribution should ex...
Posted by kiszk <gi...@git.apache.org>.
Github user kiszk commented on the issue:
https://github.com/apache/spark/pull/20571
retest this please
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #20571: [SPARK-23383][Build][Minor]Make a distribution should ex...
Posted by srowen <gi...@git.apache.org>.
Github user srowen commented on the issue:
https://github.com/apache/spark/pull/20571
Yes @yaooqinn, it's easy enough to read the logic, but, my question is _why_ treat these cases differently? Why not go all the way to failing on any unexpected input?
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #20571: [SPARK-23383][Build][Minor]Make a distribution should ex...
Posted by yaooqinn <gi...@git.apache.org>.
Github user yaooqinn commented on the issue:
https://github.com/apache/spark/pull/20571
sorry for not replying in time. the logic here is that firstly unrecognized - -options show warning message and usage information，secondly the - options end the while loop and treat the current one and following options as maven parameters which will be handled later by maven，and finally all other unrecognized options fail the process immediately.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #20571: [SPARK-23383][Build][Minor]Make a distribution should ex...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/20571
**[Test build #87539 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/87539/testReport)** for PR 20571 at commit [`b99c43c`](https://github.com/apache/spark/commit/b99c43c9d89b10ba34ae2bd3e31831abd6cd2513).
* This patch **fails due to an unknown error code, -9**.
* This patch merges cleanly.
* This patch adds no public classes.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #20571: [SPARK-23383][Build][Minor]Make a distribution should ex...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/20571
Merged build finished. Test PASSed.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #20571: [SPARK-23383][Build][Minor]Make a distribution should ex...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/20571
Merged build finished. Test FAILed.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #20571: [SPARK-23383][Build][Minor]Make a distribution should ex...
Posted by srowen <gi...@git.apache.org>.
Github user srowen commented on the issue:
https://github.com/apache/spark/pull/20571
Merged to master
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #20571: [SPARK-23383][Build][Minor]Make a distribution should ex...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/20571
**[Test build #87540 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/87540/testReport)** for PR 20571 at commit [`b99c43c`](https://github.com/apache/spark/commit/b99c43c9d89b10ba34ae2bd3e31831abd6cd2513).
* This patch passes all tests.
* This patch merges cleanly.
* This patch adds no public classes.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #20571: [SPARK-23383][Build][Minor]Make a distribution should ex...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/20571
**[Test build #87539 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/87539/testReport)** for PR 20571 at commit [`b99c43c`](https://github.com/apache/spark/commit/b99c43c9d89b10ba34ae2bd3e31831abd6cd2513).
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #20571: [SPARK-23383][Build][Minor]Make a distribution should ex...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/20571
**[Test build #87295 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/87295/testReport)** for PR 20571 at commit [`81c1b24`](https://github.com/apache/spark/commit/81c1b2407ceb478d6795438de82ac6afe65024c8).
* This patch passes all tests.
* This patch merges cleanly.
* This patch adds no public classes.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #20571: [SPARK-23383][Build][Minor]Make a distribution should ex...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/20571
Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/testing-k8s-prb-make-spark-distribution/958/
Test PASSed.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #20571: [SPARK-23383][Build][Minor]Make a distribution should ex...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/20571
Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/testing-k8s-prb-make-spark-distribution/797/
Test PASSed.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #20571: [SPARK-23383][Build][Minor]Make a distribution should ex...
Posted by yaooqinn <gi...@git.apache.org>.
Github user yaooqinn commented on the issue:
https://github.com/apache/spark/pull/20571
OK， i will push a commit very soon
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org