fix: correctly deduce crate target directory
Assuming that the target directory is in ${CARGO_MAKE_WORKSPACE_WORKING_DIRECTORY}\target is incorrect for multiple reasons; first of all, because the target directory can be customized with cargo configuration, but also because CARGO_MAKE_WORKSPACE_WORKING_DIRECTORY doesn't necessarily point to the root of the cargo workspace, since cargo make has a feature called "emulated workspaces" where you build a workspace-like structure with cargo make that's built out of ordinary cargo crates (where each has it's own root and target directory), and then that variable points to the cargo make workspace (which is not a cargo workspace, and therefore has no target directory). Using the CARGO_MAKE_CRATE_TARGET_DIRECTORY variable is a better way to locate the current build's target directory.
@microsoft-github-policy-service agree
Thanks for the PR @yuvald-sweet-security. We will look into it soon.
@gurry no problem, also note that I ended up falling back to CARGO_MAKE_WORKSPACE_WORKING_DIRECTORY if CARGO_MAKE_CRATE_TARGET_DIRECTORY doesn't exist because it seems that cargo make doesn't always set CARGO_MAKE_CRATE_TARGET_DIRECTORY early enough (but weirdly enough, it does set that variable when the crates are built through emulated workspaces)
@wmmc88 maybe I wasn't clear; the current behavior with regards to emulated workspaces is simply broken - it doesn't build because it cannot deduce the correct target directory. For example, with this directory structure:
Makefile.toml:
[env]
# this tells cargo-make that this directory acts as a workspace root
CARGO_MAKE_WORKSPACE_EMULATION = true
# a list of crate members. since we do not have a Cargo.toml, we will need to specify this in here.
CARGO_MAKE_CRATE_WORKSPACE_MEMBERS = ["um", "km"]
um:
standard Rust binary project initialized with cargo new um from the root directory
km:
standard WDK project initialized with cargo wdk new --wdm km from the root directory
km/Makefile.toml:
extend = "target/rust-driver-makefile.toml"
[config]
load_script = '''
#!@rust
//! ```cargo
//! [dependencies]
//! wdk-build = "0.4.0"
//! ```
#![allow(unused_doc_comments)]
wdk_build::cargo_make::load_rust_driver_makefile()?
'''
Running cargo make from the root directory fails with this error:
[cargo-make][1] INFO - Build Done in 1.80 seconds.
[cargo-make][1] INFO - Execute Command: "rust-script" "target\\_cargo_make_temp\\persisted_scripts\\c6f46e81df625caf4dfbb235d91efefcff124b65a2b99ad0e7baff59898dd02a.rs"
Error: IoError(Os { code: 3, kind: NotFound, message: "The system cannot find the path specified." })
[cargo-make][1] ERROR - Unable to execute rust code.
[cargo-make][1] WARN - Build Failed.
[cargo-make] ERROR - Error while running duckscript: Source: Unknown Line: 8 - Error while executing command, exit code: 1
[cargo-make] WARN - Build Failed.
This is because CARGO_MAKE_WORKSPACE_WORKING_DIRECTORY is now set to the root directory, which has no target. However, even if we go and manually create a target directory at the root, we fail with the following error:
[cargo-make][1] INFO - Execute Command: "rust-script" "target\\_cargo_make_temp\\persisted_scripts\\c6f46e81df625caf4dfbb235d91efefcff124b65a2b99ad0e7baff59898dd02a.rs"
[cargo-make][1] ERROR - Descriptor file: ".\\target/rust-driver-makefile.toml" not found.
[cargo-make][1] WARN - Build Failed.
[cargo-make] ERROR - Error while running duckscript: Source: Unknown Line: 8 - Error while executing command, exit code: 1
[cargo-make] WARN - Build Failed.
This is because wdk-build placed the auto-generated makefile into target/rust-driver-makefile.toml but the Makefile.toml inside the WDK project requests extending target/rust-driver-makefile.toml relative to itself, i.e. km/target/rust-driver-makefile.toml which doesn't exist.
I don't think that sharing rust-driver-makefile.toml between emulated workspaces is a good idea, because they might be different projects or different drivers with different versions of that makefile and there's no prima facie reason to force them to share the same build logic. However, if you wish to take that approach, then we still somehow need to tell km/Makefile.toml to extend a makefile from the workspace root - setting it to ../target/rust-driver-makefile.toml breaks building the project without the workspace (i.e. running cargo make inside km) and it doesn't seem that the extend property allows evaluation of environment variables, and also create the target directory if it doesn't exist in load_wdk_build_makefile.
@wmmc88 maybe I wasn't clear; the current behavior with regards to emulated workspaces is simply broken - it doesn't build because it cannot deduce the correct target directory. For example, with this directory structure:
Makefile.toml:[env] # this tells cargo-make that this directory acts as a workspace root CARGO_MAKE_WORKSPACE_EMULATION = true # a list of crate members. since we do not have a Cargo.toml, we will need to specify this in here. CARGO_MAKE_CRATE_WORKSPACE_MEMBERS = ["um", "km"]
um: standard Rust binary project initialized withcargo new umfrom the root directory
km: standard WDK project initialized withcargo wdk new --wdm kmfrom the root directory
km/Makefile.toml:extend = "target/rust-driver-makefile.toml" [config] load_script = ''' #!@rust //! ```cargo //! [dependencies] //! wdk-build = "0.4.0" //! ``` #![allow(unused_doc_comments)] wdk_build::cargo_make::load_rust_driver_makefile()? '''Running
cargo makefrom the root directory fails with this error:[cargo-make][1] INFO - Build Done in 1.80 seconds. [cargo-make][1] INFO - Execute Command: "rust-script" "target\\_cargo_make_temp\\persisted_scripts\\c6f46e81df625caf4dfbb235d91efefcff124b65a2b99ad0e7baff59898dd02a.rs" Error: IoError(Os { code: 3, kind: NotFound, message: "The system cannot find the path specified." }) [cargo-make][1] ERROR - Unable to execute rust code. [cargo-make][1] WARN - Build Failed. [cargo-make] ERROR - Error while running duckscript: Source: Unknown Line: 8 - Error while executing command, exit code: 1 [cargo-make] WARN - Build Failed.This is because
CARGO_MAKE_WORKSPACE_WORKING_DIRECTORYis now set to the root directory, which has notarget. However, even if we go and manually create atargetdirectory at the root, we fail with the following error:[cargo-make][1] INFO - Execute Command: "rust-script" "target\\_cargo_make_temp\\persisted_scripts\\c6f46e81df625caf4dfbb235d91efefcff124b65a2b99ad0e7baff59898dd02a.rs" [cargo-make][1] ERROR - Descriptor file: ".\\target/rust-driver-makefile.toml" not found. [cargo-make][1] WARN - Build Failed. [cargo-make] ERROR - Error while running duckscript: Source: Unknown Line: 8 - Error while executing command, exit code: 1 [cargo-make] WARN - Build Failed.This is because wdk-build placed the auto-generated makefile into
target/rust-driver-makefile.tomlbut theMakefile.tomlinside the WDK project requests extendingtarget/rust-driver-makefile.tomlrelative to itself, i.e.km/target/rust-driver-makefile.tomlwhich doesn't exist.I don't think that sharing
rust-driver-makefile.tomlbetween emulated workspaces is a good idea, because they might be different projects or different drivers with different versions of that makefile and there's no prima facie reason to force them to share the same build logic. However, if you wish to take that approach, then we still somehow need to tellkm/Makefile.tomlto extend a makefile from the workspace root - setting it to../target/rust-driver-makefile.tomlbreaks building the project without the workspace (i.e. runningcargo makeinsidekm) and it doesn't seem that theextendproperty allows evaluation of environment variables, and also create thetargetdirectory if it doesn't exist inload_wdk_build_makefile.
thanks for the thorough explanation.
So this PR is not about support custom target directories, its about fixing loading the driver makefile for emulated cargo-make workspaces. Could you update PR title and description as such.
also note that I ended up falling back to CARGO_MAKE_WORKSPACE_WORKING_DIRECTORY if CARGO_MAKE_CRATE_TARGET_DIRECTORY doesn't exist because it seems that cargo make doesn't always set CARGO_MAKE_CRATE_TARGET_DIRECTORY
I'd like to understand in what situations CARGO_MAKE_CRATE_TARGET_DIRECTORY does not exist here. I would rather not need to have fallbacks in place when determining paths from cargo-make variables- it can make debugging issues more difficult