fix: correctly deduce crate target directory

[yuvald-sweet-security]

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.

[yuvald-sweet-security]

@microsoft-github-policy-service agree

[gurry]

Thanks for the PR @yuvald-sweet-security. We will look into it soon.

[yuvald-sweet-security]

@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]

Thanks for submitting a PR! What is desired behaviour you're looking for here @yuvald-sweet-security?

This change wouldn't affect that fact that custom target directories for the actual builds are not supported and will probably not be supported for quite some time. See #252 (especailly the 2nd point).

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)

In this emulated workspace scenario, the existing behavior is desirable since it means that the makefile only needs to be copied to one place shared by all members of the emulated workspace.

Maybe I'm missing it, but I don't see the point of the change proposed here. Could you mock up some examples of something failing beforehand, and then passing after this change?

[yuvald-sweet-security]

@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]

@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.

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