Update Rust in CI to 1.72.0, clarify Wasmtime's MSRV #6900
Conversation
|
I'll offer a point in favor of option 2, with uncertain weight but maybe nonzero: not everyone uses Many of those probably carry lower weight than e.g. using a sorely-needed feature in a new release; but if the habit is instead to update whenever we have time, with no other particular benefits, maybe we could consider the above? |
|
For a little more data: the 2021 Rust survey report shows 7585 of 10214 responses use latest stable or nightly Rust in CI (74.2%); 8888 of 10868 responses use Rustup (81.8%). So a majority, but I'm definitely not comfortable excluding folks who don't fit into that "happy path", if we can help it, especially if the cases that don't are some of the more interesting ones (big software systems, etc). |
|
I think personally I just want something clear and easy to follow. I've spent way too much time in my Rust career watching debates about the Rust version either end in "ok you can only use a version of Rust that's 4 years old" to the benefit of almost no one or a not-well-defined policy which ends up reigniting discussion every time it comes up. I changed CI to pin Rust versions awhile back to avoid breakage from Rust updates (e.g. new warnings or such), and my hope is that future updates would not be blocked on "settle a MSRV policy". While developing such a policy is probably inevitable and useful, the lack of community consensus on what to do here makes figuring this out quite slog in my opinion. If you'd like I'm happy to back out the change to |
|
I'm generally in favor of staying up-to-date. What about adopting policy 1, and re-examining it if we get a report that it's breaking someone else's workflow? |
|
Ah, sorry, I didn't mean to reignite any old discussions or scars. AFAIK this is the first time I've been involved in a "MSRV discussion"; hopefully I don't have as bumpy a path with them! I agree it'd be great to set down an explicit policy for Wasmtime, to avoid any ambiguity. Maybe we should discuss and decide on something in the next Wasmtime meeting? I'll put it on the agenda. |
|
(My comment raced with @elliottt's; I'm fine with seeing this go through as-is, then we can discuss more at the meeting...) |
|
Ok sounds good! I'm happy to sit on this until the next meeting. If someone wants to use something from 1.72 I'd be in favor of merging and enabling that, but in the meantime I think it's ok to update locally and otherwise keep CI at 1.71 |
|
We talked about this in the Wasmtime biweekly just now, and reached a good amount of consensus; to summarize: we agreed that it makes sense to support a sliding window of latest stable Rust versions (stable down to stable - N; N is a parameter we still need to choose). This allows flexibility to embedders of Wasmtime by not forcing an upgrade to latest stable immediately after release -- others can design release processes that work within the N-version window. It also gives some flexibility for embedders if they ned to hold a version upgrade temporarily for whatever reason (bug or miscompile elsewhere, etc). The main points to be decided are:
On the latter point, discussion revolved around (at least) two main constraints: CI resources are limited, so that we can't simply duplicate the test runs for all Rust versions in the window; and we also would prefer "CI determinism", i.e., a PR that previously passed CI should not stop passing if a new Rust version happens to be released between runs. (This is important for the contributor experience but also for e.g. ability to respond quickly without roadblocks to emergency patches and that sort of thing.) Given the determinism constraint, we'd likely define the window by pinning two versions of rust -- "newest supported" and "oldest supported" -- defining our window manually and bumping it manually. We should do these bumps regularly when new releases of Rust occur, and not only when we actually need a feature: otherwise, compatibility pressure builds up, dependencies acrete, and we have more problems when we do bump. Given the CI resources constraint, there seem to be two main options: run all tests at newest supported and add one additional job on Linux/x86-64 that runs at oldest supported, or vice-versa. The former catches platform-specific issues due to new Rust features sooner, but does not cover the "tail" of the sliding window, so N-1/N-2 Rust could break in platform-specific ways. The latter ensures the tail of the window stays working everywhere, but may catch platform-specific issues later. Does this capture everything @alexcrichton, @fitzgen, @tschneidereit, others? My own thoughts:
|
|
Thanks for writing that up @cfallin and that all looks accurate to me! I'd also agree with your conclusions of N=2 and "test latest, one job msrv" personally. |
This commit codifies an MSRV policy for Wasmtime at "stable minus two" meaning that the latest three releases of Rust will be supported. This is enforced on CI with a full test suite job running on Linux x86_64 with the minimum supported Rust version. The full test suite will use the latest stable version. A downside of this approach is that new changes may break MSRV support on non-Linux or non-x86_64 platforms and we won't know about it, but that's deemed a minor enough risk at this time. A minor fix is applied to Wasmtime's `Cargo.toml` to support Rust 1.70.0 instead of requiring Rust 1.71.0
alexcrichton
force-pushed
the
update-rust
branch
from
aaf77ba
to
2fe6222
Compare
|
Ok I've updated this with the various variables involved, but this is to showcase what I think enforcing the proposed MSRV policy would look like. Folks should still weigh in on the specifics if they have thoughts!
|
| @@ -21,13 +21,13 @@ your editor. | |||
|
|
|||
| ### Minimum Supported `rustc` Version | |||
|
|
|||
| Wasmtime supports the current stable version of Rust. | |||
| Wasmtime supports the latest three stable releases of Rust. | |||
|
|
|||
| Cranelift supports stable Rust, and follows the [Rust Update Policy for | |||
There was a problem hiding this comment.
While we're here, could we update this for Cranelift as well? We're no longer included in the mozilla-central tree and aren't subject to their Rust version policies anymore; it's probably uncontroversial to say now that Cranelift will follow the same policy as Wasmtime?
+1 on both these things from me as well. |
And from me, thank you! |
alexcrichton
changed the title
|
Ok I've updated the OP and title of this PR as well, thanks again everyone for all the input 👍 |
github-actions
bot
added
wasmtime:api
Subscribe to Label Actioncc @peterhuene
This issue or pull request has been labeled: "wasmtime:api", "wasmtime:docs"
Thus the following users have been cc'd because of the following labels:
To subscribe or unsubscribe from this label, edit the |
…6950) * Enhance `async` configuration of `bindgen!` macro (#6942) This commit takes a leaf out of `wiggle`'s book to enable bindings generation for async host functions where only some host functions are async instead of all of them. This enhances the `async` key with a few more options: async: { except_imports: ["foo"], only_imports: ["bar"], } This is beyond what `wiggle` supports where either an allow-list or deny-list can be specified (although only one can be specified). This can be useful if either the list of sync imports or the list of async imports is small. * cranelift-interpreter: Fix SIMD shifts and rotates (#6939) * cranelift-interpreter: Fix SIMD `ishl`/`{s,u}`shr * fuzzgen: Enable a few more ops * cranelift: Fix tests for {u,s}shr * fuzzgen: Change pattern matching arms for shifts Co-Authored-By: Jamey Sharp <jsharp@fastly.com> --------- Co-authored-by: Jamey Sharp <jsharp@fastly.com> * Partially revert CLI argument changes from #6737 (#6944) * Partially revert CLI argument changes from #6737 This commit is a partial revert of #6737. That change was reverted in #6830 for the 12.0.0 release of Wasmtime and otherwise it's currently slated to get released with the 13.0.0 release of Wasmtime. Discussion at today's Wasmtime meeting concluded that it's best to couple this change with #6925 as a single release rather than spread out across multiple releases. This commit is thus the revert of #6737, although it's a partial revert in that I've kept many of the new tests added to showcase the differences before/after when the change lands. This means that Wasmtime 13.0.0 will exhibit the same CLI behavior as 12.0.0 and all prior releases. The 14.0.0 release will have both a new CLI and new argument passing semantics. I'll revert this revert (aka re-land #6737) once the 13.0.0 release branch is created and `main` becomes 14.0.0. * Update release notes * riscv64: Use `PCRelLo12I` relocation on Loads (#6938) * riscv64: Use `PCRelLo12I` relocation on Loads * riscv64: Strenghten pattern matching when emitting Load's * riscv64: Clarify some of the load address logic * riscv64: Even stronger matching * Update Rust in CI to 1.72.0, clarify Wasmtime's MSRV (#6900) * Update Rust in CI to 1.72.0 * Update CI, tooling, and docs for MSRV This commit codifies an MSRV policy for Wasmtime at "stable minus two" meaning that the latest three releases of Rust will be supported. This is enforced on CI with a full test suite job running on Linux x86_64 with the minimum supported Rust version. The full test suite will use the latest stable version. A downside of this approach is that new changes may break MSRV support on non-Linux or non-x86_64 platforms and we won't know about it, but that's deemed a minor enough risk at this time. A minor fix is applied to Wasmtime's `Cargo.toml` to support Rust 1.70.0 instead of requiring Rust 1.71.0 * Fix installation of rust * Scrape MSRV from Cargo.toml * Cranelift is the same as Wasmtime's MSRV now, more words too * Fix a typo * aarch64: Use `RegScaled*` addressing modes (#6945) This commit adds a few cases to `amode` construction on AArch64 for using the `RegScaled*` variants of `AMode`. This won't affect wasm due to this only matching the sign-extension happening before the shift, but it should otherwise help non-wasm Cranelift use cases. Closes #6742 * cranelift: Validate `iconst` ranges (#6850) * cranelift: Validate `iconst` ranges Add the following checks: `iconst.i8` immediate must be within 0 .. 2^8-1 `iconst.i16` immediate must be within 0 .. 2^16-1 `iconst.i32` immediate must be within 0 .. 2^32-1 Resolves #3059 * cranelift: Parse `iconst` according to its type Modifies the parser for textual CLIF so that V in `iconst.T V` is parsed according to T. Before this commit, something like `iconst.i32 0xffff_ffff_ffff` was valid because all `iconst` were parsed the same as an `iconst.i64`. Now the above example will throw an error. Also, a negative immediate as in `iconst.iN -X` is now converted to `2^N - X`. This commit also fixes some broken tests. * cranelift: Update tests to match new CLIF parser * Some minor fixes and features for WASI and sockets (#6948) * Use `command::add_to_linker` in tests to reduce the number of times all the `add_to_linker` are listed. * Add all `wasi:sockets` interfaces currently implemented to both the sync and async `command` functions (this enables all the interfaces in the CLI for example). * Use `tokio::net::TcpStream::try_io` whenever I/O is performed on a socket, ensuring that readable/writable flags are set/cleared appropriately (otherwise once readable a socket is infinitely readable). * Add a `with_ambient_tokio_runtime` helper function to use when creating a `tokio::net::TcpStream` since otherwise it panics due to a lack of active runtime in a synchronous context. * Add `WouldBlock` handling to return a 0-length read. * Add an `--inherit-network` CLI flag to enable basic usage of sockets in the CLI. This will conflict a small amount with #6877 but should be easy to resolve, and otherwise this targets different usability points/issues than that PR. --------- Co-authored-by: Afonso Bordado <afonso360@users.noreply.github.com> Co-authored-by: Jamey Sharp <jsharp@fastly.com> Co-authored-by: Timothée Jourde <timjrd@netc.fr>
…e#6900) * Update Rust in CI to 1.72.0 * Update CI, tooling, and docs for MSRV This commit codifies an MSRV policy for Wasmtime at "stable minus two" meaning that the latest three releases of Rust will be supported. This is enforced on CI with a full test suite job running on Linux x86_64 with the minimum supported Rust version. The full test suite will use the latest stable version. A downside of this approach is that new changes may break MSRV support on non-Linux or non-x86_64 platforms and we won't know about it, but that's deemed a minor enough risk at this time. A minor fix is applied to Wasmtime's `Cargo.toml` to support Rust 1.70.0 instead of requiring Rust 1.71.0 * Fix installation of rust * Scrape MSRV from Cargo.toml * Cranelift is the same as Wasmtime's MSRV now, more words too * Fix a typo
This PR updates Rust in CI to 1.72.0. Additionally this implements the outcome of the latest Wasmtime meeting's discussion about an MSRV policy which is to support the latest 3 releases of Rust. CI now primarily tests against the latest release of Rust but a single Linux x86_64 builder runs the full test suite on the minimum Rust version supported.