search

extendr / rextendr

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

re-ordered steps in CRAN template #390

Closed beniaminogreen closed 3 days ago

beniaminogreen commented 1 month ago

Hi all,

The new version of R CMD check now tests whether the rustc version is reported before any rust code is compiled (the new R CMD check source code is linked here).

As it stands the rextendr templates report the rustc and cargo version after the code is compiled, which does not pass the new tests. This PR switches two lines in the Makevars files so that the code first reports the rustc version, and then compiles the rust code.

I have used this change to successfully update my package, zoomerjoin so it passes the new CRAN tests, but the changes haven't fully propagated through to the CRAN package page yet. I will update the PR immediately if there end up being any issues with this approach.

Best,
Ben

Ilia-Kosenkov commented 1 month ago

There are snapshot tests which might fail since you updated templates

Ilia-Kosenkov commented 1 month ago

Hm, it did not work it seems:

❯ checking Rust compilation ... WARNING Warning: No rustc version reported prior to compilation

Ilia-Kosenkov commented 1 month ago

Let's add this version printing to not-cran templates as well.

beniaminogreen commented 1 month ago

I have now added this to the non-cran templates as well.

This PR is supposed to fix the no rustc version reported prior to compilation error that you reported. Maybe it is failing because I did not also add the changes to the non-cran templates? Will see if this next round of CI tests passes.

Ilia-Kosenkov commented 1 month ago

Now version is reported but R check does not recognize it

Ilia-Kosenkov commented 1 month ago

patt <- "rustc *[[:digit:]]+[].][[:digit:]]"

This pattern seems to be invalid

JosiahParry commented 1 month ago

Note that this is not true as of https://github.com/extendr/rextendr/pull/379/files.

The new tools/msrv.R reports version of Rust and Cargo before compilation.

See https://cran.r-project.org/web/checks/check_results_fio.html and https://cran.r-project.org/web/checks/check_results_arcpbf.html which are using this new check.

JosiahParry commented 1 month ago

@beniaminogreen please update {rextendr} and call rextendr::use_cran_defaults() again this will overwrite the existing configure and configure.win to address this problem.

We can use this PR to remove the rustc --version and cargo --version calls from the Makevars though!

beniaminogreen commented 1 month ago

@beniaminogreen please update {rextendr} and call rextendr::use_cran_defaults() again this will overwrite the existing configure and configure.win to address this problem.

We can use this PR to remove the rustc --version and cargo --version calls from the Makevars though!

Thanks for this, will do! and I'll make those changes to the makevars files.

@Ilia-Kosenkov I was looking at that regex last night and it also looks weird to me. I think there's an extra brace in there. Doesn't look like this issue affects the msrv.R fix that @JosiahParry suggests, but is it worth trying to fix? Or perhaps I am reading the regex wrong.

Best, Ben

JosiahParry commented 3 days ago

This is resolved in the main branch.