microsoft / mu_devops

Project Mu Developer Operations
https://microsoft.github.io/mu/
Other
27 stars 25 forks source link

.sync/Makefile.toml: Resolve cargo-tarpaulin breaking changes #249

Closed makubacki closed 1 year ago

makubacki commented 1 year ago

Tarpaulin 0.27 was released on September 17, 2023: https://github.com/xd009642/tarpaulin/releases/tag/0.27.0

The clap crate (Command Line Argument Parser) dependency within tarpaulin was upgraded in the 0.27 release from an old version (v2) to the latest major version (v4):

  Upgraded from clap v2 to v4. This has a few changes, notably any
  arguments which can be specified more than once require multiple
  entries so --run-types doc test needs to be turned into
  --run-types doc --run-types test

This means passing packages to a single -p parameter as a comma- separated list is no longer supported:

  [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)

Providing users the ability to pass packages separated by commas to cargo make coverage is convenient and assumed in our documentation and wrapper scripts.

This change retains the same user-facing interface to cargo make coverage while using a duckscript within the cargo makefile to transform the list to the format accepted by tarpaulin. Other commands are unchanged.

Duckscript is useful because it is readily embedded in cargo-make so no additional dependencies are required and it is cross-platform.


In addition, a minor fix is made by changing Html to html for the following issue:

  [cargo-make] Execute Command: "cargo" "tarpaulin" "--out" "Html"
    "--exclude-files" "**/tests/*" "--output-dir" "D:\\src\\mu_plus/target"
  error: invalid value 'Html' for '--out [<FMT>...]'
    [possible values: json, stdout, xml, html, lcov]

    tip: a similar value exists: 'html'
makubacki commented 1 year ago

Holding a bit for Joey to try a few options.

Main constraints are that we'd like to continue to accept a comma separated list as input and the @@split() function appears to mishandle comma separators.

Javagedes commented 1 year ago

I would consider adding individual-package-targets as a depenency for all commands for consistency's sake. It looks like they all support passing -p multiple times. If you do that, I would remove the initial -p from PACKAGE_TARGET ENV variable and let your script handle that.

makubacki commented 1 year ago

I would consider adding individual-package-targets as a depenency for all commands for consistency's sake. It looks like they all support passing -p multiple times. If you do that, I would remove the initial -p from PACKAGE_TARGET ENV variable and let your script handle that.

I think that is best too. I'll check it for each command and push an update.

makubacki commented 1 year ago

I would consider adding individual-package-targets as a depenency for all commands for consistency's sake. It looks like they all support passing -p multiple times. If you do that, I would remove the initial -p from PACKAGE_TARGET ENV variable and let your script handle that.

I think that is best too. I'll check it for each command and push an update.

Updated.