Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Rcpp::compileAttributes (dot once more) #1129

Closed
Bijaelo opened this issue Dec 21, 2020 · 9 comments
Closed

Rcpp::compileAttributes (dot once more) #1129

Bijaelo opened this issue Dec 21, 2020 · 9 comments

Comments

@Bijaelo
Copy link

Bijaelo commented Dec 21, 2020

This is related to #500 and a question on stackoverflow from 2013.

Assume you have a package with a dot in the name with.dot, and you want to provide a C++ interface. This causes an error upon compilation:

f <- file('./with.dot/src/rcpp_hello_world.cpp', 'a+')
cat('// [[Rcpp::interfaces(cpp)]]', file = f)
close(f)
wd <- setwd('./with.dot')
Rcpp::compileAttributes()
devtools::document(roclets = c('rd', 'collate', 'namespace'))
setwd(wd)

Here the compilation is executed by devtools::document and this causes an error:

Re-compiling with.dot
-  installing *source* package 'with.dot' ... (557ms)
   ** using staged installation
   ** libs
   "C:/RBuildTools/4.0/mingw64/bin/"g++ -std=gnu++11  -I"C:/Users/olive/R/R-cran/R-40~1.3/include" -DNDEBUG  -I'C:/Users/olive/Documents/R/win-library/4.0/Rcpp/include'        -O2 -Wall  -mfpmath=sse -msse2 -mstackrealign -UNDEBUG -Wall -pedantic -g -O0 -fdiagnostics-color=always -c RcppExports.cpp -o RcppExports.o
   In file included from RcppExports.cpp:4
   ../inst/include/with.dot.h:7:10:fatal error: dot_RcppExports.h: No such file or directory
    #include "with_dot_RcppExports.h"
             ^~~~~~~~~~~~~~~~~~~~~~~~
   compilation terminated.
   make: *** [C:/Users/olive/R/R-cran/R-40~1.3/etc/x64/Makeconf:230: RcppExports.o] Error 1
   ERROR: compilation failed for package 'with.dot'
-  removing 'C:/Users/olive/AppData/Local/Temp/RtmpUlqJPr/devtools_install_20b02c3c2615/with.dot'
Error in (function (command = NULL, args = character(), error_on_status = TRUE,  : 
  System command 'Rcmd.exe' failed, exit status: 1, stdout + stderr (last 10 lines):
E> ** libs
E> "C:/RBuildTools/4.0/mingw64/bin/"g++ -std=gnu++11  -I"C:/Users/olive/R/R-cran/R-40~1.3/include" -DNDEBUG  -I'C:/Users/olive/Documents/R/win-library/4.0/Rcpp/include'        -O2 -Wall  -mfpmath=sse -msse2 -mstackrealign -UNDEBUG -Wall -pedantic -g -O0 -fdiagnostics-color=always -c RcppExports.cpp -o RcppExports.o
E> In file included from RcppExports.cpp:4
E> ../inst/include/with.dot.h:7:10:fatal error: dot_RcppExports.h: No such file or directory
E>  #include "with_dot_RcppExports.h"
E>           ^~~~~~~~~~~~~~~~~~~~~~~~
E> compilation terminated.
E> make: *** [C:/Users/olive/R/R-cran/R-40~1.3/etc/x64/Makeconf:230: RcppExports.o] Error 1
E> ERROR: compilation failed for package 'with.dot'
E> * removing 'C:/Users/olive/AppData/Loc
[...]
Type .Last.error.trace to see where the error occured
In addition: Warning message:
roxygen2 requires Encoding: UTF-8 

The problem comes from the file with.dot/inst/with.dot.h trying to reference with.dot/inst/with.dot_RcppExports.h (note the dot in the file name). It seems to me, that file names should have replaced dots (.) by underscores (_) for reference purposes. I tried going through the debugger and got as far as

invisible(.Call("compileAttributes", PACKAGE = "Rcpp", 
        pkgdir, pkgname, depends, registration, cppFiles, cppFileBasenames, 
        includes, verbose, .Platform))

at which point my search stopped.


sessionInfo()
R version 4.0.3 (2020-10-10)
Platform: x86_64-w64-mingw32/x64 (64-bit)
Running under: Windows 10 x64 (build 19042)

Matrix products: default

locale:
[1] LC_COLLATE=English_Denmark.1252  LC_CTYPE=English_Denmark.1252   
[3] LC_MONETARY=English_Denmark.1252 LC_NUMERIC=C                    
[5] LC_TIME=English_Denmark.1252    

attached base packages:
[1] stats     graphics  grDevices utils     datasets  methods   base     

loaded via a namespace (and not attached):
 [1] Rcpp_1.0.5        rstudioapi_0.13   xml2_1.3.2        knitr_1.30        roxygen2_7.1.1   
 [6] magrittr_2.0.1    usethis_2.0.0     devtools_2.3.2    pkgload_1.1.0     R6_2.5.0         
[11] rlang_0.4.9       fansi_0.4.1       stringr_1.4.0     tools_4.0.3       pkgbuild_1.2.0   
[16] xfun_0.19         sessioninfo_1.1.1 cli_2.2.0         withr_2.3.0       ellipsis_0.3.1   
[21] remotes_2.2.0     assertthat_0.2.1  digest_0.6.27     rprojroot_2.0.2   lifecycle_0.2.0  
[26] crayon_1.3.4      processx_3.4.5    purrr_0.3.4       callr_3.5.1       fs_1.5.0         
[31] ps_1.5.0          testthat_3.0.1    memoise_1.1.0     glue_1.4.2        stringi_1.5.3    
[36] compiler_4.0.3    desc_1.2.0        prettyunits_1.1.1
@eddelbuettel
Copy link
Member

Well the sequence of commands involving Rcpp.package.skeleton("with.dot") (which already includes an compileAttributes()) followed by standard R CMD build and R CMD check (or also rcmdcheck::rcmdcheck(...) works fine.

It even works when I manually add // [[Rcpp::interfaces(cpp)]] to the one C++ file.

So I see nothing actionable here.

@Bijaelo
Copy link
Author

Bijaelo commented Dec 21, 2020

R CMD check works out alright, but from my (rather limited) knowledge the header files in with.dot/inst would be unable to compile as it is trying to include a file that simply does not exist. So from my understanding from reading https://r-pkgs.org/src.html#cpp-import, one would be unable to use the code in another package?

From another point, it seems like unintended behaviour to have functions generate code files that intend to include each other, but do not.

With the danger of repeating myself, the actionable piece comes from the file

  • with.dot/inst/include/with.dot.h

which contains the line : #include "with_dot_RcppExports.h". This file does not exist, as it is named with.dot_RcppExports.h. Should the file naming not more logically be replacing the dot (.) with an underscore (_), or visa-versa. It seems odd that it is renaming the file in one part of the code, but not another.

@eddelbuettel
Copy link
Member

The code is in the file src/attributes.cpp. Maybe you can give it a got to try to find a fix?

Using the "interfaces(cpp)" feature is fairly rare, doing it in packages with dots even more so -- which in aggregate appears to suggest nobody was bothered enough to go for it. Contributions welcome!

@Bijaelo
Copy link
Author

Bijaelo commented Dec 21, 2020

I think I'll skip on it, and just leave out the interface to cpp. I simply lack the experience with c++ to efficiently spot where the file text is being compiled in the source code. Another year or two maybe. 👍
Thanks for the response though. :-)

@eddelbuettel
Copy link
Member

Given that this seemed like a fairly localized issue, I took a peek and just committed what may well be an appropriate fix addressing this issue of 'interface' use and generated header files for which a possible 'dot' in a package name needs needs conversion. It covers your test case above (of adding a simple Rcpp::interfaces(Cpp) attribute in a test package with a 'dot' in its name.

@kevinushey : when you have a moment, please take a look. I am learning towards sending what is in master to CRAN once they reopen, but we can possibly discuss including this. But as said before, might just be good enough to wait with this for the next cycle...

@Bijaelo
Copy link
Author

Bijaelo commented Dec 27, 2020

Thanks Dirk, I will keep an eye out and check the Devel build later on! 👍

@eddelbuettel
Copy link
Member

Forgot to add the commit id but it should be pretty obvious as the (so far) sole commit in this new branch.

@Bijaelo
Copy link
Author

Bijaelo commented Dec 27, 2020

Got it! It was just that simple eh?

Definitely something that I am going to learn from. I am unlikely to be done prior to the next commit round and either way I'd just update the package as needed. 👍

@kevinushey
Copy link
Contributor

Forgot to add the commit id but it should be pretty obvious as the (so far) sole commit in this new branch.

LGTM! I don't have a strong opinion but would otherwise be fine with bringing that for the next Rcpp release.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants