extendr / rextendr

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

Packages with `.` in name do not compile #176

Closed malcolmbarrett closed 2 years ago

malcolmbarrett commented 2 years ago

The . part of the package name results in invalid C and Rust code, it seems

library(usethis)
library(rextendr)
path <- file.path(tempdir(), "my.pkg")
ui_silence(create_package(path))
setwd(path)
rextendr::use_extendr()
#> ✓ Creating 'src/rust/src'.
#> ✓ Setting active project to '/private/var/folders/w7/8yv1j00s0bb3pfhmqc_rvd980000gn/T/RtmpTYmcmp/my.pkg'
#> ✓ Writing 'src/entrypoint.c'
#> ✓ Writing 'src/Makevars'
#> ✓ Writing 'src/Makevars.win'
#> ✓ Writing 'src/.gitignore'
#> ✓ Writing 'src/rust/Cargo.toml'.
#> ✓ Writing 'src/rust/src/lib.rs'
#> ✓ Writing 'R/extendr-wrappers.R'
#> ✓ Finished configuring extendr for package my.pkg.
#> • Please update the system requirement in 'DESCRIPTION' file.
#> • Please run `rextendr::document()` for changes to take effect.
rextendr::document()
#> ℹ Generating extendr wrapper functions for package: my.pkg.
#> ! No library found at 'src/my.pkg.so', recompilation is required.
#> Re-compiling my.pkg
#> * installing *source* package ‘my.pkg’ ...
#> ** using staged installation
#> ** libs
#> rm -Rf my.pkg.so ./rust/target/release/libmy.pkg.a entrypoint.o
#> clang -arch arm64 -I"/Library/Frameworks/R.framework/Resources/include" -DNDEBUG   -I/opt/R/arm64/include   -fPIC  -falign-functions=64 -Wall -g -O2  -UNDEBUG -Wall -pedantic -g -O0 -c entrypoint.c -o entrypoint.o
#> entrypoint.c:4:6: error: variable has incomplete type 'void'
#> void R_init_my.pkg_extendr(void *dll);
#>      ^
#> entrypoint.c:4:15: error: expected ';' after top level declarator
#> void R_init_my.pkg_extendr(void *dll);
#>               ^
#>               ;
#> entrypoint.c:6:6: error: variable has incomplete type 'void'
#> void R_init_my.pkg(void *dll) {
#>      ^
#> entrypoint.c:6:15: error: expected ';' after top level declarator
#> void R_init_my.pkg(void *dll) {
#>               ^
#>               ;
#> 4 errors generated.
#> make: *** [entrypoint.o] Error 1
#> ERROR: compilation failed for package ‘my.pkg’
#> * removing ‘/private/var/folders/w7/8yv1j00s0bb3pfhmqc_rvd980000gn/T/RtmpTYmcmp/devtools_install_108de4e22d649/my.pkg’
#> Error in (function (command = NULL, args = character(), error_on_status = TRUE, : System command 'R' failed, exit status: 1, stdout & stderr were printed

Created on 2022-02-08 by the reprex package (v2.0.1)

Ilia-Kosenkov commented 2 years ago

This is currently not supported, mainly because of the enrty_point naming (you can see it in the trace). I tried making c++ packages that also use dot in their names, but failed due to the same reason. AFAIK, the name of the entrypoint function is determined by the package name, and I am unaware if there is any convention on how to bypass it. We need to research the topic.

As a side note, style guides like this one discourage usage of periods/dots in the package name. I suspect the reasoning is similar to the function names case -- it may affect the S3 generic dispatch (or at least complicate it).

Ilia-Kosenkov commented 2 years ago

Actually, {data.table} works fine. We need to improve naming support to address also #152.

Ilia-Kosenkov commented 2 years ago

Note to self.

Note that there are some implicit restrictions on this mechanism as the basename of the DLL needs to be both a valid file name and valid as part of a C entry point (e.g. it cannot contain ‘.’): for portable code it is best to confine DLL names to be ASCII alphanumeric plus underscore. If entry point R_initlib is not found it is also looked for with ‘.’ replaced by ‘’.

malcolmbarrett commented 2 years ago

yeah, I assume sanitizing package names to snake case is what needs to be done for both C and Rust

malcolmbarrett commented 2 years ago

Awesome, thanks @Ilia-Kosenkov!