search

r-lib / pkgbuild

Find tools needed to build R packages
https://pkgbuild.r-lib.org
Other
65 stars 33 forks source link

Enable control over debug for compile_dll #100

Closed richfitz closed 3 years ago

richfitz commented 4 years ago

This PR enables the debug flags to be controlled for compile_dll(). At present if the user has a ~/.R/Makevars (or equivalent) then the debug flags (-O0 -g) are not added, otherwise they are unconditionally added. I noticed this when code was behaving differently in a docker container to locally.

I have kept the default behaviour the same, though it might be better flipped, controlled by an option etc. Any thoughts on that would depend on this function's interaction with load_all and friends though.

I have not added tests (yet) because I am not sure you'd want to support this feature. If testing was added it would likely require pulling in mockery as a Suggests.

Motivation: I am using this within a package that loads user code - I need a bit more control than cpp11::cpp_source as I am also pairing a bit of R code with the compiled code and trying to keep the workflow as similar as a packaged workflow as possible. I notice that cpp11::cpp_source itself does not use pkgbuild::compile_dll, though it would run into this if it did.

jimhester commented 4 years ago

This is really working as desired already, pkgbuild::compile_dll() is designed for interactive package development when you do not want optimizations turned on so compilation is faster and interactive debugging is easier to interpret.

I think for your use case you are better off using withr::with_makevars() to set the optimization settings you desire explicitly before calling pkgbuild::cpp_source().

richfitz commented 4 years ago

I figured that you might not want this :)

I think for your use case you are better off using withr::with_makevars() to set the optimization settings you desire explicitly before calling pkgbuild::cpp_source().

Would that not override the user's ~/.R/Makevars if they already have one?

jimhester commented 4 years ago

It uses the existing settings if a makevars file exists, as long as they aren't the same as what you pass in to with_makevars().

from ?withr::with_makevars()

If no ‘Makevars’ file exists or the fields in ‘new’ do not exist in the existing ‘Makevars’ file then the fields are added to the new file. Existing fields which are not included in ‘new’ are appended unchanged. Fields which exist in ‘Makevars’ and in ‘new’ are modified to use the value in ‘new’.

jimhester commented 3 years ago

Thank you!

richfitz commented 3 years ago

Ah very cool - I thought you did not want this, but am very happy to see it will be in the package 🙏