Skip to content

Conversation

ZR233
Copy link

@ZR233 ZR233 commented Sep 24, 2025

Summary of the PR

For no_std usecase.

Requirements

Before submitting your PR, please make sure you addressed the following
requirements:

  • All commits in this PR have Signed-Off-By trailers (with
    git commit -s), and the commit message has max 60 characters for the
    summary and max 75 characters for each description line.
  • All added/changed functionality has a corresponding unit/integration
    test.
  • All added/changed public-facing functionality has entries in the "Upcoming
    Release" section of CHANGELOG.md (if no such section exists, please create one).
  • Any newly added unsafe code is properly documented.

Signed-off-by: 周睿 <zrufo747@outlook.com>
roypat added a commit to roypat/rust-vmm-container that referenced this pull request Sep 26, 2025
Some crates have a myriad of features, and it is very easy to
accidentally mess up imports in a way that `cargo check --all-features`
works, but only compiling with only a subset of features breaks. For
this scenario, cargo check-all-features can help, as it will attempt to
compile all feature permutations.

We cannot currently add this to the default CI, as some crates in the
rust-vmm ecosystem have incompatible features [1], but this unlocks at
least testing all feature combinations for specific crates (for example,
to ensure that no-std support does not accidentally break [2]).

once the vm-memory / xen situation is fixed, this can also be added to
the default CI.

[1]: rust-vmm/vm-memory#317
[2]: rust-vmm/vm-allocator#107

Signed-off-by: Patrick Roy <roypat@amazon.co.uk>
roypat added a commit to roypat/rust-vmm-container that referenced this pull request Sep 26, 2025
Some crates have a myriad of features, and it is very easy to
accidentally mess up imports in a way that `cargo check --all-features`
works, but only compiling with only a subset of features breaks. For
this scenario, cargo check-all-features can help, as it will attempt to
compile all feature permutations.

We cannot currently add this to the default CI, as some crates in the
rust-vmm ecosystem have incompatible features [1], but this unlocks at
least testing all feature combinations for specific crates (for example,
to ensure that no-std support does not accidentally break [2]).

once the vm-memory / xen situation is fixed, this can also be added to
the default CI.

[1]: rust-vmm/vm-memory#317
[2]: rust-vmm/vm-allocator#107

Signed-off-by: Patrick Roy <roypat@amazon.co.uk>
roypat added a commit to roypat/rust-vmm-container that referenced this pull request Sep 27, 2025
Some crates have a myriad of features, and it is very easy to
accidentally mess up imports in a way that `cargo check --all-features`
works, but only compiling with only a subset of features breaks. For
this scenario, cargo check-all-features can help, as it will attempt to
compile all feature permutations.

We can even add this to the default CI, even for crates like vhost that
have incompatible features, due to cargo-all-features allowing the
elimination of incompatible features from the test matrix [1].

Some usecases where this could have been helpful is in vm-memory, where
we have tens of feature permutations, and some of them invariably get
broken regularly [2], or in crates that have no-std support that is
offered via an opt-out 'std' feature [3]

[1]: https://crates.io/crates/cargo-all-features#options
[2]: rust-vmm/vm-memory#350
[3]: rust-vmm/vm-allocator#107

Signed-off-by: Patrick Roy <roypat@amazon.co.uk>
@roypat
Copy link
Member

roypat commented Sep 27, 2025

Hey, thanks for your contribution!

This looks fine, but I'm sorta holding off on merging this until we get our CI into a place where we can compile-test different feature permutations, to make sure this feature doesn't get accidentally broken again (unless you're in a hurry to get no_std support for this crate?), e.g. see rust-vmm/rust-vmm-container#139

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.

2 participants