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

Unregistered symbols warnings #846

Closed
kbenoit opened this issue Jul 11, 2017 · 20 comments
Closed

Unregistered symbols warnings #846

kbenoit opened this issue Jul 11, 2017 · 20 comments
Assignees

Comments

@kbenoit
Copy link
Collaborator

kbenoit commented Jul 11, 2017

In current master:

* checking compiled code ... NOTE
Warning in read_symbols_from_dll(so, rarch) :
  this requires 'objdump.exe' to be on the PATH
Warning in read_symbols_from_dll(so, rarch) :
  this requires 'objdump.exe' to be on the PATH
Warning in read_symbols_from_dll(so, rarch) :
  this requires 'objdump.exe' to be on the PATH
File 'quanteda/libs/i386/quanteda.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.
* checking examples ... OK
** found \donttest examples: check also with --run-donttest
@kbenoit kbenoit changed the title Unregistered symbols NOTEs Unregistered symbols warnings Jul 11, 2017
@HaiyanLW
Copy link
Collaborator

Updating init.c should work.

@kbenoit
Copy link
Collaborator Author

kbenoit commented Jul 12, 2017

Can you or @koheiw do this? I can't understand why the

#' @useDynLib quanteda, .registration = TRUE  

in quanteda.R doesn't take care of this automatically. Can we make this automatic?

Using tools::package_native_routine_registration_skeleton() does not help either:

> tools::package_native_routine_registration_skeleton(".", "src/init.c")
Error in native_routine_registration_db_from_ff_call_db(calls, dir, character_only) : 
  no native symbols were extracted

@HaiyanLW
Copy link
Collaborator

> tools::package_native_routine_registration_skeleton(".")

should work, but I don't have "r-devel" installed now, so the easiest way is to add the new .cpp function to init.c. I can do it if @koheiw let me know what the new .cpp function is.

@kbenoit
Copy link
Collaborator Author

kbenoit commented Jul 12, 2017

That did not work for me, as I commented above. Could you add manually this in a PR?

@HaiyanLW
Copy link
Collaborator

What is the new added .cpp function?

@kbenoit
Copy link
Collaborator Author

kbenoit commented Jul 12, 2017

@koheiw knows

@HaiyanLW
Copy link
Collaborator

@koheiw What's the new .cpp you added in the current master?

@koheiw
Copy link
Collaborator

koheiw commented Jul 13, 2017

tokens_ngrams_mt2.cpp is the latest. You can remove it as it is only a clone of tokens_ngrams_mt.cpp, for development.

@HaiyanLW
Copy link
Collaborator

Do you mean tokens_ngrams_mt2.cpp is the only new added c function? If this is the case, just remove it and nothing needed to do with init.c to solve this issue.

@kbenoit
Copy link
Collaborator Author

kbenoit commented Jul 15, 2017

The way to regenerate init.c is:

tools::package_native_routine_registration_skeleton(".", character_only = FALSE)

The unintuitive character_only = FALSE is necessary if you are not calling this for the first time.

However: Even after applying this update, on Windows (only) I am still seeing:

* checking compiled code ... NOTE
File 'quanteda/libs/x64/quanteda.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.

even though both functions are called in the function void R_init_quanteda(DllInfo *dll) inside init.c. Maybe we have to call that function from somewhere?

@HaiyanLW
Copy link
Collaborator

Is the new generated init.c in the current master now? I notice R_useDynamicSymbols is set to FALSE in void R_init_quanteda(DllInfo *dll) function inside init.c, would that be the reason?

@HaiyanLW
Copy link
Collaborator

Also is .registration = TRUE set as such in the useDynLib inside NAMESPACE after you re-generated init.c?

@kbenoit
Copy link
Collaborator Author

kbenoit commented Jul 17, 2017

Not sure... I suggest you branch and test this, with a regenerated init.c. The useDynLib inside NAMESPACE comes from the Roxygen directive in quanteda.R.

@HaiyanLW
Copy link
Collaborator

Ok, I can test it tomorrow on Windows.

@HaiyanLW
Copy link
Collaborator

Not sure why the useDynLib has always been mentioned.

@kbenoit
Copy link
Collaborator Author

kbenoit commented Jul 17, 2017

Tell me the branch and I will pull it and run the tests. You can also always using devtools::build_win().

@kbenoit kbenoit modified the milestone: CRAN v0.9.9.9000 Jul 18, 2017
@HaiyanLW
Copy link
Collaborator

I ran the tools::package_native_routine_registration_skeleton(".", character_only = FALSE) command on my Windows machine and the returned content is exactly same as what inside init.c in the current master. Also built without any note

@kbenoit
Copy link
Collaborator Author

kbenoit commented Jul 18, 2017

I'm still seeing the note in the CHECK --as-cran on my Windows 10 system. And it's in Appveyor. This is insanely frustrating.

@mpadge
Copy link
Contributor

mpadge commented Jul 19, 2017

@kbenoit There's been an Rcpp update which inserts underscores before all RcppExports functions. If you've got a pre-generated skeleton, you simply need to manually insert the underscores in all functions there and you should be good to go. Simply check all R/RcppExports.R and src/RcppExports.cpp and ensure your init.c matches.

@kbenoit
Copy link
Collaborator Author

kbenoit commented Jul 20, 2017

Thanks @mpadge, you may have saved a lot more of my hair from being pulled out!!

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

4 participants