You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@spark.apache.org by pwendell <gi...@git.apache.org> on 2014/05/06 23:13:33 UTC

[GitHub] spark pull request: SPARK-1737: Warn rather than fail when Java 7+...

GitHub user pwendell opened a pull request:

    https://github.com/apache/spark/pull/669

    SPARK-1737: Warn rather than fail when Java 7+ is used to create distributions

    Also moves a few lines of code around in make-distribution.sh.

You can merge this pull request into a Git repository by running:

    $ git pull https://github.com/pwendell/spark make-distribution

Alternatively you can review and apply these changes as the patch at:

    https://github.com/apache/spark/pull/669.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 #669
    
----
commit 46918ecf05e9a828c4cf7fa07465967c1a199d4f
Author: Patrick Wendell <pw...@gmail.com>
Date:   2014-05-06T21:11:25Z

    SPARK-1737: Warn rather than fail when Java 7+ is used to create distributions.

----


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] spark pull request: SPARK-1737: Warn rather than fail when Java 7+...

Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the pull request:

    https://github.com/apache/spark/pull/669#issuecomment-42360207
  
    Merged build started. 


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] spark pull request: SPARK-1737: Warn rather than fail when Java 7+...

Posted by pwendell <gi...@git.apache.org>.
Github user pwendell commented on the pull request:

    https://github.com/apache/spark/pull/669#issuecomment-42359866
  
    /cc @andrewor14


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] spark pull request: SPARK-1737: Warn rather than fail when Java 7+...

Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the pull request:

    https://github.com/apache/spark/pull/669#issuecomment-42362697
  
     Merged build triggered. 


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] spark pull request: SPARK-1737: Warn rather than fail when Java 7+...

Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the pull request:

    https://github.com/apache/spark/pull/669#issuecomment-42360182
  
     Merged build triggered. 


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] spark pull request: SPARK-1737: Warn rather than fail when Java 7+...

Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the pull request:

    https://github.com/apache/spark/pull/669#issuecomment-42368012
  
    Merged build finished. All automated tests passed.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] spark pull request: SPARK-1737: Warn rather than fail when Java 7+...

Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the pull request:

    https://github.com/apache/spark/pull/669#issuecomment-42362711
  
    Merged build started. 


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] spark pull request: SPARK-1737: Warn rather than fail when Java 7+...

Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the pull request:

    https://github.com/apache/spark/pull/669#issuecomment-42368014
  
    All automated tests passed.
    Refer to this link for build results: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/14735/


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] spark pull request: SPARK-1737: Warn rather than fail when Java 7+...

Posted by andrewor14 <gi...@git.apache.org>.
Github user andrewor14 commented on a diff in the pull request:

    https://github.com/apache/spark/pull/669#discussion_r12350571
  
    --- Diff: make-distribution.sh ---
    @@ -39,18 +39,11 @@
     # 5) ./bin/spark-shell --master spark://my-master-ip:7077
     #
     
    +set -o pipefail
     # Figure out where the Spark framework is installed
     FWDIR="$(cd `dirname $0`; pwd)"
     DISTDIR="$FWDIR/dist"
     
    -set -o pipefail
    -VERSION=$(mvn help:evaluate -Dexpression=project.version 2>/dev/null | grep -v "INFO" | tail -n 1)
    -if [ $? != 0 ]; then
    -    echo -e "You need Maven installed to build Spark."
    -    echo -e "Download Maven from https://maven.apache.org/"
    -    exit -1;
    -fi
    -
     if [ -z "$JAVA_HOME" ]; then
    --- End diff --
    
    Should we require the user to set `JAVA_HOME` anymore? Or should we use `JAVA_HOME` if it's specified, and just `java` if it's not?


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] spark pull request: SPARK-1737: Warn rather than fail when Java 7+...

Posted by andrewor14 <gi...@git.apache.org>.
Github user andrewor14 commented on a diff in the pull request:

    https://github.com/apache/spark/pull/669#discussion_r12350451
  
    --- Diff: make-distribution.sh ---
    @@ -59,10 +52,17 @@ fi
     JAVA_CMD="$JAVA_HOME"/bin/java
     JAVA_VERSION=$("$JAVA_CMD" -version 2>&1)
     if ! [[ "$JAVA_VERSION" =~ "1.6" ]]; then
    -  echo "Error: JAVA_HOME must point to a JDK 6 installation (see SPARK-1703)."
    +  echo "***NOTE***: JAVA_HOME is not set to a JDK 6 installation. The resulting"
    +  echo "***NOTE***: distribution will not support Java 6. See SPARK-1703."
    --- End diff --
    
    This might look weird:
    
    ```
    ***NOTE***: JAVA_HOME is not set to a JDK 6 installation. The resulting
    ***NOTE***: distribution will not support Java 6. See SPARK-1703.
    ```
    
    I think one all-caps asterisk-bounded warning is enough


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] spark pull request: SPARK-1737: Warn rather than fail when Java 7+...

Posted by asfgit <gi...@git.apache.org>.
Github user asfgit closed the pull request at:

    https://github.com/apache/spark/pull/669


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] spark pull request: SPARK-1737: Warn rather than fail when Java 7+...

Posted by andrewor14 <gi...@git.apache.org>.
Github user andrewor14 commented on a diff in the pull request:

    https://github.com/apache/spark/pull/669#discussion_r12351430
  
    --- Diff: make-distribution.sh ---
    @@ -39,18 +39,11 @@
     # 5) ./bin/spark-shell --master spark://my-master-ip:7077
     #
     
    +set -o pipefail
     # Figure out where the Spark framework is installed
     FWDIR="$(cd `dirname $0`; pwd)"
     DISTDIR="$FWDIR/dist"
     
    -set -o pipefail
    -VERSION=$(mvn help:evaluate -Dexpression=project.version 2>/dev/null | grep -v "INFO" | tail -n 1)
    -if [ $? != 0 ]; then
    -    echo -e "You need Maven installed to build Spark."
    -    echo -e "Download Maven from https://maven.apache.org/"
    -    exit -1;
    -fi
    -
     if [ -z "$JAVA_HOME" ]; then
    --- End diff --
    
    ok, sounds good


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] spark pull request: SPARK-1737: Warn rather than fail when Java 7+...

Posted by pwendell <gi...@git.apache.org>.
Github user pwendell commented on a diff in the pull request:

    https://github.com/apache/spark/pull/669#discussion_r12350607
  
    --- Diff: make-distribution.sh ---
    @@ -39,18 +39,11 @@
     # 5) ./bin/spark-shell --master spark://my-master-ip:7077
     #
     
    +set -o pipefail
     # Figure out where the Spark framework is installed
     FWDIR="$(cd `dirname $0`; pwd)"
     DISTDIR="$FWDIR/dist"
     
    -set -o pipefail
    -VERSION=$(mvn help:evaluate -Dexpression=project.version 2>/dev/null | grep -v "INFO" | tail -n 1)
    -if [ $? != 0 ]; then
    -    echo -e "You need Maven installed to build Spark."
    -    echo -e "Download Maven from https://maven.apache.org/"
    -    exit -1;
    -fi
    -
     if [ -z "$JAVA_HOME" ]; then
    --- End diff --
    
    I think in general when building it's good to make them set JAVA_HOME explicitly. Otherwise it's really hard to reason about which java is getting detected.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] spark pull request: SPARK-1737: Warn rather than fail when Java 7+...

Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the pull request:

    https://github.com/apache/spark/pull/669#issuecomment-42368016
  
    All automated tests passed.
    Refer to this link for build results: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/14737/


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] spark pull request: SPARK-1737: Warn rather than fail when Java 7+...

Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the pull request:

    https://github.com/apache/spark/pull/669#issuecomment-42368010
  
    Merged build finished. All automated tests passed.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---