crate-ci/azure-pipelines

Cargo check should use `--all-targets`

Closed this issue · 7 comments

Currently the cargo check script uses --bins --examples --tests

- script: cargo check --all --bins --examples --tests

These options are not complete. Benches are missing for example.

--all-targets is the better option (https://doc.rust-lang.org/cargo/commands/cargo-check.html).

It does not use --all-targets because it implies --benches, which will generally not compile on stable/beta since #[bench] is nightly-only :)

I would love to be able to switch to --all-targets, though I think that issue would have to be solved first.

epage commented

iirc benches is stable, the harness is not (allowing you to use criterion).

However, we are then split on whether to focus on custom harness users or the built-in harness users.

Sorry, what I mean is that if a binary or library has benchmarks that use the built-in harness, then running cargo check --all-targets will fail when using stable. This would probably be very unexpected from the author's perspective. I'm not sure what the best way around this is though, because as you say we would like to check things like criterion benchmarks. May be worth filing a cargo issue for this?

epage commented

I was just clarifying the situation, that for some people it can work on stable. As long as we have something step that is ensuring benches compile, I have no problem directly calling out each type of target rather than using --all-targets.

There's this:

- ${{ if parameters.benches }}:

Though it's not currently settable from stages, and not called out in the parameters list at the top of the file, so that should probably be fixed.

I wasn't aware of the issues with nightly. I just remembered that the option --all-targets exists and was wondering why it was not used. Thanks for the discussion, I learnt stuff.

I opened #26 to track the benches issue.