Skip to content

Commit

Permalink
Implement libr crate for dynamic runtime symbol resolution (#205)
Browse files Browse the repository at this point in the history
* POC of `libr` crate for dynamic runtime symbol resolution

* Temp support for globals

* Better initialization routine

* Use `libr::has::` module

* Add mutable global support

* Pull default up into attribute

* Try a Windows specific function

* Expose individual initialization functions

* Update unix startup code to use libr

* Get tests working again with libr setup

* Remove hard coded shared library path

* Add windows specific types file for `Rstart`

* Fix up cfg passthrough

* Expose `Rgraphapp` as a 2nd order export

* Fix thinkos

* Needlessly complicating things

* Rename

* Remove remaining `extern "C" {` usages

* Start `extern "C" {` removal from shim

* Move all globals over

* Start chipping away at list of libR-sys imports

* Another chunk of function exports

* `R_CStackLimit` is a mutable global

* More functions

* Power through last easy functions

* Move over `SEXPTYPE`s

* Move over `Rboolean`

* Move over `R_xlen_t`

* Move over `Rbyte`

* Move over `Rcomplex`

* Move over `SEXP`

* Account for awkward variadic function signatures

* Remove windows usage of `libR_shim`

* Move types to their own file

* Expose graphics device types and functions

Using an opaque pointer trick

* Tweak import for windows

* Rip out `libR-sys` and `libR-shim`

* Remove `-undefined dynamic_lookup`, yay safety!

* No longer need `post-install.sh`

* Remove `install_name_tool` usage in CI

* Remove unsetting of `DYLD_INSERT_LIBRARIES`

We don't actually set this particular env var anyways, so this didn't do anything, but we definitely don't need it now!

* Remove comment about testing ark

* Add note about how CI works

* No longer need R or `dll2lib.R` in build steps

* No longer need `dll2lib.R`

* Don't need this panic import

* Use `libr::*` in the most annoying places

* Fiddle with test imports

* Preemptively open and leak the 4 R DLLs on Windows

So other R packages like stats can find and use them without needing the DLL folder on the PATH

* Add `RLibraries` struct to reduce duplication

* Tweak unused library field names

* Use `get()` and `set()` for mutable globals

* Allow unused imports on unix, where we don't use this file yet

* Open on unix with `RTLD_GLOBAL`

* Source `ldpaths` to set appropriate library path env var

* Simpler approach with `bash -c`

* Use `sh` and `printf` for simplicity and portability
  • Loading branch information
DavisVaughan authored Jan 25, 2024
1 parent dff73bb commit df81c99
Show file tree
Hide file tree
Showing 99 changed files with 3,003 additions and 1,963 deletions.
13 changes: 0 additions & 13 deletions .cargo/config.toml
Original file line number Diff line number Diff line change
@@ -1,16 +1,3 @@

[target.x86_64-apple-darwin]
rustflags = [
"-C", "link-arg=-undefined",
"-C", "link-arg=dynamic_lookup",
]

[target.aarch64-apple-darwin]
rustflags = [
"-C", "link-arg=-undefined",
"-C", "link-arg=dynamic_lookup",
]

# Increase stack space on Windows to 8m
# See https://github.com/posit-dev/positron/issues/1975

Expand Down
3 changes: 3 additions & 0 deletions .github/workflows/amalthea-ci.yml
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,9 @@ jobs:
run: |
cargo build --verbose
# Ubuntu runners already have a version of R installed.
# Unit tests "automatically" find and set `R_HOME` if it isn't set by
# calling `R RHOME`. See the testing version of `start_r()` for this.
- name: Run Unit Tests
id: amalthea-test
run: |
Expand Down
54 changes: 1 addition & 53 deletions .github/workflows/release.yml
Original file line number Diff line number Diff line change
Expand Up @@ -123,29 +123,12 @@ jobs:
run: |
echo "~/${{matrix.homebrew_folder}}/bin" >> $GITHUB_PATH
# Determine R_HOME (for building ark)
- name: Find Homebrew R installation
id: r_installation
run: |
# Path to the Homebrew build of R, e.g. /Users/user229818/homebrew/Cellar/r
R_FOLDER=~/${{matrix.homebrew_folder}}/Cellar/r
# Get the first (and generally) only installed version, e.g. 4.2.2
R_VERSION=$(ls ${R_FOLDER} | head -1)
# Form the path to the R binary, e.g. /Users/user229818/homebrew/Cellar/r/4.2.2/bin/R
R_EXECUTABLE="${R_FOLDER}/${R_VERSION}/bin/R"
# Invoke the R binary to determine its RHOME directory (usually lib/R)
R_HOME=$(${R_EXECUTABLE} RHOME)
# Output the result for consumption in later steps
echo "Using R at ${R_HOME}"
echo "r_home=${R_HOME}" >> $GITHUB_OUTPUT
# Compile
- name: Compile ARK (${{ matrix.arch }})
env:
npm_config_arch: ${{ matrix.arch }}
ARK_BUILD_TYPE: ${{ matrix.flavor }}
RUST_TARGET: ${{ matrix.rust_target_prefix }}-apple-darwin
R_HOME: ${{ steps.r_installation.outputs.r_home }}
CARGO_FLAGS:
PKG_CONFIG_ALLOW_CROSS: 1
run: |
Expand All @@ -158,22 +141,10 @@ jobs:
# Enter the build directory
pushd target/${{ matrix.rust_target_prefix }}-apple-darwin/${{ matrix.flavor }}
# On macOS, we use install_name_tool to fix up the link to libR.dylib.
#
# Note that we still try to link with '-undefined dynamic_lookup', just to ensure that
# linking succeeds when we compile against a version of R compiled for a different
# architecture. This is mostly relevant when producing x86_64 builds of ark on an arm64
# machine.
#
# However, because using libR-sys still implies that the path to the R library ends up in
# the library load list, we have to modify that after the fact anyhow.
OLD_LIBR_PATH=$(otool -L ark | grep libR.dylib | cut -c2- | cut -d ' ' -f1)
echo "Fixing path to shared R library at ${OLD_LIBR_PATH}..."
install_name_tool -change "${OLD_LIBR_PATH}" "@rpath/libR.dylib" ark
# Compress the kernel to an archive
ARCHIVE="$GITHUB_WORKSPACE/ark-${{ needs.get_version.outputs.ARK_VERSION }}-${{ matrix.flavor }}-darwin-${{ matrix.arch }}.zip"
zip -Xry $ARCHIVE ark
popd
# Create build artifact
Expand Down Expand Up @@ -204,33 +175,10 @@ jobs:
- name: Checkout sources
uses: actions/checkout@v4

- name: Setup R
uses: r-lib/actions/setup-r@v2
with:
r-version: '4.3.2'

- name: Find R installation
id: r_installation
shell: bash
run: |
R_HOME="C:\R"
R_SCRIPT="${R_HOME}\bin\x64\Rscript.exe"
echo "Using R at ${R_HOME}"
echo "Using Rscript at ${R_SCRIPT}"
# Output the result for consumption in later steps
echo "r_home=${R_HOME}" >> $GITHUB_OUTPUT
echo "r_script=${R_SCRIPT}" >> $GITHUB_OUTPUT
- name: Generate LIB from DLL
shell: cmd
run: |
${{ steps.r_installation.outputs.r_script }} "scripts\windows\dll2lib.R"
- name: Compile ARK
env:
ARK_BUILD_TYPE: ${{ matrix.flavor }}
RUST_TARGET: ${{ matrix.rust_target_prefix }}-pc-windows-msvc
R_HOME: ${{ steps.r_installation.outputs.r_home }}
shell: cmd
run: |
cargo clean
Expand Down
13 changes: 1 addition & 12 deletions .vscode/tasks.json
Original file line number Diff line number Diff line change
Expand Up @@ -3,18 +3,7 @@
"tasks": [
{
"type": "shell",
"osx": {
"command": "cargo build && scripts/post-install.sh"
},
"windows": {
// Note that the environment variable, R_HOME, must be set prior to `cargo build`
// in order to build the ark dependency libR-sys. Once libR-sys is built,
// R_HOME is not required for subsequent runs of `cargo build`.
// Prior to running `cargo build`, execute something like this:
// PowerShell: $env:R_HOME = "C:\\Program Files\\R\\R-4.3.2"
// bash: export R_HOME="C:\\Program Files\\R\\R-4.3.2"
"command": "cargo build"
},
"command": "cargo build",
"problemMatcher": [
"$rustc"
],
Expand Down
Loading

0 comments on commit df81c99

Please sign in to comment.