feat(cargo-wdk): support target architecture override through config.toml and CARGO_BUILD_TARGET#494
Conversation
- Removed the TargetArch enum and simplified target architecture handling in the CLI. - Updated the command execution function to accept an optional working directory. - Enhanced error reporting for command failures, including status codes and stderr output. - Cleaned up the `run` method in the `WdkBuild` struct to prepare for future enhancements. - Improved test assertions for driver packaging tasks and added a utility function to determine target folders based on architecture. - Updated the Cargo configuration for the WDM driver test to specify the target architecture explicitly.
BuildAction to respect other forms of target overrides
There was a problem hiding this comment.
Pull Request Overview
This PR refactors the cargo-wdk build command to better respect Cargo's target architecture override mechanisms by removing the TargetArch enum and simplifying architecture handling. Instead of detecting architecture via rustc --print host-tuple, the code now uses cargo rustc -- --print cfg to detect the effective target architecture that Cargo will build for, respecting configuration settings from .cargo/config.toml or environment variables.
- Removed
TargetArchenum and related CLI architecture detection logic - Updated command execution to support optional working directories and enhanced error reporting
- Improved driver packaging task error messages and test utilities
Reviewed Changes
Copilot reviewed 12 out of 12 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
crates/cargo-wdk/tests/wdm-driver/.cargo/config.toml |
Added explicit target architecture configuration |
crates/cargo-wdk/tests/build_command_test.rs |
Updated test assertions and added utility for target folder detection |
crates/cargo-wdk/src/providers/wdk_build.rs |
Added placeholder for future WDK build number field |
crates/cargo-wdk/src/providers/mod.rs |
Enhanced command error reporting with status codes and stderr |
crates/cargo-wdk/src/providers/exec.rs |
Added working directory support and improved debug logging |
crates/cargo-wdk/src/cli.rs |
Removed TargetArch enum and simplified build command handling |
crates/cargo-wdk/src/actions/mod.rs |
Removed TargetArch enum definition |
crates/cargo-wdk/src/actions/build/tests.rs |
Updated test cases to use new architecture handling |
crates/cargo-wdk/src/actions/build/package_task.rs |
Changed driver model parameter to reference and minor formatting cleanup |
crates/cargo-wdk/src/actions/build/mod.rs |
Implemented new target architecture detection and package root path construction |
crates/cargo-wdk/src/actions/build/error.rs |
Added new error types for architecture detection |
crates/cargo-wdk/src/actions/build/build_task.rs |
Updated to use simplified architecture handling |
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
…ecture and artifact paths for the packaging process - Added tests for building KMDF drivers with explicit target architecture options. - Created a new test fixture for `kmdf-driver-with-target-override` to validate target override scenarios
There was a problem hiding this comment.
Pull Request Overview
Copilot reviewed 16 out of 17 changed files in this pull request and generated 3 comments.
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
…g logic - Added new error variants to `BuildActionError` and `BuildTaskError` for better error reporting. - Enhanced the `build_and_package` function to handle driver binary paths and target architecture more effectively. - Modified tests to reflect changes in the build process and ensure accurate artifact handling. - Renamed `Profile::Dev` to `Profile::Debug` for clarity and consistency.
There was a problem hiding this comment.
Pull Request Overview
Copilot reviewed 17 out of 18 changed files in this pull request and generated 6 comments.
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
There was a problem hiding this comment.
Pull Request Overview
Copilot reviewed 26 out of 38 changed files in this pull request and generated 3 comments.
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
There was a problem hiding this comment.
Pull Request Overview
Copilot reviewed 26 out of 38 changed files in this pull request and generated 3 comments.
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
There was a problem hiding this comment.
Pull Request Overview
Copilot reviewed 26 out of 38 changed files in this pull request and generated no new comments.
Comments suppressed due to low confidence (1)
crates/cargo-wdk/src/actions/mod.rs:39
- The profile parsing accepts 'debug' but Cargo's standard development profile is called 'dev'. This creates inconsistency with Cargo conventions.
fn from_str(s: &str) -> Result<Self, Self::Err> {
match s.to_lowercase().as_str() {
"debug" => std::result::Result::Ok(Self::Debug),
"release" => std::result::Result::Ok(Self::Release),
_ => Err(format!("'{s}' is not a valid profile")),
}
}
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 18 out of 19 changed files in this pull request and generated 1 comment.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
…n nuget_wdk_content_root_path function
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 18 out of 19 changed files in this pull request and generated 1 comment.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
… get target architecture
… get target architecture
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 18 out of 19 changed files in this pull request and generated no new comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
….toml` and `CARGO_BUILD_TARGET` (#494)
This PR includes changes to the
buildcommand to respect other forms of target configuration vis config.toml (build.target) and environment (CARGO_BUILD_TARGET) along with the--targetCLI option.Logical Changes
cargo buildalready supported the above forms of target specification. What we needed was to ensure packaging step uses the same target as build step and uses the correct output location of binaries.In this PR is we now run
cargo build --message-format=json(instead ofcargo build) and extract the output location of binaries from the resulting JSON. To get the target arch we runcargo rustc --printright aftercargo build --message-format=jsonand parse the output for thetarget_archfield. [1] [2] [3].Test Changes
Added
kmdf-driver-with-target-overridetest fixture incargo-wdk/testsand the cases incrates/cargo-wdk/tests/build_command_test.rs[4]CI Workflow Changes
To support the integration test cases that run
cargo wdk buildin a cross-compilation setting when using Nuget-based WDK, two additional environment variables have been added:FullVersionNumber: Denotes the complete version number (including QFE) of the WDK and SDK nuget packages that were downloaded [5]NugetPackagesRoot: The root directory where the nuget packages required to build a driver are downloaded [6]The variables are set in the
install-wdkaction and used inbuild_command_testwhere some form of target override is tested. UsingNugetPackagesRootandFullVersionNumbertheWDKContentRootenv var is set to point to the correct WDK content based on the target architecture.Closes #435
Closes #348
Screenshots:
If
build.targetis configured visconfig.tomlorCARGO_BUILD_TARGET,cargo wdk buildfails during the packaging phase: