Skip to content
New issue

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

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

Already on GitHub? # to your account

Enable control over debug for compile_dll #100

Merged
merged 3 commits into from
Oct 16, 2020
Merged

Enable control over debug for compile_dll #100

merged 3 commits into from
Oct 16, 2020

Conversation

richfitz
Copy link
Contributor

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.

richfitz added a commit to mrc-ide/dust that referenced this pull request Jul 22, 2020
See r-lib/pkgbuild#100 - there may be
better ways of doing this
@jimhester
Copy link
Member

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
Copy link
Contributor Author

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
Copy link
Member

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
Copy link
Member

Thank you!

@jimhester jimhester merged commit d9717aa into r-lib:master Oct 16, 2020
@richfitz
Copy link
Contributor Author

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

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

Successfully merging this pull request may close these issues.

2 participants