Skip to content

feat: add linker directive for VHF lib + move static linker directives from wdk-build to wdk-sys#685

Open
Alan632 wants to merge 16 commits into
microsoft:mainfrom
Alan632:vhf_lib_linker_args-v2
Open

feat: add linker directive for VHF lib + move static linker directives from wdk-build to wdk-sys#685
Alan632 wants to merge 16 commits into
microsoft:mainfrom
Alan632:vhf_lib_linker_args-v2

Conversation

@Alan632

@Alan632 Alan632 commented Jun 18, 2026

Copy link
Copy Markdown
Contributor

Summary

Adds conditionally compiled linker directives for the VHF library tied to the "hid" feature.

Moves build script emitted static linker directives (cargo::rustc-link-lib=static=*) from wdk-build/src/lib.rs into bindgen generated files as conditionally compiled attributes. The Rust source insertion happens in wdk-sys/build.rs while the backend logic and Rust source string build lives in wdk-build/src/lib.rs (closely following the pattern the bindgen generated headers use). This allows conditional compilation of linker directives (and eventually link args) without having to rely on cross crate feature signaling, and guards against issues from version drift (in case multiple versions of wdk-build are used in one project).

This PR is a redesign of and supersedes PR!653.

Verification

Verified both kmdf and umdf drivers built with "hid" and Vhf functions linked against VhfKm.lib and VhfUm.lib respectively. Additionally, inspected each driver's linker .map file for evidence of the respective Vhf symbols.

Bindgen generated files are also inspected post build for the presence of the link attributes.

… directives from wdk-build/src/lib.rs into wdk-sys/build.rs
Copilot AI review requested due to automatic review settings June 18, 2026 18:48
@Alan632 Alan632 self-assigned this Jun 18, 2026

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

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 how native WDK libraries are linked by moving static rustc-link-lib directives out of wdk-build’s configure_binary_build prints and into wdk-sys’s generated bindings via #[link(...)] attributes. It also adds conditional linker directives for the VHF library when the hid feature is enabled, selecting VhfKm vs VhfUm based on driver model.

Changes:

  • Emit base driver-model-specific native library link directives into bindgen output (ntddk.rs/windows.rs) from wdk-sys/build.rs.
  • Add hid-gated #[link] directives for VhfKm (KMDF/WDM) and VhfUm (UMDF).
  • Remove the corresponding cargo::rustc-link-lib=static=* emissions from wdk-build::Config::configure_binary_build (leaving link-arg prints in place).

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 3 comments.

File Description
crates/wdk-sys/build.rs Adds a reusable #[link] directive helper and injects conditional link directives into generated bindings (including VHF for hid).
crates/wdk-build/src/lib.rs Stops emitting static rustc-link-lib lines (now handled by wdk-sys), retaining cdylib link-args and documenting future move.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread crates/wdk-sys/build.rs Outdated
Comment thread crates/wdk-build/src/lib.rs
Comment thread crates/wdk-sys/build.rs Outdated
@codecov-commenter

codecov-commenter commented Jun 18, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 80.61%. Comparing base (21ec8f8) to head (0b5deed).

Additional details and impacted files
@@            Coverage Diff             @@
##             main     #685      +/-   ##
==========================================
+ Coverage   79.93%   80.61%   +0.67%     
==========================================
  Files          26       26              
  Lines        5633     5829     +196     
  Branches     5633     5829     +196     
==========================================
+ Hits         4503     4699     +196     
  Misses       1002     1002              
  Partials      128      128              

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

Copilot AI review requested due to automatic review settings June 18, 2026 23:04

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 2 out of 2 changed files in this pull request and generated 3 comments.

Comment thread crates/wdk-build/src/lib.rs Outdated
Comment thread crates/wdk-build/src/lib.rs
Comment thread crates/wdk-build/src/lib.rs
Comment thread crates/wdk-build/src/lib.rs Outdated
Comment thread crates/wdk-build/src/lib.rs Outdated
Comment thread crates/wdk-sys/build.rs Outdated
.with_codegen_config((CodegenConfig::TYPES | CodegenConfig::VARS).complement())
.header_contents(&format!("{outfile_name}-input.h"), &header_contents);

add_link_directives(builder, &config.link_directives(ApiSubset::Base))

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

If you mirror the header_contents implementation, you would just have a bindgen_library_link_raw_lines (or similar name) to get the formatted rawlines and just pass that directly to raw_line. That would make it so that you don't need this free function and can continue to chain the builder construction like with header_contents

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

That bindgen_library_link_raw_lines would call a config.libraries(api_subsets) just the same as header_contents calls headers

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Redesigned the implementation to closely mirror the header_contents pattern.

Comment thread crates/wdk-build/src/lib.rs Outdated
Comment thread crates/wdk-build/src/lib.rs Outdated
Comment thread crates/wdk-build/src/lib.rs Outdated
Alan Ngo added 2 commits June 19, 2026 11:29
…utes out when building non-driver test wdk-sys-tests (wdk-sys-tests does not call configure_binary_build thus no search paths are emitted and ungated link attributes fail the build/test)
…on, library selection, and link directive string creation)

-removed raw strings in lieu of types to tightly constrain LinkDirective creation w/o needing assert checks
-removed and compacted unneeded functions from original implementation
Copilot AI review requested due to automatic review settings June 19, 2026 23:42

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

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

… attributes to ensure wdk-sys builds correctly when compiled for test

- add unit tests
- add PartialEq and Eq traits to LinkKind, LinkModifier, and LinkDirective
@Alan632 Alan632 marked this pull request as ready for review June 23, 2026 00:45
Copilot AI review requested due to automatic review settings June 23, 2026 00:45

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

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

@leon-xd leon-xd left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Great work. Few changes here and there but the overall shape is very solid!

Comment thread tests/wdk-sys-tests/Cargo.toml
Comment thread crates/wdk-build/src/lib.rs Outdated
Comment thread crates/wdk-build/src/lib.rs Outdated
Comment thread crates/wdk-build/src/lib.rs Outdated
Comment thread crates/wdk-build/src/lib.rs Outdated
Comment thread crates/wdk-build/src/lib.rs Outdated
Comment thread crates/wdk-build/src/lib.rs
Comment thread crates/wdk-build/src/lib.rs Outdated
Comment thread crates/wdk-build/src/lib.rs Outdated
Comment thread tests/wdk-sys-tests/Cargo.toml
Copilot AI review requested due to automatic review settings July 2, 2026 16:45

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 4 out of 4 changed files in this pull request and generated 2 comments.

}

#[test]
fn render_static_directive_includes_no_bundle_modifier() {
}

#[test]
fn base_directives_are_all_static_no_bundle() {
Copilot AI review requested due to automatic review settings July 2, 2026 16:51

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

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

Alan Ngo added 3 commits July 6, 2026 12:41
- add doctest to wdk-sys-tests
- add cdylib driver unit test to mixed-package-kmdf-workspace, and update Cargo.toml
Copilot AI review requested due to automatic review settings July 6, 2026 21:01

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 7 out of 7 changed files in this pull request and generated 4 comments.

//! Like the driver unit test in `tests/mixed-package-kmdf-workspace`, it
//! references a bindgen generated WDM type (`DRIVER_OBJECT`) via a const
//! `size_of` — so the doctest actually links `wdk-sys`'s generated bindings
//! while touching only a type (never a KM function
}

#[test]
fn render_static_directive_includes_no_bundle_modifier() {
}

#[test]
fn base_directives_are_all_static_no_bundle() {
//!
//! This is a sample KMDF driver that demonstrates how to use the crates in
//! windows-driver-rs to create a skeleton of a kmdf driver.
//!
/// [`ApiSubset`] rather than a set like
/// [`Config::bindgen_header_contents`].
/// Each generated bindings file requests only the native libraries
/// introduced by its own [`ApiSubset`]. This avoids duplicate `#[link]`

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

nit: i think we can delete this entire paragraph, the first sentence is pretty much saying the same thing as the paragraph above and not sure how necessary the second sentence is but i'm fine to keep it if you feel it's necessary

@leon-xd leon-xd left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

good changes

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.

5 participants