1# Coding guidelines 2 3For the most part, Wasmtime and Cranelift follow common Rust conventions and 4[pull request] (PR) workflows, though we do have a few additional things to 5be aware of. 6 7[pull request]: https://help.github.com/articles/about-pull-requests/ 8 9### `rustfmt` 10 11All PRs must be formatted according to rustfmt, and this is checked in the 12continuous integration tests. You can format code locally with: 13 14```sh 15$ cargo fmt 16``` 17 18at the root of the repository. You can find [more information about rustfmt 19online](https://github.com/rust-lang/rustfmt) too, such as how to configure 20your editor. 21 22### Compiler Warnings and Lints 23 24Wasmtime promotes all compiler warnings to errors in CI, meaning that the `main` 25branch will never have compiler warnings for the version of Rust that's being 26tested on CI. Compiler warnings change over time, however, so it's not always 27guaranteed that Wasmtime will build with zero warnings given an arbitrary 28version of Rust. If you encounter compiler warnings on your version of Rust 29please feel free to send a PR fixing them. 30 31During local development, however, compiler warnings are simply warnings and the 32build and tests can still succeed despite the presence of warnings. This can be 33useful because warnings are often quite prevalent in the middle of a 34refactoring, for example. By the time you make a PR, though, we'll require that 35all warnings are resolved or otherwise CI will fail and the PR cannot land. 36 37Compiler lints are controlled through the `[workspace.lints.rust]` table in the 38`Cargo.toml` at the root of the Wasmtime repository. A few allow-by-default 39lints are enabled such as `trivial_numeric_casts`, and you're welcome to enable 40more lints as applicable. Lints can additionally be enabled on a per-crate basis 41such as placing this in a `src/lib.rs` file: 42 43```rust 44#![warn(trivial_numeric_casts)] 45``` 46 47Using `warn` here will allow local development to continue while still causing 48CI to promote this warning to an error. 49 50### Clippy 51 52All PRs are gated on `cargo clippy` passing for all workspace crates and 53targets. All clippy lints, however, are allow-by-default and thus disabled. The 54Wasmtime project selectively enables Clippy lints on an opt-in basis. Lints can 55be controlled for the entire workspace via `[workspace.lints.clippy]`: 56 57```toml 58[workspace.lints.clippy] 59# ... 60manual_strip = 'warn' 61``` 62 63or on a per-crate or module basis by using attributes: 64 65```rust 66#![warn(clippy::manual_strip)] 67``` 68 69In Wasmtime we've found that the default set of Clippy lints is too noisy to 70productively use other Clippy lints, hence the allow-by-default behavior. 71Despite this though there are numerous useful Clippy lints which are desired for 72all crates or in some cases for a single crate or module. Wasmtime encourages 73contributors to enable Clippy lints they find useful through workspace or 74per-crate configuration. 75 76Like compiler warnings in the above section all Clippy warnings are turned into 77errors in CI. This means that `cargo clippy` should always produce no warnings 78on Wasmtime's `main` branch if you're using the same compiler version that CI 79does (typically current stable Rust). This means, however, that if you enable a 80new Clippy lint for the workspace you'll be required to fix the lint for all 81crates in the workspace to land the PR in CI. 82 83Clippy can be run locally with: 84 85```shell 86$ cargo clippy --workspace --all-targets 87``` 88 89Contributors are welcome to enable new lints and send PRs for this. Feel free to 90reach out if you're not sure about a lint as well. 91 92### Minimum Supported `rustc` Version (MSRV) 93 94Wasmtime and Cranelift support the latest three stable releases of Rust. This 95means that if the latest version of Rust is 1.72.0 then Wasmtime supports Rust 961.70.0, 1.71.0, and 1.72.0. CI will test by default with 1.72.0 and there will 97be one job running the full test suite on Linux x86\_64 on 1.70.0. 98 99Some of the CI jobs depend on nightly Rust, for example to run rustdoc with 100nightly features, however these use pinned versions in CI that are updated 101periodically and the general repository does not depend on nightly features. 102 103Updating Wasmtime's MSRV is done by editing the `rust-version` field in the 104workspace root's `Cargo.toml` 105 106Note that this policy is subject to change over time (notably it might be 107extended to include more rustc versions). Current Wasmtime users don't require a 108larger MSRV window to justify the maintenance needed to have a larger window. If 109your use case requires a larger MSRV range though please feel free to contact 110maintainers to raise your use case (e.g. an issue, in a Wasmtime meeting, on 111Zulip, etc). 112 113### Dependencies of Wasmtime 114 115Wasmtime and Cranelift have a higher threshold than default for adding 116dependencies to the project. All dependencies are required to be "vetted" 117through the [`cargo vet` tool](https://mozilla.github.io/cargo-vet/). This is 118checked on CI and will run on all modifications to `Cargo.lock`. 119 120A "vet" for Wasmtime is not a meticulous code review of a dependency for 121correctness but rather it is a statement that the crate does not contain 122malicious code and is safe for us to run during development and (optionally) 123users to run when they run Wasmtime themselves. Wasmtime's vet entries are used 124by other organizations which means that this isn't simply for our own personal 125use. Wasmtime additionally uses vet entries from other organizations as well 126which means we don't have to vet everything ourselves. 127 128New vet entries are required to be made by trusted contributors to Wasmtime. 129This is all configured in the `supply-chain` folder of Wasmtime. These files 130generally aren't hand-edited though and are instead managed through the `cargo 131vet` tool itself. Note that our `supply-chain/audits.toml` additionally contains 132entries which indicates that authors are trusted as opposed to vets of 133individual crates. This lowers the burden of updating version of a crate from a 134trusted author. 135 136When put together this means that contributions to Wasmtime and Cranelift which 137update existing dependencies or add new dependencies will not be mergeable by 138default (CI will fail). This is expected from our project's configuration and 139this situation will be handled one of a few ways: 140 141Note that this process is not in place to prevent new dependencies or prevent 142updates, but rather it ensures that development of Wasmtime is done with a 143trusted set of code that has been reviewed by trusted parties. We welcome 144dependency updates and new functionality, so please don't be too alarmed when 145contributing and seeing a failure of `cargo vet` on CI! 146 147### `cargo vet` for Contributors 148 149If you're a contributor to Wasmtime and you've landed on this documentation, 150hello and thanks for your contribution! Here's some guidelines for changing the 151set of dependencies in Wasmtime: 152 153* If a new dependency is being added it might be worth trying to slim down 154 what's required or avoiding the dependency altogether. Avoiding new 155 dependencies is best when reasonable, but it is not always reasonable to do 156 so. This is left to the judgement of the author and reviewer. 157 158* When updating dependencies this should be done for a specific purpose relevant 159 to the PR-at-hand. For example if the PR implements a new feature then the 160 dependency update should be required for the new feature. Otherwise it's best 161 to leave dependency updates to their own PRs. It's ok to update dependencies 162 "just for the update" but we prefer to have that as separate PRs. 163 164Dependency additions or updates require action on behalf of project maintainers 165so we ask that you don't run `cargo vet` yourself or update the `supply-chain` 166folder yourself. Instead a maintainer will review your PR and perform the `cargo 167vet` entries themselves. Reviewers will typically make a separate pull request 168to add `cargo vet` entries and once that lands yours will be added to the queue. 169 170### `cargo vet` for Maintainers 171 172Maintainers of Wasmtime are required to explicitly vet and approve all 173dependency updates and modifications to Wasmtime. This means that when reviewing 174a PR you should ensure that contributors are not modifying the `supply-chain` 175directory themselves outside of commits authored by other maintainers. Otherwise 176though to add vet entries this is done through one of a few methods: 177 178* For a PR where maintainers themselves are modifying dependencies the `cargo 179 vet` entries can be included inline with the PR itself by the author. The 180 reviewer knows that the author of the PR is themself a maintainer. 181 182* PRs that "just update dependencies" are ok to have at any time. You can do 183 this in preparation for a future feature or for a future contributor. This 184 more-or-less is the same as the previous categories. 185 186* For contributors who should not add vet entries themselves maintainers should 187 review the PR and add vet entries either in a separate PR or as part of the 188 contributor's PR itself. As a separate PR you'll check out the branch, run 189 `cargo vet`, then rebase away the contributor's commits and push your `cargo 190 vet` commit alone to merge. For pushing directly to the contributor's own PR 191 be sure to read the notes below. 192 193Note for the last case it's important to ensure that if you push directly to a 194contributor's PR any future updates pushed by the contributor either contain or 195don't overwrite your vet entries. Also verify that if the PR branch is rebased 196or force-pushed, the details of your previously pushed vetting remain the same: 197e.g., versions were not bumped and descriptive reasons remain the same. If 198pushing a vetting commit to a contributor's PR and also asking for more changes, 199request that the contributor make the requested fixes in an additional commit 200rather than force-pushing a rewritten history, so your existing vetting commit 201remains untouched. These guidelines make it easier to verify no tampering has 202occurred. 203 204### Policy for adding `cargo vet` entries 205 206For maintainers this is intended to document the project's policy on adding 207`cargo vet` entries. The goal of this policy is to not make dependency updates 208so onerous that they never happen while still achieving much of the intended 209benefit of `cargo vet` in protection against supply-chain style attacks. 210 211* For dependencies **that receive at least 10,000 downloads a day** on crates.io 212 it's ok to add an entry to `exemptions` in `supply-chain/config.toml`. This 213 does not require careful review or review at all of these dependencies. The 214 assumption here is that a supply chain attack against a popular crate is 215 statistically likely to be discovered relatively quickly. Changes to `main` in 216 Wasmtime take at least 2 weeks to be released due to our release process, so 217 the assumption is that popular crates that are victim of a supply chain attack 218 would be discovered during this time. This policy additionally greatly helps 219 when updating dependencies on popular crates that are common to see without 220 increasing the burden too much on maintainers. 221 222* For other dependencies a manual vet is required. The `cargo vet` tool will 223 assist in adding a vet by pointing you towards the source code, as published 224 on crates.io, to be browsed online. Manual review should be done to ensure 225 that "nothing nefarious" is happening. For example `unsafe` should be 226 inspected as well as use of ambient system capabilities such as `std::fs`, 227 `std::net`, or `std::process`, and build scripts. Note that you're not 228 reviewing for correctness, instead only for whether a supply-chain attack 229 appears to be present. 230 231This policy intends to strike a rough balance between usability and security. 232It's always recommended to add vet entries where possible, but the first bullet 233above can be used to update an `exemptions` entry or add a new entry. Note that 234when the "popular threshold" is used **do not add a vet entry** because the 235crate is, in fact, not vetted. This is required to go through an 236`[[exemptions]]` entry. 237