feat(r/sedonadb): Use the "bootstrap.R" mechanism to copy necessary Rust source codes #146
Conversation
There was a problem hiding this comment.
I suppose this file should be added to .Rbuildignore.
There was a problem hiding this comment.
Ah, thanks for catching!
paleolimbot
left a comment
paleolimbot
left a comment
There was a problem hiding this comment.
Awesome!
I have https://paleolimbot.r-universe.dev/builds set up that should sync once this merges and we'll see!
| - name: Test R Package | ||
| run: | | ||
| R -e 'library(testthat); test_local("r/sedonadb", MultiReporter$new(list(CheckReporter$new(), LocationReporter$new())))' |
There was a problem hiding this comment.
It might make sense to keep this and add the CMD check version in a separate workflow that doesn't run whenever somebody touches rust/**. I think the R CMD INSTALL version more effectively uses the cache (but if the CMD check version is still in the ~15 minute range we should totally do that).
There was a problem hiding this comment.
Thanks, makes sense! I reverted r.yml.
There was a problem hiding this comment.
add the CMD check version in a separate workflow
The reason I used the check-r-package action was simply that it looked easier to me. So, if not, I don't have strong opinion to use R CMD check.
There was a problem hiding this comment.
FYI, r-polars has two jobs: a fast job that builds in debug mode and runs only test, and a job workflow that builds in release mode and runs R CMD check.
If you're interested, check them out.
https://github.com/pola-rs/r-polars/blob/66d7b73b6496dc4494fae72f8540a6958cec0c9a/.github/workflows/test-r.yml#L47-L197
There was a problem hiding this comment.
Let's get CMD check passing in a future PR! As I think you discovered here, there's some C file deep in the dependency stack that CMD check objects to (we either need to patch the file, remove the file after build, or ignore just that specific error).
4 participants
I found @paleolimbot invented a cool mechanism (r-lib/pkgbuild#157) that can be used for vendoring the necessary source code when installed via
pkgbuild(i.e. pak). Currently, the README requires users to install the sedonadb R package by:This pull request will allows us to simplify the installation to this:
(and probably enables it to publish the R package to R-universe?)