-
Notifications
You must be signed in to change notification settings - Fork 28.3k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[SPARK-5434] [EC2] Preserve spaces in EC2 path #4224
Conversation
A minor issue, but we might want to backport this. Shall I create a JIRA? |
Test build #26169 has started for PR 4224 at commit
|
Test build #26169 has finished for PR 4224 at commit
|
Test PASSed. |
LGTM. Would be good to create a JIRA especially if we want to backport. @JoshRosen might have more ideas on backporting |
@@ -20,6 +20,6 @@ | |||
|
|||
# Preserve the user's CWD so that relative paths are passed correctly to | |||
#+ the underlying Python script. | |||
SPARK_EC2_DIR="$(dirname $0)" | |||
SPARK_EC2_DIR="$(dirname "$0")" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
jw - what does it mean to nest quotes inside of other quotes like this?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
$()
invokes a subshell, so the quotes inside the subshell are processed independently of those outside.
So here we're making sure dirname
gets exactly 1 argument by quoting the $0
, in case the value of $0
has spaces.
Then, we're making sure the output of dirname
in turn is captured as 1 argument when passed to SPARK_EC2_DIR
by quoting the whole subshell, in case that output has spaces.
Welcome to the most deceptive feature of Bash: Word Splitting.
Ok I'm merging this into master and marking it as backport needed (since we're in the middle of a 1.2.1 release) |
Fixes [SPARK-5434](https://issues.apache.org/jira/browse/SPARK-5434). Simple demonstration of the problem and the fix: ``` $ spacey_path="/path/with some/spaces" $ dirname $spacey_path usage: dirname path $ echo $? 1 $ dirname "$spacey_path" /path/with some $ echo $? 0 ``` Author: Nicholas Chammas <nicholas.chammas@gmail.com> Closes #4224 from nchammas/patch-1 and squashes the following commits: 960711a [Nicholas Chammas] [EC2] Preserve spaces in EC2 path
I backported this to 1.2 |
Fixes SPARK-5434.
Simple demonstration of the problem and the fix: