Skip to content

Conversation

@leon-xd
Copy link
Contributor

@leon-xd leon-xd commented Mar 20, 2025

BREAKING CHANGES:

  • introduces MSRV via rust-version to virtual manifest, currently set to 1.82.0 1.84.0

Overview

With the introduction of Rust 2024, bindgen implemented new options for supporting different editions of Rust. bindgen's default behavior is to generate Rust code for the latest edition and toolchain.

We need to overwrite this default behavior with whatever edition wdk-build uses, as we would like to manually update from 2021 -> 2024. This PR also introduces logic to specify the rust version & toolchain to bindgen.

Files changed

cargo.toml

  • added rust-version, set to 1.82.0 1.84.0
  • bump resolver to 3
  • bump bindgen to 0.71.0 for Builder::rust-edition
  • bump cargo-metadata to 0.19.2 for Edition::E2024
  • add semver crate
  • bump thiserror to 2.0.12 for cargo-metadata version dependency

crates/wdk-build/cargo.toml

  • added relevant crates
  • added nightly feature

crates/wdk-build/src/bindgen.rs

  • impl TryFrom for conversion between cargo-metadata and bindgen rust edition enums
  • fn get_rust_target determines proper rust version/toolchain for bindgen
  • fn get_rust_edition determines proper rust edition for bindgen
  • added calls to target and edition for wdk_default

crates/wdk-build/src/cargo_make.rs

  • fixed code that was incompatible with older version of cargo-metadata

crates/wdk-build/src/lib.rs

  • added ConfigError variants for new errors thrown by updated code

@leon-xd
Copy link
Contributor Author

leon-xd commented Mar 22, 2025

I have created an issue to the bindgen team to discuss why this build is failing: rust-lang/rust-bindgen#3177

Switching from bindgen 0.69 -> 0.71 means size and alignment checks happen at compile-time rather than runtime. Looking into the old versions of the USB header bindings, these were generated in old builds unsuccessfully, but since we never used these functions this was never discovered until now.

@leon-xd leon-xd requested a review from wmmc88 March 24, 2025 23:42
@wmmc88 wmmc88 requested a review from Copilot March 25, 2025 20:00
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR updates the project’s Rust version, edition handling, and bindgen configuration to align with Rust 2024 and the latest toolchain. Key changes include:

  • Enhancements in crates/wdk-build/src/bindgen.rs to convert cargo_metadata editions to bindgen’s RustEdition and to set the rust target and edition on the builder.
  • Updates to Cargo.toml and workspace manifests that bump dependency versions, update the resolver, and specify the MSRV.
  • Adjustments to crates/wdk-sys and crates/wdk-build/src/cargo_make.rs to support the new nightly feature configuration and updated target kind checks.

Reviewed Changes

Copilot reviewed 15 out of 15 changed files in this pull request and generated no comments.

Show a summary per file
File Description
crates/wdk-build/src/bindgen.rs Introduced BindgenRustEditionWrapper, added get_rust_target and get_rust_edition functions, and updated builder configuration.
Cargo.toml Upgraded dependency versions, updated resolver, and specified a new rust-version.
crates/wdk-sys/Cargo.toml Expanded nightly feature configuration.
crates/wdk-build/src/lib.rs Updated ConfigError variants with clearer error messages.
crates/wdk-build/Cargo.toml Declared rust-version and semver workspace settings, and added nightly feature.
crates/wdk-build/src/cargo_make.rs Modified target kind check for cdylib using cargo_metadata enum.
Comments suppressed due to low confidence (2)

crates/wdk-build/src/bindgen.rs:190

  • Consider adding unit tests for 'get_rust_target' to validate its behavior under both nightly and stable toolchain configurations, ensuring the correct Rust target is selected.
fn get_rust_target() -> Result<bindgen::RustTarget, ConfigError> {

crates/wdk-build/src/bindgen.rs:223

  • Consider adding unit tests for 'get_rust_edition' to ensure that the conversion from cargo metadata editions to bindgen's RustEdition is handled correctly, including error cases for unsupported editions.
fn get_rust_edition() -> Result<bindgen::RustEdition, ConfigError> {

hamzamust
hamzamust previously approved these changes Mar 27, 2025
hamzamust
hamzamust previously approved these changes Mar 28, 2025
wmmc88
wmmc88 previously approved these changes Mar 29, 2025
Copy link
Collaborator

@wmmc88 wmmc88 left a comment

Choose a reason for hiding this comment

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

LGTM!

@wmmc88 wmmc88 requested a review from Copilot March 31, 2025 19:27
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR updates the Rust toolchain configuration for wdk-build by specifying a new MSRV and Rust edition for bindgen, while updating several dependencies to support these changes. Key changes include:

  • Bumping versions for rust-bindgen, cargo-metadata, semver, and thiserror, and adding MSRV via rust-version.
  • Adding new logic to determine the Rust target and edition for bindgen in wdk-build.
  • Adjusting dependencies in Cargo.toml files (including nightly features) to support the updated configuration.

Reviewed Changes

Copilot reviewed 15 out of 15 changed files in this pull request and generated no comments.

Show a summary per file
File Description
crates/wdk-sys/Cargo.toml Updated nightly dependencies to include wdk-build/nightly
crates/wdk-build/src/lib.rs Added new error variants and updated error messages in ConfigError
crates/wdk-build/src/cargo_make.rs Updated target matching to use cargo_metadata’s TargetKind enum
crates/wdk-build/src/bindgen.rs Added functions for determining rust target/edition and adjusted builder usage
crates/wdk-build/Cargo.toml Enabled rust-version and semver at the workspace level
Cargo.toml Upgraded dependency versions and bumped resolver version
Comments suppressed due to low confidence (2)

crates/wdk-build/src/bindgen.rs:194

  • The documentation comment references ConfigError::SemverError while the actual variant is named RustVersionParseError. Update the comment to match the correct error variant.
// Returns `ConfigError::MsrvNotSupportedByBindgen` if the MSRV is not supported by bindgen, or `ConfigError::SemverError` if the MSRV cannot be parsed as a semver version.

crates/wdk-build/src/bindgen.rs:251

  • [nitpick] The variable name 'wdk_sys_cargo_metadata' might be misleading as it pertains to the 'wdk-build' package. Consider renaming it to 'wdk_build_metadata' for clarity.
let wdk_sys_cargo_metadata = MetadataCommand::new().exec()?

@leon-xd leon-xd dismissed stale reviews from wmmc88 and hamzamust via b027e13 April 1, 2025 21:41
@leon-xd leon-xd requested review from Copilot, hamzamust and wmmc88 April 1, 2025 21:42
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR updates the rust-version and rust edition configuration for the wdk-default bindgen builder to support Rust 2024 features and enforce an updated MSRV.

  • Specifies the rust version in Cargo manifests and adjusts the cargo resolver version.
  • Updates dependencies (bindgen, cargo_metadata, semver, thiserror) to support new rust edition and nightly features.
  • Enhances error handling and configuration in the wdk-build crate, including improved target and edition determination.

Reviewed Changes

Copilot reviewed 15 out of 15 changed files in this pull request and generated no comments.

Show a summary per file
File Description
crates/wdk-sys/Cargo.toml Updated the nightly feature list to also include wdk-build/nightly.
crates/wdk-build/src/lib.rs Modified error variants and improved documentation for configuration errors.
crates/wdk-build/src/cargo_make.rs Adjusted target kind comparison to use the cargo_metadata enum rather than raw string literal.
crates/wdk-build/src/bindgen.rs Added functions for Rust target and edition retrieval with enhanced error handling.
crates/wdk-build/Cargo.toml Introduced rust-version and semver workspace settings along with a new nightly feature.
Cargo.toml Upgraded resolver and dependency versions to align with the new configuration settings.
Comments suppressed due to low confidence (2)

crates/wdk-build/src/cargo_make.rs:980

  • Ensure that the usage of cargo_metadata::TargetKind::CDyLib matches the expected target kind value, as this change from string comparison may lead to unexpected mismatches if the enum value differs from the previous literal.
.any(|target| target.kind.contains(&cargo_metadata::TargetKind::CDyLib))

crates/wdk-build/src/bindgen.rs:191

  • The documentation references ConfigError::SemverError, but the implemented variant is RustVersionParseError. Please update the documentation to reflect the correct error variant.
/// Returns `ConfigError::MsrvNotSupportedByBindgen` if the MSRV is not supported by bindgen, or `ConfigError::SemverError` if the MSRV cannot be parsed as a

hamzamust
hamzamust previously approved these changes Apr 1, 2025
wmmc88
wmmc88 previously approved these changes Apr 1, 2025
@leon-xd leon-xd dismissed stale reviews from wmmc88 and hamzamust via 3bb00b9 April 1, 2025 22:34
@leon-xd leon-xd requested review from hamzamust and wmmc88 April 1, 2025 23:14
@leon-xd leon-xd added this pull request to the merge queue Apr 1, 2025
Merged via the queue into microsoft:main with commit 0ae8259 Apr 2, 2025
62 checks passed
@leon-xd leon-xd deleted the bindgen-rust-version-edition branch April 2, 2025 00:13
svasista-ms pushed a commit to svasista-ms/windows-drivers-rs that referenced this pull request Apr 3, 2025
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