search

ropensci / redland-bindings

Redland librdf language bindings
http://librdf.org/bindings/
Other
17 stars 3 forks source link

generate NAMESPACE from multiple sources #9

Closed mbjones closed 9 years ago

mbjones commented 9 years ago

The R NAMESPACE file is generated by SWIG for the classes and code that SWIG generates. However, this NAMESPACE file is incomplete, as it is missing the proper DynLib calls, and it is missing the statements that would normally be generated from ROxygen. I have added a NAMESPACE.in file which contains these additions, and the makefile then generates the true NAMESPACE by concatenating NAMESPACE.in with the generated NAMESPACE file. However, if one runs devtools::document() after make, then it will overwrite the NAMESPACE file and omit all of the SWIG-generated lines, because SWIG doesn't generate ROxygen @export tags. So, we need to make all of this build infrastructure robust so both make and document() can work properly.

gothub commented 9 years ago

This can be accomplished as a custom roclet, which would have to be called explicitly, i.e.

devtools:document(roclet="mergedNamespace")

One possible approach is to merge in a file containing the SWIG generated classes into the NAMESPACE file after the default namespace roclet has run.

gothub commented 9 years ago

Since roxygen2 3.0.0, you can set the default roclets to run per project by adding/modifying DESCRIPTION file, i.e.

Roxygen: list(roclets = c("collate", "rd", "namespace", "mergedNamespace))

mbjones commented 9 years ago

@sbpcs59 I reviewed your implementation of the NAMESPACE merge, and I like how you did it in by adding a custom roclet. The only part that is weird is that I don't see how NAMESPACE.librdf is generated, as you no longer tell SWIG to generate the NAMESPACE file. It seems to me that we want SWIG to generate that file for us to keep up as the Redland.i changes. Can you clarify?

mbjones commented 9 years ago

To fix this, I generated the SWIG NAMESPACE file and then save this to the redland/inst/build directory, where it is available to ROxygen2. I updated the roclet to use this new location rather than the previous NAMESPACE.librdf file (which was generating an error because it shouldn't be located at the package root). This issue is closed by SHA 1051cdc5.

gothub commented 9 years ago

Hi Matt,

Thanks for having a look at the roclet implementation.

My premise for not using the NAMESPACE file created by swig is that swig creates an unusable NAMESPACE file. It appears to generate both an 'export' directive and an 'exportMethod' directive in the generated NAMESPACE file for each function. The redland API as implemented in the redland.R file contains functions, not s4 methods, so the exportMethod directives from NAMESPACE fail when the redland package is installed via ' R CMD INSTALL --no-multiarch --with-keep.source redland'. At least this is what happens when I build on Ubuntu 14.04.1. This error doesn't show up with the 'devtools::load_all()' function.

Instead of using the NAMESPACE file generated by swig, I manually created a NAMESPACE file that contains the current set of librdf functions without the 'exportMethod' directives. Also I 'normalized' the static NAMESPACE.librdf file to have one export call per line, i.e. 'export(librdf_new_world' instead of the multi-line directives created by swig which are harder to process via a roclet and you have to detect and deal with the entire multi-line directive.

My reasoning on this is that the librdf API is relatively stable and won't need to be updated often, but our redland R package API will change often, so I optimized for that, knowing that if the librdf API changed, then the static NAMESPACE.swig would have to be updated manually.

I intend to log bugs in the librdf bug tracking system for the following:

- have swig -namespace create a valid NAMESPACE file
- address problem detailed in

https://github.com/ropensci/redland-bindings/issues/4

Is the redland package building/installing for you?

On Wed, Dec 24, 2014 at 3:52 PM, Matt Jones notifications@github.com wrote:

To fix this, I generated the SWIG NAMESPACE file and then save this to the redland/inst/build directory, where it is available to ROxygen2. I updated the roclet to use this new location rather than the previous NAMESPACE.librdf file (which was generating an error because it shouldn't be located at the package root). This issue is closed by SHA 1051cdc https://github.com/ropensci/redland-bindings/commit/1051cdc5f4605aa7a7cc98db5fd020697a5dda51.

— Reply to this email directly or view it on GitHub https://github.com/ropensci/redland-bindings/issues/9#issuecomment-68079084 .

Peter Slaughter, Software Engineer National Center for Ecological Analysis and Synthesis 735 State St, Santa Barbara, CA 93101 voice: 805 892 2533

gothub commented 9 years ago

The NAMESPACE file generated by SWIG causes an error when the package is installed, due to the exportMethods directives that it generates for SWIG function calls.

gothub commented 9 years ago

The mergeNamespace roclet was rewritten to dynamically determine the set of SWIG functions to add to the NAMESPACE file by loading the wrapper file 'R/redland.R' and inspecting each loaded function. This fix is in commit https://github.com/ropensci/redland-bindings/commit/db9d67831724b712a8ddfe84873a26571fac6136