feat: switch from extendr to the savvy framework

[yutannihilation]

This is a proof of concept of migration from the extendr framework to the savvy framework. The bonus is that you can compile the package for webR. But, there are some caveats.

• savvy doesn't provide the default argument. You need to write a wrapper manually.
• savvy doesn't support Option as the argument, so you have to handle it on R's side.

[eitsupi]

Thanks for working on this!
I will take a look and want to merge this.

[yutannihilation]

Thanks for the update! I have no idea why the build on Windows fails, but the error message refers to unwind_protect_impl, which is compiled by invoking a C compiler in build.rs, so I might do something wrong there. Let me check.

 undefined reference to `unwind_protect_impl'

[eitsupi]

Thanks for the quick glance!

[yutannihilation]

Here's what I found so far:

• I said Windows above, but it seems it also fails on release-lib action with similar errors on other platforms as well
• I dumped the symbols of libprqlr.a, and found unwind_protect_impl is marked as "U" (undefined), which is expected to be found in other libraries, just like symbols like STRING_ELT are in libR. (I just googled around about nm and know very little, so I might be wrong here).

$ nm src/rust/target/x86_64-pc-windows-gnu/release/libprqlr.a | grep -A4 -B4 unwind_protect_impl
                 U SETCDR
                 U STRING_ELT
                 U strlen
                 U TYPEOF
                 U unwind_protect_impl

savvy-1747ec4ca039e54f.savvy.4c8d39534a29233-cgu.2.rcgu.o:
0000000000000000 b .bss
0000000000000000 d .data
--
                 U SETCAR
                 U SETCDR
                 U strlen
                 U TYPEOF
                 U unwind_protect_impl

• I added -vv to cargo build to see what commands are actually invoked in build.rs. Then, I found how cc crate works; it adds rust-link-lib and rustc-link-search so that the object is liked at the final build stage (?) rather than link it directly when building savvy. It seems rust-cache cannot reproduce these flags, and that's probably the cause (at least, should be related somehow).

[savvy 0.2.9] cargo:rustc-link-lib=static=unwind_protect
[savvy 0.2.9] cargo:rustc-link-search=native=C:/Users/Yutani/Documents/GitHub/prqlr/src/rust/target\x86_64-pc-windows-gnu\release\build\savvy-a17089b8b4baf766\out
[savvy 0.2.9] cargo:rerun-if-changed=src/unwind_protect_wrapper.c

So, my guess is this is that how cc crate works is not very compatible with how rust-cache action. I'm hoping there are some way to avoid this nicely, but I have no idea at the moment.

[eitsupi]

So, my guess is this is that how cc crate works is not very compatible with how rust-cache action.

Maybe this? Swatinem/rust-cache#161

[eitsupi]

Ok, rerunning seems to have fixed the failure on Windows as well. It was probably a network error or something.

[yutannihilation]

Oh, curious. release-lib actions fail even without rust-cache? Then, my assumption might be wrong.

[eitsupi]

Hmm, it seems that the method of copying the pre-built libprqlr.a and installing the R package is failed.

[eitsupi]

Oh, curious. release-lib actions fail even without rust-cache? Then, my assumption might be wrong.

The R CMD check workflow on Windows now succeeds, so perhaps that was relevant.

[yutannihilation]

Thanks, it seems to matter, and probably release-lib is a different problem.

Sorry, let me correct. I said

• unwind_protect_impl is marked as "U" (undefined),

but, in the later section, it is actually included (cf. nm symbol both "U" and "T", what does that mean?). So, I think release-lib should succeed.

$ nm src/rust/target/x86_64-pc-windows-gnu/release/libprqlr.a | grep -A20 unwind_protect_wrapper.o
unwind_protect_wrapper.o:
0000000000000000 b .bss
0000000000000000 d .data
0000000000000000 p .pdata$not_so_long_jump
0000000000000000 p .pdata$unwind_protect_impl
0000000000000000 r .rdata$zzz
0000000000000000 t .text
0000000000000000 t .text$not_so_long_jump
0000000000000000 t .text$unwind_protect_impl
0000000000000000 r .xdata$not_so_long_jump
0000000000000000 r .xdata$unwind_protect_impl
                 U __imp_longjmp
                 U __imp_R_NilValue
                 U __intrinsic_setjmpex
0000000000000000 T not_so_long_jump
                 U R_MakeUnwindCont
                 U R_PreserveObject
                 U R_UnwindProtect
                 U SETCAR
0000000000000000 T unwind_protect_impl

I thought I could reproduce the failure on my local by

cp src/rust/target/x86_64-pc-windows-gnu/release/libprqlr.a tools/
cd src/rust && cargo clean

(build package)

But, it doesn't fail. I'm wondering what's the difference.

[eitsupi]

Thanks.
The binaries actually used on CI can be downloaded from the CI summary.
https://github.com/eitsupi/prqlr/actions/runs/7683446634?pr=252

[yutannihilation]

Thanks, they are actually different. We are getting closer...

GHA Windows:

$ nm ~/Downloads/libs-x86_64-pc-windows-gnu/libprqlr-0.11.1-x86_64-pc-windows-gnu/libprqlr.a | grep unwind_protect_impl                 U unwind_protect_impl
                 U unwind_protect_impl
                 U unwind_protect_impl
                 U unwind_protect_impl
                 U unwind_protect_impl
                 U unwind_protect_impl

local Windows:

$ nm src/rust/target/x86_64-pc-windows-gnu/release/libprqlr.a | grep unwind_protect_impl
                 U unwind_protect_impl
                 U unwind_protect_impl
                 U unwind_protect_impl
                 U unwind_protect_impl
                 U unwind_protect_impl
                 U unwind_protect_impl
0000000000000000 p .pdata$unwind_protect_impl
0000000000000000 t .text$unwind_protect_impl
0000000000000000 r .xdata$unwind_protect_impl
0000000000000000 T unwind_protect_impl

[yutannihilation]

I'll run checks on my repo and report here if it succeeds, so don't worry about the approval.

1st run (this should succeed as there's no cache for rust-cache):
https://github.com/yutannihilation/prqlr/actions/runs/7689177307 (R-CMD-check, succeeded)
https://github.com/yutannihilation/prqlr/actions/runs/7689178444 (release-lib, failed)

2nd run:
https://github.com/yutannihilation/prqlr/actions/runs/7689215631 (R-CMD-check)

[yutannihilation]

Ah, okay. I misunderstood about the meaning of -p. This won't work.

Package Selection:
  -p, --package [<SPEC>]  Package to clean artifacts for

[yutannihilation]

This should succeed.

[eitsupi]

Seems passed all the tests, thank you for the awesome work!

I would like to check it works locally and merge it within a few days.

[yutannihilation]

Thanks. The last problem was that I forgot the absence of R_INCLUDE_DIR makes it skip building unwind_protect.c.

https://github.com/yutannihilation/savvy/blob/ad13597494e6232fdfe38612d1103616aee782ec/build.rs#L6

Please let me know if you find any problem on your local!

[eitsupi]

It seems that pre-built binary is working fine on arm64 Linux!

root ➜ /tmp $ export NOT_CRAN=true
root ➜ /tmp $ export LIBPRQLR_PATH=/tmp/libprqlr.a
root ➜ /tmp $ radian
R version 4.2.2 (2022-10-31) -- "Innocent and Trusting"
Platform: aarch64-unknown-linux-gnu (64-bit)

r$> remotes::install_github("eitsupi/prqlr#252")
Downloading GitHub repo yutannihilation/prqlr@savvy
── R CMD build ─────────────────────────────────────────────────────────────────────────────────────────────────────────
✔  checking for file ‘/tmp/RtmpkPtlZk/remotes86429df08f/yutannihilation-prqlr-5f38e75/DESCRIPTION’ (675ms)
─  preparing ‘prqlr’:
✔  checking DESCRIPTION meta-information ...
─  cleaning src
─  running ‘cleanup’
─  checking for LF line-endings in source and make files and shell scripts
─  checking for empty or unneeded directories
─  building ‘prqlr_0.7.0.9000.tar.gz’

Installing package into ‘/usr/local/lib/R/site-library’
(as ‘lib’ is unspecified)
* installing *source* package ‘prqlr’ ...
** using staged installation

---------------------- [LIBRARY BINARY FOUND] ----------------------
The library was found at </tmp/libprqlr.a>. No need to build it.
--------------------------------------------------------------------


------------------------ [COPYING BINARY] ------------------------
Copying </tmp/libprqlr.a> to </tmp/RtmpCbxnGA/R.INSTALL1401a6291ed/prqlr/tools/libprqlr.a>.
------------------------------------------------------------------

** libs
rm -Rf "prqlr.so" "/tmp/RtmpCbxnGA/R.INSTALL1401a6291ed/prqlr/src/rust/target//release/libprqlr.a" "init.o"
gcc -I"/usr/local/lib/R/include" -DNDEBUG   -I/usr/local/include   -fpic  -g -O2 -fstack-protector-strong -Wformat -Werror=format-security -Wdate-time -D_FORTIFY_SOURCE=2 -g  -c init.c -o init.o
if [ -f "/tmp/RtmpCbxnGA/R.INSTALL1401a6291ed/prqlr/src/../tools/libprqlr.a" ]; then \
        mkdir -p "/tmp/RtmpCbxnGA/R.INSTALL1401a6291ed/prqlr/src/rust/target//release" ; \
        mv "/tmp/RtmpCbxnGA/R.INSTALL1401a6291ed/prqlr/src/../tools/libprqlr.a" "/tmp/RtmpCbxnGA/R.INSTALL1401a6291ed/prqlr/src/rust/target//release/libprqlr.a" ; \
        exit 0; \
fi && \
if [ -f "/tmp/RtmpCbxnGA/R.INSTALL1401a6291ed/prqlr/src/rust/vendor.tar.xz" ]; then \
        mkdir -p "/tmp/RtmpCbxnGA/R.INSTALL1401a6291ed/prqlr/src/rust/vendor" && \
        /usr/bin/tar --extract --xz --file "/tmp/RtmpCbxnGA/R.INSTALL1401a6291ed/prqlr/src/rust/vendor.tar.xz" -C "/tmp/RtmpCbxnGA/R.INSTALL1401a6291ed/prqlr/src/rust/vendor" && \
        mkdir -p "/tmp/RtmpCbxnGA/R.INSTALL1401a6291ed/prqlr/src/.cargo" && \
        cp "/tmp/RtmpCbxnGA/R.INSTALL1401a6291ed/prqlr/src/rust/vendor-config.toml" "/tmp/RtmpCbxnGA/R.INSTALL1401a6291ed/prqlr/src/.cargo/config.toml"; \
fi && \
if [ "true" != "true" ]; then \
        export CARGO_HOME="/tmp/RtmpCbxnGA/R.INSTALL1401a6291ed/prqlr/src/.cargo"; \
        export CARGO_BUILD_JOBS=2; \
fi && \
export PATH="/usr/local/sbin:/usr/local/bin:/usr/sbin:/usr/bin:/sbin:/bin:/root/.cargo/bin" && \
if [ "" != "wasm32-unknown-emscripten" ]; then \
  cargo build --lib --manifest-path="/tmp/RtmpCbxnGA/R.INSTALL1401a6291ed/prqlr/src/rust/Cargo.toml" --target-dir "/tmp/RtmpCbxnGA/R.INSTALL1401a6291ed/prqlr/src/rust/target" --target="" \
    --profile="release" --features=""; \
else \
  export CC="gcc" && \
  export CFLAGS="-g -O2 -fstack-protector-strong -Wformat -Werror=format-security -Wdate-time -D_FORTIFY_SOURCE=2 -g " && \
  export CARGO_PROFILE_RELEASE_PANIC="abort" && \
  cargo +nightly build --lib --manifest-path="/tmp/RtmpCbxnGA/R.INSTALL1401a6291ed/prqlr/src/rust/Cargo.toml" --target-dir "/tmp/RtmpCbxnGA/R.INSTALL1401a6291ed/prqlr/src/rust/target" --target="" \
    --profile="release" --features="" -Zbuild-std=panic_abort,std; \
fi
if [ "true" != "true" ]; then \
        rm -Rf "/tmp/RtmpCbxnGA/R.INSTALL1401a6291ed/prqlr/src/.cargo" "/tmp/RtmpCbxnGA/R.INSTALL1401a6291ed/prqlr/src/rust/vendor" "/tmp/RtmpCbxnGA/R.INSTALL1401a6291ed/prqlr/src/rust/target//release/build"; \
fi
gcc -shared -L/usr/local/lib/R/lib -L/usr/local/lib -o prqlr.so init.o -L/tmp/RtmpCbxnGA/R.INSTALL1401a6291ed/prqlr/src/rust/target//release -lprqlr -L/usr/local/lib/R/lib -lR
installing to /usr/local/lib/R/site-library/00LOCK-prqlr/00new/prqlr/libs
** R
** inst
** byte-compile and prepare package for lazy loading
** help
*** installing help indices
*** copying figures
** building package indices
** installing vignettes
** testing if installed package can be loaded from temporary location
** checking absolute paths in shared objects and dynamic libraries
** testing if installed package can be loaded from final location
** testing if installed package keeps a record of temporary installation path
* DONE (prqlr)

r$> library(prqlr)

    "from mtcars | filter cyl > 6 | select {cyl, mpg}" |>
      prql_compile() |>
      cat()
SELECT
  cyl,
  mpg
FROM
  mtcars
WHERE
  cyl > 6

-- Generated by PRQL compiler version:0.11.1 (https://prql-lang.org)

Let's merging this.

[yutannihilation]

Thank you!

[eitsupi]

It looks like the wasm build on R-universe is failing (Not only this package).

Perhaps we need to open an issue in the webR repository and update the Dockerfile for the container image used for the build?

[yutannihilation]

Ah, yes, they are not ready for Rust yet. Sorry I didn't share it clearly. This is the relevant discussion:

r-wasm/actions#13

[eitsupi]

Thank you. I'm concerned about increasing the image size, but I think it would be great if Rust was added to the base image.

[yutannihilation]

Just for sharing. I understood the problem a bit wrong. In the case of R-universe, cargo is installed by these lines:

https://github.com/r-universe-org/build-wasm/blob/51bbee600c268dd272ad6b62446079ebe043d453/Dockerfile#L27-L30

However, since it's installed via apt, the toolchain is fixed to stable (and probably wasm32-unknown-emscripten is not available). Currently it fails with this error:

     cargo +nightly build --lib --release --manifest-path=./rust/Cargo.toml --target-dir ./rust/target --target wasm32-unknown-emscripten -Zbuild-std=panic_abort,std; \
  fi
error: no such command: `+nightly`

	Cargo does not handle `+toolchain` directives.
	Did you mean to invoke `cargo` through `rustup` instead?

ref: https://github.com/r-universe/yutannihilation/actions/runs/7848104729/job/21418576850#step:5:324

But, as it turned out adding Rust is not very lightweight (my comment), I think R-universe's way, installing it only when it's needed, is appropriate. I'll try proposing some better handling of Rust toolchain to r-universe-org/build-wasm.

edit: Wait, it seems I'm looking at wrong lines. Hmm...

# Install some common runtime libs