Switch to output release/debug after compile #2909

Merged
merged 1 commit into from Jul 26, 2016

Conversation

Projects
None yet
5 participants
@jonathandturner
Contributor

jonathandturner commented Jul 23, 2016

This is similar to #2896 but instead of inline, it puts an indicator at the bottom showing if it was a release or debug build.

jturner-23759:cargo jturner$ ./target/release/cargo build
Finished debug (unoptimized + debuginfo) build.
jturner-23759:cargo jturner$ ./target/release/cargo build --release
Finished release (optimized) build.
@rust-highfive

This comment has been minimized.

Show comment
Hide comment
@rust-highfive

rust-highfive Jul 23, 2016

Thanks for the pull request, and welcome! The Rust team is excited to review your changes, and you should hear from @alexcrichton (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.

Thanks for the pull request, and welcome! The Rust team is excited to review your changes, and you should hear from @alexcrichton (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.

@jonathandturner

This comment has been minimized.

Show comment
Hide comment
@jonathandturner

jonathandturner Jul 23, 2016

Contributor

Image to see a better example:

screen shot 2016-07-22 at 9 03 25 pm

Contributor

jonathandturner commented Jul 23, 2016

Image to see a better example:

screen shot 2016-07-22 at 9 03 25 pm

@jonas-schievink

This comment has been minimized.

Show comment
Hide comment
@jonas-schievink

jonas-schievink Jul 23, 2016

I feel like "Finished" should be the same color as "Compiling" (and aligned to it)

I feel like "Finished" should be the same color as "Compiling" (and aligned to it)

src/cargo/ops/cargo_rustc/job_queue.rs
@@ -76,7 +78,7 @@ impl<'a> JobState<'a> {
}
impl<'a> JobQueue<'a> {
- pub fn new<'cfg>(cx: &Context<'a, 'cfg>) -> JobQueue<'a> {
+ pub fn new<'cfg>(cx: &Context<'a, 'cfg>, is_release: bool, is_doc_all: bool) -> JobQueue<'a> {

This comment has been minimized.

@alexcrichton

alexcrichton Jul 23, 2016

Member

I think these booleans are actually accessible through cx.build_config, so perhaps they don't need to be passed in?

@alexcrichton

alexcrichton Jul 23, 2016

Member

I think these booleans are actually accessible through cx.build_config, so perhaps they don't need to be passed in?

src/cargo/ops/cargo_rustc/job_queue.rs
+ if self.is_release {
+ "release (optimized)"
+ } else {
+ "debug (unoptimized + debuginfo)"

This comment has been minimized.

@alexcrichton

alexcrichton Jul 23, 2016

Member

It might be worth checking the default profile here to tweak how this is printed. I believe this can be calculated when the job queue is created by taking a look at Context::lib_profile with the argument of cx.resolve.root(). The Profile can then tell us whether it's optimized (opt_level > 0) or if debuginfo is enabled.

That way we can basically correctly say:

Finished release (optimized + debuginfo) build
@alexcrichton

alexcrichton Jul 23, 2016

Member

It might be worth checking the default profile here to tweak how this is printed. I believe this can be calculated when the job queue is created by taking a look at Context::lib_profile with the argument of cx.resolve.root(). The Profile can then tell us whether it's optimized (opt_level > 0) or if debuginfo is enabled.

That way we can basically correctly say:

Finished release (optimized + debuginfo) build
src/cargo/ops/cargo_rustc/job_queue.rs
@@ -192,6 +196,14 @@ impl<'a> JobQueue<'a> {
}
if self.queue.is_empty() {
+ if !self.is_doc_all {
+ try!(writeln!(cx.config.shell().err(), "Finished {} build.",

This comment has been minimized.

@alexcrichton

alexcrichton Jul 23, 2016

Member

I'd nix the period here

@alexcrichton

alexcrichton Jul 23, 2016

Member

I'd nix the period here

@alexcrichton

This comment has been minimized.

Show comment
Hide comment
@alexcrichton

alexcrichton Jul 23, 2016

Member

I personally like this being set apart a bit so you know it's not just another crate being compiled, but I could go both ways.

Member

alexcrichton commented Jul 23, 2016

I personally like this being set apart a bit so you know it's not just another crate being compiled, but I could go both ways.

@jonathandturner

This comment has been minimized.

Show comment
Hide comment
@jonathandturner

jonathandturner Jul 24, 2016

Contributor

I can go either way re: styling. Shouldn't be too hard to fix up the tests now that they're updated.

Contributor

jonathandturner commented Jul 24, 2016

I can go either way re: styling. Shouldn't be too hard to fix up the tests now that they're updated.

@jonathandturner

This comment has been minimized.

Show comment
Hide comment
@jonathandturner

jonathandturner Jul 25, 2016

Contributor

Now:

screen shot 2016-07-25 at 11 29 23 am

Contributor

jonathandturner commented Jul 25, 2016

Now:

screen shot 2016-07-25 at 11 29 23 am

@alexcrichton

This comment has been minimized.

Show comment
Hide comment
@alexcrichton

alexcrichton Jul 25, 2016

Member

Looks good to me, thanks @jonathandturner! Could you also squash the commits down?

Member

alexcrichton commented Jul 25, 2016

Looks good to me, thanks @jonathandturner! Could you also squash the commits down?

@alexcrichton

This comment has been minimized.

Show comment
Hide comment
Member

alexcrichton commented Jul 25, 2016

@bors

This comment has been minimized.

Show comment
Hide comment
@bors

bors Jul 25, 2016

Contributor

⌛️ Testing commit cd955f1 with merge dc46aba...

Contributor

bors commented Jul 25, 2016

⌛️ Testing commit cd955f1 with merge dc46aba...

bors added a commit that referenced this pull request Jul 25, 2016

Auto merge of #2909 - jonathandturner:debug_release_at_bottom, r=alex…
…crichton

Switch to output release/debug after compile

This is similar to #2896 but instead of inline, it puts an indicator at the bottom showing if it was a release or debug build.

```
jturner-23759:cargo jturner$ ./target/release/cargo build
Finished debug (unoptimized + debuginfo) build.
jturner-23759:cargo jturner$ ./target/release/cargo build --release
Finished release (optimized) build.
```
@bors

This comment has been minimized.

Show comment
Hide comment
Contributor

bors commented Jul 26, 2016

☀️ Test successful - cargo-cross-linux, cargo-linux-32, cargo-linux-64, cargo-mac-32, cargo-mac-64, cargo-win-gnu-32, cargo-win-gnu-64, cargo-win-msvc-32, cargo-win-msvc-64
Approved by: alexcrichton
Pushing dc46aba to master...

@bors bors merged commit cd955f1 into rust-lang:master Jul 26, 2016

3 checks passed

continuous-integration/appveyor/pr AppVeyor build succeeded
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
homu Test successful
Details

@jonathandturner jonathandturner deleted the jonathandturner:debug_release_at_bottom branch Jul 26, 2016

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment