[SPARK-8603] [sparkR] In windows, Incorrect file seperator passed to … #7025
Conversation
…Java and Scripts from R
|
ok to test |
|
Test build #36035 has finished for PR 7025 at commit
|
|
The code change looks ok but I need to test it on a windows machine to see what is going on. Will try to do this soon |
There was a problem hiding this comment.
Is anything which is not Unix guaranteed to be Windows? It seems like Windows is actually the special case here, so can we check its platform directly rather than assuming "not unix" is Windows?
There was a problem hiding this comment.
According to https://stat.ethz.ch/R-manual/R-devel/library/base/html/Platform.html, .Platform$OS.type have only two options: "unix" or "windows".
There was a problem hiding this comment.
@sun-rui @JoshRosen I have a window machine. Although I am not used to R, if changing the condition to .Platform$OS.type != "windows" and rebasing are only things to be done, I can make a PR based on this. (The author might have to be @prakashpc though).
There was a problem hiding this comment.
@HyukjinKwon, cool, thanks! Could you double-check if this issue exists on Windows ,and this PR is the right fix. What I am wondering is that why system2 can't handle path separator since system2 is meant to be portable.
There was a problem hiding this comment.
@sun-rui I will try to test and then submit a PR. Thank you!
|
Hey @prakashpc, I think this PR is pretty close to being merge-ready in case you'd like to address my comments upthread. If you no longer have time to work on this, though, would you mind closing this in order to cut down on the PR review queue? Thanks! |
|
Can one of the admins verify this patch? |
|
ping @prakashpc |
…indows
## What changes were proposed in this pull request?
This PR corrects SparkR to use `shell()` instead of `system2()` on Windows.
Using `system2(...)` on Windows does not process windows file separator `\`. `shell(tralsate = TRUE, ...)` can treat this problem. So, this was changed to be chosen according to OS.
Existing tests were failed on Windows due to this problem. For example, those were failed.
```
8. Failure: sparkJars tag in SparkContext (test_includeJAR.R#34)
9. Failure: sparkJars tag in SparkContext (test_includeJAR.R#36)
```
The cases above were due to using of `system2`.
In addition, this PR also fixes some tests failed on Windows.
```
5. Failure: sparkJars sparkPackages as comma-separated strings (test_context.R#128)
6. Failure: sparkJars sparkPackages as comma-separated strings (test_context.R#131)
7. Failure: sparkJars sparkPackages as comma-separated strings (test_context.R#134)
```
The cases above were due to a weird behaviour of `normalizePath()`. On Linux, if the path does not exist, it just prints out the input but it prints out including the current path on Windows.
```r
# On Linus
path <- normalizePath("aa")
print(path)
[1] "aa"
# On Windows
path <- normalizePath("aa")
print(path)
[1] "C:\\Users\\aa"
```
## How was this patch tested?
Jenkins tests and manually tested in a Window machine as below:
Here is the [stdout](https://gist.github.com/HyukjinKwon/4bf35184f3a30f3bce987a58ec2bbbab) of testing.
Closes #7025
Author: hyukjinkwon <gurwls223@gmail.com>
Author: Hyukjin Kwon <gurwls223@gmail.com>
Author: Prakash PC <prakash.chinnu@gmail.com>
Closes #13165 from HyukjinKwon/pr/7025.
(cherry picked from commit 1c40373)
Signed-off-by: Shivaram Venkataraman <shivaram@cs.berkeley.edu>
8 participants
[SPARK-8603] [sparkR] In windows, Incorrect file seperator passed to Java and Scripts from R
R supports both file separator in the file path (Unix : / and Windows: ) irrespective of the operating system but when passing file path to Java or script program, it has to be converted according to operating system