feat: switch from extendr to the savvy framework (#252)

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. --------- Co-authored-by: eitsupi <[email protected]>
PRQL · Jan 30, 2024 · 2b00b41 · 2b00b41
1 parent 0756505
commit 2b00b41
Show file tree

Hide file tree

Showing 25 changed files with 284 additions and 261 deletions.
diff --git a/.github/workflows/check.yml b/.github/workflows/check.yml
@@ -93,6 +93,9 @@ jobs:
         env:
           NOT_CRAN: "true"
         run: |
+          # make sure savvy is built from source because rust-cache doesn't work well.
+          (find ~/.cargo/registry/ src/rust/target -name 'savvy-*' | xargs rm -rf) || true
+
           Rscript -e 'pkgbuild::compile_dll()'
           echo "LIBPRQLR_PATH=$(pwd)/src/rust/target/${{ steps.rust-target.outputs.TARGET }}/release/libprqlr.a" >>$GITHUB_ENV
 

diff --git a/.github/workflows/release-lib.yml b/.github/workflows/release-lib.yml
@@ -55,6 +55,9 @@ jobs:
           LIB_VERSION="$(cargo metadata --format-version=1 --no-deps | jq --raw-output '.packages[0].version')"
           echo "LIB_VERSION=${LIB_VERSION}" >>"$GITHUB_ENV"
           rustup target add ${{ matrix.target }}
+          # savvy needs R_INCLUDE_DIR envvar
+          echo R_INCLUDE_DIR=$(Rscript -e 'cat(normalizePath(R.home("include")))') >> "$GITHUB_ENV"
+
 
       - uses: Swatinem/rust-cache@v2
         with:
@@ -82,6 +85,9 @@ jobs:
           PRQLR_PROFILE: release
         working-directory: src
         run: |
+          # make sure savvy is built from source because rust-cache doesn't work well.
+          (find ~/.cargo/registry/ rust/target -name 'savvy-*' | xargs rm -rf) || true
+
           LIB_PATH="$(pwd)/rust/target/${TARGET}/${PRQLR_PROFILE}/${LIB_NAME}.a"
           ARTIFACT_NAME="${LIB_NAME}-${LIB_VERSION}-${TARGET}.tar.gz"
           make -f Makevars${{ runner.os == 'Windows' && '.win' || '.in' }} "${LIB_PATH}"

diff --git a/DESCRIPTION b/DESCRIPTION
@@ -34,10 +34,8 @@ RoxygenNote: 7.2.3
 SystemRequirements: Cargo (Rust's package manager), rustc
 VignetteBuilder: knitr
 Config/testthat/edition: 3
-Config/rextendr/version: 0.3.1
 Config/Needs/dev:
     devtools,
-    rextendr,
     lintr,
     styler,
     purrr,
@@ -46,4 +44,4 @@ Config/Needs/dev:
     tidyr
 Config/Needs/website:
     pkgdown
-Config/prqlr/LibVersion: 0.11.0
+Config/prqlr/LibVersion: 0.11.1
diff --git a/LICENSE.note b/LICENSE.note
@@ -235,22 +235,6 @@ License:     Apache-2.0 OR MIT
 
 -------------------------------------------------------------
 
-Name:        extendr-api
-Version:     0.6.0
-Repository:  https://github.com/extendr/extendr
-Authors:     andy-thomason, Thomas Down, Mossa Merhi Reimert, Claus O. Wilke, Hiroaki Yutani, Ilia A. Kosenkov, Michael Milton
-License:     MIT
-
--------------------------------------------------------------
-
-Name:        extendr-macros
-Version:     0.6.0
-Repository:  https://github.com/extendr/extendr
-Authors:     andy-thomason, Thomas Down, Mossa Merhi Reimert, Claus O. Wilke, Hiroaki Yutani, Ilia A. Kosenkov
-License:     MIT
-
--------------------------------------------------------------
-
 Name:        gimli
 Version:     0.28.1
 Repository:  https://github.com/gimli-rs/gimli
@@ -331,14 +315,6 @@ License:     Apache-2.0 OR MIT
 
 -------------------------------------------------------------
 
-Name:        libR-sys
-Version:     0.6.0
-Repository:  https://github.com/extendr/libR-sys
-Authors:     andy-thomason, Thomas Down, Mossa Merhi Reimert, Claus O. Wilke, Ilia A. Kosenkov, Hiroaki Yutani
-License:     MIT
-
--------------------------------------------------------------
-
 Name:        libc
 Version:     0.2.151
 Repository:  https://github.com/rust-lang/libc
@@ -419,14 +395,6 @@ License:     Apache-2.0 OR MIT
 
 -------------------------------------------------------------
 
-Name:        paste
-Version:     1.0.14
-Repository:  https://github.com/dtolnay/paste
-Authors:     David Tolnay
-License:     Apache-2.0 OR MIT
-
--------------------------------------------------------------
-
 Name:        proc-macro2
 Version:     1.0.71
 Repository:  https://github.com/dtolnay/proc-macro2
@@ -531,6 +499,38 @@ License:     Apache-2.0 OR BSL-1.0
 
 -------------------------------------------------------------
 
+Name:        savvy
+Version:     0.2.9
+Repository:  https://github.com/yutannihilation/savvy/
+Authors:     Hiroaki Yutani
+License:     MIT
+
+-------------------------------------------------------------
+
+Name:        savvy-bindgen
+Version:     0.2.9
+Repository:  https://github.com/yutannihilation/savvy/
+Authors:     Hiroaki Yutani
+License:     MIT
+
+-------------------------------------------------------------
+
+Name:        savvy-ffi
+Version:     0.2.9
+Repository:  https://github.com/yutannihilation/savvy/
+Authors:     Hiroaki Yutani
+License:     MIT
+
+-------------------------------------------------------------
+
+Name:        savvy-macro
+Version:     0.2.9
+Repository:  https://github.com/yutannihilation/savvy/
+Authors:     Hiroaki Yutani
+License:     MIT
+
+-------------------------------------------------------------
+
 Name:        semver
 Version:     1.0.20
 Repository:  https://github.com/dtolnay/semver

diff --git a/R/compile.R b/R/compile.R
@@ -1,9 +1,11 @@
 #' @title Compile a PRQL query into a SQL query
 #' @param prql_query a PRQL query string.
-#' @param target a compile target name to use. If not specified, the target contained in the query will be used.
+#' @param target a compile target name to use. If not specified (`NULL`),
+#' the target contained in the query will be used.
 #' All available target names can be listed with the [prql_get_targets] function.
-#' @param format a logical flag. Whether to format the SQL query.
-#' @param signature_comment a logical flag. Whether to add a signature comment to the output SQL query.
+#' @param format a logical flag (default: `TRUE`). Whether to format the SQL query.
+#' @param signature_comment a logical flag. (default: `TRUE`).
+#' Whether to add a signature comment to the output SQL query.
 #' @return a SQL query string
 #' @seealso [prql_get_targets]
 #' @examples
@@ -38,8 +40,8 @@ prql_compile <- function(
     target = getOption("prqlr.target", default = NULL),
     format = getOption("prqlr.format", default = TRUE),
     signature_comment = getOption("prqlr.signature_comment", default = TRUE)) {
-  compile(prql_query, target, format, signature_comment) |>
-    unwrap()
+  target <- target %||% "sql.any"
+  compile(prql_query, target, format, signature_comment)
 }
 
 #' @title prql-compiler's version

diff --git a/R/utils.R b/R/utils.R
@@ -1,50 +1 @@
-#' @title Rust-like unwrapping of result. Useful to do error handling on the R side.
-#' @param result a list where either element ok or err is NULL, or both if ok is a literal NULL.
-#' @param call context of error or string used for error printing, could be omitted.
-#' @return the ok-element of list , or a error will be thrown
-#' @examples
-#' # export unwrap for this example, only for internal use
-#' unwrap <- prqlr:::unwrap
-#'
-#' # unwrap a result with the ok-value "foo"
-#' unwrap(list(ok = "foo", err = NULL))
-#'
-#' # unwrap an err-result
-#' tryCatch(
-#'   unwrap(ok = NULL, err = "something happend on the rust side"),
-#'   error = function(e) as.character(e)
-#' )
-#' @noRd
-unwrap <- function(result, call = sys.call(1L)) {
-  # if not result
-  if (!inherits(result, "rust_result") && (!is.list(result) || !all(names(result) %in% c("ok", "err")))) {
-    stop("Internal error: cannot unwrap non result")
-  }
-
-  # if result is ok (ok can be be valid null, hence OK if both ok and err is null)
-  if (is.null(result$err)) {
-    return(result$ok)
-  }
-
-  # if result is error, raise R errors. Modify the error formatting as necessary.
-  if (is.null(result$ok) && !is.null(result$err)) {
-    stop(result$err)
-
-    # add some custom error context, or just delete
-    # stop(
-    #   paste(
-    #     result$err,
-    #     paste(
-    #       "\nwhen calling:\n",
-    #       paste(
-    #         capture.output(print(call)),collapse="\n")
-    #     )
-    #   )
-    # )
-  }
-
-  # if not ok XOR error, then it must be another internal error.
-  stop("Internal error: result object corrupted")
-}
-
 `%||%` <- function(x, y) if (is.null(x)) y else x
diff --git a/R/extendr-wrappers.R → R/wrappers.R b/R/extendr-wrappers.R → R/wrappers.R
@@ -1,14 +1,5 @@
-# Generated by extendr: Do not edit by hand
-
-# nolint start
-
-#
-# This file was created with the following call:
-#   .Call("wrap__make_prqlr_wrappers", use_symbols = TRUE, package_name = "prqlr")
-
-#' @docType package
-#' @usage NULL
 #' @useDynLib prqlr, .registration = TRUE
+#' @keywords internal
 NULL
 
 #' @title Compile a PRQL query into a SQL query
@@ -18,29 +9,40 @@ NULL
 #' @param signature_comment a logical flag. Whether to add a signature comment to the output SQL query.
 #' @return a list contains a SQL string or an error message.
 #' @noRd
-compile <- function(prql_query, target = "sql.any", format = TRUE, signature_comment = TRUE) .Call(wrap__compile, prql_query, target, format, signature_comment)
+compile <- function(prql_query, target, format, signature_comment) {
+  .Call(compile__impl, prql_query, target, format, signature_comment)
+}
 
 #' @noRd
-prql_to_pl <- function(prql_query) .Call(wrap__prql_to_pl, prql_query)
+prql_to_pl <- function(prql_query) {
+  .Call(prql_to_pl__impl, prql_query)
+}
 
 #' @noRd
-pl_to_rq <- function(pl_json) .Call(wrap__pl_to_rq, pl_json)
+pl_to_rq <- function(pl_json) {
+  .Call(pl_to_rq__impl, pl_json)
+}
 
 #' @noRd
-rq_to_sql <- function(rq_json) .Call(wrap__rq_to_sql, rq_json)
+rq_to_sql <- function(rq_json) {
+  .Call(rq_to_sql__impl, rq_json)
+}
 
 #' @title prql-compiler's version
 #' @return a prql-compiler's version string
 #' @noRd
-compiler_version <- function() .Call(wrap__compiler_version)
+compiler_version <- function() {
+  .Call(compiler_version__impl)
+}
 
 #' @title Get available target names
 #' @description Get available target names for the `target` option of the [prql_compile()] function.
 #' @return a character vector of target names.
 #' @examples
 #' prql_get_targets()
 #' @export
-prql_get_targets <- function() .Call(wrap__prql_get_targets)
+prql_get_targets <- function() {
+  .Call(prql_get_targets__impl)
+}
 
 
-# nolint end
diff --git a/Taskfile.yml b/Taskfile.yml
@@ -32,7 +32,7 @@ tasks:
     internal: true
     desc: Install Rust packages for development.
     cmds:
-      - cargo install cargo-license cargo-outdated
+      - cargo install cargo-license cargo-outdated savvy-cli
 
   cargo-update:
     desc: Update the lock files.
@@ -97,7 +97,7 @@ tasks:
       - "{{.CARGO_LOCK}}"
       - "{{.RUST_SOURCE}}"
     deps:
-      - build-extendr-wrappers
+      - build-wrappers
     cmds:
       - Rscript -e 'devtools::test()'
 
@@ -112,7 +112,7 @@ tasks:
       - "{{.CARGO_LOCK}}"
       - "{{.RUST_SOURCE}}"
     deps:
-      - build-extendr-wrappers
+      - build-wrappers
     cmds:
       - Rscript -e 'devtools::run_examples(document = FALSE)'
 
@@ -128,7 +128,7 @@ tasks:
       - "{{.CARGO_LOCK}}"
       - "{{.RUST_SOURCE}}"
     deps:
-      - build-extendr-wrappers
+      - build-wrappers
     cmds:
       - Rscript -e
         'devtools::load_all();
@@ -145,13 +145,13 @@ tasks:
     status:
       - Rscript -e 'if (desc::desc_get("RoxygenNote") < packageVersion("roxygen2")) quit(status = 1)'
     deps:
-      - build-extendr-wrappers
+      - build-wrappers
     cmds:
       - Rscript -e 'devtools::document()'
 
-  build-extendr-wrappers:
+  build-wrappers:
     internal: true
-    desc: Build the extendr wrappers.
+    desc: Build the Rust wrappers.
     sources:
       - src/Makevars*
       - configure*
@@ -161,11 +161,9 @@ tasks:
     deps:
       - build-vendor-sources
     generates:
-      - R/extendr-wrappers.R
-    status:
-      - Rscript -e 'if (desc::desc_get("Config/rextendr/version") < packageVersion("rextendr")) quit(status = 1)'
+      - R/wrappers.R
     cmds:
-      - Rscript -e 'rextendr::document()'
+      - savvy-cli update .
 
   build-license-note:
     internal: true
@@ -204,7 +202,7 @@ tasks:
     generates:
       - README.md
     deps:
-      - build-extendr-wrappers
+      - build-wrappers
     cmds:
       - Rscript -e
         'devtools::load_all();
@@ -228,7 +226,7 @@ tasks:
       - "{{.R_SOURCE}}"
       - "{{.VIGNETTES}}"
     deps:
-      - build-extendr-wrappers
+      - build-wrappers
     cmds:
       - Rscript -e
         'devtools::load_all();

diff --git a/configure b/configure
@@ -106,7 +106,14 @@ detect_target_option "$@"
 check_bin_lib
 check_cargo
 
-RUST_TARGET="${TARGET:-$(rustc -vV | grep host | cut -d' ' -f2)}"
+# cf. https://github.com/r-wasm/rwasm/issues/18#issuecomment-1910198843
+if [ "$(uname)" = "Emscripten" ]; then
+  HOST_TRIPLE="wasm32-unknown-emscripten"
+else
+  HOST_TRIPLE="$(rustc -vV | grep host | cut -d' ' -f2)"
+fi
+
+RUST_TARGET="${TARGET:-${HOST_TRIPLE}}"
 sed -e "s|@RUST_TARGET@|${RUST_TARGET}|" src/Makevars.in >src/Makevars
 
 exit 0