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

feat: embed version and package metadata in binary #243

Draft
wants to merge 18 commits into
base: main
Choose a base branch
from
Draft
2 changes: 2 additions & 0 deletions crates/wdk-build/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,8 @@ categories = ["development-tools::build-utils", "development-tools::ffi"]

[build-dependencies]
rustversion.workspace = true
serde = "1.0"
serde_json = "1.0"
Comment on lines +14 to +15
Copy link
Collaborator

Choose a reason for hiding this comment

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

These should be workspace dependencies


[dependencies]
anyhow.workspace = true
Expand Down
80 changes: 58 additions & 22 deletions crates/wdk-build/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,8 @@ mod utils;

mod bindgen;

mod resource_compile;

use std::{env, path::PathBuf};

use cargo_metadata::MetadataCommand;
Expand Down Expand Up @@ -313,6 +315,39 @@ impl Config {
Ok(())
}

/// Returns Resource Compile path to build and link based off of
/// the architecture platform
///
/// # Errors
///
/// This function will return an error if any of the required paths do not
/// exist.
pub fn get_rc_root_path(&self) -> Result<String, ConfigError> {
Copy link
Collaborator

Choose a reason for hiding this comment

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

This looks like the same logic as in: https://github.com/CHMANG/windows-drivers-rs/blob/37f74cae7207571347706806121088917f5040ba/crates/wdk-build/src/cargo_make.rs#L516-L517

Can this logic be commonized to a single place in utils as a free function that takes in a wdk content root path? that way it can be called in both setup_path and in this function.

also can this Config method be named get_bin_path since all the arch-specific bins are here.

let bin_directory = self.wdk_content_root.join("bin");

// Add Windows SDK library paths based on logic from WindowsDriver.KernelMode.props &
Copy link
Collaborator

Choose a reason for hiding this comment

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

This comment is copy-pasted. please clean up so that it makes sense

// WindowsDriver.UserMode.props in
// NI(22H2) WDK
let sdk_version = utils::get_latest_windows_sdk_version(bin_directory.as_path())?;
let windows_sdk_rc_path = bin_directory
.join(sdk_version)
.join(self.cpu_architecture.as_windows_str());

if !windows_sdk_rc_path.is_dir() {
return Err(ConfigError::DirectoryNotFound {
directory: windows_sdk_rc_path.to_string_lossy().into(),
});
}

let rc_path = windows_sdk_rc_path
.canonicalize()?
.strip_extended_length_path_prefix()?
.to_string_lossy()
.into_owned();

Ok(rc_path)
}

/// Return header include paths required to build and link based off of the
/// configuration of `Config`
///
Expand All @@ -323,54 +358,51 @@ impl Config {
pub fn get_include_paths(&self) -> Result<Vec<PathBuf>, ConfigError> {
// FIXME: consider deprecating in favor of iter
let mut include_paths = vec![];

let include_directory = self.wdk_content_root.join("Include");

// Add windows sdk include paths
// Based off of logic from WindowsDriver.KernelMode.props &
// WindowsDriver.UserMode.props in NI(22H2) WDK
let sdk_version = utils::get_latest_windows_sdk_version(include_directory.as_path())?;
let windows_sdk_include_path = include_directory.join(sdk_version);

let crt_include_path = windows_sdk_include_path.join("km/crt");
if !crt_include_path.is_dir() {
let km_include_path = windows_sdk_include_path.join("km");
if !km_include_path.is_dir() {
return Err(ConfigError::DirectoryNotFound {
directory: crt_include_path.to_string_lossy().into(),
directory: km_include_path.to_string_lossy().into(),
});
}
include_paths.push(
crt_include_path
km_include_path
.canonicalize()?
.strip_extended_length_path_prefix()?,
);

let km_or_um_include_path = windows_sdk_include_path.join(match self.driver_config {
DriverConfig::Wdm | DriverConfig::Kmdf(_) => "km",
DriverConfig::Umdf(_) => "um",
});
if !km_or_um_include_path.is_dir() {
let kit_shared_include_path = windows_sdk_include_path.join("shared");
if !kit_shared_include_path.is_dir() {
return Err(ConfigError::DirectoryNotFound {
directory: km_or_um_include_path.to_string_lossy().into(),
directory: kit_shared_include_path.to_string_lossy().into(),
});
}
include_paths.push(
km_or_um_include_path
kit_shared_include_path
.canonicalize()?
.strip_extended_length_path_prefix()?,
);

let kit_shared_include_path = windows_sdk_include_path.join("shared");
if !kit_shared_include_path.is_dir() {
let um_include_path = windows_sdk_include_path.join("um");
if !um_include_path.is_dir() {
return Err(ConfigError::DirectoryNotFound {
directory: kit_shared_include_path.to_string_lossy().into(),
directory: um_include_path.to_string_lossy().into(),
});
}
include_paths.push(
kit_shared_include_path
um_include_path
.canonicalize()?
.strip_extended_length_path_prefix()?,
);

Comment on lines +367 to +402
Copy link
Collaborator

Choose a reason for hiding this comment

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

Why are you changing the include path logic? This changes how all the bindings are generated and break it.

The logic here strictly matches what happens in the vcxproj files of the WDK. If RC compilation requires different includes, it should be handled elsewhere

// Add other driver type-specific include paths
match &self.driver_config {
DriverConfig::Wdm => {}
Expand Down Expand Up @@ -407,7 +439,7 @@ impl Config {
);
}
}

Ok(include_paths)
}

Expand Down Expand Up @@ -689,7 +721,11 @@ impl Config {
for path in library_paths {
println!("cargo::rustc-link-search={}", path.display());
}

resource_compile::generate_and_compile_rcfile(
self.get_include_paths()?,
self.get_rc_root_path()?,
);

match &self.driver_config {
DriverConfig::Wdm => {
// Emit WDM-specific libraries to link to
Expand Down
217 changes: 217 additions & 0 deletions crates/wdk-build/src/resource_compile.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,217 @@
use std::{
env,
fs,
path::{Path, PathBuf},
process::Command,
};
use cargo_metadata::MetadataCommand;
Copy link
Collaborator

Choose a reason for hiding this comment

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

I think this file should probably be restructured such that it is some ResourceCompile builder, rather than a set of free functions. I think the process of compiling the rc file should be separated from the process of writing the rc file from a template to filesystem. This way, we could reuse it for different rc files, if needed


// function to generate and compile RC file
pub fn generate_and_compile_rcfile(include_paths: Vec<PathBuf>, rc_exe_rootpath: String) {
// Initialize an empty vector to store modified include arguments
let mut includeargs: Vec<String> = Vec::new();

// Iterate over each include path
for include_path in include_paths {
// Convert the include path to a string
if let Some(include_str) = include_path.to_str() {
// Append "/I" and the include path to the modified vector
includeargs.push("/I".to_string());
includeargs.push(include_str.to_string());
} else {
println!("Non-Unicode path is not supported: {:?}", include_path);
}
}

let (companyname, copyright, productname) = get_packagemetadatadetails();
let (productversion, description, fileversion, name) = get_packagedetails();
Copy link
Collaborator

Choose a reason for hiding this comment

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

Rather than use tuples, please use purpose-built structs for type safety

getandset_rcfile(
companyname,
copyright,
productname,
productversion,
description,
fileversion,
name,
&includeargs,
rc_exe_rootpath,
);
}
// function to get and set RC File with package metadata
fn getandset_rcfile(
Copy link
Collaborator

Choose a reason for hiding this comment

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

please follow rust conventions for function names

companyname: String,
copyright: String,
productname: String,
productversion:String,
description:String,
fileversion:String,
name:String,
includeargs: &Vec<String>,
rc_exe_rootpath:String,
) {
println!("Set and create rc file... ");
Copy link
Collaborator

Choose a reason for hiding this comment

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

rather than using println and eprintln, please using tracing crate's macros for info, error, warn, etc.

let rcfile_path = "resources.rc";
if fs::metadata(&rcfile_path).is_ok() {
// File exists, so let's remove it
if let Err(err) = fs::remove_file(&rcfile_path) {
eprintln!("Error deleting file: {}", err);
} else {
println!("File deleted successfully!");
}
} else {
println!("File does not exist.");
}

let ver_filetype = "VFT_DRV";
let ver_filesubtype = "VFT2_DRV_SYSTEM";
let ver_originalfilename = "VER_INTERNALNAME_STR";

// Create the RC file content
let rc_content = format!(
r#"#include <windows.h>
#include <ntverp.h>
Copy link
Collaborator

Choose a reason for hiding this comment

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

Is there an MSDN doc that this template is pulled from? Are these all required? optional?

#define VER_FILETYPE {file_type}
#define VER_FILESUBTYPE {file_subtype}
#define VER_INTERNALNAME_STR "{name}"
#define VER_ORIGINALFILENAME_STR {original_filename}

#undef VER_FILEDESCRIPTION_STR
#define VER_FILEDESCRIPTION_STR "{description}"

#undef VER_PRODUCTNAME_STR
#define VER_PRODUCTNAME_STR VER_FILEDESCRIPTION_STR

#define VER_FILEVERSION {fileversion},0
#define VER_FILEVERSION_STR "{productversion}.0"

#undef VER_PRODUCTVERSION
#define VER_PRODUCTVERSION VER_FILEVERSION

#undef VER_PRODUCTVERSION_STR
#define VER_PRODUCTVERSION_STR VER_FILEVERSION_STR

#define VER_LEGALCOPYRIGHT_STR {copyright}
#ifdef VER_COMPANYNAME_STR

#undef VER_COMPANYNAME_STR
#define VER_COMPANYNAME_STR {companyname}
#endif

#undef VER_PRODUCTNAME_STR
#define VER_PRODUCTNAME_STR {productname}

#include "common.ver""#,
file_type = ver_filetype,
file_subtype = ver_filesubtype,
original_filename = ver_originalfilename
);

std::fs::write("resources.rc", rc_content).expect("Unable to write RC file");
Copy link
Collaborator

Choose a reason for hiding this comment

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

error handling is needed in this module rather than panicking

invoke_rc(&includeargs, rc_exe_rootpath);
}

// function to invoke RC.exe
fn invoke_rc(includeargs: &Vec<String>, rc_exe_rootpath: String) {

let resource_script = "resources.rc";
let rc_exe_path = format!("{}\\rc.exe", rc_exe_rootpath);
let rc_exe_path = Path::new(&rc_exe_path);
if !rc_exe_path.exists() {
eprintln!(
"Error: rc.exe path does not exist : {}",
rc_exe_path.display()
);
std::process::exit(1); // Exit with a non-zero status code
}

let mut command = Command::new(rc_exe_path);
command.args(includeargs).arg(resource_script);
println!("Command executed: {:?}", command);

let status = command.status();

match status {
Ok(exit_status) => {
if exit_status.success() {
println!("Resource compilation successful!");
println!("cargo:rustc-link-arg=resources.res");
} else {
println!("Resource compilation failed.");
std::process::exit(1); // Exit with a non-zero status code
}
}
Err(err) => {
eprintln!("Error running rc.exe: {}", err);
std::process::exit(1); // Exit with a non-zero status code
}
}
}
// function to get package metadata details
fn get_packagemetadatadetails() -> (String, String, String) {
// Run the 'cargo metadata' command and capture its output
let path = env::var("CARGO_MANIFEST_DIR").unwrap();
let meta = MetadataCommand::new()
Copy link
Collaborator

Choose a reason for hiding this comment

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

Rather than do additional parsing here, do you think it would be better to bake this into the WDK config struct @ayodejiige ?

.manifest_path("./Cargo.toml")
.current_dir(&path)
.exec()
.unwrap();
let root = meta.root_package().unwrap();
let metadata = &root.metadata;

// Extract metadata values with default fallbacks
let companyname = metadata
.get("wdk")
.and_then(|wdk| wdk.get("driver-model"))
.and_then(|driver_model| driver_model.get("companyname"))
.map(|s| s.to_string())
.unwrap_or_else(|| "Company name not found in metadata".to_string());
let copyrightname = metadata
.get("wdk")
.and_then(|wdk| wdk.get("driver-model"))
.and_then(|driver_model| driver_model.get("copyright"))
.map(|s| s.to_string())
.unwrap_or_else(|| "Copyright name not found in metadata".to_string());
let productname = metadata
.get("wdk")
.and_then(|wdk| wdk.get("driver-model"))
.and_then(|driver_model| driver_model.get("productname"))
.map(|s| s.to_string())
.unwrap_or_else(|| "Product name not found in metadata".to_string());

(companyname, copyrightname, productname)
}
// function to get package details
fn get_packagedetails() -> (String, String, String, String) {
let mut fileversion = String::new();
let mut description = String::new();
let mut productversion = String::new();
let mut name = String::new();

match fs::read_to_string("Cargo.toml") {
Copy link
Collaborator

Choose a reason for hiding this comment

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

This is very hacky to get the crate version. You should use the env vars that cargo provides you: https://doc.rust-lang.org/cargo/reference/environment-variables.html#environment-variables-cargo-sets-for-crates

Ok(text1) => {
for line in text1.lines() {
if line.starts_with("version") {
let start = line.find('"').unwrap_or(0) + 1;
let end = line.rfind('"').unwrap_or(0);
productversion = line[start..end].to_string();
let versionparts: Vec<&str> = productversion.split('.').collect();
fileversion = versionparts.join(",");
}
if line.starts_with("description") {
let start = line.find('"').unwrap_or(0) + 1;
let end = line.rfind('"').unwrap_or(0);
description = line[start..end].to_string();
}
if line.starts_with("name") {
let start = line.find('"').unwrap_or(0) + 1;
let end = line.rfind('"').unwrap_or(0);
name = line[start..end].to_string();
}
}
}
Err(_) => {
eprintln!("Error reading Cargo.toml");
}
}
(productversion, description, fileversion, name)
}
3 changes: 3 additions & 0 deletions examples/sample-wdm-driver/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,9 @@ publish = false

[package.metadata.wdk.driver-model]
driver-type = "WDM"
companyname = "Microsoft Corporation"
copyright = "(C) 2024 Microsoft. All rights reserved."
productname = "Surface"
Comment on lines +15 to +17
Copy link
Collaborator

Choose a reason for hiding this comment

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

These fields should be using kebab case per toml conventions

Copy link
Collaborator

Choose a reason for hiding this comment

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

They also should not be a part of the driver-model metadata section. The driver-model section contains things in the driver model tab of visual studio. I think you probably want a separate metadata section for this? What do you think @ayodejiige


[lib]
crate-type = ["cdylib"]
Expand Down
Loading