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

Request: add a way to check for build-time dependencies #1788

Open
wch opened this issue May 26, 2018 · 2 comments
Open

Request: add a way to check for build-time dependencies #1788

wch opened this issue May 26, 2018 · 2 comments
Labels
feature
Milestone

Comments

@wch
Copy link
Member

wch commented May 26, 2018

(I'm creating this issue to track a discussion that started elsewhere.) In general, it's a bad idea to run code from another package at build time, because it can bake-in some dependencies such that the resulting binary package may not work on another system, or it may not work after the build-time dependency is upgraded.

Here's an example where a package would not work if built on one machine (like a CRAN build server) and run on another:
rstudio/htmltools#22
This was a common and dangerous enough problem that we added an explicit check for it:
https://github.com/wch/htmltools/blob/9ebe77c5/R/html_dependency.R#L67-L75

Here's an example where upgrading the build-time dependency package would break the target package:
rstudio/httpuv#85 (comment)
In this case, we ended up adding an awkward workaround that will have to be maintained for a long while in the future:
https://github.com/rstudio/httpuv/blob/5b84ed80/src/RcppExports-legacy.cpp

It could save a lot of trouble if devtools could do some sort of check that warned when a package calls a function from other packages at build-time.

I should mention that there are some exceptions, where it is usually OK to do this. For example R6::R6Class() is commonly called at build time, but it is designed to encapsulate all code that will be needed in the future so it this won't cause these problems. Also, base functions like new.env() and local() should generally be OK.

@jimhester jimhester added the feature label Jun 29, 2018
@jimhester
Copy link
Member

jimhester commented Jul 18, 2018

I agree this is a useful thing to check, however I am not sure devtools is the right place for it. It seems more appropriate as a lintr linter or part of goodpractice.

@jennybc
Copy link
Member

jennybc commented Apr 8, 2019

Another example shared by @wch

knitr caching evaluate::evaluate() at build time: yihui/knitr#1441

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

No branches or pull requests

3 participants