-
Notifications
You must be signed in to change notification settings - Fork 26
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
Make rextendr packages CRAN compatible by default #394
Conversation
welp! back to the drawing board! |
Can't you do something like
|
…ck for cran and non-cran packages
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I've taken this as an opportunity to refactor the way in which Makevars are done.
Summary
This PR now removes use_cran_defaults()
and instead, uses a single Makevars
and Makevars.in
to support CRAN environment and regular development environment. This is done in the following way:
Makevars
&Makevars.win
are nowMakevars.in
andMakevars.win.in
- The
configure
script generates theMakevars
andMakevars.win
files substituting the@CRAN_FLAGS@
this approach is taken fromdata.table
see https://github.com/Rdatatable/data.table/blob/master/src/Makevars.in. It also deletes thesrc/Makevars
andsrc/Makevars.win
before generating the respective files - Now
src/Makevars{.win}
are in the.Rbuildignore
and.gitignore
when usinguse_extendr()
ensuring that the file is always rebuilt - When
NOT_CRAN=false
thevendor.tar.xz
is unbundled and the flags-j 2 --offline
are set
To ensure all of this is done correctly, two new tests are added which run rcmdcheck::rcmdcheck()
and rcmdcheck::rcmdcheck(env = c("NOT_CRAN"="false"))
to check both scenarios.
The function use_cran_defaults()
has been removed.
Next steps
If you both approve of this direction, I would like to make the following changes
Within this PR
- remove
vignettes/articles/cran-compliance.qmd
as an alternative i am happy to write a standalone help doc e.g.?rextendr::cran
- remove mentions of
use_cran_defaults()
from the NEWS.md as the function will have been introduced and removed before a new CRAN release
Follow up PRs
- write a function to generate an
inst/AUTHORS
file as was done by @DavZim in {rtiktoken} see https://github.com/DavZim/rtiktoken/blob/master/inst/AUTHORS - write a page in the user guide on publishing to CRAN
I still believe checking |
Thanks @Ilia-Kosenkov, that's fine. That can be addressed. But are you okay with this plan (assuming we change the NOT_CRAN to check based on a true value)? Having one |
What happens when NOT_CRAN is true but I don't have vendor/ only vendor.tar.xz? Ie I installed the package from GitHub or from local. Is the archive unpacked correctly? |
@DavZim the Makevars is only ran when building the package from source. This template will unpack the archive if |
RUSTFLAGS
modification additive
I've moved this as a draft so I can adjust the |
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files
|
As a followup from #393 and comments from @CGMossa and @Ilia-Kosenkov, this PR makes adds
--print=native-static-libs
to any existingRUSTFLAGS
ensuring that they are not overwritten.