From 9a02d5b761bea3b5f51c7f6e67989b30f21509f3 Mon Sep 17 00:00:00 2001 From: NateD-MSFT <34494373+NateD-MSFT@users.noreply.github.com> Date: Fri, 31 May 2024 13:15:02 -0500 Subject: [PATCH] fix: skip infverif task for sample drivers built with certain GE WDK versions (#143) Signed-off-by: NateD-MSFT <34494373+NateD-MSFT@users.noreply.github.com> Co-authored-by: Melvin Wang --- crates/sample-kmdf-driver/Makefile.toml | 8 +- crates/wdk-build/rust-driver-makefile.toml | 1 + .../rust-driver-sample-makefile.toml | 78 ++++++++ crates/wdk-build/src/cargo_make.rs | 172 +++++++++++++++++- crates/wdk-build/src/lib.rs | 12 +- crates/wdk-build/src/utils.rs | 160 ++++++++++++++++ 6 files changed, 419 insertions(+), 12 deletions(-) create mode 100644 crates/wdk-build/rust-driver-sample-makefile.toml diff --git a/crates/sample-kmdf-driver/Makefile.toml b/crates/sample-kmdf-driver/Makefile.toml index 64a2b957..f91ac160 100644 --- a/crates/sample-kmdf-driver/Makefile.toml +++ b/crates/sample-kmdf-driver/Makefile.toml @@ -1,4 +1,4 @@ -extend = "../wdk-build/rust-driver-makefile.toml" - -[env] -WDK_BUILD_ADDITIONAL_INFVERIF_FLAGS = "/msft" +extend = [ + { path = "../wdk-build/rust-driver-makefile.toml" }, + { path = "../wdk-build/rust-driver-sample-makefile.toml" }, +] diff --git a/crates/wdk-build/rust-driver-makefile.toml b/crates/wdk-build/rust-driver-makefile.toml index 253f7e26..d383f584 100644 --- a/crates/wdk-build/rust-driver-makefile.toml +++ b/crates/wdk-build/rust-driver-makefile.toml @@ -80,6 +80,7 @@ script = ''' wdk_build::cargo_make::validate_and_forward_args(); wdk_build::cargo_make::setup_path()?; +wdk_build::cargo_make::setup_wdk_version()?; ''' [tasks.copy-inx-to-output] diff --git a/crates/wdk-build/rust-driver-sample-makefile.toml b/crates/wdk-build/rust-driver-sample-makefile.toml new file mode 100644 index 00000000..8b5cf74c --- /dev/null +++ b/crates/wdk-build/rust-driver-sample-makefile.toml @@ -0,0 +1,78 @@ +# This file is used to extend the standard rust-driver-makefile to build official sample drivers. See examples at https://github.com/microsoft/Windows-rust-drivers-samples +# Using this file requires extending both the standard makefile and this makefile in order, as follows: +# extend = [ { path = "target/rust-driver-makefile.toml" }, { path = "target/rust-driver-sample-makefile.toml" } ] + +# This plugin replaces the embedded `CARGO_MAKE_CURRENT_TASK_INITIAL_MAKEFILE_DIRECTORY` in the rust-script doc comment with its evaluated value. See https://github.com/sagiegurari/cargo-make/issues/1081 for more context. +[plugins.impl.rust-condition-script-cargo_make_current_task_initial_makefile_directory-substitution] +script = ''' +# Parse task json into variables +taskjson = json_parse ${task.as_json} + +# Convert backslashes to forward slashes +rust-driver-toolchain-path = replace ${taskjson.env.CARGO_MAKE_CURRENT_TASK_INITIAL_MAKEFILE_DIRECTORY} "\\" "/" + +# Replace the ${CARGO_MAKE_CURRENT_TASK_INITIAL_MAKEFILE_DIRECTORY} variable in the condition_script with the rust-driver-toolchain-path +taskjson.condition_script = replace ${taskjson.condition_script} "\${CARGO_MAKE_CURRENT_TASK_INITIAL_MAKEFILE_DIRECTORY}" "${rust-driver-toolchain-path}" + +# Unset the private flag since it breaks cm_plugin_run_custom_task +unset taskjson.private + +# Reencode variables into json +taskjson = json_encode taskjson + +# Run the invoking task, with the modified condition_script +cm_plugin_run_custom_task ${taskjson} +''' + + +[tasks.wdk-samples-setup] +private = true +install_crate = { crate_name = "rust-script", min_version = "0.30.0" } +plugin = "rust-env-update" +script_runner = "@rust" +script = ''' +//! ```cargo +//! [dependencies] +//! wdk-build = { path = ".", version = "0.2.0" } +//! ``` +#![allow(unused_doc_comments)] +let env_string = std::env::var_os(wdk_build::cargo_make::WDK_VERSION_ENV_VAR) + .map_or_else( + || panic!("Couldn't read WDK build version that should have been set in init"), + |os_env_string| os_env_string.to_string_lossy().into_owned(), + ); +wdk_build::cargo_make::setup_infverif_for_samples(&env_string)?; +''' + +[tasks.infverif] +dependencies = ["wdk-samples-setup", "stampinf"] +plugin = "rust-condition-script-cargo_make_current_task_initial_makefile_directory-substitution" +condition_script = ''' +#!@rust + +//! ```cargo +//! [dependencies] +//! wdk-build = { path = "${CARGO_MAKE_CURRENT_TASK_INITIAL_MAKEFILE_DIRECTORY}", version = "0.2.0" } +//! ``` +#![allow(unused_doc_comments)] + +use core::ops::RangeFrom; + +// This range is inclusive of 25798. FIXME: update with range end after /sample flag is added to InfVerif CLI +const MISSING_SAMPLE_FLAG_WDK_BUILD_NUMBER_RANGE: RangeFrom = 25798..; + +std::panic::catch_unwind(|| { + let wdk_version = std::env::var(wdk_build::cargo_make::WDK_VERSION_ENV_VAR).expect("WDK_BUILD_DETECTED_VERSION should always be set by wdk-build-init cargo make task"); + let wdk_build_number = str::parse::(&wdk_build::utils::get_wdk_version_number(&wdk_version).unwrap()).expect(&format!("Couldn't parse WDK version number! Version number: {}", wdk_version)); + + if MISSING_SAMPLE_FLAG_WDK_BUILD_NUMBER_RANGE.contains(&wdk_build_number) { + // cargo_make will interpret returning an error from the rust-script condition_script as skipping the task + return Err::<(), String>(format!("Skipping InfVerif. InfVerif in WDK Build {wdk_build_number} is bugged and does not contain the /samples flag.").into()); + } + Ok(()) +}).unwrap_or_else(|_|{ + // panic contents would have already been printed to console + eprintln!("condition_script for infverif task panicked while executing. Defaulting to running inverif task."); + Ok(()) +})? +''' diff --git a/crates/wdk-build/src/cargo_make.rs b/crates/wdk-build/src/cargo_make.rs index c2846d5d..cd27282f 100644 --- a/crates/wdk-build/src/cargo_make.rs +++ b/crates/wdk-build/src/cargo_make.rs @@ -10,7 +10,7 @@ use std::path::{Path, PathBuf}; -use cargo_metadata::MetadataCommand; +use cargo_metadata::{camino::Utf8Path, MetadataCommand}; use clap::{Args, Parser}; use crate::{ @@ -19,7 +19,18 @@ use crate::{ ConfigError, }; +/// The filename of the main makefile for Rust Windows drivers. +pub const RUST_DRIVER_MAKEFILE_NAME: &str = "rust-driver-makefile.toml"; +/// The filename of the samples makefile for Rust Windows drivers. +pub const RUST_DRIVER_SAMPLE_MAKEFILE_NAME: &str = "rust-driver-sample-makefile.toml"; + const PATH_ENV_VAR: &str = "Path"; +/// The environment variable that [`setup_wdk_version`] stores the WDK version +/// in. +pub const WDK_VERSION_ENV_VAR: &str = "WDK_BUILD_DETECTED_VERSION"; +/// The first WDK version with the new `InfVerif` behavior. +const MINIMUM_SAMPLES_FLAG_WDK_VERSION: i32 = 25798; +const WDK_INF_ADDITIONAL_FLAGS_ENV_VAR: &str = "WDK_BUILD_ADDITIONAL_INFVERIF_FLAGS"; /// The name of the environment variable that cargo-make uses during `cargo /// build` and `cargo test` commands @@ -492,6 +503,76 @@ pub fn setup_path() -> Result<(), ConfigError> { Ok(()) } +/// Adds the WDK version to the environment in the full string form of +/// 10.xxx.yyy.zzz, where x, y, and z are numerical values. +/// +/// # Errors +/// +/// This function returns a [`ConfigError::WDKContentRootDetectionError`] if the +/// WDK content root directory could not be found, or if the WDK version is +/// ill-formed. +pub fn setup_wdk_version() -> Result { + let Some(wdk_content_root) = detect_wdk_content_root() else { + return Err(ConfigError::WDKContentRootDetectionError); + }; + let version = get_latest_windows_sdk_version(&wdk_content_root.join("Lib"))?; + + if let Ok(existing_version) = std::env::var(WDK_VERSION_ENV_VAR) { + if version == existing_version { + // Skip updating. This can happen in certain recursive + // cargo-make cases. + return Ok(version); + } + // We have a bad version string set somehow. Return an error. + return Err(ConfigError::WDKContentRootDetectionError); + } + + println!("FORWARDING ARGS TO CARGO-MAKE:"); + if !crate::utils::validate_wdk_version_format(&version) { + return Err(ConfigError::WDKVersionStringFormatError { version }); + } + + append_to_space_delimited_env_var(WDK_VERSION_ENV_VAR, &version); + forward_env_var_to_cargo_make(WDK_VERSION_ENV_VAR); + Ok(version) +} + +/// Sets the `WDK_INFVERIF_SAMPLE_FLAG` environment variable to contain the +/// appropriate flag for building samples. +/// +/// # Errors +/// +/// This function returns a [`ConfigError::WDKContentRootDetectionError`] if +/// an invalid WDK version is provided. +/// +/// # Panics +/// +/// This function will panic if the function for validating a WDK version string +/// is ever changed to no longer validate that each part of the version string +/// is an i32. +pub fn setup_infverif_for_samples + ToString + ?Sized>( + version: &S, +) -> Result<(), ConfigError> { + let validated_version_string = crate::utils::get_wdk_version_number(version)?; + // This print signifies the start of the forwarding and signals to the + // `rust-env-update` plugin that it should forward args. This is also used to + // signal that the auto-generated help from `clap` was not executed. + println!("FORWARDING ARGS TO CARGO-MAKE:"); + // Safe to unwrap as we called .parse::().is_ok() in our call to + // validate_wdk_version_format above. + let version = validated_version_string + .parse::() + .expect("Unable to parse the build number of the WDK version string as an int!"); + let sample_flag = if version > MINIMUM_SAMPLES_FLAG_WDK_VERSION { + "/samples" // Note: Not currently implemented, so in samples TOML we currently skip infverif + } else { + "/msft" + }; + append_to_space_delimited_env_var(WDK_INF_ADDITIONAL_FLAGS_ENV_VAR, sample_flag); + forward_env_var_to_cargo_make(WDK_INF_ADDITIONAL_FLAGS_ENV_VAR); + Ok(()) +} + /// Returns the path to the WDK build output directory for the current /// cargo-make flow /// @@ -553,9 +634,9 @@ pub fn copy_to_driver_package_folder>(path_to_copy: P) -> Result< Ok(()) } -/// Symlinks `rust-driver-toolchain.toml` to the `target` folder where it can be +/// Symlinks `rust-driver-makefile.toml` to the `target` folder where it can be /// extended from a `Makefile.toml`. This is necessary so that paths in the -/// `rust-driver-toolchain.toml` can to be relative to +/// `rust-driver-makefile.toml` can to be relative to /// `CARGO_MAKE_CURRENT_TASK_INITIAL_MAKEFILE_DIRECTORY` /// /// # Errors @@ -566,13 +647,61 @@ pub fn copy_to_driver_package_folder>(path_to_copy: P) -> Result< /// - [`ConfigError::MultipleWDKBuildCratesDetected`] if there are multiple /// versions of the WDK build crate detected /// - [`ConfigError::IoError`] if there is an error creating or updating the -/// symlink to `rust-driver-toolchain.toml` +/// symlink to `rust-driver-makefile.toml` /// /// # Panics /// /// This function will panic if the `CARGO_MAKE_WORKSPACE_WORKING_DIRECTORY` /// environment variable is not set pub fn load_rust_driver_makefile() -> Result<(), ConfigError> { + load_wdk_build_makefile(RUST_DRIVER_MAKEFILE_NAME) +} + +/// Symlinks `rust-driver-sample-makefile.toml` to the `target` folder where it +/// can be extended from a `Makefile.toml`. This is necessary so that paths in +/// the `rust-driver-sample-makefile.toml` can to be relative to +/// `CARGO_MAKE_CURRENT_TASK_INITIAL_MAKEFILE_DIRECTORY` +/// +/// # Errors +/// +/// This function returns: +/// - [`ConfigError::CargoMetadataError`] if there is an error executing or +/// parsing `cargo_metadata` +/// - [`ConfigError::MultipleWDKBuildCratesDetected`] if there are multiple +/// versions of the WDK build crate detected +/// - [`ConfigError::IoError`] if there is an error creating or updating the +/// symlink to `rust-driver-sample-makefile.toml` +/// +/// # Panics +/// +/// This function will panic if the `CARGO_MAKE_WORKSPACE_WORKING_DIRECTORY` +/// environment variable is not set +pub fn load_rust_driver_sample_makefile() -> Result<(), ConfigError> { + load_wdk_build_makefile(RUST_DRIVER_SAMPLE_MAKEFILE_NAME) +} + +/// Symlinks a [`wdk_build`] `cargo-make` makefile to the `target` folder where +/// it can be extended from a downstream `Makefile.toml`. This is necessary so +/// that paths in the [`wdk_build`] makefile can be relative to +/// `CARGO_MAKE_CURRENT_TASK_INITIAL_MAKEFILE_DIRECTORY` +/// +/// # Errors +/// +/// This function returns: +/// - [`ConfigError::CargoMetadataError`] if there is an error executing or +/// parsing `cargo_metadata` +/// - [`ConfigError::MultipleWDKBuildCratesDetected`] if there are multiple +/// versions of the WDK build crate detected +/// - [`ConfigError::IoError`] if there is an error creating or updating the +/// symlink to the makefile. +/// +/// # Panics +/// +/// This function will panic if the `CARGO_MAKE_WORKSPACE_WORKING_DIRECTORY` +/// environment variable is not set +fn load_wdk_build_makefile + AsRef + AsRef>( + makefile_name: S, +) -> Result<(), ConfigError> { let cargo_metadata = MetadataCommand::new().exec()?; let wdk_build_package_matches = cargo_metadata @@ -593,15 +722,16 @@ pub fn load_rust_driver_makefile() -> Result<(), ConfigError> { .manifest_path .parent() .expect("The parsed manifest_path should have a valid parent directory") - .join("rust-driver-makefile.toml"); + .join(&makefile_name); let cargo_make_workspace_working_directory = std::env::var(CARGO_MAKE_WORKSPACE_WORKING_DIRECTORY_ENV_VAR).unwrap_or_else(|_| { panic!("{CARGO_MAKE_WORKSPACE_WORKING_DIRECTORY_ENV_VAR} should be set by cargo-make.") }); - let destination_path = - Path::new(&cargo_make_workspace_working_directory).join("target/rust-driver-makefile.toml"); + let destination_path = Path::new(&cargo_make_workspace_working_directory) + .join("target") + .join(&makefile_name); // Only create a new symlink if the existing one is not already pointing to the // correct file @@ -706,3 +836,31 @@ fn forward_env_var_to_cargo_make>(env_var_name: S) { ); } } + +#[cfg(test)] +mod tests { + use crate::ConfigError; + + const WDK_TEST_OLD_INF_VERSION: &str = "10.0.22061.0"; + const WDK_TEST_NEW_INF_VERSION: &str = "10.0.26100.0"; + + #[test] + fn check_env_passing() -> Result<(), ConfigError> { + crate::cargo_make::setup_infverif_for_samples(WDK_TEST_OLD_INF_VERSION)?; + let env_string = std::env::var_os(crate::cargo_make::WDK_INF_ADDITIONAL_FLAGS_ENV_VAR) + .map_or_else( + || panic!("Couldn't get OS string"), + |os_env_string| os_env_string.to_string_lossy().into_owned(), + ); + assert_eq!(env_string.split(' ').last(), Some("/msft")); + + crate::cargo_make::setup_infverif_for_samples(WDK_TEST_NEW_INF_VERSION)?; + let env_string = std::env::var_os(crate::cargo_make::WDK_INF_ADDITIONAL_FLAGS_ENV_VAR) + .map_or_else( + || panic!("Couldn't get OS string"), + |os_env_string| os_env_string.to_string_lossy().into_owned(), + ); + assert_eq!(env_string.split(' ').last(), Some("/samples")); + Ok(()) + } +} diff --git a/crates/wdk-build/src/lib.rs b/crates/wdk-build/src/lib.rs index fcf89519..cb55083e 100644 --- a/crates/wdk-build/src/lib.rs +++ b/crates/wdk-build/src/lib.rs @@ -13,7 +13,9 @@ #![cfg_attr(nightly_toolchain, feature(assert_matches))] mod bindgen; -mod utils; +/// Module for utility code related to the cargo-make experience for building +/// drivers. +pub mod utils; pub mod cargo_make; @@ -119,6 +121,14 @@ pub enum ConfigError { )] WDKContentRootDetectionError, + /// Error returned when the WDK version string does not match the expected + /// format + #[error("The WDK version string provided ({version}) was not in a valid format.")] + WDKVersionStringFormatError { + /// The incorrect WDK version string. + version: String, + }, + /// Error returned when `cargo_metadata` execution or parsing fails #[error(transparent)] CargoMetadataError(#[from] cargo_metadata::Error), diff --git a/crates/wdk-build/src/utils.rs b/crates/wdk-build/src/utils.rs index accebc67..e2acbf97 100644 --- a/crates/wdk-build/src/utils.rs +++ b/crates/wdk-build/src/utils.rs @@ -26,14 +26,27 @@ use crate::{CPUArchitecture, ConfigError}; /// Errors that may occur when stripping the extended path prefix from a path #[derive(Debug, Error, PartialEq, Eq)] pub enum StripExtendedPathPrefixError { + /// Error raised when the provided path is empty. #[error("provided path is empty")] EmptyPath, + /// Error raised when the provided path has no extended path prefix to + /// strip. #[error("provided path has no extended path prefix to strip")] NoExtendedPathPrefix, } + +/// A trait for dealing with paths with extended-length prefixes. pub trait PathExt { + /// The kinds of errors that can be returned when trying to deal with an + /// extended path prefix. type Error; + /// Strips the extended length path prefix from a given path. + /// # Errors + /// + /// Returns an error defined by the implementer if unable to strip the + /// extended path length prefix. + fn strip_extended_length_path_prefix(&self) -> Result; } @@ -68,6 +81,7 @@ where /// Detect `WDKContentRoot` Directory. Logic is based off of Toolset.props in /// NI(22H2) WDK +#[must_use] pub fn detect_wdk_content_root() -> Option { // If WDKContentRoot is present in environment(ex. running in an eWDK prompt), // use it @@ -244,6 +258,15 @@ fn read_registry_key_string_value( /// Searches a directory and determines the latest windows SDK version in that /// directory +/// +/// # Errors +/// +/// Returns a `ConfigError::DirectoryNotFound` error if the directory provided +/// does not exist. +/// +/// # Panics +/// +/// Panics if the path provided is not valid Unicode. pub fn get_latest_windows_sdk_version(path_to_search: &Path) -> Result { Ok(path_to_search .read_dir()? @@ -272,6 +295,12 @@ pub fn get_latest_windows_sdk_version(path_to_search: &Path) -> Result CPUArchitecture { let target_arch = std::env::var("CARGO_CFG_TARGET_ARCH").expect( "Cargo should have set the CARGO_CFG_TARGET_ARCH environment variable when executing \ @@ -283,6 +312,62 @@ pub fn detect_cpu_architecture_in_build_script() -> CPUArchitecture { }) } +/// Validates that a given string matches the WDK version format (10.xxx.yyy.zzz +/// where xxx, yyy, and zzz are numeric and not necessarily 3 digits long). +pub fn validate_wdk_version_format>(version_string: S) -> bool { + let version = version_string.as_ref(); + let version_parts: Vec<&str> = version.split('.').collect(); + + // First, check if we have "10" as our first value + if !version_parts.first().is_some_and(|first| *first == "10") { + return false; + } + + // Now check that we have four entries. + if version_parts.len() != 4 { + return false; + } + + // Finally, confirm each part is numeric. + if !version_parts + .iter() + .all(|version_part| version_part.parse::().is_ok()) + { + return false; + } + + true +} + +/// Returns the version number from a full WDK version string. +/// +/// # Errors +/// +/// This function returns a [`ConfigError::WDKVersionStringFormatError`] if the +/// version string provided is ill-formed. +/// +/// # Panics +/// +/// If the WDK version format validation function is ever changed not to +/// validate that there are 4 substrings in the WDK version string, this +/// function will panic. +pub fn get_wdk_version_number + ToString + ?Sized>( + version_string: &S, +) -> Result { + if !validate_wdk_version_format(version_string) { + return Err(ConfigError::WDKVersionStringFormatError { + version: version_string.to_string(), + }); + } + + let version_substrings = version_string.as_ref().split('.').collect::>(); + let version_substring = version_substrings.get(2).expect( + "WDK version string was validated to be well-formatted, but we couldn't get the \ + appropriate substring!", + ); + Ok((*version_substring).to_string()) +} + #[cfg(test)] mod tests { use windows::Win32::UI::Shell::{FOLDERID_ProgramFiles, SHGetKnownFolderPath, KF_FLAG_DEFAULT}; @@ -345,4 +430,79 @@ mod tests { ) ); } + + #[test] + fn validate_wdk_strings() { + let test_string = "10.0.12345.0"; + assert_eq!( + get_wdk_version_number(test_string).ok(), + Some("12345".to_string()) + ); + let test_string = "10.0.5.0"; + assert_eq!( + get_wdk_version_number(test_string).ok(), + Some("5".to_string()) + ); + let test_string = "10.0.0.0"; + assert_eq!( + get_wdk_version_number(test_string).ok(), + Some("0".to_string()) + ); + let test_string = "11.0.0.0"; + assert_eq!( + format!("{}", get_wdk_version_number(test_string).err().unwrap()), + format!( + "The WDK version string provided ({}) was not in a valid format.", + test_string + ) + ); + let test_string = "10.0.12345.0.0"; + assert_eq!( + format!("{}", get_wdk_version_number(test_string).err().unwrap()), + format!( + "The WDK version string provided ({}) was not in a valid format.", + test_string + ) + ); + let test_string = "10.0.12345.a"; + assert_eq!( + format!("{}", get_wdk_version_number(test_string).err().unwrap()), + format!( + "The WDK version string provided ({}) was not in a valid format.", + test_string + ) + ); + let test_string = "10.0.12345"; + assert_eq!( + format!("{}", get_wdk_version_number(test_string).err().unwrap()), + format!( + "The WDK version string provided ({}) was not in a valid format.", + test_string + ) + ); + let test_string = "10.0.1234!5.0"; + assert_eq!( + format!("{}", get_wdk_version_number(test_string).err().unwrap()), + format!( + "The WDK version string provided ({}) was not in a valid format.", + test_string + ) + ); + let test_string = "Not a real version!"; + assert_eq!( + format!("{}", get_wdk_version_number(test_string).err().unwrap()), + format!( + "The WDK version string provided ({}) was not in a valid format.", + test_string + ) + ); + let test_string = ""; + assert_eq!( + format!("{}", get_wdk_version_number(test_string).err().unwrap()), + format!( + "The WDK version string provided ({}) was not in a valid format.", + test_string + ) + ); + } }