Skip to content
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

Add a "cargo bench --debug" option #6446

Closed
wants to merge 1 commit into from

Conversation

asomers
Copy link

@asomers asomers commented Dec 15, 2018

It builds the benchmarks in debug mode and then runs them. It's useful
in a CI context where it's important that the benches aren't broken, but
it's not important how fast they run.

Fixes #6445

It builds the benchmarks in debug mode and then runs them.  It's useful
in a CI context where it's important that the benches aren't broken, but
it's not important how fast they run.

Fixes rust-lang#6445
@rust-highfive
Copy link

Thanks for the pull request, and welcome! The Rust team is excited to review your changes, and you should hear from @ehuss (or someone else) soon.

If any changes to this PR are deemed necessary, please add them as extra commits. This ensures that the reviewer can see what has changed since they last reviewed the code. Due to the way GitHub handles out-of-date commits, this should also make it reasonably obvious what issues have or haven't been addressed. Large or tricky changes may require several passes of review and changes.

Please see the contribution instructions for more information.

@alexcrichton
Copy link
Member

Thanks for the PR! I've sort of long wanted to have an inverse to --release across the board on all of Cargo's subcommands, enabled by default if nothing else is passed. I think this flag may best be named --dev, however, as that's what the profile in Cargo.toml is called as well.

I think that I'd personally prefer to see this added across the board rather than just cargo bench if we add it, and I think for other subcommands we'll want to be sure to reject --dev --release but otherwise accept them in isolation.

As another aside, I think it's a longstanding bug that cargo bench also doesn't accept --release!

Would you be willing to help tackle some of these surrounding issues as well?

@asomers
Copy link
Author

asomers commented Dec 17, 2018

In fact, I just discovered that cargo test --benches does almost the same thing as what this PR does (but in a workspace, it also runs unit tests; that's a different bug). So that can satisfy my needs. But I agree that it would be nice if --debug or --dev were supported across the board. So are you saying that you want to add the following commands?

cargo bench --dev # build and run benchmarks in debug mode
cargo bench --release # has no effect except to be exclusive with --dev
cargo build --dev # has no effect except to be exclusive with --release
cargo check --dev # has no effect except to be exclusive with --release
cargo doc --dev # has no effect except to be exclusive with --release
cargo run --dev # has no effect except to be exclusive with --release
cargo test --dev # has no effect except to be exclusive with --release
cargo clean --dev # Not sure what to do here?
cargo install --dev # synonym for --debug

@alexcrichton
Copy link
Member

That looks right to me yeah!

I forgot that we had cargo install --debug, that's a mistake on our part letting that through :(

@alexcrichton
Copy link
Member

FWIW cargo clean is sorta hard no matter how you slice it today, but the idea is it only clears out target/debug vs target/release

@dwijnand
Copy link
Member

cargo clean --dev clearing out just target/debug would be lovely and mirror cargo clean --release as of #6349.

@bors
Copy link
Contributor

bors commented Jan 7, 2019

☔ The latest upstream changes (presumably #6505) made this pull request unmergeable. Please resolve the merge conflicts.

@alexcrichton
Copy link
Member

Ok we discussed this at a recent Cargo triage meeting and the conclusion was that we're unlikely to land this as-is for now. Instead @ehuss pointed out that a strategy like rust-lang/rfcs#2678 is likely going to be our best bet where --profile=debug can be passed anywhere. Thanks for the PR though!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add a "cargo bench --debug" option
6 participants