Skip to content

Conversation

roypat
Copy link
Member

@roypat roypat commented Sep 26, 2025

run cargo check for the x86_64-pc-windows-gnu target, to ensure we don't accidentally break building the crate on windows.

Summary of the PR

Please summarize here why the changes in this PR are needed.

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.

@roypat
Copy link
Member Author

roypat commented Sep 26, 2025

Ah, needs #348 first

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 <[email protected]>
@roypat roypat force-pushed the check-windows branch 2 times, most recently from 0f8c1aa to 5164ba0 Compare September 27, 2025 19:04
roypat added a commit to rust-vmm/rust-vmm-container that referenced this pull request Sep 30, 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 <[email protected]>
bonzini
bonzini previously approved these changes Oct 1, 2025
@roypat roypat force-pushed the check-windows branch 2 times, most recently from 41f1ed9 to 81819a8 Compare October 1, 2025 13:49
roypat added 5 commits October 1, 2025 15:09
run `cargo check` for the x86_64-pc-windows-gnu target, to ensure we
don't accidentally break building the crate on windows.

Hackily add rawfd and xen features to the cargo-all-features denylist
for just this test.

Signed-off-by: Patrick Roy <[email protected]>
The tests module in mmap/windows.rs assumed that `crate::bitmap` was
always available, but it is not. Hide the imports / usage of that crate
behind appropriate cfgs.

Fixes: 4757a4a ("do not backdoor-enable backend-bitmap in unittests")
Signed-off-by: Patrick Roy <[email protected]>
Otherwise the crate does not compile on windows with
--no-default-features --features backend-bitmap (not a particularly
useful combination of features, since it gives bitmaps without mmap
backends, which would usually drag in the winapi crate, but it should
still work).

Signed-off-by: Patrick Roy <[email protected]>
the rawfd and xen features do not work on windows (and don't even
compile), so explicitly give a compile_error!() in this case.

Signed-off-by: Patrick Roy <[email protected]>
@roypat
Copy link
Member Author

roypat commented Oct 1, 2025

@bonzini, sorry, this one ended up conflicting with #352, could you have another look? the only change should be that now this uses cargo-all-features as well

Copy link
Member

@bonzini bonzini left a comment

Choose a reason for hiding this comment

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

yes, still looks good :)

@roypat roypat enabled auto-merge (rebase) October 1, 2025 15:23
@roypat roypat merged commit ebfefb0 into rust-vmm:main Oct 1, 2025
2 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.

3 participants