You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@spark.apache.org by zuotingbing <gi...@git.apache.org> on 2017/03/28 07:22:05 UTC
[GitHub] spark pull request #17452: [SPARK-20123]$SPARK_HOME variable might have spac...
GitHub user zuotingbing opened a pull request:
https://github.com/apache/spark/pull/17452
[SPARK-20123]$SPARK_HOME variable might have spaces in it(e.g. $SPARK\u2026
JIRA Issue: https://issues.apache.org/jira/browse/SPARK-20123
## What changes were proposed in this pull request?
If $SPARK_HOME or $FWDIR variable contains spaces, then use "./dev/make-distribution.sh --r --name custom-spark --tgz -Psparkr -Phadoop-2.4 -Phive -Phive-thriftserver -Pmesos -Pyarn" build spark will failed.
## How was this patch tested?
manual tests
You can merge this pull request into a Git repository by running:
$ git pull https://github.com/zuotingbing/spark spark-bulid
Alternatively you can review and apply these changes as the patch at:
https://github.com/apache/spark/pull/17452.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 #17452
----
commit ab6c29daa0c5c84b9e05f0ea57238dbd0003a2a4
Author: zuotingbing <zu...@zte.com.cn>
Date: 2017-03-28T07:08:05Z
[SPARK-20123]$SPARK_HOME variable might have spaces in it(e.g. $SPARK_HOME=/home/spark build/spark), then build spark failed.
----
---
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.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request #17452: [SPARK-20123][build]$SPARK_HOME variable might ha...
Posted by zuotingbing <gi...@git.apache.org>.
Github user zuotingbing commented on a diff in the pull request:
https://github.com/apache/spark/pull/17452#discussion_r109072645
--- Diff: dev/make-distribution.sh ---
@@ -217,43 +217,43 @@ fi
# Make R package - this is used for both CRAN release and packing R layout into distribution
if [ "$MAKE_R" == "true" ]; then
echo "Building R source package"
- R_PACKAGE_VERSION=`grep Version $SPARK_HOME/R/pkg/DESCRIPTION | awk '{print $NF}'`
+ R_PACKAGE_VERSION=`grep Version "$SPARK_HOME/R/pkg/DESCRIPTION" | awk '{print $NF}'`
pushd "$SPARK_HOME/R" > /dev/null
# Build source package and run full checks
# Do not source the check-cran.sh - it should be run from where it is for it to set SPARK_HOME
- NO_TESTS=1 "$SPARK_HOME/"R/check-cran.sh
+ NO_TESTS=1 "$SPARK_HOME/R/check-cran.sh"
# Move R source package to match the Spark release version if the versions are not the same.
# NOTE(shivaram): `mv` throws an error on Linux if source and destination are same file
if [ "$R_PACKAGE_VERSION" != "$VERSION" ]; then
- mv $SPARK_HOME/R/SparkR_"$R_PACKAGE_VERSION".tar.gz $SPARK_HOME/R/SparkR_"$VERSION".tar.gz
+ mv "$SPARK_HOME/R/SparkR_$R_PACKAGE_VERSION.tar.gz" "$SPARK_HOME/R/SparkR_$VERSION.tar.gz"
fi
# Install source package to get it to generate vignettes rds files, etc.
- VERSION=$VERSION "$SPARK_HOME/"R/install-source-package.sh
+ VERSION=$VERSION "$SPARK_HOME/R/install-source-package.sh"
popd > /dev/null
else
echo "Skipping building R source package"
fi
# Copy other things
-mkdir "$DISTDIR"/conf
-cp "$SPARK_HOME"/conf/*.template "$DISTDIR"/conf
+mkdir "$DISTDIR/conf"
+cp "$SPARK_HOME"/conf/*.template "$DISTDIR/conf"
--- End diff --
If you quote `f* bar` , `f* bar` will be treated as one argument. The error is missing the destination file.
---
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.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #17452: [SPARK-20123][build]$SPARK_HOME variable might have spac...
Posted by srowen <gi...@git.apache.org>.
Github user srowen commented on the issue:
https://github.com/apache/spark/pull/17452
Yes it wouldn't work with shell expansion. I don't think that is the case here but it is fine either way. Consistency is the most important thing.
---
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.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request #17452: [SPARK-20123][build]$SPARK_HOME variable might ha...
Posted by zuotingbing <gi...@git.apache.org>.
Github user zuotingbing commented on a diff in the pull request:
https://github.com/apache/spark/pull/17452#discussion_r108623175
--- Diff: R/check-cran.sh ---
@@ -40,7 +40,7 @@ fi
if [ -d "$SPARK_JARS_DIR" ]; then
# Build a zip file containing the source package with vignettes
- SPARK_HOME="${SPARK_HOME}" "$R_SCRIPT_PATH/"R CMD build $FWDIR/pkg
+ SPARK_HOME="${SPARK_HOME}" "$R_SCRIPT_PATH/"R CMD build "$FWDIR"/pkg
--- End diff --
Sure, but i do not think it is a good choice to quote whole arguments that include a variable.
e.g. cp "/home/spark build/spark/conf/*.template" "/home/test" will failed.
It seems to me that quote the variable is better than whole arguments, what is your suggestion?
Thanks!
---
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.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #17452: [SPARK-20123]$SPARK_HOME variable might have spaces in i...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/17452
Can one of the admins verify this patch?
---
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.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request #17452: [SPARK-20123][build]$SPARK_HOME variable might ha...
Posted by srowen <gi...@git.apache.org>.
Github user srowen commented on a diff in the pull request:
https://github.com/apache/spark/pull/17452#discussion_r108978223
--- Diff: dev/make-distribution.sh ---
@@ -217,43 +217,43 @@ fi
# Make R package - this is used for both CRAN release and packing R layout into distribution
if [ "$MAKE_R" == "true" ]; then
echo "Building R source package"
- R_PACKAGE_VERSION=`grep Version $SPARK_HOME/R/pkg/DESCRIPTION | awk '{print $NF}'`
+ R_PACKAGE_VERSION=`grep Version "$SPARK_HOME/R/pkg/DESCRIPTION" | awk '{print $NF}'`
pushd "$SPARK_HOME/R" > /dev/null
# Build source package and run full checks
# Do not source the check-cran.sh - it should be run from where it is for it to set SPARK_HOME
- NO_TESTS=1 "$SPARK_HOME/"R/check-cran.sh
+ NO_TESTS=1 "$SPARK_HOME/R/check-cran.sh"
# Move R source package to match the Spark release version if the versions are not the same.
# NOTE(shivaram): `mv` throws an error on Linux if source and destination are same file
if [ "$R_PACKAGE_VERSION" != "$VERSION" ]; then
- mv $SPARK_HOME/R/SparkR_"$R_PACKAGE_VERSION".tar.gz $SPARK_HOME/R/SparkR_"$VERSION".tar.gz
+ mv "$SPARK_HOME/R/SparkR_$R_PACKAGE_VERSION.tar.gz" "$SPARK_HOME/R/SparkR_$VERSION.tar.gz"
fi
# Install source package to get it to generate vignettes rds files, etc.
- VERSION=$VERSION "$SPARK_HOME/"R/install-source-package.sh
+ VERSION=$VERSION "$SPARK_HOME/R/install-source-package.sh"
popd > /dev/null
else
echo "Skipping building R source package"
fi
# Copy other things
-mkdir "$DISTDIR"/conf
-cp "$SPARK_HOME"/conf/*.template "$DISTDIR"/conf
+mkdir "$DISTDIR/conf"
+cp "$SPARK_HOME"/conf/*.template "$DISTDIR/conf"
--- End diff --
Hm, I don't see that this works.
```
touch foo
cp "f* bar"
usage ... source_file target_file
```
What am I missing?
---
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.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request #17452: [SPARK-20123][build]$SPARK_HOME variable might ha...
Posted by srowen <gi...@git.apache.org>.
Github user srowen commented on a diff in the pull request:
https://github.com/apache/spark/pull/17452#discussion_r109116625
--- Diff: dev/make-distribution.sh ---
@@ -217,43 +217,43 @@ fi
# Make R package - this is used for both CRAN release and packing R layout into distribution
if [ "$MAKE_R" == "true" ]; then
echo "Building R source package"
- R_PACKAGE_VERSION=`grep Version $SPARK_HOME/R/pkg/DESCRIPTION | awk '{print $NF}'`
+ R_PACKAGE_VERSION=`grep Version "$SPARK_HOME/R/pkg/DESCRIPTION" | awk '{print $NF}'`
pushd "$SPARK_HOME/R" > /dev/null
# Build source package and run full checks
# Do not source the check-cran.sh - it should be run from where it is for it to set SPARK_HOME
- NO_TESTS=1 "$SPARK_HOME/"R/check-cran.sh
+ NO_TESTS=1 "$SPARK_HOME/R/check-cran.sh"
# Move R source package to match the Spark release version if the versions are not the same.
# NOTE(shivaram): `mv` throws an error on Linux if source and destination are same file
if [ "$R_PACKAGE_VERSION" != "$VERSION" ]; then
- mv $SPARK_HOME/R/SparkR_"$R_PACKAGE_VERSION".tar.gz $SPARK_HOME/R/SparkR_"$VERSION".tar.gz
+ mv "$SPARK_HOME/R/SparkR_$R_PACKAGE_VERSION.tar.gz" "$SPARK_HOME/R/SparkR_$VERSION.tar.gz"
fi
# Install source package to get it to generate vignettes rds files, etc.
- VERSION=$VERSION "$SPARK_HOME/"R/install-source-package.sh
+ VERSION=$VERSION "$SPARK_HOME/R/install-source-package.sh"
popd > /dev/null
else
echo "Skipping building R source package"
fi
# Copy other things
-mkdir "$DISTDIR"/conf
-cp "$SPARK_HOME"/conf/*.template "$DISTDIR"/conf
+mkdir "$DISTDIR/conf"
+cp "$SPARK_HOME"/conf/*.template "$DISTDIR/conf"
--- End diff --
OK the line above is correct. I must be crazy, I thought I read it earlier as being one big quoted argument. If it wasn't that way before then I must have misread it and you can ignore this comment.
---
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.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request #17452: [SPARK-20123][build]$SPARK_HOME variable might ha...
Posted by felixcheung <gi...@git.apache.org>.
Github user felixcheung commented on a diff in the pull request:
https://github.com/apache/spark/pull/17452#discussion_r108976857
--- Diff: dev/make-distribution.sh ---
@@ -217,43 +217,43 @@ fi
# Make R package - this is used for both CRAN release and packing R layout into distribution
if [ "$MAKE_R" == "true" ]; then
echo "Building R source package"
- R_PACKAGE_VERSION=`grep Version $SPARK_HOME/R/pkg/DESCRIPTION | awk '{print $NF}'`
+ R_PACKAGE_VERSION=`grep Version "$SPARK_HOME/R/pkg/DESCRIPTION" | awk '{print $NF}'`
pushd "$SPARK_HOME/R" > /dev/null
# Build source package and run full checks
# Do not source the check-cran.sh - it should be run from where it is for it to set SPARK_HOME
- NO_TESTS=1 "$SPARK_HOME/"R/check-cran.sh
+ NO_TESTS=1 "$SPARK_HOME/R/check-cran.sh"
# Move R source package to match the Spark release version if the versions are not the same.
# NOTE(shivaram): `mv` throws an error on Linux if source and destination are same file
if [ "$R_PACKAGE_VERSION" != "$VERSION" ]; then
- mv $SPARK_HOME/R/SparkR_"$R_PACKAGE_VERSION".tar.gz $SPARK_HOME/R/SparkR_"$VERSION".tar.gz
+ mv "$SPARK_HOME/R/SparkR_$R_PACKAGE_VERSION.tar.gz" "$SPARK_HOME/R/SparkR_$VERSION.tar.gz"
fi
# Install source package to get it to generate vignettes rds files, etc.
- VERSION=$VERSION "$SPARK_HOME/"R/install-source-package.sh
+ VERSION=$VERSION "$SPARK_HOME/R/install-source-package.sh"
popd > /dev/null
else
echo "Skipping building R source package"
fi
# Copy other things
-mkdir "$DISTDIR"/conf
-cp "$SPARK_HOME"/conf/*.template "$DISTDIR"/conf
+mkdir "$DISTDIR/conf"
+cp "$SPARK_HOME"/conf/*.template "$DISTDIR/conf"
cp "$SPARK_HOME/README.md" "$DISTDIR"
cp -r "$SPARK_HOME/bin" "$DISTDIR"
cp -r "$SPARK_HOME/python" "$DISTDIR"
# Remove the python distribution from dist/ if we built it
if [ "$MAKE_PIP" == "true" ]; then
- rm -f $DISTDIR/python/dist/pyspark-*.tar.gz
+ rm -f "$DISTDIR"/python/dist/pyspark-*.tar.gz
--- End diff --
`rm -f "$DISTDIR/python/dist/pyspark-*.tar.gz"`?
---
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.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request #17452: [SPARK-20123][build]$SPARK_HOME variable might ha...
Posted by srowen <gi...@git.apache.org>.
Github user srowen commented on a diff in the pull request:
https://github.com/apache/spark/pull/17452#discussion_r108360431
--- Diff: R/check-cran.sh ---
@@ -20,14 +20,14 @@
set -o pipefail
set -e
-FWDIR="$(cd `dirname "${BASH_SOURCE[0]}"`; pwd)"
-pushd $FWDIR > /dev/null
+FWDIR="$(cd "$(dirname "${BASH_SOURCE[0]}")"; pwd)"
--- End diff --
This removed the backtick so it no longer works
---
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.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request #17452: [SPARK-20123][build]$SPARK_HOME variable might ha...
Posted by srowen <gi...@git.apache.org>.
Github user srowen commented on a diff in the pull request:
https://github.com/apache/spark/pull/17452#discussion_r108363062
--- Diff: R/check-cran.sh ---
@@ -40,7 +40,7 @@ fi
if [ -d "$SPARK_JARS_DIR" ]; then
# Build a zip file containing the source package with vignettes
- SPARK_HOME="${SPARK_HOME}" "$R_SCRIPT_PATH/"R CMD build $FWDIR/pkg
+ SPARK_HOME="${SPARK_HOME}" "$R_SCRIPT_PATH/"R CMD build "$FWDIR"/pkg
--- End diff --
You could quote whole arguments that include a variable for completeness
---
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.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request #17452: [SPARK-20123][build]$SPARK_HOME variable might ha...
Posted by zuotingbing <gi...@git.apache.org>.
Github user zuotingbing commented on a diff in the pull request:
https://github.com/apache/spark/pull/17452#discussion_r108969951
--- Diff: dev/make-distribution.sh ---
@@ -217,43 +217,43 @@ fi
# Make R package - this is used for both CRAN release and packing R layout into distribution
if [ "$MAKE_R" == "true" ]; then
echo "Building R source package"
- R_PACKAGE_VERSION=`grep Version $SPARK_HOME/R/pkg/DESCRIPTION | awk '{print $NF}'`
+ R_PACKAGE_VERSION=`grep Version "$SPARK_HOME/R/pkg/DESCRIPTION" | awk '{print $NF}'`
pushd "$SPARK_HOME/R" > /dev/null
# Build source package and run full checks
# Do not source the check-cran.sh - it should be run from where it is for it to set SPARK_HOME
- NO_TESTS=1 "$SPARK_HOME/"R/check-cran.sh
+ NO_TESTS=1 "$SPARK_HOME/R/check-cran.sh"
# Move R source package to match the Spark release version if the versions are not the same.
# NOTE(shivaram): `mv` throws an error on Linux if source and destination are same file
if [ "$R_PACKAGE_VERSION" != "$VERSION" ]; then
- mv $SPARK_HOME/R/SparkR_"$R_PACKAGE_VERSION".tar.gz $SPARK_HOME/R/SparkR_"$VERSION".tar.gz
+ mv "$SPARK_HOME/R/SparkR_$R_PACKAGE_VERSION.tar.gz" "$SPARK_HOME/R/SparkR_$VERSION.tar.gz"
fi
# Install source package to get it to generate vignettes rds files, etc.
- VERSION=$VERSION "$SPARK_HOME/"R/install-source-package.sh
+ VERSION=$VERSION "$SPARK_HOME/R/install-source-package.sh"
popd > /dev/null
else
echo "Skipping building R source package"
fi
# Copy other things
-mkdir "$DISTDIR"/conf
-cp "$SPARK_HOME"/conf/*.template "$DISTDIR"/conf
+mkdir "$DISTDIR/conf"
+cp "$SPARK_HOME"/conf/*.template "$DISTDIR/conf"
--- End diff --
Of course i use ### cp "$SPARK_HOME"/conf/*.template "$DISTDIR/conf" to get the debug info. You can write a test.sh and $ sh -x test.sh for debug.
---
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.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request #17452: [SPARK-20123][build]$SPARK_HOME variable might ha...
Posted by zuotingbing <gi...@git.apache.org>.
Github user zuotingbing commented on a diff in the pull request:
https://github.com/apache/spark/pull/17452#discussion_r108898299
--- Diff: dev/make-distribution.sh ---
@@ -217,43 +217,43 @@ fi
# Make R package - this is used for both CRAN release and packing R layout into distribution
if [ "$MAKE_R" == "true" ]; then
echo "Building R source package"
- R_PACKAGE_VERSION=`grep Version $SPARK_HOME/R/pkg/DESCRIPTION | awk '{print $NF}'`
+ R_PACKAGE_VERSION=`grep Version "$SPARK_HOME/R/pkg/DESCRIPTION" | awk '{print $NF}'`
pushd "$SPARK_HOME/R" > /dev/null
# Build source package and run full checks
# Do not source the check-cran.sh - it should be run from where it is for it to set SPARK_HOME
- NO_TESTS=1 "$SPARK_HOME/"R/check-cran.sh
+ NO_TESTS=1 "$SPARK_HOME/R/check-cran.sh"
# Move R source package to match the Spark release version if the versions are not the same.
# NOTE(shivaram): `mv` throws an error on Linux if source and destination are same file
if [ "$R_PACKAGE_VERSION" != "$VERSION" ]; then
- mv $SPARK_HOME/R/SparkR_"$R_PACKAGE_VERSION".tar.gz $SPARK_HOME/R/SparkR_"$VERSION".tar.gz
+ mv "$SPARK_HOME/R/SparkR_$R_PACKAGE_VERSION.tar.gz" "$SPARK_HOME/R/SparkR_$VERSION.tar.gz"
fi
# Install source package to get it to generate vignettes rds files, etc.
- VERSION=$VERSION "$SPARK_HOME/"R/install-source-package.sh
+ VERSION=$VERSION "$SPARK_HOME/R/install-source-package.sh"
popd > /dev/null
else
echo "Skipping building R source package"
fi
# Copy other things
-mkdir "$DISTDIR"/conf
-cp "$SPARK_HOME"/conf/*.template "$DISTDIR"/conf
+mkdir "$DISTDIR/conf"
+cp "$SPARK_HOME"/conf/*.template "$DISTDIR/conf"
--- End diff --
oh NO, it might be wrong if we quote the arg $SPARK_HOME/conf/*.template as a whole.
It works well already and the debug info as follows :
+ mkdir '/home/spark build/spark/dist/conf'
+ cp '/home/spark build/spark/conf/docker.properties.template' '/home/spark build/spark/conf/fairscheduler.xml.template' '/home/spark build/spark/conf/log4j.properties.template' '/home/spark build/spark/conf/metrics.properties.template' '/home/spark build/spark/conf/slaves.template' '/home/spark build/spark/conf/spark-defaults.conf.template' '/home/spark build/spark/conf/spark-env.sh.template' '/home/spark build/spark/dist/conf'
---
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.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request #17452: [SPARK-20123][build]$SPARK_HOME variable might ha...
Posted by zuotingbing <gi...@git.apache.org>.
Github user zuotingbing commented on a diff in the pull request:
https://github.com/apache/spark/pull/17452#discussion_r108400132
--- Diff: R/check-cran.sh ---
@@ -20,14 +20,14 @@
set -o pipefail
set -e
-FWDIR="$(cd `dirname "${BASH_SOURCE[0]}"`; pwd)"
-pushd $FWDIR > /dev/null
+FWDIR="$(cd "$(dirname "${BASH_SOURCE[0]}")"; pwd)"
--- End diff --
you can use like this:
FWDIR="$(cd "`dirname "${BASH_SOURCE[0]}"`"; pwd)"
But i used the "$( )", it also works. You can test this.
Thank you.
---
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.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #17452: [SPARK-20123][build]$SPARK_HOME variable might have spac...
Posted by zuotingbing <gi...@git.apache.org>.
Github user zuotingbing commented on the issue:
https://github.com/apache/spark/pull/17452
OK\uff0cwill do. Thanks!
---
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.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request #17452: [SPARK-20123][build]$SPARK_HOME variable might ha...
Posted by srowen <gi...@git.apache.org>.
Github user srowen commented on a diff in the pull request:
https://github.com/apache/spark/pull/17452#discussion_r108885749
--- Diff: dev/make-distribution.sh ---
@@ -217,43 +217,43 @@ fi
# Make R package - this is used for both CRAN release and packing R layout into distribution
if [ "$MAKE_R" == "true" ]; then
echo "Building R source package"
- R_PACKAGE_VERSION=`grep Version $SPARK_HOME/R/pkg/DESCRIPTION | awk '{print $NF}'`
+ R_PACKAGE_VERSION=`grep Version "$SPARK_HOME/R/pkg/DESCRIPTION" | awk '{print $NF}'`
pushd "$SPARK_HOME/R" > /dev/null
# Build source package and run full checks
# Do not source the check-cran.sh - it should be run from where it is for it to set SPARK_HOME
- NO_TESTS=1 "$SPARK_HOME/"R/check-cran.sh
+ NO_TESTS=1 "$SPARK_HOME/R/check-cran.sh"
# Move R source package to match the Spark release version if the versions are not the same.
# NOTE(shivaram): `mv` throws an error on Linux if source and destination are same file
if [ "$R_PACKAGE_VERSION" != "$VERSION" ]; then
- mv $SPARK_HOME/R/SparkR_"$R_PACKAGE_VERSION".tar.gz $SPARK_HOME/R/SparkR_"$VERSION".tar.gz
+ mv "$SPARK_HOME/R/SparkR_$R_PACKAGE_VERSION.tar.gz" "$SPARK_HOME/R/SparkR_$VERSION.tar.gz"
fi
# Install source package to get it to generate vignettes rds files, etc.
- VERSION=$VERSION "$SPARK_HOME/"R/install-source-package.sh
+ VERSION=$VERSION "$SPARK_HOME/R/install-source-package.sh"
popd > /dev/null
else
echo "Skipping building R source package"
fi
# Copy other things
-mkdir "$DISTDIR"/conf
-cp "$SPARK_HOME"/conf/*.template "$DISTDIR"/conf
+mkdir "$DISTDIR/conf"
+cp "$SPARK_HOME"/conf/*.template "$DISTDIR/conf"
--- End diff --
I think the two args need quoting separately?
---
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.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request #17452: [SPARK-20123][build]$SPARK_HOME variable might ha...
Posted by srowen <gi...@git.apache.org>.
Github user srowen commented on a diff in the pull request:
https://github.com/apache/spark/pull/17452#discussion_r108898852
--- Diff: dev/make-distribution.sh ---
@@ -217,43 +217,43 @@ fi
# Make R package - this is used for both CRAN release and packing R layout into distribution
if [ "$MAKE_R" == "true" ]; then
echo "Building R source package"
- R_PACKAGE_VERSION=`grep Version $SPARK_HOME/R/pkg/DESCRIPTION | awk '{print $NF}'`
+ R_PACKAGE_VERSION=`grep Version "$SPARK_HOME/R/pkg/DESCRIPTION" | awk '{print $NF}'`
pushd "$SPARK_HOME/R" > /dev/null
# Build source package and run full checks
# Do not source the check-cran.sh - it should be run from where it is for it to set SPARK_HOME
- NO_TESTS=1 "$SPARK_HOME/"R/check-cran.sh
+ NO_TESTS=1 "$SPARK_HOME/R/check-cran.sh"
# Move R source package to match the Spark release version if the versions are not the same.
# NOTE(shivaram): `mv` throws an error on Linux if source and destination are same file
if [ "$R_PACKAGE_VERSION" != "$VERSION" ]; then
- mv $SPARK_HOME/R/SparkR_"$R_PACKAGE_VERSION".tar.gz $SPARK_HOME/R/SparkR_"$VERSION".tar.gz
+ mv "$SPARK_HOME/R/SparkR_$R_PACKAGE_VERSION.tar.gz" "$SPARK_HOME/R/SparkR_$VERSION.tar.gz"
fi
# Install source package to get it to generate vignettes rds files, etc.
- VERSION=$VERSION "$SPARK_HOME/"R/install-source-package.sh
+ VERSION=$VERSION "$SPARK_HOME/R/install-source-package.sh"
popd > /dev/null
else
echo "Skipping building R source package"
fi
# Copy other things
-mkdir "$DISTDIR"/conf
-cp "$SPARK_HOME"/conf/*.template "$DISTDIR"/conf
+mkdir "$DISTDIR/conf"
+cp "$SPARK_HOME"/conf/*.template "$DISTDIR/conf"
--- End diff --
Sorry, yes that's also correct. But right now this is just one big argument to cp, not 2 or more. Doesn't it need to be more like `cp "$SPARK_HOME"/conf/*.template "$DISTDIR/conf"`?
---
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.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request #17452: [SPARK-20123][build]$SPARK_HOME variable might ha...
Posted by felixcheung <gi...@git.apache.org>.
Github user felixcheung commented on a diff in the pull request:
https://github.com/apache/spark/pull/17452#discussion_r109092653
--- Diff: dev/make-distribution.sh ---
@@ -217,43 +217,43 @@ fi
# Make R package - this is used for both CRAN release and packing R layout into distribution
if [ "$MAKE_R" == "true" ]; then
echo "Building R source package"
- R_PACKAGE_VERSION=`grep Version $SPARK_HOME/R/pkg/DESCRIPTION | awk '{print $NF}'`
+ R_PACKAGE_VERSION=`grep Version "$SPARK_HOME/R/pkg/DESCRIPTION" | awk '{print $NF}'`
pushd "$SPARK_HOME/R" > /dev/null
# Build source package and run full checks
# Do not source the check-cran.sh - it should be run from where it is for it to set SPARK_HOME
- NO_TESTS=1 "$SPARK_HOME/"R/check-cran.sh
+ NO_TESTS=1 "$SPARK_HOME/R/check-cran.sh"
# Move R source package to match the Spark release version if the versions are not the same.
# NOTE(shivaram): `mv` throws an error on Linux if source and destination are same file
if [ "$R_PACKAGE_VERSION" != "$VERSION" ]; then
- mv $SPARK_HOME/R/SparkR_"$R_PACKAGE_VERSION".tar.gz $SPARK_HOME/R/SparkR_"$VERSION".tar.gz
+ mv "$SPARK_HOME/R/SparkR_$R_PACKAGE_VERSION.tar.gz" "$SPARK_HOME/R/SparkR_$VERSION.tar.gz"
fi
# Install source package to get it to generate vignettes rds files, etc.
- VERSION=$VERSION "$SPARK_HOME/"R/install-source-package.sh
+ VERSION=$VERSION "$SPARK_HOME/R/install-source-package.sh"
popd > /dev/null
else
echo "Skipping building R source package"
fi
# Copy other things
-mkdir "$DISTDIR"/conf
-cp "$SPARK_HOME"/conf/*.template "$DISTDIR"/conf
+mkdir "$DISTDIR/conf"
+cp "$SPARK_HOME"/conf/*.template "$DISTDIR/conf"
cp "$SPARK_HOME/README.md" "$DISTDIR"
cp -r "$SPARK_HOME/bin" "$DISTDIR"
cp -r "$SPARK_HOME/python" "$DISTDIR"
# Remove the python distribution from dist/ if we built it
if [ "$MAKE_PIP" == "true" ]; then
- rm -f $DISTDIR/python/dist/pyspark-*.tar.gz
+ rm -f "$DISTDIR"/python/dist/pyspark-*.tar.gz
--- End diff --
ok I see that other thread...
---
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.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #17452: [SPARK-20123][build]$SPARK_HOME variable might have spac...
Posted by srowen <gi...@git.apache.org>.
Github user srowen commented on the issue:
https://github.com/apache/spark/pull/17452
Merged to master
---
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.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request #17452: [SPARK-20123][build]$SPARK_HOME variable might ha...
Posted by zuotingbing <gi...@git.apache.org>.
Github user zuotingbing commented on a diff in the pull request:
https://github.com/apache/spark/pull/17452#discussion_r108978377
--- Diff: dev/make-distribution.sh ---
@@ -217,43 +217,43 @@ fi
# Make R package - this is used for both CRAN release and packing R layout into distribution
if [ "$MAKE_R" == "true" ]; then
echo "Building R source package"
- R_PACKAGE_VERSION=`grep Version $SPARK_HOME/R/pkg/DESCRIPTION | awk '{print $NF}'`
+ R_PACKAGE_VERSION=`grep Version "$SPARK_HOME/R/pkg/DESCRIPTION" | awk '{print $NF}'`
pushd "$SPARK_HOME/R" > /dev/null
# Build source package and run full checks
# Do not source the check-cran.sh - it should be run from where it is for it to set SPARK_HOME
- NO_TESTS=1 "$SPARK_HOME/"R/check-cran.sh
+ NO_TESTS=1 "$SPARK_HOME/R/check-cran.sh"
# Move R source package to match the Spark release version if the versions are not the same.
# NOTE(shivaram): `mv` throws an error on Linux if source and destination are same file
if [ "$R_PACKAGE_VERSION" != "$VERSION" ]; then
- mv $SPARK_HOME/R/SparkR_"$R_PACKAGE_VERSION".tar.gz $SPARK_HOME/R/SparkR_"$VERSION".tar.gz
+ mv "$SPARK_HOME/R/SparkR_$R_PACKAGE_VERSION.tar.gz" "$SPARK_HOME/R/SparkR_$VERSION.tar.gz"
fi
# Install source package to get it to generate vignettes rds files, etc.
- VERSION=$VERSION "$SPARK_HOME/"R/install-source-package.sh
+ VERSION=$VERSION "$SPARK_HOME/R/install-source-package.sh"
popd > /dev/null
else
echo "Skipping building R source package"
fi
# Copy other things
-mkdir "$DISTDIR"/conf
-cp "$SPARK_HOME"/conf/*.template "$DISTDIR"/conf
+mkdir "$DISTDIR/conf"
+cp "$SPARK_HOME"/conf/*.template "$DISTDIR/conf"
cp "$SPARK_HOME/README.md" "$DISTDIR"
cp -r "$SPARK_HOME/bin" "$DISTDIR"
cp -r "$SPARK_HOME/python" "$DISTDIR"
# Remove the python distribution from dist/ if we built it
if [ "$MAKE_PIP" == "true" ]; then
- rm -f $DISTDIR/python/dist/pyspark-*.tar.gz
+ rm -f "$DISTDIR"/python/dist/pyspark-*.tar.gz
--- End diff --
it is wrong if we quote the arg `$DISTDIR/python/dist/pyspark-*.tar.gz` as a whole.
---
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.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request #17452: [SPARK-20123][build]$SPARK_HOME variable might ha...
Posted by srowen <gi...@git.apache.org>.
Github user srowen commented on a diff in the pull request:
https://github.com/apache/spark/pull/17452#discussion_r108407452
--- Diff: R/check-cran.sh ---
@@ -20,14 +20,14 @@
set -o pipefail
set -e
-FWDIR="$(cd `dirname "${BASH_SOURCE[0]}"`; pwd)"
-pushd $FWDIR > /dev/null
+FWDIR="$(cd "$(dirname "${BASH_SOURCE[0]}")"; pwd)"
--- End diff --
Ah OK, it does seem to work. There are other instances below though that I suppose should be consistent
---
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.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request #17452: [SPARK-20123][build]$SPARK_HOME variable might ha...
Posted by asfgit <gi...@git.apache.org>.
Github user asfgit closed the pull request at:
https://github.com/apache/spark/pull/17452
---
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.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #17452: [SPARK-20123][build]$SPARK_HOME variable might have spac...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/17452
**[Test build #3631 has started](https://amplab.cs.berkeley.edu/jenkins/job/NewSparkPullRequestBuilder/3631/testReport)** for PR 17452 at commit [`bcf9599`](https://github.com/apache/spark/commit/bcf95991282904c259979c4cfe6b6df293df8edb).
---
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.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request #17452: [SPARK-20123][build]$SPARK_HOME variable might ha...
Posted by srowen <gi...@git.apache.org>.
Github user srowen commented on a diff in the pull request:
https://github.com/apache/spark/pull/17452#discussion_r108977947
--- Diff: dev/make-distribution.sh ---
@@ -217,43 +217,43 @@ fi
# Make R package - this is used for both CRAN release and packing R layout into distribution
if [ "$MAKE_R" == "true" ]; then
echo "Building R source package"
- R_PACKAGE_VERSION=`grep Version $SPARK_HOME/R/pkg/DESCRIPTION | awk '{print $NF}'`
+ R_PACKAGE_VERSION=`grep Version "$SPARK_HOME/R/pkg/DESCRIPTION" | awk '{print $NF}'`
pushd "$SPARK_HOME/R" > /dev/null
# Build source package and run full checks
# Do not source the check-cran.sh - it should be run from where it is for it to set SPARK_HOME
- NO_TESTS=1 "$SPARK_HOME/"R/check-cran.sh
+ NO_TESTS=1 "$SPARK_HOME/R/check-cran.sh"
# Move R source package to match the Spark release version if the versions are not the same.
# NOTE(shivaram): `mv` throws an error on Linux if source and destination are same file
if [ "$R_PACKAGE_VERSION" != "$VERSION" ]; then
- mv $SPARK_HOME/R/SparkR_"$R_PACKAGE_VERSION".tar.gz $SPARK_HOME/R/SparkR_"$VERSION".tar.gz
+ mv "$SPARK_HOME/R/SparkR_$R_PACKAGE_VERSION.tar.gz" "$SPARK_HOME/R/SparkR_$VERSION.tar.gz"
fi
# Install source package to get it to generate vignettes rds files, etc.
- VERSION=$VERSION "$SPARK_HOME/"R/install-source-package.sh
+ VERSION=$VERSION "$SPARK_HOME/R/install-source-package.sh"
popd > /dev/null
else
echo "Skipping building R source package"
fi
# Copy other things
-mkdir "$DISTDIR"/conf
-cp "$SPARK_HOME"/conf/*.template "$DISTDIR"/conf
+mkdir "$DISTDIR/conf"
+cp "$SPARK_HOME"/conf/*.template "$DISTDIR/conf"
cp "$SPARK_HOME/README.md" "$DISTDIR"
cp -r "$SPARK_HOME/bin" "$DISTDIR"
cp -r "$SPARK_HOME/python" "$DISTDIR"
# Remove the python distribution from dist/ if we built it
if [ "$MAKE_PIP" == "true" ]; then
- rm -f $DISTDIR/python/dist/pyspark-*.tar.gz
+ rm -f "$DISTDIR"/python/dist/pyspark-*.tar.gz
--- End diff --
Doesn't that fail to work because `*` isn't expanded?
---
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.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #17452: [SPARK-20123][build]$SPARK_HOME variable might have spac...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/17452
**[Test build #3631 has finished](https://amplab.cs.berkeley.edu/jenkins/job/NewSparkPullRequestBuilder/3631/testReport)** for PR 17452 at commit [`bcf9599`](https://github.com/apache/spark/commit/bcf95991282904c259979c4cfe6b6df293df8edb).
* This patch passes all tests.
* This patch merges cleanly.
* This patch adds no public classes.
---
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.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org