Skip to content

Commit efa6c11

Browse files
svasista-msCopilot
andauthored
fix(cargo-wdk): use --message-format=json-render-diagnostics in the build command to render compiler errors instead of the plain json option (#645)
Fixes #613 ## Problem When `cargo wdk build` encounters a compilation error, the error output is hard to debug: 1. JSON wall — `--message-format=json` sends even the compiler diagnostics as JSON to `stdout` instead of redirecting it to `stderr`. So, compiler errors are not directly visible instead they are buried deep in the output json. This is affecting test outputs because in case of failures, the errors cannot be seen easily in the `json` output. 2. Redundant error content — `BuildTaskError::CargoBuild` wraps the full `CommandError` (containing `stdout` JSON), even though the user may have already seen the real compiler errors on `stderr` in real time. ## Fixes The following changes fix the above problems: 1. Switch to `--message-format=json-render-diagnostics` This option instructs cargo to render diagnostics from `rustc` directly to `stderr` instead of putting it in the `json` output. stdout: only machine-parseable JSON (artifacts, build-finished) — no diagnostic messages stderr: human-readable compiler errors and warnings, rendered by Cargo (visible to the user in real time since stderr is inherited) 2. Omit `stdout` from `CommandError::CommandFailed` before returning from `BuildTask`: When cargo build fails, `BuildTask::run()` now maps the error to a new instance of `CommandFailed` but with the `stdout` field set to `String::new()` to avoid printing machine-readable (`json`) output on the screen. ## Before Running `kmdf_driver_cross_compiles_with_cli_option_successfully` test after removing `aarch64-pc-windows-msvc` target: <img width="947" height="572" alt="image" src="https://github.com/user-attachments/assets/efb391d3-02e8-47fd-907e-91ffe5c1d4ba" /> _____ Compiler diagnostics not printed to `stderr`, instead it is buried in the json output when `cargo wdk build` fails: <img width="1547" height="822" alt="image" src="https://github.com/user-attachments/assets/4a0cf5c0-a9d4-4a8c-be63-c1237d6ed835" /> _____ Output seen when tests fail due to compilation errors: <img width="1936" height="1162" alt="image" src="https://github.com/user-attachments/assets/ac4a5e53-5a95-483d-898c-0e751e790c36" /> ## After Running `kmdf_driver_cross_compiles_with_cli_option_successfully` test after removing `aarch64-pc-windows-msvc` target: <img width="1344" height="824" alt="image" src="https://github.com/user-attachments/assets/f95f94a9-9076-46e3-a706-c07c7ca95b65" /> _____ Compiler diagnostics available on terminal: <img width="1028" height="576" alt="image" src="https://github.com/user-attachments/assets/39d6ba3f-5b6f-4fdb-bbb9-0b39c21874be" /> _____ Output seen when tests fail due to compilation errors: <img width="1867" height="1231" alt="image" src="https://github.com/user-attachments/assets/b282efcd-c748-4129-b2c1-4f5c6b297900" /> --------- Co-authored-by: Copilot <copilot@github.com>
1 parent 5748ea2 commit efa6c11

3 files changed

Lines changed: 68 additions & 22 deletions

File tree

crates/cargo-wdk/src/actions/build/build_task.rs

Lines changed: 66 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,7 @@ use wdk_build::CpuArchitecture;
1717
use crate::providers::exec::CommandExec;
1818
use crate::{
1919
actions::{Profile, build::error::BuildTaskError, to_target_triple},
20+
providers::error::CommandError,
2021
trace,
2122
};
2223

@@ -91,7 +92,7 @@ impl<'a> BuildTask<'a> {
9192
) -> Result<impl Iterator<Item = Result<Message, std::io::Error>>, BuildTaskError> {
9293
debug!("Running cargo build");
9394
let mut args = vec!["build".to_string()];
94-
args.push("--message-format=json".to_string());
95+
args.push("--message-format=json-render-diagnostics".to_string());
9596
args.push("-p".to_string());
9697
args.push(self.package_name.to_string());
9798
if let Some(path) = self.manifest_path.to_str() {
@@ -120,7 +121,16 @@ impl<'a> BuildTask<'a> {
120121
// is respected
121122
let output = self
122123
.command_exec
123-
.run("cargo", &args, None, Some(self.working_dir))?;
124+
.run("cargo", &args, None, Some(self.working_dir))
125+
.map_err(|mut err| {
126+
// Drop stdout from CommandFailed so the noisy
127+
// --message-format=json-render-diagnostics output isn't bubbled up
128+
// in the wrapped error.
129+
if let CommandError::CommandFailed { stdout, .. } = &mut err {
130+
stdout.clear();
131+
}
132+
BuildTaskError::CargoBuild(err)
133+
})?;
124134

125135
debug!("cargo build done");
126136
Ok(Message::parse_stream(std::io::Cursor::new(output.stdout)))
@@ -205,7 +215,7 @@ mod tests {
205215
let verbosity = clap_verbosity_flag::Verbosity::default();
206216
let expected_args = vec![
207217
"build".to_string(),
208-
"--message-format=json".to_string(),
218+
"--message-format=json-render-diagnostics".to_string(),
209219
"-p".to_string(),
210220
"my-driver".to_string(),
211221
"--manifest-path".to_string(),
@@ -266,7 +276,7 @@ mod tests {
266276
}
267277

268278
#[test]
269-
fn run_returns_error_when_cargo_command_fails() {
279+
fn run_returns_command_failed_error_with_empty_stdout_when_cargo_build_exits_nonzero() {
270280
let working_dir = PathBuf::from("C:/abs/driver");
271281
let mut mock = MockCommandExec::new();
272282
mock.expect_run().return_once(|_, _, _, _| {
@@ -292,22 +302,58 @@ mod tests {
292302
);
293303

294304
let err = task.run().err().expect("expected cargo failure");
295-
let BuildTaskError::CargoBuild(command_error) = err else {
296-
panic!("expected cargo build error");
305+
let BuildTaskError::CargoBuild(CommandError::CommandFailed {
306+
command,
307+
args,
308+
stdout,
309+
}) = err
310+
else {
311+
panic!("expected CargoBuild(CommandFailed) error, got: {err:?}");
297312
};
298-
match command_error {
299-
CommandError::CommandFailed {
300-
command,
301-
args,
302-
stdout,
303-
} => {
304-
assert_eq!(command, "cargo");
305-
assert_eq!(args, vec!["build".to_string()]);
306-
assert_eq!(stdout, "error");
307-
}
308-
CommandError::IoError(_, _, err) => {
309-
panic!("expected CommandFailed, got IoError: {err}")
310-
}
311-
}
313+
assert_eq!(command, "cargo");
314+
assert!(
315+
args.contains(&"build".to_string()),
316+
"expected args to contain 'build', got: {args:?}"
317+
);
318+
assert!(
319+
stdout.is_empty(),
320+
"expected stdout to be omitted, got: {stdout}"
321+
);
322+
}
323+
324+
#[test]
325+
fn run_returns_io_error_when_cargo_build_command_invocation_fails() {
326+
let working_dir = PathBuf::from("C:/abs/driver");
327+
let mut mock = MockCommandExec::new();
328+
mock.expect_run().return_once(|_, _, _, _| {
329+
Err(CommandError::from_io_error(
330+
"cargo",
331+
&["build"],
332+
std::io::Error::new(std::io::ErrorKind::NotFound, "program not found"),
333+
))
334+
});
335+
336+
let task = BuildTask::new(
337+
"my-driver",
338+
&working_dir,
339+
None,
340+
None,
341+
clap_verbosity_flag::Verbosity::default(),
342+
&mock,
343+
);
344+
345+
let err = task
346+
.run()
347+
.err()
348+
.expect("expected command invocation failure");
349+
let BuildTaskError::CargoBuild(CommandError::IoError(command, args, io_err)) = err else {
350+
panic!("expected CargoBuild(IoError) error, got: {err:?}");
351+
};
352+
assert_eq!(command, "cargo");
353+
assert!(
354+
args.contains(&"build".to_string()),
355+
"expected args to contain 'build', got: {args:?}"
356+
);
357+
assert_eq!(io_err.kind(), std::io::ErrorKind::NotFound);
312358
}
313359
}

crates/cargo-wdk/src/actions/build/error.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -49,7 +49,7 @@ pub enum BuildTaskError {
4949
#[error("Empty manifest path found error")]
5050
EmptyManifestPath,
5151
#[error("Error running cargo build command")]
52-
CargoBuild(#[from] CommandError),
52+
CargoBuild(#[source] CommandError),
5353
#[error(transparent)]
5454
FileIo(#[from] FileError),
5555
}

crates/cargo-wdk/src/actions/build/tests.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1935,7 +1935,7 @@ impl TestBuildAction {
19351935
.to_string();
19361936
let mut expected_cargo_build_args: Vec<String> = vec![
19371937
"build",
1938-
"--message-format=json",
1938+
"--message-format=json-render-diagnostics",
19391939
"-p",
19401940
&driver_name,
19411941
"--manifest-path",

0 commit comments

Comments
 (0)