Remove/update r_output_test for Windows compatibility

[extendr]

Remove/update r_output_test for Windows compatibility

[Ilia-Kosenkov]

(cargo test -- --nocapture --test-threads=1) *>&1 > test.log

   Compiling winapi-x86_64-pc-windows-gnu v0.4.0
   Compiling winapi v0.3.9
   Compiling proc-macro2 v1.0.24
   Compiling memchr v2.3.4
   Compiling unicode-xid v0.2.1
   Compiling autocfg v1.0.1
   Compiling cc v1.0.66
   Compiling version_check v0.9.2
   Compiling glob v0.3.0
   Compiling libc v0.2.81
   Compiling lazy_static v1.4.0
   Compiling log v0.4.11
   Compiling bitflags v1.2.1
   Compiling unicode-width v0.1.8
   Compiling cfg-if v0.1.10
   Compiling quick-error v1.2.3
   Compiling regex-syntax v0.6.21
   Compiling strsim v0.8.0
   Compiling vec_map v0.8.2
   Compiling bindgen v0.53.3
   Compiling rustc-hash v1.1.0
   Compiling lazycell v1.3.0
   Compiling shlex v0.1.1
   Compiling peeking_take_while v0.1.2
   Compiling syn v1.0.54
   Compiling ndarray v0.13.1
   Compiling rawpointer v0.2.1
   Compiling extendr-api v0.1.10 (C:\Users\[redacted]\source\repos\extendr\extendr-api)
   Compiling thread_local v1.0.1
   Compiling nom v5.1.2
   Compiling textwrap v0.11.0
   Compiling num-traits v0.2.14
   Compiling num-integer v0.1.44
   Compiling num-complex v0.2.4
   Compiling humantime v1.3.0
   Compiling clang-sys v0.29.3
   Compiling matrixmultiply v0.2.4
   Compiling aho-corasick v0.7.15
   Compiling which v3.1.1
   Compiling quote v1.0.7
   Compiling libloading v0.5.2
   Compiling regex v1.4.2
   Compiling cexpr v0.4.0
   Compiling atty v0.2.14
   Compiling winapi-util v0.1.5
   Compiling clap v2.33.3
   Compiling termcolor v1.1.2
   Compiling env_logger v0.7.1
   Compiling extendr-macros v0.1.10 (C:\Users\[redacted]\source\repos\extendr\extendr-macros)
   Compiling libR-sys v0.1.10 (C:\Users\[redacted]\source\repos\libR-sys)
    Finished test [unoptimized + debuginfo] target(s) in 45.71s
     Running target\debug\deps\extendr_api-6e8158411f5aa083.exe

running 8 tests
test args::tests::test_args ... ok
test engine::tests::test_engine ... ok
test robj::tests::parse_test ... ok
test robj::tests::test_debug ... ok
test robj::tests::test_from_robj ... ok
test robj::tests::test_to_robj ... ok
test tests::export_test ... ok
test tests::r_output_test ... ok

test result: ok. 8 passed; 0 failed; 0 ignored; 0 measured; 0 filtered out

     Running target\debug\deps\extendr_macros-8ca9d8d6a277d051.exe

running 0 tests

test result: ok. 0 passed; 0 failed; 0 ignored; 0 measured; 0 filtered out

   Doc-tests extendr-api

running 11 tests
test src\lib.rs - (line 18) ... ignored
test src\lib.rs - (line 28) ... ignored
test src\lib.rs - (line 38) ... ignored
test src\lib.rs - (line 49) ... ignored
test src\lib.rs - (line 58) ... ignored
test src\rmacros.rs - c (line 26) ... ok
test src\rmacros.rs - data_frame (line 95) ... ok
test src\rmacros.rs - list (line 76) ... ok
test src\rmacros.rs - r (line 10) ... ok
test src\rmacros.rs - read_table (line 61) ... ok
test src\rmacros.rs - rep (line 45) ... ok

test result: ok. 6 passed; 0 failed; 5 ignored; 0 measured; 0 filtered out

   Doc-tests extendr-macros

running 0 tests

test result: ok. 0 passed; 0 failed; 0 ignored; 0 measured; 0 filtered out

[lebensterben]

what's the value for "capabilities("fifo")"

[Ilia-Kosenkov]

@lebensterben,
surprisngly, TRUE. I hope that someone else can test it in a Windows environment before making any decisions.

[clauswilke]

Just to be clear, is PR extendr/libR-sys#12 required for the tests to work on Windows?

[lebensterben]

@lebensterben, ...

@Ilia-Kosenkov I think you should report to the upstream in the meantime.

[Ilia-Kosenkov]

@clauswilke ,
Yes, otherwise I cannot reliably build it. With your PR, I just need to set $R_HOME and switch to stable-x86_64-pc-wndows-gnu default toolchain to run things pretty much 'as-is'.

@lebensterben,
My quick attempt at googling fifo issues yielded no results, there is still a possibility this issue is not universal and is caused by something on my two machines.
I will try to reproduce it elsewhere, so far I can confirm it fails not only on 4.0.3 but also on older 3.6.3.

[lebensterben]

My personal opinion is old versions of R and non x86-64 architectures are not worth supporting at this stage. And preferably not even Windows. Unless there is at least one core contributor who routinely uses R on those platform/OS.

[clauswilke]

@Ilia-Kosenkov I see. I can merge the PR in principle, but I am still pondering whether the case without R_HOME can be made to work also. Could you do one quick test for me? Could you start R and let me know what the output from this line is:

cat(R.home(), R.home('include'), R.home('bin'), sep = '\n')

I wonder whether the x64 or i386 is included or not. We could add it manually if it is missing, and that might fix the build when R_HOME isn't set.

[Ilia-Kosenkov]

@clauswilke
Executed from R-4.0.3 x64

C:/PROGRA~1/R/R-40~1.3
C:/PROGRA~1/R/R-40~1.3/include
C:/PROGRA~1/R/R-40~1.3/bin/x64     

The name mangling as a legacy Windows feature, paths are resolved correctly, its just their representation in the current situation.

[clauswilke]

Ok, so this explains why it doesn't work without R_HOME: We're taking the output as printed, but it's not actually a valid path in Windows. Is there any way to get R to print the full path?

In any case, I've merged the PR. Could you prepare a PR that addresses the current issue by enabling the tests on Windows and modifying the test that fails?

[Ilia-Kosenkov]

@clauswilke,
I will do that later.

However, the problem with paths was, as far as I remember, in i386 version.
Rust build process was launched as x64 no matter what target was chosen, so when building against i386 we pull x64 libraries from R variables. Your PR puts things in order and allows consistent i386 builds (but no tests so far).

Thre are two options to fix paths: fs::path_real, which returns correct *nix-style path, and normalizePath, which gets Windows-style (i.e. with \ separators). I guess any path within R should be constructed using either fs::path or file.path, and then normalized using normalizePath before exporting to rust toolchain.
It should not harm builds on other systems as it will just handle paths carefully using dedicated library, and not by simply appending strings with random slashes.

[clauswilke]

@Ilia-Kosenkov Thanks, this is useful. I'll prepare another PR to libR-sys to address these issues. I think I now know what to do to make it work without R_HOME.

[clauswilke]

@Ilia-Kosenkov I've been trying to create a new way of finding all the correct paths (extendr/libR-sys#18), but one final piece is missing on Windows. I can't call R again from the command line once I have determined the R home. It's very strange, but it's probably got something to do with the path layout on Windows that I'm not aware of.

The problem is somewhere here:
https://github.com/clauswilke/libR-sys/blob/1f3f75a305293af79b436e6ea0dc54ca6a9b9d90/build.rs#L72-L78
or here:
https://github.com/clauswilke/libR-sys/blob/1f3f75a305293af79b436e6ea0dc54ca6a9b9d90/build.rs#L87-L93

Could you let me know where the R executable lives on Windows and what it is called? And can you take the R home directory on Windows, append something like bin\x64\R.exe, and then run the equivalent of this command:

R -s -e "cat(normalizePath(R.home('include')), sep = '\n')"

In other words, something like:

$(R_HOME)\bin\x64\R.exe -s -e "cat(normalizePath(R.home('include')), sep = '\n')"

[Ilia-Kosenkov]

@clauswilke ,
If I set R_HOME to point to a correct folder, then it works fine.
However, to get correct paths in Windows CMD you need something like this:
"%R_HOME%\bin\x64\R.exe" -s -e "cat(normalizePath(R.home('include')), sep = '\n')"
And the output is C:\Program Files\R\R-4.0.3\include, which seems correct to me.

[clauswilke]

I see. This is frustrating, because at that point in the code the R home should be known and set correctly. I have made an update that prints out some debugging info. Could you run that without setting R_HOME and let me know what it prints out after the crash? Just like lines 99 to 101 here: https://github.com/extendr/libR-sys/runs/1524671980?check_suite_focus=true#step:6:99

On GitHub actions, the output is:

  R home: C:\R
  R library: C:\R\bin\x64
  R binary: C:\R\bin\x64\R.exe

I don't know anything about Windows, but this does look a bit suspicious.

[clauswilke]

@Ilia-Kosenkov Never mind, I've fixed the issue. It was a single quotes vs. double quotes problem.

[Ilia-Kosenkov]

@clauswilke,
Ok, glad to hear that

[clauswilke]

@Ilia-Kosenkov Now that we've resolved most of the Windows build issues, would you mind preparing a PR with the revised testing code you wrote when you first filed this issue? I'd like to be able to turn on tests on Windows. I think the code you proposed is fine, I'd just like to ask you to add a few comments explaining what you're doing and why.

[Ilia-Kosenkov]

@clauswilke,
Ok, will do that