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
77 changes: 55 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,36 @@ 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");

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 +355,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 +436,7 @@ impl Config {
);
}
}

Ok(include_paths)
}

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

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

match &self.driver_config {
DriverConfig::Wdm => {
// Emit WDM-specific libraries to link to
Expand Down
223 changes: 223 additions & 0 deletions crates/wdk-build/src/resource_compile.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,223 @@
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_rc_file(include_paths: Vec<PathBuf>, rc_exe_root_path: String) {
// Initialize an empty vector to store modified include arguments
let mut include_args: 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
include_args.push("/I".to_string());
include_args.push(include_str.to_string());
} else {
println!("Non-Unicode path is not supported: {:?}", include_path);
}
}

let (company_name, copyright, product_name) = get_package_metadata_details();
let (product_version, description, file_version, name) = get_package_details();

get_and_set_rc_file(
company_name,
copyright,
product_name,
product_version,
description,
file_version,
name,
&include_args,
rc_exe_root_path,
);
}

// Function to get and set RC File with package metadata
fn get_and_set_rc_file(
company_name: String,
copyright: String,
product_name: String,
product_version: String,
description: String,
file_version: String,
name: String,
include_args: &Vec<String>,
rc_exe_root_path: 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 rc_file_path = "resources.rc";
if fs::metadata(&rc_file_path).is_ok() {
// File exists, so let's remove it
if let Err(err) = fs::remove_file(&rc_file_path) {
eprintln!("Error deleting file: {}", err);
} else {
println!("File deleted successfully!");
}
} else {
println!("File does not exist.");
}

let ver_file_type = "VFT_DRV";
let ver_file_subtype = "VFT2_DRV_SYSTEM";
let ver_original_filename = "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 {file_version},0
#define VER_FILEVERSION_STR "{product_version}.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 {company_name}
#endif

#undef VER_PRODUCTNAME_STR
#define VER_PRODUCTNAME_STR {product_name}

#include "common.ver""#,
file_type = ver_file_type,
file_subtype = ver_file_subtype,
original_filename = ver_original_filename
);

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(&include_args, rc_exe_root_path);
}

// Function to invoke RC.exe
fn invoke_rc(include_args: &Vec<String>, rc_exe_root_path: String) {
let resource_script = "resources.rc";
let rc_exe_path = format!("{}\\rc.exe", rc_exe_root_path);
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(include_args).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_package_metadata_details() -> (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 company_name = 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 copyright_name = 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 product_name = 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());

(company_name, copyright_name, product_name)
}

// Function to get package details
fn get_package_details() -> (String, String, String, String) {
let mut file_version = String::new();
let mut description = String::new();
let mut product_version = 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(text) => {
for line in text.lines() {
if line.starts_with("version") {
let start = line.find('"').unwrap_or(0) + 1;
let end = line.rfind('"').unwrap_or(0);
product_version = line[start..end].to_string();
let version_parts: Vec<&str> = product_version.split('.').collect();
file_version = version_parts.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");
}
}

(product_version, description, file_version, 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