-
Notifications
You must be signed in to change notification settings - Fork 29k
SPARK-3337 Paranoid quoting in shell to allow install dirs with spaces within. #2229
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-3337 Paranoid quoting in shell to allow install dirs with spaces within. #2229
Conversation
|
QA tests have started for PR 2229 at commit
|
|
QA tests have finished for PR 2229 at commit
|
|
Great idea. I've been itching to do this myself. retest this please |
|
QA tests have started for PR 2229 at commit
|
|
QA tests have finished for PR 2229 at commit
|
4bcd7e8 to
11642ad
Compare
|
QA tests have started for PR 2229 at commit
|
11642ad to
3b9ff28
Compare
|
Tested it by have spark directory as "Apache Spark". all scripts seemed to work. |
|
QA tests have started for PR 2229 at commit
|
|
QA tests have finished for PR 2229 at commit
|
|
QA tests have finished for PR 2229 at commit
|
3b9ff28 to
ded203f
Compare
|
QA tests have started for PR 2229 at commit
|
|
Tests timed out after a configured wait of |
|
retest this please |
|
QA tests have started for PR 2229 at commit
|
|
QA tests have finished for PR 2229 at commit
|
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.
You probably don't need to fix this one since it's only a number (not a big deal)
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.
You are right !
|
QA tests have started for PR 2229 at commit
|
|
QA tests have finished for PR 2229 at commit
|
sbt/sbt-launch-lib.bash
Outdated
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.
Him what about this one? If we have multiple arguments this is gonna group them together. Do we want that?
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.
Ah yes, getting rid of them(and alike) as well.
7b6eee7 to
b35fe1c
Compare
|
retest this please |
|
QA tests have started for PR 2229 at commit
|
|
Tests timed out after a configured wait of |
b35fe1c to
5596f9d
Compare
|
test this please. |
5596f9d to
d4ad660
Compare
|
QA tests have started for PR 2229 at commit
|
|
This patch is most prone to merge conflicts, would be good to merge it soon. @andrewor14 thoughts? |
|
QA tests have finished for PR 2229 at commit
|
|
Yup. I played around with this locally with spaces and it also works for me. Merging this into master. Thanks. |
...
Tested ! TBH, it isn't a great idea to have directory with spaces within. Because emacs doesn't like it then hadoop doesn't like it. and so on...