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

re-ordered steps in CRAN template #390

Open
wants to merge 3 commits into
base: main
Choose a base branch
from

Conversation

beniaminogreen
Copy link

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
Copy link
Member

There are snapshot tests which might fail since you updated templates

@Ilia-Kosenkov
Copy link
Member

Hm, it did not work it seems:

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

@Ilia-Kosenkov
Copy link
Member

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

@beniaminogreen
Copy link
Author

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
Copy link
Member

Now version is reported but R check does not recognize it

@Ilia-Kosenkov
Copy link
Member

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

This pattern seems to be invalid

@JosiahParry
Copy link
Contributor

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
Copy link
Contributor

@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
Copy link
Author

@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

inst/templates/Makevars Outdated Show resolved Hide resolved
inst/templates/Makevars.win Outdated Show resolved Hide resolved
inst/templates/cran/Makevars Outdated Show resolved Hide resolved
inst/templates/cran/Makevars.win Show resolved Hide resolved
inst/templates/cran/Makevars.win Outdated Show resolved Hide resolved
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants