extendr / rextendr

An R package that helps scaffolding extendr-enabled packages or compiling Rust code dynamically
https://extendr.github.io/rextendr/
Other
181 stars 27 forks source link

Change `CARGO_HOME` to temp dir if not `NOT_CRAN=true` in Makefiles #233

Closed eitsupi closed 1 year ago

eitsupi commented 1 year ago

CRAN policy prohibits adding files to the home directory during R package installation. This PR modifies the Makefile to write cargo cache to the temporary directory if not NOT_CRAN=true so that packages can be created according to this policy.

See also https://github.com/r-rust/faq#how-to-avoid-writing-in-home

Makefiles are copied from https://github.com/cran/prqlr/tree/0.3.0

Ilia-Kosenkov commented 1 year ago

Should it also tolerate NOT_CRAN=1?

eitsupi commented 1 year ago

Thanks for your response.

I used TRUE too here because I have seen other packages (arrow) use TRUE, but given that the original devtools setting is true, I don't think 1 needs to be considered. https://github.com/r-lib/devtools/blob/aa3f88bc693d049e1b524ca1a86b300e2c954765/R/R.R#L27 https://github.com/r-lib/devtools/commit/4a444805dadf02ccbba7d0776753ebc661fb9bcc

yutannihilation commented 1 year ago

I believe true is the only one possible value. The devtools package sets it to true and the testthat package expects true. I also searched the CRAN packages, but it seems almost no package expects TRUE (or 1).

https://github.com/search?q=org%3Acran+NOT_CRAN&type=code

eitsupi commented 1 year ago

Thanks for your review. I removed NOT_CRAN=TRUE condition.

yutannihilation commented 1 year ago

As it seems there's no opposition, I'm merging.

eitsupi commented 1 year ago

Thanks!