cargo-hack icon indicating copy to clipboard operation
cargo-hack copied to clipboard

Allow specifying the target to run the test with

Open MOZGIII opened this issue 3 years ago • 5 comments

Some context first: we are building a blockchain on substrate, and have a monorepo with a bunch of crates. Some crates are intended to be built in both std and no_std modes, but the issue is that when building in no_std mode, wasm target is assumed. Crates have an std flag, which enables std mode in dependencies, and when it's absent - the dependent crates also disable std mode (and switch to assuming wasm). The problem is we can't test no_std mode with cargo hack.

It would be great if we could use a presence of a certain feature in the features set under test to use a different compile target (in our case - wasm32-unknown-unknown).

An example.

Let us have a crate with features std = ["dep1/std", "dep2/std"], feat1 = ["std", "codec"], feat2 = ["some-non-std-feature"].

We'd like to test in the following way:

  • default-features = false features: [std] target: native (since std is explicitly enabled)
  • default-features = false features: [feat1] target: native (since std is implicitly enabled via feat1)
  • default-features = false features: [feat2] target: WASM (since std is effectively not enabled)
  • default-features = false features: [] target: WASM (since std is effectively not enabled)

We could specify all this manually somehow if we had just one or two crates, but we have way too much of them, and the logic to determine the target is so simple, yet tricky, so I thought it would be a good fit for cargo-hack. I'm sure if cargo-hack offers this functionality other projects will pick it up as well.

MOZGIII avatar Nov 15 '21 09:11 MOZGIII

Do you mean you need an option to process the following two lines in one line?

cargo hack check --each-feature
cargo hack check --each-feature --skip std --target wasm32-unknown-unknown

taiki-e avatar Nov 15 '21 13:11 taiki-e

Those two lines are not going to cut it. What they don't handle is:

  1. We don't need all crates to be tested under wasm32-unknown-unknown - some of them simply don't support no_std. yet, some of them work in both std and no_std mode. When we work at no_std mode, we always work in wasm. I.e. the second line breaks here. We can work around this by having a list of all the crates that need to be tested in wasm target - but it's hard to maintain.
  2. When we test in the std mode, the tests still choose feat2, which is only supposed to work in no_std mode; we have to list all such features in the command, which works but is messy and hard to maintain: cargo hack check --feature-powerset --no-dev-deps --exclude-no-default-features --skip runtime-benchmarks --skip try-runtime ....

MOZGIII avatar Nov 15 '21 15:11 MOZGIII

Overall, I think baking in the logic to derive target from a feature is kind of overly specific, but I've figured that each crate can have some instructions on how it should be tested by cargo-hack in it's Cargo.toml metadata - which can be a more general solution to the same issue. What if we add some extra information there?

MOZGIII avatar Nov 15 '21 15:11 MOZGIII

Thanks for the explanation. If I understand correctly, what this feature request is asking for is something like:

  • A way to specify options such as --exclude and --skip per-crate (optionally per-target).

    The package.metadata in Cargo.toml seems to be a reasonable choice, since it can get as json via cargo-metadata.

    Something like the following:

    # Default options if no per-target option is specified.
    [package.metadata.cargo-hack]
    skip = ["feat1"]
    exclude = false # If true, same as `--exclude crate_name`
    
    # Options when built for specific target (wasm32-unknown-unknown, in this case)
    [package.metadata.cargo-hack.target.wasm32-unknown-unknown]
    skip = ["std"]
    

And the following is the feature that I initially thought this feature request to be asking for.

  • A way to handle multiple targets in one command.

    For this, cargo already has two unsettling features.

    Implementing an ability that emulates multitarget is okay, but this ability does not seem to be a blocker to meet this feature request.

taiki-e avatar Nov 20 '21 15:11 taiki-e

Thanks for the explanation. If I understand correctly, what this feature request is asking for is something like:

Yep. something like this.

I tried to implement it manually with some scripting, but it ended up non-trivial with bash and cargo metadata.

One thing to note is a difficulty with how substrate projects manage std/no_std switching. We only have a flag for std, which is also the default, but can be turned off with --no-default-features; one problem I encountered was that no_std environment only works in wasm target - i.e. there are some compile-time assertions on pointer sizes etc, so it's not possible to test/check no_std with native target.

This means we need to check some featured only with wasm target, some only with non-wasm; and some with both. The presence of the std feature in the selected features is a solid indicator of whether wasm or non-wasm target should be used.

Can we actually detect whether the std feature is active with a particular feature set to pick the target? This will greatly reduce the amount of manually entered info that we'd have to keep in the Cargo.toml metadata.

Some extra context of our case

We have, by in large, three kinds of crates:

  • builds with std and some non-wasm target (the usual rust);
  • builds both without std for wasm target, and with std for non-wasm target (usually, the non-wasm variant builds second, and embeds the wasm variant, wrapping it with the host/wasm glue code); these typically can also run tests in std mode.

What would be nice is to run the checks on the individual crates in the no_std mode, as we've been hit by a lack of said tests a couple of time already.

MOZGIII avatar Nov 20 '21 16:11 MOZGIII