Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: skip infverif task for sample drivers built with certain GE WDK versions #143

Merged
merged 47 commits into from
May 31, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
47 commits
Select commit Hold shift + click to select a range
5cd6fc7
Initial stab at getting the WDK version into environment
NateD-MSFT Apr 23, 2024
6038348
Implement logic to select InfVerif flag. HAS BUG
NateD-MSFT Apr 26, 2024
67f3717
Introduce WDK_IS_SAMPLE flag to make simpler environment logic
NateD-MSFT Apr 26, 2024
1dc5701
Quick TODO to improve parsing
NateD-MSFT Apr 26, 2024
b261b0a
Refactor set_sample_infverif
NateD-MSFT Apr 26, 2024
db8da44
Cleanup and add test case.
NateD-MSFT Apr 26, 2024
f760c30
/samples, not /sample
NateD-MSFT Apr 26, 2024
b6e2951
Update to not run infverif for samples
NateD-MSFT Apr 26, 2024
15e99c7
Merge branch 'main' into vnext-wdk-support
NateD-MSFT Apr 26, 2024
ab732ab
Fix cargo format bug
NateD-MSFT Apr 26, 2024
e7c3263
Merge branch 'vnext-wdk-support' of https://github.com/NateD-MSFT/win…
NateD-MSFT Apr 26, 2024
dca2606
Cargo auto-fixes
NateD-MSFT Apr 27, 2024
963d12c
Clippy fixes
NateD-MSFT Apr 27, 2024
5d575f3
Undo accidental change to types.rs
NateD-MSFT Apr 27, 2024
bd557e5
Refactors to address PR feedback.
NateD-MSFT May 1, 2024
df43c89
Fix TOML formatting
NateD-MSFT May 1, 2024
43366e2
Merge branch 'main' into vnext-wdk-support
NateD-MSFT May 1, 2024
ef2eba6
Fix new clippy lint.
NateD-MSFT May 1, 2024
f3de079
Fix doc bug.
NateD-MSFT May 1, 2024
db51319
Fix a formatting issue that slipped in.
NateD-MSFT May 1, 2024
31b4bd1
Refactor `validate_wdk_version_format` into utils.rs
NateD-MSFT May 1, 2024
dcaeb14
Use a single collection instead of iterators
NateD-MSFT May 2, 2024
810e31b
Comment fix
NateD-MSFT May 2, 2024
2ce9838
Apply suggestions from code review
NateD-MSFT May 3, 2024
15c5458
Refactor and add a new enum value. Still has a bug
NateD-MSFT May 8, 2024
6a66900
Merge branch 'vnext-wdk-support' of https://github.com/NateD-MSFT/win…
NateD-MSFT May 8, 2024
0cbea24
This borrow actually is required here
NateD-MSFT May 8, 2024
75c09a2
Significant refactoring and renaming
NateD-MSFT May 9, 2024
754e1f9
BUGGY: Attempt to refactor samples logic into makefiles
NateD-MSFT May 9, 2024
cb6ce1d
TOML formatting fix
NateD-MSFT May 9, 2024
2ad53a5
Various typos and language fixes
NateD-MSFT May 9, 2024
fb129de
BUGGY: Add new plugin to work with cargo make paths
NateD-MSFT May 9, 2024
140208d
Merge branch 'vnext-wdk-support' of https://github.com/NateD-MSFT/win…
NateD-MSFT May 9, 2024
916fe8f
Fix an updated function name
NateD-MSFT May 9, 2024
504a7d5
Merge branch 'main' into vnext-wdk-support
NateD-MSFT May 9, 2024
337f363
Merge branch 'main' into vnext-wdk-support
NateD-MSFT May 9, 2024
bb5da2e
Update crates/wdk-build/rust-driver-sample-makefile.toml
NateD-MSFT May 13, 2024
a33c93d
Fix conditional-script behavior
NateD-MSFT May 16, 2024
f0cf0e4
Clippy/fmt fixes
NateD-MSFT May 16, 2024
71ad073
Merge branch 'vnext-wdk-support' of https://github.com/NateD-MSFT/win…
NateD-MSFT May 16, 2024
9871806
Merge branch 'main' into vnext-wdk-support
NateD-MSFT May 16, 2024
cb9b7d5
Remove pub marker on utils
NateD-MSFT May 16, 2024
d38bddf
Merge branch 'vnext-wdk-support' of https://github.com/NateD-MSFT/win…
NateD-MSFT May 16, 2024
52c9ce6
Add version field to WDKVersionStringFormatError
NateD-MSFT May 23, 2024
193d123
Merge branch 'main' into vnext-wdk-support
NateD-MSFT May 30, 2024
5f2ac57
Fix a missing reference in make script
NateD-MSFT May 30, 2024
8cb3311
A few more fixes to the make script
NateD-MSFT May 30, 2024
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 4 additions & 4 deletions crates/sample-kmdf-driver/Makefile.toml
Original file line number Diff line number Diff line change
@@ -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" },
]
1 change: 1 addition & 0 deletions crates/wdk-build/rust-driver-makefile.toml
Original file line number Diff line number Diff line change
Expand Up @@ -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]
Expand Down
78 changes: 78 additions & 0 deletions crates/wdk-build/rust-driver-sample-makefile.toml
Original file line number Diff line number Diff line change
@@ -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]
wmmc88 marked this conversation as resolved.
Show resolved Hide resolved
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
wmmc88 marked this conversation as resolved.
Show resolved Hide resolved
const MISSING_SAMPLE_FLAG_WDK_BUILD_NUMBER_RANGE: RangeFrom<u32> = 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");
wmmc88 marked this conversation as resolved.
Show resolved Hide resolved
let wdk_build_number = str::parse::<u32>(&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(())
})?
'''
172 changes: 165 additions & 7 deletions crates/wdk-build/src/cargo_make.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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::{
Expand All @@ -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
Expand Down Expand Up @@ -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<String, ConfigError> {
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<S: AsRef<str> + 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::<i32>().is_ok() in our call to
// validate_wdk_version_format above.
let version = validated_version_string
.parse::<i32>()
.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
///
Expand Down Expand Up @@ -553,9 +634,9 @@ pub fn copy_to_driver_package_folder<P: AsRef<Path>>(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
Expand All @@ -566,13 +647,61 @@ pub fn copy_to_driver_package_folder<P: AsRef<Path>>(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<S: AsRef<str> + AsRef<Utf8Path> + AsRef<Path>>(
makefile_name: S,
) -> Result<(), ConfigError> {
let cargo_metadata = MetadataCommand::new().exec()?;

let wdk_build_package_matches = cargo_metadata
Expand All @@ -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
Expand Down Expand Up @@ -706,3 +836,31 @@ fn forward_env_var_to_cargo_make<S: AsRef<str>>(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(())
}
}
12 changes: 11 additions & 1 deletion crates/wdk-build/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Expand Down Expand Up @@ -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),
Expand Down
Loading
Loading