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

Add rust-toolchain files for all Rust repos #28

Open
15 of 17 tasks
bstrie opened this issue Nov 16, 2022 · 9 comments
Open
15 of 17 tasks

Add rust-toolchain files for all Rust repos #28

bstrie opened this issue Nov 16, 2022 · 9 comments
Assignees

Comments

@bstrie
Copy link
Contributor

bstrie commented Nov 16, 2022

The Enarx organization owns several Rust repos. However, the CI for PRs tends to test against whatever the stable release of Rust is at the moment the PR is submitted. Because Rust adds new warnings and clippy lints over time, and because our CI requires builds to pass without warnings, this means that PRs to Rust repos sometimes require fixing unrelated warnings before the CI will pass, which is annoying and a barrier to contribution.

Ideally, for each Rust crate in the Enarx organization we would determine the oldest Rust release that we would like to support, and then codify that version in a rust-toolchain file in the repo, and then have CI use this file to determine which Rust version to test against. This will both enforce MSRV and prevent the need to fix unrelated warnings when filing PRs.

The first step will be to add rust-toolchain files and rust-version fields to each of the following repos:

  • vfs (?)
  • iocuddle
  • mmledger
  • flagset
  • lset
  • crt0stack
  • xsave
  • vdso
  • mmarinus
  • nbytes
  • primordial
  • ciborium
  • snp
  • rcrt1
  • sgx
  • testaso
  • noted

Once this is done, then we can determine how our CI will use this information.

@bstrie
Copy link
Contributor Author

bstrie commented Nov 16, 2022

While we're at it, start using the rust-version Cargo.toml key in each crate: https://doc.rust-lang.org/cargo/reference/manifest.html#the-rust-version-field

@bstrie
Copy link
Contributor Author

bstrie commented Nov 16, 2022

It looks like many of these repos already have a specified MSRV, but we still need to make sure their clippy workflow uses the MSRV rather than the latest stable.

@dpal dpal self-assigned this Nov 17, 2022
@rvolosatovs
Copy link
Member

rvolosatovs commented Nov 17, 2022

@dpal given that this may involve changing Cargo.toml settings etc. I think @bstrie is best suited for this job

For deterministic CI behavior, we can also just copy-paste this directory https://github.com/enarx/vfs/tree/main/.github/workflows, which will enable dependabot, automatic updates, cargo fmt, cargo clippy and cargo nextest on Linux and MacOS runners run via nix (and so using a locked-in rust toolchain version)

@dpal
Copy link

dpal commented Nov 17, 2022

@dpal given that this may involve changing Cargo.toml settings etc. I think @bstrie is best suited for this job

@rvolosatovs, very much so, however, I want to understand the process too. Are you sure that this directory from the vfs repo is the best to copy around? For example, it has your name explicitly mentioned in one of the flows. I do not know how to interpret all the details of each of the workflow configuration files but should it be linked to some group rather than an individual?

@dpal
Copy link

dpal commented Nov 17, 2022

This issue is related to the issue #30 and should be solved once we have shared workflows.

@rvolosatovs
Copy link
Member

@dpal given that this may involve changing Cargo.toml settings etc. I think @bstrie is best suited for this job

@rvolosatovs, very much so, however, I want to understand the process too. Are you sure that this directory from the vfs repo is the best to copy around? For example, it has your name explicitly mentioned in one of the flows. I do not know how to interpret all the details of each of the workflow configuration files but should it be linked to some group rather than an individual?

This is more about Rust specifics.

The directory I linked is regarding CI, the project uses https://github.com/rvolosatovs/nixify to bootstrap the nix flake and then leverages the provided functionality to use it in CI. We use https://github.com/rvolosatovs/nixify also in Enarx, Steward, Drawbridge, Benefice.
My username appears in the workflow, because the automated flake lock update PR assigns me for review, since I'm de-facto "Nix expert" and mostly people are afraid of touching anything nix-related.

We can adopt https://github.com/rvolosatovs/nixify in other Enarx projects as well and benefit from consistent CI experience, deterministic CI behavior, automatic updates and caching. That's up to discussion and can happen in parallel to this issue

@rvolosatovs
Copy link
Member

@dpal I filed #31 for the CI part

@dpal
Copy link

dpal commented Nov 18, 2022

@bstrie will create a checklist and start going repo by repo. Ben will move it into "In Progress" once he starts doing things under the umbrella of this ticket.

@dpal dpal assigned rvolosatovs and unassigned dpal Nov 18, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: New
Development

No branches or pull requests

3 participants