new R warnings re: registration of symbols
Closed this issue · 34 comments
R will now warn during R CMD check when symbols are not explicitly registered with dynamic lookup disabled. This implies that any R package using Rcpp::compileAttributes() will be getting these warnings. From anytime:
* checking compiled code ... NOTE
File ‘anytime/libs/anytime.so’:
Found no calls to: ‘R_registerRoutines’, ‘R_useDynamicSymbols’
It is good practice to use registered native symbols and to disable
symbol search.
See ‘Writing portable packages’ in the ‘Writing R Extensions’ manual.
It looks like the expectation is that this will be on for CRAN submissions, so we might have to take the time to update compileAttributes() to handle manually generating the registration code.
Ewww.
I noticed that, but then I also noticed this today: wch/r-source@db321a9
Did you use r-devel as of yesterday / earlier or current? With
edd@max:~$ RD --version | head -1
R Under development (unstable) (2017-01-16 r71994) -- "Unsuffered Consequences"
edd@max:~$
I get a clean check -- no issues for anytime_0.2.0.tar.gz under r-devel. Pheww
We may want to keep an eye on it. Then again, we may want to make registration better anyway (time permitting...)
It's disabled temporarily; effectively until the documentation necessary is available in the R manuals:
So I think this will likely be affecting us (and client Rcpp packages) soon.
I have a demo implementation (pretty much standalone) here:
https://github.com/kevinushey/sourcetools/blob/master/R/register.R
We might want to cannibalize some of this code and integrate it with compileAttributes().
Lovely stuff. I keep noticing the various incremental commit which BDR put into R-devel, but have not had time to look at this. Automating the registration via compileAttributes() would appear to be the thing to do.
I am set up for reverse depends checks over CRAN. Given other lists of packages (i.e. if we bothered to systematically search GitHub, say) we could do that too.
And it's all for a good cause. Omelettes and breaking eggs comes to mind.
If there were only a couple of packages we end up breaking we could to this:
-
Generate the code as in Kevin's prototype
-
Asking the users whose packages are broken to provide some "by convention" means of providing an additional initialization function which we would run from our generated code.
Mind you, normal package builds do not tickle new runs of compileAttributes(). So we'd have to rig up a new best bed anyway. No hurry.
We could first test this with our own packages.
I agree that we want to make sure we never break packages (especially for packages that call compileAttributes() on every build since that will be exceedingly annoying for developers)
Perhaps we could compromise with a solution like the following:
-
Emit the registration code to the tail end of
RcppExports.cpp-- that is, implement a function there (with C linkage) called e.g.Rcpp_init_pkgname. This function can return an array ofCallMethodDefs, or something similar. -
Emit an
R_init_pkg.cfile if none already exists / we don't detect a call toR_init_pkgsomewhere in the package sources. This file will take care of callingRcpp_init_pkgnameand passing the R call method definitions to R's registration routines. If that file does exist and doesn't call our Rcpp initialization routine, then we'll notify the user with a message or something similar.
I do think that it will be relatively rare for Rcpp users to also have their own unique registration code. I imagine most Rcpp users who are registering routines are just doing something less automated to accomplish what we're setting out to do right now; it's unlikely many users are mixing e.g. .C, .External and .Fortran interface calling functions. That said, we should be prepared to accommodate those users as well.
I have just received this NOTE on a submission to CRAN although it did not show up when my package was checked with win-builder (this has been changed and does now).
* checking compiled code ... NOTE
File 'strataG/libs/i386/strataG.dll':
Found no calls to: 'R_registerRoutines', 'R_useDynamicSymbols'
File 'strataG/libs/x64/strataG.dll':
Found no calls to: 'R_registerRoutines', 'R_useDynamicSymbols'
It is good practice to register native routines and to disable symbol
search.
See 'Writing portable packages' in the 'Writing R Extensions' manual.
Unfortunately, after reading the section referenced in "Writing R Extensions" and the thread here, I am no more enlightened as to what is triggering this NOTE or how to repair it for my package to be accepted.
Is there something else I can read that would help or is someone able to guide me as to what I should include?
Thanks in advance.
You need to read the documentation from "r-devel", ie R 3.4.0 -- which become "r-release" around April (though no date has been set). Try eg here.
Hi kevin @kevinushey. My package has the same problem (i.e. Found no calls to: 'R_registerRoutines', 'R_useDynamicSymbols' ). I didn't understand how to fix this from the r-devel documentation. Could you let me know specifically what should be done to bypass this NOTE?
Thanks a ton,
Andy
What part of the previous message (ie: read "r-devel" documentation) was unclear?
@eddelbuettel As I said, "I didn't understand how to fix this from the r-devel documentation.", so I was hoping if Rcpp could provide some guidance. I'm only a user of Rcpp so I don't know what is causing the NOTE at the C level.
What makes you think our role is tutoring you to read documentation that other people have written? Complaining to us is like complaining to the makers of your editor, or OS, or whatnot. Misplaced.
You wrote an R package. R updated its requirements. Read the R docs.
We're all volunteers here too, and asking us to read them aloud to you is ... a little weird.
We will get to this eventually. But we're all busy, and we do this as side-jobs and favours to the world at large. So have them patience.
Or, ideally, do some work to actually help us.
As both a user of RCpp and a developer of packages that get used by people with a variety of backgrounds, I deeply appreciate the effort and work you and your co-authors put into maintaining it and helping out such a large community.
I did not get the impression that @andyyao95 was asking you to read the r-devel documentation to him. He said that he had read it and did not understand it. I too have read it and have not fully understood what I need to do to be in compliance with the CRAN requirements. Because I have not found anyone to explain it to me, I am googling and testing and learning by trial and error. Perhaps I will narrow it down and find a resolution.
We are all overworked and do much of this community coding as a labour of love. I try to remember that none of us were born with knowledge and we all had to learn some way. Sometimes reading clarifies things. When it doesn't, some of us are lucky enough to have knowledgeable people around who can help explain difficult concepts in a way that the text cannot. I am sure you too have had this experience in your career and been on our side of the ignorance wall.
I sincerely hope this doesn't offend you, however being in the same place as @andyyao95, I thought it might be helpful to try to explain the perspective of one who tries to solve a problem as much as possible before taking others time to seek help, but still needs to seek help from time to time.
The tl;dr version of this -- you must now define a routine in your package called R_init_<pkg> which takes care of registering symbols, as described in https://cran.r-project.org/doc/manuals/r-devel/R-exts.html#Registering-native-routines.
Since examples are much easier to digest than R's documentation, the idea is that you (at least, right now) need to manually generate a file similar to this: https://github.com/kevinushey/sourcetools/blob/master/src/sourcetools_init.c
FWIW, it's likely sufficient to just add this to your package somewhere:
void R_init_<pkg>(DllInfo* info) {
R_registerRoutines(info, NULL, NULL, NULL, NULL);
R_useDynamicSymbols(info, TRUE);
}replacing <pkg> with the name of your package. (That would, in theory, silence the R CMD check warnings, although I have not yet tested.) Note that for now dynamics symbols are required as the R-level interface generated by Rcpp relies on this.
It would also be necessary to add .registration = TRUE in the useDynLib() call in the NAMESPACE file, e.g. as in https://github.com/kevinushey/sourcetools/blob/master/NAMESPACE#L13.
In the (hopefully near-ish) future, Rcpp will automatically take care of generating this code for you as part of the compileAttributes() step; unfortunately, the Rcpp team hasn't had time to take on this work just yet.
It's also worth examining how Rcpp registers its own routines:
https://github.com/RcppCore/Rcpp/blob/master/src/Rcpp_init.cpp
although the logic there is somewhat more complicated as Rcpp does a bit more work.
Hi All, I tried to follow @kevinushey suggestion by:
- creating a C file in my scr/ section of my package markovchain, named registerDynamicSymbols.c
- pushing the following code:
// RegisteringDynamic Symbols
#include <R.h>
#include <Rinternals.h>
#include <R_ext/Rdynload.h>
void R_init_markovchain(DllInfo* info) {
R_registerRoutines(info, NULL, NULL, NULL, NULL);
R_useDynamicSymbols(info, TRUE);
}
I then tested it with winbuilder but still NOTES are raised:
File 'markovchain/libs/i386/markovchain.dll':
Found no calls to: 'R_registerRoutines', 'R_useDynamicSymbols'
File 'markovchain/libs/x64/markovchain.dll':
Found no calls to: 'R_registerRoutines', 'R_useDynamicSymbols'
I hope that forthoming version of Rcpp could help on this.
@spedygiorgio It really is not all that complicated. Here are the steps:
- Get yourself r-devel. Just build it, or use a Docker instance, or ...
- Run it on a package. Eg I just did for (the GitHub version of) RcppArmadillo as I knew I had not (yet) registered its
fastLm()function. And surely,RD CMD check(using r-devel) warns. - The key: be lazy and just call
tools::package_native_routine_registration_skeleton(".")from r-devel as has been suggested before. In my case it identifies four entry points, and sets up the file. I copied that tosrc/init.c. - Rebuild, re-check. Worst case you need to also adapt
NAMESPACE.
@spedygiorgio FWIW I recall reading on R-devel that winbuilder has had some problems related to this check; if things look okay locally but fail on winbuilder in this specific case you may be fine.
I brought the fact that winbuilder does not catch this check to the attention of Uwe Ligges last week and he modified it. It now throws the NOTE.
For what it is worth my now-updated RcppArmadillo passes cleanly here (with a just-updated build of R-devel) and also on win-builder (which took a jolly long time to reply, well over an hour).
Seems that on my packages things worked worse. As suggested by @eddelbuettel I moved to the R Devel, then I ran tools::package_native_routine_registration_skeleton(".") and pasted the output into a init.c. And also, I included the , .registration = TRUE within my useDynLib(markovchain).
Not only the error " Found no calls to: 'R_registerRoutines', 'R_useDynamicSymbols'" keep to be shown but I also receive a bunch of "undocumented code object".
Sorry, but that just seems to indicate that you need to work on your package. Maybe the r-help or r-package-devel lists can help you with that,
There never was an Rcpp issue here, so could we maybe stop this off-topic thread here?
I had the same problems of @spedygiorgio, here's how I fixed :
- I did what Dirk suggested 2 posts above (using
tools::package_native...etc..) - at this point all my Rcpp functions became visible and consequently "undocumented"
- I changed NAMESPACE file by deleting the line
exportPattern("^[[:alpha:]]+")line (added by Rcpp skeleton function) and replaced by explicit export directives :export(functionToMakeVisible1,functionToMakeVisible2,...)(this solves the undocumented issue) CMD check --as-cranusing R-devel on Windows still gave theFound no calls to: 'R_registerRoutines'error but only on x64 architecture- I tested on https://win-builder.r-project.org/ and went OK, so I suspect there's something wrong/different with local and win-builder builds
@digEmAll thanks for the suggestion, I will try it!
I would also to add that a part of this problem is that tools::package_native_routine_registration_skeleton(".") could not work well when in Rcpp export directives are "." (to make the function hidden)...
@eddelbuettel , certainly I have to work on my package but consider that all my external code has been always written in Rcpp... And before R 3.4 all worked properly... So I believe it is an Rcpp issue... Not for Rcpp team faults, but for CRAN architecture revisions...
@digEmAll: The registration only covers the R-to-C(++) call (as far as I understand it) and should have no bearing on what is exported, and what needs documentation.
The issue is likely the presence of e.g.
exportPattern(<everything>)
in the NAMESPACE file -- this will inadvertently export the native symbol objects, which is likely not intended by most users.
And it's definitely not Rcpp's fault if we don't work with new R-devel routines that nobody told us were coming ...
@kevinushey :
yes, as far as I understood from -> here, you can replace the .Call("c_function",PACKAGE="package",arg1,arg2) with .Call(c_function,arg1,arg2) (note the c_function with no quotes) so probably the symbol c_function is made available in the package and exportPattern(<permissiveRegexp>) will make it available for the rest of the world as well.
BTW, it seems that modification can give some speed improvements. Maybe it could be added in the next Rcpp compileAttributes version :)
I managed to compile my package and removing the warning under ubuntu, which does not have R-devel binaries, by just downloading R-devel source and compiling it (takes around 30 minutes, maybe less. I am on an fairly old PC).
Download here: https://cran.r-project.org/sources.html and unpack.
cd R-devel
./configure
make
Then doing this (quick and dirty):
cd 'package-dir'
~/R-devel/bin/R
in the R console paste
tools::package_native_routine_registration_skeleton(".")
and copy the resulting c ouput in a file under src. Also modify your NAMESPACE file to include this line useDynLib(packagename, .registration = TRUE).
Then build your package as your used to. This fixed the problem for my package (see commit hildebra/Rarefaction@33896d9).
- You made your life too difficult.
- You don't need to build R-devel to access the
package_native_routine_registration_skeleton()helper. As discussed on the mailing lists, you can just fetch the function from the R-devel sources. - We do provide Docker containers with r-devel pre-built. Running Docker on Ubuntu is trivial.
- I have explained numerous times how to build r-devel well; the Dockerfiles are one example.
- I am glad the issue is fixed for you too; it is now still not an Rcpp issue so I am now locking this thread.
Closing this as it turned into a "R packaging help" issue for which this is not the right forum.