-
Notifications
You must be signed in to change notification settings - Fork 2.5k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Provide an option to cargo check
everything except benchmarks
#10958
Comments
E.g. there could be a I'd rather not have to manually configure RA for each and every package and then remember to change the configuration when a libs crate is added to a package. |
Sorry for the subtlety between these flags. |
What if my Cargo.toml contains
will |
FWIW for now I am simply sticking to nightly rustc for crates where |
No. It won't. See https://doc.rust-lang.org/stable/cargo/commands/cargo-test.html#target-selection (or you can check the doc with the old new friend
I believe it's fairly stable. At least it's a documented behaviour and changes against that should be under meticulous reviews. |
That doesn't achieve my goal then, of checking everything except for the benchmarks. |
Personally, I would lean towards the following route until benches are stable
|
Oh now I see. You want to run all Cargo targets including those disabled by default. I would suggest the following command: cargo check --test '*' --bin '*' --example '*' And I totally agree using criterion is a viable choice. |
Can you say more about why RA wants to check all targets? Like, in what situation does it do that? Would it be possible for it to run checks for a narrower subset? |
Well usually I'd like my IDE to tell me when I do some change that breaks examples/tests/benchmarks. So it makes a lot of sense to check all targets I think? In Miri I used to accidentally break benchmarks fairly regularly and only got caught on CI when I created a PR, which was rather annoying. |
Note that |
Oh wait. It should check the lib, but won't test it. Since test targets depends on the lib target, it will always get the lib target checked. Sorry for misreading your requirements. |
But that will still skip lib crates if they have no tests, I would assume? I should probably just test this locally...
also FWIW
So this won't work at all for lib-only crates. |
Yes. I want to do exactly what |
hi, I stumbled on this problem as well and created a pr to fix this: #14163 |
#14163 proposed a solution that demote to a warning if lib is missing when
#10958 (comment) wants a way to build all targets but benchmarks. It seems like #14163 achieves that but want to hear back from @RalfJung. Also, @ryoqun could you share your use case so we can know the problem space more? |
Not sure what you want to hear back about from me? The key goal hasn't changed, and is described here: have a command that builds all parts of a crate that one can expect to build successfully on a stable toolchain. Well the ideal version would actually be context-sensitive: build benchmarks on a nightly toolchain but not on stable. Basically, "cargo please build everything that should actually work on this toolchain" -- it almost never makes sense to even try building benchmarks on a stable toolchain. So maybe |
@weihanglo thanks for your quick reply.
The context is quite involved... Firstly, I want to run To recap, To begin with, individual The unwanted effect is referring to the well-known possibility of false-negatives of compilation errors of feature-gated code (again, one of primary motivations of That's especially problematic for our special feature F, actually called The
Hmm, can
I think it's possible to write bench targets without relying on the unstable bench feature (i.e. rust-lang/rust#66287), meaning there's a possibility of stable-compatible bench targets. So, I wonder this context-sensitivity could work? At least, it needs a bit smarter? |
That issue is about the unstable feature being unstable? What does it say about using it on stable? |
I meant that it's possible to create That's said. It's rather rare use case. Usually, |
RE unstable benches, see #10958 (comment) @ryoqun it sounds like you are wanting to run non-dev targets across each workspace member to avoid unification. For this, you want |
@epage Thanks for replying! Sorry to rehash this, but speaking of the behavior difference of #10958 (comment) cc: @weihanglo :
As far as I read the docs, there's no explicit mention about this additional check only if https://doc.rust-lang.org/stable/cargo/commands/cargo-check.html#target-selection
For that consistency perspective as well, I'd still argue to demote the process-terminating error into just a warning one... |
|
oh, thanks for quick reply. I meant using different machines. It's easy to run parallel jobs using multiple machines with CI providers, but it's not easy to request bigger machine with 2x cores... |
Depending on your project, there would be a shared overhead between If you have found this is faster (and either uses less compute minutes or you are fine with it using more which is more likely) and you are fine with taking up more of your max concurrent jobs for the duration of the shared overhead, this sounds fairly limited in who all would benefit from this and seems something not to design towards. |
IDE plugins like RA like to invoke
cargo check
with--all-targets
to ensure that tests and examples are checked as well, not just the main crate sources. However, this often fails on stable since it will also check benchmarks, which essentially always need nightly features (Cc crossbeam-rs/crossbeam#890).So I thought I could work around this by telling RA to run
"cargo" "check" "--lib" "--bins" "--tests" "--examples" "--workspace" "--message-format=json"
instead -- however, this command sadly will fail when there is no lib target. Is there a way to say "please check everything except for the benchmarks"?The documentation claims that
--all-targets
is equivalent to specifying--lib --bins --tests --benches --examples
, but that is wrong:--lib --bins --tests --benches --examples
will fail when there is no lib target, but--all-targets
will still work.The text was updated successfully, but these errors were encountered: