[SPARK-5188][BUILD] make-distribution.sh should support curl, not only wget to get Tachyon #3988

sarutak · 2015-01-10T00:56:48Z

When we use make-distribution.sh with --with-tachyon option, Tachyon will be downloaded by wget command but some systems don't have wget by default (MacOS X doesn't have).
Other scripts like build/mvn, build/sbt support not only wget but also curl so make-distribution.sh should support curl too.

…et tachyon

SparkQA · 2015-01-10T02:02:15Z

Test build #25349 has finished for PR 3988 at commit 83b49b5.

This patch passes all tests.
This patch merges cleanly.
This patch adds no public classes.

nchammas · 2015-01-10T03:24:54Z

build/mvn

@@ -48,11 +48,11 @@ install_app() {
    # check if we already have the tarball
    # check if we have curl installed
    # download application
-    [ ! -f "${local_tarball}" ] && [ -n "`which curl 2>/dev/null`" ] && \
+    [ ! -f "${local_tarball}" ] && [ -n "`type curl 2>/dev/null`" ] && \


~~Why are we replacing which with type? What's the difference between the two commands?~~ I'm guessing you want to avoid the external process call and variation of behavior across POSIX systems for which.

FWIW, the approach recommended in this answer, which I agree with, is to use command -v, though honestly one way or the other it doesn't seem like a big deal.

sarutak · 2015-01-20T16:45:27Z

@nchammas Thanks for your advise. I modified to use command command.

nchammas · 2015-01-20T17:19:00Z

build/mvn

@@ -48,11 +48,11 @@ install_app() {
    # check if we already have the tarball
    # check if we have curl installed
    # download application
-    [ ! -f "${local_tarball}" ] && [ -n "`which curl 2>/dev/null`" ] && \
+    [ ! -f "${local_tarball}" ] && command -v curl &>/dev/null && \


Instead of &>/dev/null, which seems a little confusing, can't we use

if [ $(command -v curl) ];

to test for the existence of curl?

We don't have to redirect output if we do it this way, and it's much more readable. I suggest using this pattern throughout.

SparkQA · 2015-01-20T17:56:33Z

Test build #25824 has finished for PR 3988 at commit 1e4c7e0.

This patch passes all tests.
This patch merges cleanly.
This patch adds no public classes.

…ands

nchammas · 2015-01-20T18:45:38Z

LGTM.

@sarutak Did you test this on OS X?

SparkQA · 2015-01-20T19:40:44Z

Test build #25828 has finished for PR 3988 at commit 2db9fbf.

This patch passes all tests.
This patch merges cleanly.
This patch adds no public classes.

sarutak · 2015-01-20T20:07:28Z

@nchammas I confirmed all my changes work but there is another problem in build/mvn, which is not related to my changes. It's a blocker so I separated the PR #4124 to fix the issue.

pwendell · 2015-01-20T20:43:16Z

make-distribution.sh

@@ -224,14 +224,24 @@ cp -r "$SPARK_HOME/ec2" "$DISTDIR"
 if [ "$SPARK_TACHYON" == "true" ]; then
  TACHYON_VERSION="0.5.0"
  TACHYON_URL="https://github.com/amplab/tachyon/releases/download/v${TACHYON_VERSION}/tachyon-${TACHYON_VERSION}-bin.tar.gz"
+  TACHYON_TGZ="tachyon-${TACHYON_VERSION}-bin.tar.gz"


can you define this first and then use this in TACHYON_URL also?

pwendell · 2015-01-20T20:44:34Z

LGTM pending a minor comment regarding style.

SparkQA · 2015-01-20T22:12:57Z

Test build #25841 has finished for PR 3988 at commit a3e908b.

This patch passes all tests.
This patch merges cleanly.
This patch adds no public classes.

SparkQA · 2015-01-20T23:53:10Z

Test build #25851 has finished for PR 3988 at commit 7cc8255.

This patch passes all tests.
This patch merges cleanly.
This patch adds no public classes.

SparkQA · 2015-01-23T02:56:17Z

Test build #25990 has finished for PR 3988 at commit e24e01b.

This patch fails Spark unit tests.
This patch merges cleanly.
This patch adds no public classes.

sarutak · 2015-01-23T02:57:35Z

retest this please.

SparkQA · 2015-01-23T03:53:43Z

Test build #25994 has finished for PR 3988 at commit e24e01b.

This patch fails Spark unit tests.
This patch merges cleanly.
This patch adds no public classes.

sarutak · 2015-01-23T06:37:34Z

retest this please.

SparkQA · 2015-01-23T07:48:52Z

Test build #26001 has finished for PR 3988 at commit e24e01b.

This patch passes all tests.
This patch merges cleanly.
This patch adds no public classes.

pwendell · 2015-01-26T22:43:21Z

@sarutak can you bring this up to date?

SparkQA · 2015-01-27T01:59:33Z

Test build #26130 has finished for PR 3988 at commit 010e884.

This patch passes all tests.
This patch merges cleanly.
This patch adds no public classes.

sarutak · 2015-01-27T02:02:50Z

It's now up-to-date.

SparkQA · 2015-01-28T06:11:23Z

Test build #26205 has finished for PR 3988 at commit 0f546e0.

This patch passes all tests.
This patch merges cleanly.
This patch adds no public classes.

Modified make-distribution.sh so that we use curl, not only wget to g…

83b49b5

…et tachyon

nchammas reviewed Jan 10, 2015
View reviewed changes

Used "command" command instead of "type" command

1e4c7e0

nchammas reviewed Jan 20, 2015
View reviewed changes

Removed redirection from the logic which checks the existence of comm…

2db9fbf

…ands

pwendell reviewed Jan 20, 2015
View reviewed changes

Fixed style

a3e908b

Fix to use \$MVN instead of mvn

7cc8255

sarutak force-pushed the SPARK-5188 branch from eb57bc8 to 7cc8255 Compare January 20, 2015 22:44

sarutak added 2 commits January 20, 2015 14:48

Merge branch 'master' of git://git.apache.org/spark into SPARK-5188

3caa4cb

Merge branch 'master' of git://git.apache.org/spark into SPARK-5188

3daf1f1

Merge branch 'master' of git://git.apache.org/spark into SPARK-5188

e24e01b

sarutak added 2 commits January 27, 2015 09:49

Fixed a merge conflict

163687e

Merge branch 'SPARK-5188' of github.com:sarutak/spark into SPARK-5188

010e884

Merge branch 'master' of git://git.apache.org/spark into SPARK-5188

0f546e0

asfgit closed this in e902dc4 Jan 28, 2015

sarutak deleted the SPARK-5188 branch April 11, 2015 05:24

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[SPARK-5188][BUILD] make-distribution.sh should support curl, not only wget to get Tachyon #3988

[SPARK-5188][BUILD] make-distribution.sh should support curl, not only wget to get Tachyon #3988

sarutak commented Jan 10, 2015

SparkQA commented Jan 10, 2015

nchammas Jan 10, 2015

nchammas Jan 10, 2015

sarutak commented Jan 20, 2015

nchammas Jan 20, 2015

SparkQA commented Jan 20, 2015

nchammas commented Jan 20, 2015

SparkQA commented Jan 20, 2015

sarutak commented Jan 20, 2015

pwendell Jan 20, 2015

pwendell commented Jan 20, 2015

SparkQA commented Jan 20, 2015

SparkQA commented Jan 20, 2015

SparkQA commented Jan 23, 2015

sarutak commented Jan 23, 2015

SparkQA commented Jan 23, 2015

sarutak commented Jan 23, 2015

SparkQA commented Jan 23, 2015

pwendell commented Jan 26, 2015

SparkQA commented Jan 27, 2015

sarutak commented Jan 27, 2015

SparkQA commented Jan 28, 2015

[SPARK-5188][BUILD] make-distribution.sh should support curl, not only wget to get Tachyon #3988

[SPARK-5188][BUILD] make-distribution.sh should support curl, not only wget to get Tachyon #3988

Conversation

sarutak commented Jan 10, 2015

SparkQA commented Jan 10, 2015

nchammas Jan 10, 2015

Choose a reason for hiding this comment

nchammas Jan 10, 2015

Choose a reason for hiding this comment

sarutak commented Jan 20, 2015

nchammas Jan 20, 2015

Choose a reason for hiding this comment

SparkQA commented Jan 20, 2015

nchammas commented Jan 20, 2015

SparkQA commented Jan 20, 2015

sarutak commented Jan 20, 2015

pwendell Jan 20, 2015

Choose a reason for hiding this comment

pwendell commented Jan 20, 2015

SparkQA commented Jan 20, 2015

SparkQA commented Jan 20, 2015

SparkQA commented Jan 23, 2015

sarutak commented Jan 23, 2015

SparkQA commented Jan 23, 2015

sarutak commented Jan 23, 2015

SparkQA commented Jan 23, 2015

pwendell commented Jan 26, 2015

SparkQA commented Jan 27, 2015

sarutak commented Jan 27, 2015

SparkQA commented Jan 28, 2015