microsoft / mu_basecore

Project Mu BaseCore
https://microsoft.github.io/mu/
Other
242 stars 124 forks source link

[Bug]: Fail builds on types of cargo failures #575

Closed makubacki closed 1 year ago

makubacki commented 1 year ago

Is there an existing issue for this?

Current Behavior

We recently encountered an issue where cargo make errors did not get noticed for a while because the build continued.

For example:

  [cargo-make] Execute Command: "cargo" "tarpaulin" "-p" \
    "HelloWorldRustDxe,RustBootServicesAllocatorDxe"

  cargo_tarpaulin::config: Creating config
  cargo_tarpaulin: Running Tarpaulin
  cargo_tarpaulin: Building project
  cargo_tarpaulin::cargo: Cleaning project
  error: invalid character `,` in pkgid:
    `HelloWorldRustDxe,RustBootServicesAllocatorDxe`, characters must
    be Unicode XID characters (numbers, `-`, `_`, or most letters)
  cargo_tarpaulin: Cargo failed to run! Error: cargo run failed
  Error: "Cargo failed to run! Error: cargo run failed"
  [cargo-make] ERROR - Error while executing command, exit code: 1
  [cargo-make] WARN - Build Failed.

Expected Behavior

It would be helpful for the build to fail on errors with clear references to errors.

Steps To Reproduce

Review the MsCorePkg pipeline output as of commit dec8f57d3a5dc3f270f6b18940e99ba114d25f7a.

Build Environment

- Mu Plus at commit dec8f57d3a5dc3f270f6b18940e99ba114d25f7a
- Build agent details
  - https://github.com/actions/runner-images/blob/win22/20230918.1/images/win/Windows2022-Readme.md
  - https://github.com/actions/runner-images/releases/tag/win22%2F20230918.1

Version Information

- Rust 1.71.1
- Cargo-make 0.37.1
- Cargo-tarpaulin 0.27.0

Urgency

Medium

Are you going to fix this?

Someone else needs to fix it

Do you need maintainer feedback?

No maintainer feedback needed

Anything else?

No response

Javagedes commented 1 year ago

This is in regard to stuart_ci_build plugins. Specifically RustHostUnitTestPlugin that does not fail when the command itself fails due to an error unrelated to individual test failures.