Initial Windows support for Ark #140

DavisVaughan · 2023-11-07T22:22:45Z

Branched from #172

This PR does the initial work to get Windows support up and running enough that I think we can merge it in. That will allow us to then iterate on a few things at the same time - including getting a GitHub Actions workflow that builds an executable for us.

I'd like to take this chance to document some Windows peculiarities that we have to work around. It may be useful for later.

Platform specific folder structure

We now have a decent chunk of platform specific code, so I've introduced a new convention to handle this. Each crate has the following structure:

sys.rs
sys/unix.rs
sys/windows.rs
sys/unix/
sys/windows/

In sys.rs, we always do:

cfg_if::cfg_if! {
    if #[cfg(unix)] {
        mod unix;
        pub use self::unix::*;
    } else if #[cfg(windows)] {
        mod windows;
        pub use self::windows::*;
    }
}

This allows us to create 2 identical APIs in sys/unix.rs and sys/windows.rs, and then use them in a platform agnostic way with sys::module::fn() from outside of sys/. This is inspired by the Rust std library.

Write console buffer on Windows

On Windows, the write_console() buffer that R provides us has some weird quirks. In general, the buffer that comes through will be in the system encoding, so we typically need to convert it to UTF-8 to then put it in a rust string and forward it to the frontend. Okay, no problem.

The weirdness is that if EmbeddedUTF8 is set for R, then R surrounds UTF-8 text with a 3-byte escape like \x02\xFF\xFE<text>\x03\xFF\xFE to "mark" it. There could be more text in the buffer before or after this that is just in the system encoding. We have to break the buffer into the system encoded bits and the UTF-8 text bits, re-encode the system encoded bits to UTF-8, drop the 3-byte escapes, and put everything back together. This is done in console_to_utf8().

Interrupts

On the unix side, we use R_pending_interrupts to flag when R should handle an interrupt. On the windows side, this is apparently called UserBreak, and is mostly handled the same way, except I don't see R_suspended_interrupts for Windows.

Global variables from libR-sys

libR-sys exposes both functions and global variables from R for us to use. However, either it, bindgen, or rustc have an issue where global variables bindings are not being written in such a way that they work for Windows. Take R_NilValue for example. It is exposed in libR-sys through:

extern "C" {
  pub static mut R_NilValue: SEXP;
}

And libR-sys sets println!("cargo:rustc-link-lib=dylib=R");. Now, AFAICT that should be enough because it tells rustc to link to R dynamically.

And it does work fine on Mac, but on Windows global variables MUST be marked with dllimport otherwise the linker cannot find them, and this does not seem to be happening with this setup. You must also add:

#[link(name = "R", kind = "dylib")]
extern "C" {
  pub static mut R_NilValue: SEXP;
}

If you do that, then it works right on Windows. This causes quite an issue because we import many global variables from R.

To work around this, I have created a new subcrate, libR-shim, in this PR #172. It re-exports everything we need from libR-sys except for the global variables, which it then manually provides bindings for tagged with the additional #link[] attribute. This wasn't fun to do, but it seemed like the best way.

Practically, all this means is that we now import libR_shim::* instead of libR_sys::*, and if we are missing anything in the shim, we have to re-export it before we can use it.

libR-sys has been removed as a dependency from ark and harp, so you can't accidentally use libR_sys::R_NilValue, which would be bad.

R setup

On Unix, we have access to a number of ptr_* global variables that we use to set hooks like ReadConsole() (ptr_R_ReadConsole) and friends. On Windows, those don't actually exist. They would be declared in Rinterface.h, but they are behind a #ifdef R_INTERFACE_PTRS, and that is only defined on Unix.
https://github.com/wch/r-source/blob/55cd975c538ad5a086c2085ccb6a3037d5a0cb9a/src/include/Rinterface.h#L131

Instead, we set things like this through Rstart, a struct that holds startup options that is passed to R_DefParamsEx(), which sets up some initial options, and then R_SetParams(), which assigns the things in Rstart into global variables that R manages from then on.
https://github.com/wch/r-source/blob/55cd975c538ad5a086c2085ccb6a3037d5a0cb9a/src/include/R_ext/RStartup.h#L127-L129

The problem is, libR-sys doesn't include Rstart in the generated bindings on Windows. They would need to include #include <R_ext/RStartup.h> in their wrapper.h file here (https://github.com/extendr/libR-sys/blob/6fc2b5f2371fe20ec53d2cdf845a6e13dfdd3cfe/wrapper.h#L42), and they would also need to #define Win32 to generate the full Windows bindings for it.

For now, I have actually gone and done this manually using a fork of libR-sys. The generated bindings for Rstart lives in interface_types.rs, which we then use in interface.rs. Hopefully we can remove this in the future?

DLLs and LIBs

At cargo build time, ark / libR-sys need R.lib to exist at C:\\Program Files\\R\\R-4.3.2\\bin\\x64\\R.lib. But R doesn't ship with a lib file, it ships with R.dll. To workaround this, we use a script from RStudio called dll2lib.R (see the comment below) which uses Visual Studio utilities to generate the R.lib from the dll.

You typically have to set R_HOME before cargo build the first time you build the libR-sys dependency, I do that with

# command prompt
set R_HOME=C:\Program Files\R\R-4.3.2
# powershell
$env:R_HOME = "C:\Program Files\R\R-4.3.2"

After building it once, you don't need that to iterate on ark itself.

At ark load time, i.e. when the chosen version of R is being initialized, ark.exe needs to be able to dynamically link to the R.dll that goes with the user's R version. Windows uses the PATH as one place where it looks for libraries to dynamically link against, so in positron-r we add C:\\Program Files\\R\\R-<VERSION>\\bin\\x64 to the PATH of the subprocess that ark is run in.

Still TODO after merging

Create a GitHub Actions workflow for building ark on Windows
ark.exe needs a Windows Application Manifest so that it can declare UTF-8 as the default codepage. Right now 1252 comes through instead. RStudio actually produces rsession.exe and rsession-utf8.exe and ships them both, one has UTF-8 support in the manifest and the other does not. We may have to do the same for ark. produce separate UTF-8 rsession binary for ucrt builds rstudio/rstudio#10524. There is a rust crate to help us with this https://lib.rs/crates/embed-manifest.
Stream capturing on Windows, i.e. so system2() commands show stdout on the screen. Use https://github.com/rstudio/rstudio/blob/main/src/cpp/core/system/Win32OutputCapture.cpp as a guide.
Right now we set the RSTART_VERSION to 0 in R_DefParamsEx(), but in theory we should use 1. That may "just work", I haven't fully tested it. It would give us access to some additional hooks in newer versions of R (like a cleanup hook).
The positron-r build process needs to utilize dll2lib.R during the build process.
We need to merge some variation of Barebones work for R discovery on Windows positron#1900, particularly for updating the PATH to know about R's bin directory where R.dll lives, since the PATH is how Windows finds runtime dynamic library dependencies (crazy). That PR also has some kind of hardcoded support for detecting an R installation in the first place, which needs to be generalized.
Either update to dev winsafe if this is fixed, or write our own wrappers around the bare bones equivalents to avoid the +1 issue, or manually shorten the string in the meantime if we know this is going to be appended. I don't think MultiByteToWideChar and WideCharToMultiByte need to add 1 to num_bytes rodrigocfd/winsafe#111

Outdated notes below, possibly still useful for historical purposes:

I got stuck for awhile at Rust's (rustc's) linking step where it threw out something like: "Can't find R.lib"

And indeed in the R_HOME/bin directory there is an R.dll but no R.lib. It seems like in RStudio for Windows they totally regenerate the R.lib on the fly using dll2lib.R?? It uses this script:
https://github.com/rstudio/rstudio/blob/d26c0d74ad873a0f48b9153c2b0e56111cae2932/src/cpp/tools/dll2lib.R

It's kind of bonkers, but this works (from Command Prompt):

Start Command Prompt with admin privileges
cd "C:\Program Files\Microsoft Visual Studio\2022\Community\Common7\Tools\"
VsDevCmd.bat -clean_env -no_logo
VsDevCmd.bat -arch=amd64 -startdir=none -host_arch=amd64 -winsdk=10.0.22000.0 -no_logo
Run dll2lib.R in an R session started from that Command Prompt session. Skip the Sys.setenv() line at the beginning.
You should end up with an R.lib (and some others) in your R_HOME/bin

(These commands were taken from https://github.com/rstudio/rstudio/blob/d26c0d74ad873a0f48b9153c2b0e56111cae2932/package/win32/make-package.bat#L179-L188, I think the idea is that it ends up putting some required locations on the path, i.e. for dumpbin.exe, lib.exe, and maybe cl.exe (a c compiler))

That allowed it to find R.lib, and now I am stuck with a new linker error:

  = note:    Creating library D:\Users\davis-vaughan\files\programming\positron\amalthea\target\debug\deps\ark.lib and object D:\Users\davis-vaughan\files\programming\positron\amalthea\target\debug\deps\ark.exp
          libark-80e335538b8b7a46.rlib(ark-80e335538b8b7a46.2tsak1yhuntf56wy.rcgu.o) : error LNK2019: unresolved external symbol R_running_as_main_program referenced in function _ZN3ark9interface7start_r17h6836a63f3dd11975E
          libark-80e335538b8b7a46.rlib(ark-80e335538b8b7a46.2tsak1yhuntf56wy.rcgu.o) : error LNK2019: unresolved external symbol ptr_R_WriteConsole referenced in function _ZN3ark9interface7start_r17h6836a63f3dd11975E
          libark-80e335538b8b7a46.rlib(ark-80e335538b8b7a46.2tsak1yhuntf56wy.rcgu.o) : error LNK2019: unresolved external symbol ptr_R_WriteConsoleEx referenced in function _ZN3ark9interface7start_r17h6836a63f3dd11975E
          libark-80e335538b8b7a46.rlib(ark-80e335538b8b7a46.2tsak1yhuntf56wy.rcgu.o) : error LNK2019: unresolved external symbol ptr_R_ShowMessage referenced in function _ZN3ark9interface7start_r17h6836a63f3dd11975E
          libark-80e335538b8b7a46.rlib(ark-80e335538b8b7a46.2tsak1yhuntf56wy.rcgu.o) : error LNK2019: unresolved external symbol ptr_R_Busy referenced in function _ZN3ark9interface7start_r17h6836a63f3dd11975E
          libark-80e335538b8b7a46.rlib(ark-80e335538b8b7a46.2tsak1yhuntf56wy.rcgu.o) : error LNK2019: unresolved external symbol R_wait_usec referenced in function _ZN3ark9interface7start_r17h6836a63f3dd11975E
          libark-80e335538b8b7a46.rlib(ark-80e335538b8b7a46.2tsak1yhuntf56wy.rcgu.o) : error LNK2019: unresolved external symbol R_PolledEvents referenced in function _ZN3ark9interface7start_r17h6836a63f3dd11975E
          libark-80e335538b8b7a46.rlib(ark-80e335538b8b7a46.3v147ityszidpou2.rcgu.o) : error LNK2001: unresolved external symbol R_PolledEvents
          libark-80e335538b8b7a46.rlib(ark-80e335538b8b7a46.2tsak1yhuntf56wy.rcgu.o) : error LNK2019: unresolved external symbol R_checkActivity referenced in function _ZN3ark9interface5RMain14process_events17h8139846d39dd97b5E
          libark-80e335538b8b7a46.rlib(ark-80e335538b8b7a46.2tsak1yhuntf56wy.rcgu.o) : error LNK2019: unresolved external symbol R_InputHandlers referenced in function _ZN3ark9interface5RMain14process_events17h8139846d39dd97b5E
          libark-80e335538b8b7a46.rlib(ark-80e335538b8b7a46.2tsak1yhuntf56wy.rcgu.o) : error LNK2019: unresolved external symbol R_runHandlers referenced in function _ZN3ark9interface5RMain14process_events17h8139846d39dd97b5E
          D:\Users\davis-vaughan\files\programming\positron\amalthea\target\debug\deps\ark.exe : fatal error LNK1120: 11 unresolved externals

I think we knew this was coming. Things like ptr_R_WriteConsole are declared in an R header file, but are only defined in Unix specific files. We define them ourselves at runtime, but this is too late for Rust's linker check, I think 😢

On Windows there is no -undefined dynamic_linking flag that we can set, so we need to do something else here, like work with the Rstart struct and set these pointers through that (although that is also tricky because it has changed over time, most recently in R 4.2.0, and RStudio does some crazy stuff there to support many versions)

DavisVaughan · 2023-11-21T18:23:20Z

Figured out where the dumpbin.exe and lib.exe executables live so I got dll2lib.R down to 1 file to run

path <- "C:\\Program Files\\Microsoft Visual Studio\\2022\\Community\\VC\\Tools\\MSVC\\14.37.32822\\bin\\Hostx86\\x86"

if (!dir.exists(path)) {
  stop("Visual Studio tools directory is incorrect or the tools have not been installed.")
}

# Put the path containing the tools on the PATH.
Sys.setenv(PATH = paste(path, Sys.getenv("PATH"), sep = ";"))

# Find R DLLs.
dlls <- list.files(R.home("bin"), pattern = "dll$", full.names = TRUE)

message("Generating .lib files for DLLs in ", R.home("bin"))

# Generate corresponding 'lib' file for each DLL.
for (dll in dlls) {

   # check to see if we've already generated our exports
   def <- sub("dll$", "def", dll)
   if (file.exists(def))
      next

   # Call it on R.dll to generate exports.
   command <- sprintf("dumpbin.exe /EXPORTS /NOLOGO %s", dll)
   message("> ", command)
   output <- system(paste(command), intern = TRUE)

   # Remove synonyms.
   output <- sub("=.*$", "", output)

   # Find start, end markers
   start <- grep("ordinal\\s+hint\\s+RVA\\s+name", output)
   end <- grep("^\\s*Summary\\s*$", output)
   contents <- output[start:(end - 1)]
   contents <- contents[nzchar(contents)]

   # Remove forwarded fields (not certain that this does anything)
   contents <- grep("forwarded to", contents, invert = TRUE, value = TRUE, fixed = TRUE)

   # parse into a table
   tbl <- read.table(text = contents, header = TRUE, stringsAsFactors = FALSE)
   exports <- tbl$name

   # sort and re-format exports
   exports <- sort(exports)
   exports <- c("EXPORTS", paste("\t", tbl$name, sep = ""))

   # Write the exports to a def file
   def <- sub("dll$", "def", dll)
   cat(exports, file = def, sep = "\n")

   # Call 'lib.exe' to generate the library file.
   outfile <- sub("dll$", "lib", dll)
   fmt <- "lib.exe /def:%s /out:%s /machine:%s"
   cmd <- sprintf(fmt, def, outfile, .Platform$r_arch)
   system(cmd)

}

DavisVaughan · 2023-12-05T19:31:48Z

crates/amalthea/src/sys.rs

+cfg_if::cfg_if! {
+    if #[cfg(unix)] {
+        mod unix;
+        pub use self::unix::*;
+    } else if #[cfg(windows)] {
+        mod windows;
+        pub use self::windows::*;
+    }
+}


Each crate now has a sys.rs file that looks like this. Typically the way this works is that, for example, the crate level signals module will call sys::signals::fn() for a platform specific version of fn() that it needs. i.e. handle_interrupt_request() is one such fn().

You typically won't call sys::signals::* outside of the signals module. The signals module itself then either re-exposes functions through pub use or exposes its own slightly higher level wrappers around the system specific utilities.

So then other files call crate::signals::*

This is modeled after the Rust std library

DavisVaughan · 2023-12-05T19:32:37Z

crates/amalthea/src/sys/windows/stream_capture.rs

+    }
+
+    pub fn listen(&self) {
+        // TODO: Windows


Stub for stream capturing on Windows. A big TODO, but not critical to "just make it work"

DavisVaughan · 2023-12-05T19:38:34Z

crates/ark/src/interface.rs

-    fn initialize_signal_handlers() {
-        // Reset the signal block.
-        //
-        // This appears to be necessary on macOS; 'sigprocmask()' specifically
-        // blocks the signals in _all_ threads associated with the process, even
-        // when called from a spawned child thread. See:
-        //
-        // https://github.com/opensource-apple/xnu/blob/0a798f6738bc1db01281fc08ae024145e84df927/bsd/kern/kern_sig.c#L1238-L1285
-        // https://github.com/opensource-apple/xnu/blob/0a798f6738bc1db01281fc08ae024145e84df927/bsd/kern/kern_sig.c#L796-L839
-        //
-        // and note that 'sigprocmask()' uses 'block_procsigmask()' to apply the
-        // requested block to all threads in the process:
-        //
-        // https://github.com/opensource-apple/xnu/blob/0a798f6738bc1db01281fc08ae024145e84df927/bsd/kern/kern_sig.c#L571-L599
-        //
-        // We may need to re-visit this on Linux later on, since 'sigprocmask()' and
-        // 'pthread_sigmask()' may only target the executing thread there.
-        //
-        // The behavior of 'sigprocmask()' is unspecified after all, so we're really
-        // just relying on what the implementation happens to do.
-        let mut sigset = SigSet::empty();
-        sigset.add(SIGINT);
-        sigprocmask(SigmaskHow::SIG_BLOCK, Some(&sigset), None).unwrap();


When you see a big deletion, it didn't get removed, just moved to the unix/* equivalent file, i.e. unix/interface.rs

crates/ark/src/interface.rs

DavisVaughan · 2023-12-05T19:43:09Z

crates/ark/src/sys/unix/interface.rs

The Unix specific startup code from interface.rs has been moved here. Not much has changed really. Unix specific callables from the R api are imported at the bottom of here now.

DavisVaughan · 2023-12-05T20:04:42Z

crates/ark/src/sys/windows/interface_types.rs

Manually generated bindings for Rstart using a forked version of libR-sys

I'm not entirely sure how this is going to behave on different versions of R. Hopefully it "just works" since it has C representation and older versions of R simply won't access fields like EmitEmbeddedUTF8, added in R 4.0.0

(Similar to what RStudio does)

DavisVaughan · 2023-12-05T20:05:37Z

crates/ark/src/sys/windows/signals.rs

+pub fn initialize_signal_handlers() {
+    // TODO: Windows
+}
+
+pub fn initialize_signal_block() {
+    // TODO: Windows
+}


Uncertain if we actually need these

Signal masking/blocking is POSIX only.

We don't need a SIGINT handler since we can set the global pending var from the Control thread right? Technically we don't need this handler on Unix either, but I guess we want to support sending interrupts from e.g. a terminal. Though we could just let the default SIGINT handler from R set up for that purpose?

DavisVaughan · 2023-12-05T20:06:28Z

crates/ark/src/sys/windows/signals.rs

+pub fn interrupts_pending() -> bool {
+    unsafe { UserBreak == Rboolean_TRUE }
+}
+
+pub fn set_interrupts_pending(pending: bool) {
+    if pending {
+        unsafe { UserBreak = Rboolean_TRUE };
+    } else {
+        unsafe { UserBreak = Rboolean_FALSE };
+    }
+}
+
+#[link(name = "R", kind = "dylib")]
+extern "C" {
+    static mut UserBreak: libR_shim::Rboolean;
+}


Apparently UserBreak is the Windows equivalent (ish) to R_pending_interrupts, so I've created some helpers to wrap these in a platform agnostic way, and we use those everywhere now

DavisVaughan · 2023-12-05T20:07:57Z

crates/ark/src/sys/windows/strings.rs

+    let x = MultiByteToWideChar(code_page, flags, x)?;
+
+    // `WC::NoValue` doesn't exist, so we make it unsafely:
+    // https://github.com/rodrigocfd/winsafe/issues/110
+    let flags = unsafe { WC::from_raw(0) };
+    let default_char = None;
+    let used_default_char = None;
+
+    let x = WideCharToMultiByte(CP::UTF8, flags, &x, default_char, used_default_char)?;


I currently have a locally patched version of winsafe for rodrigocfd/winsafe#111

It won't actually break anything if you don't have a patched version, console output just won't look quite right (extra spaces and new lines typically)

DavisVaughan · 2023-12-05T20:09:40Z

crates/harp/src/line_ending.rs

+ *
+ */
+
+pub use sys::line_ending::NATIVE_LINE_ENDING;


I took the opportunity to move this platform specific code into the new folder structure too

crates/ark/src/sys/windows/interface.rs

jennybc · 2023-12-06T02:13:07Z

With this PR and its positron companion (posit-dev/positron#1900) and following these instructions, I have successfully built ark and used R inside positron!

The only coffee table I banged my shins on is that, when you run the The Code Known As dll2lib.R (#140 (comment)), you need to do so as administrator. I saved that code to positron\extensions\positron-r\scripts for lack of any better ideas. I had to tweak the path a bit because I seem to have ended up with a slightly different (newer) Visual Studio build:

# What Davis used (subtle difference in ?version?)
# path <- "C:\\Program Files\\Microsoft Visual Studio\\2022\\Community\\VC\\Tools\\MSVC\\14.37.32822\\bin\\Hostx86\\x86"
path <- "C:\\Program Files\\Microsoft Visual Studio\\2022\\Community\\VC\\Tools\\MSVC\\14.38.33130\\bin\\Hostx86\\x86"

Then it's important to launch RStudio (or what have you) as administrator before you try to execute the code that creates the .lib files for the DLLs.

I also forgot to set the R_HOME env var before my first attempt to do cargo build, but that was pure user error. @DavisVaughan makes it clear that this is necessary.

jennybc · 2023-12-06T22:55:39Z

The recent work on R_HOME and R_USER are working for me 🎉
And now I'm also getting the full (vs 8.3) path to R:

> R.home()
[1] "C:/Program Files/R/R-4.3.2"

jennybc

I am not reviewing the implementation, but rather confirming that I can build ARK on Windows from this branch and, together with posit-dev/positron#1900, can launch a dev build of Positron on Windows, with a functional R interpreter.

lionel-

LGTM!

lionel- · 2023-12-07T12:44:06Z

crates/ark/src/control.rs

-        // TODO: Windows.
-        // TODO: Needs to send a SIGINT to the whole process group so that
-        // processes started by R will also be interrupted.
+        sys::control::handle_interrupt_request();


This namespace makes me think that it's from the stdlib rather than something from ark, but I guess I just need to get used to it.

crates/ark/src/interface.rs

lionel- · 2023-12-07T12:51:35Z

crates/ark/src/main.rs

-    let mut sigset = SigSet::empty();
-    sigset.add(SIGINT);
-    sigprocmask(SigmaskHow::SIG_BLOCK, Some(&sigset), None).unwrap();
+    signals::initialize_signal_block();


Can we use ark::signals:: for this sort of stdlib-sounding namespaces? Same with traps:: and sys::. We're lucky because ark:: is very short so that might be a good convention for us.

lionel- · 2023-12-07T12:53:34Z

crates/ark/src/sys/unix/interface.rs

@@ -0,0 +1,133 @@
+/*


These abstractions look good

lionel- · 2023-12-07T12:56:02Z

crates/ark/src/sys/windows/console.rs

+///
+///    \002\377\376 <text> \003\377\376
+///
+/// strangely, we see these escapes around text that is not UTF-8


this was an rstudio comment i copied over

crates/ark/src/sys/windows/console.rs

lionel- · 2023-12-07T13:14:47Z

crates/ark/src/sys/windows/signals.rs

+pub fn initialize_signal_handlers() {
+    // TODO: Windows
+}
+
+pub fn initialize_signal_block() {
+    // TODO: Windows
+}


Signal masking/blocking is POSIX only.

We don't need a SIGINT handler since we can set the global pending var from the Control thread right? Technically we don't need this handler on Unix either, but I guess we want to support sending interrupts from e.g. a terminal. Though we could just let the default SIGINT handler from R set up for that purpose?

jennybc · 2023-12-07T16:52:09Z

One more important thing I just ran into and then @DavisVaughan reminded me, from a slack conversation:

https://github.com/posit-dev/amalthea/blob/728dd25af69990369dfcd71e49781ddc091b61ed/crates/ark/src/r_task.rs#L92

This timeout is currently too short for us in the AWS Windows cloud machines (generally very sluggish and laggy for me) and the way you'll find out is you'll get crashes while trying to do normal things, like install a package. Increase from 5 to 5000 to get unstuck for now.

Very nix specific

Based partially on https://crosvm.dev/book/contributing/style_guide_platform_specific_code.html

Does nothing on Windows

This seems to handle interrupting a for loop and an input request like `readline()`

Also patches over winsafe issue directly in ark, so others get the patch too

Since this seems to be POSIX only, I think

DavisVaughan force-pushed the test/windows branch from ecd242b to d292666 Compare November 21, 2023 20:32

DavisVaughan force-pushed the test/windows branch 2 times, most recently from 6040061 to b7f20dc Compare November 30, 2023 20:55

DavisVaughan changed the base branch from main to feature/libR-shim November 30, 2023 21:08

DavisVaughan force-pushed the test/windows branch from b2336c0 to 28dbb53 Compare December 5, 2023 19:01

DavisVaughan force-pushed the feature/libR-shim branch from 18233dd to ec160e0 Compare December 5, 2023 19:03

DavisVaughan force-pushed the test/windows branch 3 times, most recently from b2336c0 to c4c9d5f Compare December 5, 2023 19:11

DavisVaughan commented Dec 5, 2023

View reviewed changes

DavisVaughan marked this pull request as ready for review December 5, 2023 20:11

jennybc reviewed Dec 6, 2023

View reviewed changes

crates/ark/src/sys/windows/interface.rs Outdated Show resolved Hide resolved

DavisVaughan changed the title ~~Windows testing~~ Initial Windows support for Ark Dec 6, 2023

DavisVaughan requested a review from lionel- December 6, 2023 23:26

jennybc approved these changes Dec 7, 2023

View reviewed changes

jennybc mentioned this pull request Dec 7, 2023

Barebones work for R discovery on Windows posit-dev/positron#1900

Merged

lionel- approved these changes Dec 7, 2023

View reviewed changes

DavisVaughan changed the base branch from feature/libR-shim to main December 7, 2023 20:56

DavisVaughan added 8 commits December 7, 2023 15:57

Don't register SIGBUS on Windows

1b2306c

Use tree-sitter-r without Unix specific include

9683147

Gate signal blocking

4e39223

Gate signal block / unblock and interrupt handler

a613109

Gate handle_interrupt_request()

4640482

Don't compile stream_capture.rs on Windows

b2b7b31

Very nix specific

Trait based approach to platform specific behavior

e0e7555

Based partially on https://crosvm.dev/book/contributing/style_guide_platform_specific_code.html

Another restructure based on how std::sys works

ee9a155

DavisVaughan added 25 commits December 7, 2023 15:57

Some consistency fixes

5e727b8

Extract out platform specific R setup

b36baad

Intermediate typos

17d6d3c

Add in platform specific polled events scope

86f5a98

Does nothing on Windows

Platform specific launch.json

700004f

Commit the current state of our dark magic

fa3f878

Fix missing include on unix

486ec1d

Fix up rebase issues

53dfa68

Post rebase cleanup

28e5f10

Remove unused import on windows

cdf2832

Better logging of write_console() issues

554159b

Add system_to_utf8() helper

1b3ee3e

Handle embedded UTF-8 peculiarities

9d91fd6

Handle interrupts on Windows

a834741

This seems to handle interrupting a for loop and an input request like `readline()`

Move more common functions to the shim

61488b9

Use a more specific file name

11e0951

Add some more TODOs for Windows

693c040

Look up R_HOME and R_USER dynamically

0eb1f26

Also patches over winsafe issue directly in ark, so others get the patch too

Add in dll2lib.R

0ad4bf0

Freeing functions are too new to call

f6ec4c8

Clean up signal handler/block comments

cc50439

Remove one more TODO

72e9864

Since this seems to be POSIX only, I think

Fully prefix sys, otherwise use the function

61755f7

Remove unused windows import

ff29f80

Join strings once at the end

466cec2

DavisVaughan force-pushed the test/windows branch from 46fbca5 to 466cec2 Compare December 7, 2023 20:58

Remove dangling harp dep on libR-sys

957212e

DavisVaughan merged commit 36e6fe6 into main Dec 7, 2023
1 check passed

DavisVaughan deleted the test/windows branch December 7, 2023 21:07

DavisVaughan mentioned this pull request Dec 7, 2023

Bump ark to 0.1.30 posit-dev/positron#1928

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Initial Windows support for Ark #140

Initial Windows support for Ark #140

DavisVaughan commented Nov 7, 2023 •

edited

Loading

DavisVaughan commented Nov 21, 2023 •

edited

Loading

DavisVaughan Dec 5, 2023

DavisVaughan Dec 5, 2023

DavisVaughan Dec 5, 2023

DavisVaughan Dec 5, 2023

DavisVaughan Dec 5, 2023

DavisVaughan Dec 5, 2023

lionel- Dec 7, 2023

DavisVaughan Dec 5, 2023

DavisVaughan Dec 5, 2023

DavisVaughan Dec 5, 2023

jennybc commented Dec 6, 2023

jennybc commented Dec 6, 2023

jennybc left a comment

lionel- left a comment

lionel- Dec 7, 2023

lionel- Dec 7, 2023

lionel- Dec 7, 2023

lionel- Dec 7, 2023

DavisVaughan Dec 7, 2023

lionel- Dec 7, 2023

jennybc commented Dec 7, 2023 •

edited

Loading

Initial Windows support for Ark #140

Initial Windows support for Ark #140

Conversation

DavisVaughan commented Nov 7, 2023 • edited Loading

Platform specific folder structure

Write console buffer on Windows

Interrupts

Global variables from libR-sys

R setup

DLLs and LIBs

Still TODO after merging

Outdated notes below, possibly still useful for historical purposes:

DavisVaughan commented Nov 21, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jennybc commented Dec 6, 2023

jennybc commented Dec 6, 2023

jennybc left a comment

Choose a reason for hiding this comment

lionel- left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jennybc commented Dec 7, 2023 • edited Loading

DavisVaughan commented Nov 7, 2023 •

edited

Loading

DavisVaughan commented Nov 21, 2023 •

edited

Loading

jennybc commented Dec 7, 2023 •

edited

Loading