extendr / rextendr

An R package that helps scaffolding extendr-enabled packages or compiling Rust code dynamically
https://extendr.github.io/rextendr/
Other
180 stars 27 forks source link

Building on Windows #2

Closed CGMossa closed 3 years ago

CGMossa commented 3 years ago

For Windows builds, the DLL is named without the "lib"-prefix.

E.g. in rust_source:


  # load shared library
  libfilename <- if (.Platform$OS.type == "windows") {
    paste0(libname, get_dynlib_ext())
  } else {
    paste0("lib", libname, get_dynlib_ext())
  }

  the$shared_lib <- file.path(dir, "target", "debug", libfilename)
  dyn.load(the$shared_lib, local = TRUE, now = TRUE)

Common CARGO_TARGET_DIR

A common "trick" to reduce cargo's disk usage, and compile time, is to set CARGO_TARGET_DIR or other things, to one common place for all cargo packages. This means that, when rextendr tries to locate the target, it might not be able to do so properly, as those would be in the common target directory, and not in the project (which is in a temporary location) directory.

Thus I've added:

  status <- system2(
    command = "cargo",
    args = c(
      "build",
      #"--release",  # release vs debug should be configurable at some point; for now, debug compiles faster
      sprintf("--lib --manifest-path=%s --target-dir %s/target/", file.path(dir, "Cargo.toml"), dir)
    ),
    stdout = stdout,
    stderr = stdout
  )

to the rust_source.

Edited: Moved previous points to #4 and #5.

I'll make a PR immediately as I get some comment on these points.

clauswilke commented 3 years ago

Thanks for looking into these things. Could I ask you to open separate issues for separate items, so we can knock them off one-by-one? You see the problem arise with your PR, which addresses the first two items but not the next two. So I think we need a new issue for dylib vs cdylib and possibly a new issue for unloading dlls (though I don't think this will be possible; other comparable packages also don't unload).

clauswilke commented 3 years ago

Closing this issue as the build-on-windows PR has been merged.