r-lib / devtools

Tools to make an R developer's life easier
https://devtools.r-lib.org
Other
2.4k stars 758 forks source link

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

Open wch opened 6 years ago

wch commented 6 years ago

(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: https://github.com/rstudio/htmltools/issues/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: https://github.com/rstudio/httpuv/pull/85#issuecomment-331534372 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 commented 6 years ago

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 commented 5 years ago

Another example shared by @wch

knitr caching evaluate::evaluate() at build time: https://github.com/yihui/knitr/issues/1441