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

feat: System checks on SystemRequirements (cargo, rustc, msrv) #379

Merged
merged 10 commits into from
Sep 7, 2024

Conversation

albersonmiranda
Copy link
Contributor

@albersonmiranda albersonmiranda commented Aug 31, 2024

What type of PR is this? (check all applicable)

  • New Feature
  • Bug Fix
  • Documentation Update
  • Style Update
  • Refactor
  • Tests
  • Other tasks

Description

This PR updates use_cran_defaults() to check SystemRequirements field in DESCRIPTION and perform a system check for both Rust and Cargo tools. If any of them is not found, build fails with a message to install them. If they are found, it checks for the minimum version of rustc. If it is lower than specified in SystemRequirements, build fails with a message stating both installed and minimum version required. Finally, if all tests pass, it prints the version of cargo and rustc found, which will be used to build the package.

Related Tickets & Documents

Added/updated tests?

Goal is code coverage percentage at 80% and above.

  • Yes
  • No, and this is why: please replace this line with details on why tests
    have not been included
  • I need help with writing tests

Release checklist

  • Bump version
  • Update NEWS.md

@CGMossa
Copy link
Member

CGMossa commented Sep 5, 2024

Just wanted to say, this looks impressive already. Keep up the good work @albersonmiranda!

@albersonmiranda
Copy link
Contributor Author

@Ilia-Kosenkov After extendr/extendr#844 it is now passing R CMD Check locally and it's ready to review.

@albersonmiranda albersonmiranda marked this pull request as ready for review September 6, 2024 10:20
@Ilia-Kosenkov
Copy link
Member

I got it that the version in DESCRIPTION is annoying for snapshot tests, I will fix it after you merge this.

NEWS.md Outdated Show resolved Hide resolved
Copy link
Contributor

@JosiahParry JosiahParry left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is a great improvement on the existing configure and configure.win approach.

{fio} is already on CRAN using this approach. Excited to see it enable more robust pre-installation checks in the future as well.

@JosiahParry JosiahParry enabled auto-merge (squash) September 7, 2024 16:43
@JosiahParry JosiahParry changed the title feat: System checks on systemRequirements feat: System checks on SystemRequirements (cargo, rustc, msrv) Sep 7, 2024
@JosiahParry JosiahParry merged commit 52cd2e5 into extendr:main Sep 7, 2024
17 checks passed
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.

Perform system checks on systemRequirements
4 participants