Minor assembly directory fix in load-spark-env-sh

[dstripelis]

Set the current dir path $FWDIR and same at $ASSEMBLY_DIR1, $ASSEMBLY_DIR2
otherwise $SPARK_HOME cannot be visible from spark-env.sh -- no SPARK_HOME variable is assigned there.
I am using the Spark-1.3.0 source code package and I come across with this when trying to start the master: sbin/start-master.sh

[AmplabJenkins]

Can one of the admins verify this patch?

[srowen]

(You could give this a more specific title than "Update load-spark-env.sh")
This is borderline important enough for a JIRA, but I think we might consider this a minor add-on fix for SPARK-4924, maybe.

I'm not sure about this. For example spark-class sources this script with . "$SPARK_HOME"/bin/load-spark-env.sh and pyspark does similarly. So these have SPARK_HOME set.

However run-example uses . "$FWDIR"/bin/load-spark-env.sh, and scripts in sbin use . "$SPARK_PREFIX/bin/load-spark-env.sh" Clearly they don't expect SPARK_HOME necessarily.

CC @vanzin since this used to refer to FWDIR actually:
517975d

The lines you reference don't exist in 1.3.0 though. Are you sure you're using 1.3.0?

[vanzin]

Hmm. I see. spark-daemon.sh calls this script directly, but does not set SPARK_HOME. Makes sense.

One comment: doing the following in a script that is sourced is a little weird:

FWDIR="$(cd "`dirname "$0"`"/..; pwd)"

That's because $0 actually refers to the script that is sourcing this one, not load-spark-env.sh. In Spark, though, the code above probably still works because of where all the scripts are.

I'd put a comment in the code about that, maybe, or use $BASH_SOURCE instead. Other than that, looks like a correct change to me.

[dstripelis]

To be more precise, I am running Spark on HPC and I have made the build on scala-2.11; however when I started the master, the error I was receiving was
echo "Failed to find Spark assembly in $assembly_folder" 1>&2
echo "You need to build Spark before running this program." 1>&2
which were coming from the compute-classpath.sh script
So, I figured that the scala version was not passed correctly, since when we are doing the sourcing
. "$FWDIR"/bin/load-spark-env.sh inside the the compute-classpath.sh, the scala_version was assigned explicitly to 2.10, clearly it could not find the assembly of scala-2.11 in (assembly/target/scala-2.11). After some closer investigation I saw that when the check of if a scala-2.11 directory exists (in the load-spark-env.sh) the $FWD_DIR did not have a valid value and it was mistakenly giving a scala version of 2.10 since it couldn't find the correct path; as such I suggested this change.

and yes this is on Spark 1.3.0, I have downloaded the package from the following mirror:
http://www.apache.org/dyn/closer.cgi/spark/spark-1.3.0/spark-1.3.0.tgz

[srowen]

@Raschild what about Marcelo's comment about $BASH_SOURCE? does this affect master or only 1.3 and before?

[dstripelis]

@srowen yes you could use something like
FWDIR="$(cd "dirname "${BASH_SOURCE[0]}""/..; pwd)"

@vanzin yes you are right; I only used because it is the same first command in the compute-classpath.sh because I thought it will be wise to have some command cohesion...

From what I see it is the same issue in Spark-1.2.0 package...I haven't look at previous versions!

[srowen]

Yes I was wonder whether it affects master rather than older versions, but it looks like it does. Up to you which approach you would like to take in the end but I'm happy to merge your final decision shortly.

[srowen]

ok to test

[SparkQA]

Test build #29665 has started for PR 5261 at commit b9babcd.

[SparkQA]

Test build #29665 has finished for PR 5261 at commit b9babcd.

• This patch fails Spark unit tests.
• This patch merges cleanly.
• This patch adds no public classes.
• This patch does not change any dependencies.

[AmplabJenkins]

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

[srowen]

Jenkins, retest this please.

[SparkQA]

Test build #29668 has started for PR 5261 at commit b9babcd.

[SparkQA]

Test build #29668 has finished for PR 5261 at commit b9babcd.

• This patch passes all tests.
• This patch merges cleanly.
• This patch adds no public classes.
• This patch does not change any dependencies.

[AmplabJenkins]

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

[srowen]

@Raschild this is waiting on your comment and/or change. Can you make the title more descriptive please?

[dstripelis]

@srowen
Changed the title. Hope this one is more indicative!

[srowen]

@Raschild ... hm I would have thought something like [MINOR] fix assembly dir location in load-spark-env-sh is more descriptive. Marcelo still has an outstanding question for you about BASH_SOURCE. What do you think? sounds like it should either have a comment or change.

I'm still a little confused about which branch this applies to. Yes, I see you're applying this to master, fine. However in 1.3, the two lines you change don't reference SPARK_HOME. I assume the net change in previous branches is just to set FWDIR differently?

I'd like to get this resolved one way or the other.

[dstripelis]

@srowen - Title Updated
I applied Marcello's comment but still it couldn't work. The whole point for $FWDIR is in these two lines:
ASSEMBLY_DIR2="$FWDIR/assembly/target/scala-2.11"
ASSEMBLY_DIR1="$FWDIR/assembly/target/scala-2.10"
if you do not set up properly the directory then the if condition will fall into the scala-2.10 and this is when the error occurs:
if [ -d "$ASSEMBLY_DIR2" ]; then export SPARK_SCALA_VERSION="2.11" else export SPARK_SCALA_VERSION="2.10" fi
Overall from what I have seen this applies to most of the spark development packages. And yes as you have stated before the net change in previous branches is just to set FWDIR differently! Otherwise, another way to work around it, could be to source a script with all the global variables [ i.e. $SPARK_HOME. ]

[srowen]

I've merged this for master; I will come back and cherry-pick into 1.3 / 1.2 after the 1.3 release, just as I don't want to merge into the 1.3 branch with any possible functional change this second.