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

Make rustc shim's verbose output include crate_name being compiled. #82782

Conversation

pnkfelix
Copy link
Member

@pnkfelix pnkfelix commented Mar 4, 2021

This change is mainly motivated by an issue with the environment printing I added in PR 82403: multiple rustc invocations progress in parallel, and the environment output, spanning multiple lines, gets interleaved in ways make it difficult to extra the enviroment settings.

(This aforementioned difficulty is more of a hiccup than an outright show-stopper, because the environment variables tend to be the same for all of the rustc invocations, so it doesn't matter too much if one mixes up which lines one is looking at. But still: Better to fix it.)

This change is mainly motivated by an issue with the environment
printing I added in PR 82403: multiple rustc invocations progress
in parallel, and the environment output, spanning multiple lines,
gets interleaved in ways make it difficult to extra the enviroment settings.

(This aforementioned difficulty is more of a hiccup than an outright
show-stopper, because the environment variables tend to be the same for all of
the rustc invocations, so it doesn't matter too much if one mixes up which lines
one is looking at. But still: Better to fix it.)
@Mark-Simulacrum
Copy link
Member

Do you care about cargo test? We emit test:false/true for the rustc-timing lines

@Mark-Simulacrum Mark-Simulacrum self-assigned this Mar 5, 2021
@Mark-Simulacrum Mark-Simulacrum added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Mar 5, 2021
@pnkfelix
Copy link
Member Author

pnkfelix commented Mar 5, 2021

Do you care about cargo test? We emit test:false/true for the rustc-timing lines

I don't understand your question.

Are you asking whether I should consider adding such contextual info (the boolean value of test) to the output I'm adding here?

Or are you saying I may want to add similar contextual info elsewhere? (Note that the [RUSTC-TIMING] lines already include the crate-name, which must be Some for those lines to be emitted in the first place...)

(I didn't do a thorough review of where else in this code such contextual info might be of use. I wanted to address the more immediate problem of this particular series of lines and how confusing they can be when many instances are running in parallel.)

@Mark-Simulacrum
Copy link
Member

Sorry, I'm saying - if you care about distinguishing between rustc and rustc --test invocations, then you should add that information to the output.

…-SHIM]` unconditionally.

1. I added `--test` based on review feedback from simulacrum: I decided I would
rather include such extra context than get confused later on by its absence.
(However, I chose to encode it differently than how `[RUSTC-TIMING]` does... I
don't have much basis for doing so, other than `--test` to me more directly
reflects what it came from.)

2. I also decided to include `[RUSTC-SHIM]` at start of all of these lines
driven by the verbosity level, to make to clear where these lines of text
originate from. (Basically, I skimmed over the output and realized that a casual
observer might not be able to tell where this huge set of new lines were coming
from.)
@pnkfelix
Copy link
Member Author

pnkfelix commented Mar 5, 2021

Took @Mark-Simulacrum 's advice and added --test to output when it was part of the given arguments.

Also added [RUSTC-SHIM] to the prefix so that its more apparent where these lines originate. (Its easy enough for me to M-x kill-rectangle them, and I assume others can regexp delete them in a similar manner when desired.)

Here is what some sample bits of output look like now:

...
[RUSTC-SHIM] rustc rustc_driver env[40]: "RUSTFLAGS"="-Zmacro-backtrace -Clink-args=-Wl,-rpath,$ORIGIN/../lib -Ztls-model=initial-exec -Zunstable-options -Wrustc::internal -Cprefer-dynamic"
[RUSTC-SHIM] rustc rustc_driver env[41]: "RUST_TEST_THREADS"="128"
[RUSTC-SHIM] rustc rustc_driver working directory: /media/pnkfelix/Rust/rust.git
...
[RUSTC-SHIM] rustc --test rustc_main env[0]: "CARGO"="/media/pnkfelix/Rust/rust.git/objdir-default/build/x86_64-unknown-linux-gnu/stage0/bin/cargo"
[RUSTC-SHIM] rustc --test rustc_main env[1]: "CARGO_BIN_NAME"="rustc-main"
[RUSTC-SHIM] rustc --test rustc_main env[2]: "CARGO_CRATE_NAME"="rustc_main"
...

@Mark-Simulacrum
Copy link
Member

@bors r+ rollup

@bors
Copy link
Contributor

bors commented Mar 5, 2021

📌 Commit 444a756 has been approved by Mark-Simulacrum

@bors bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Mar 5, 2021
bors added a commit to rust-lang-ci/rust that referenced this pull request Mar 5, 2021
…laumeGomez

Rollup of 7 pull requests

Successful merges:

 - rust-lang#80845 (Make ItemKind::ExternCrate looks like hir::ItemKind::ExternCrate to make transition over hir::ItemKind simpler)
 - rust-lang#82708 (Warn on `#![doc(test(...))]` on items other than the crate root and use future incompatible lint)
 - rust-lang#82714 (Detect match arm body without braces)
 - rust-lang#82736 (Bump optimization from mir_opt_level 2 to 3 and 3 to 4 and make "release" be level 2 by default)
 - rust-lang#82782 (Make rustc shim's verbose output include crate_name being compiled.)
 - rust-lang#82797 (Update tests names to start with `issue-`)
 - rust-lang#82809 (rustdoc: Use substrings instead of split to grab enum variant paths)

Failed merges:

r? `@ghost`
`@rustbot` modify labels: rollup
@bors bors merged commit ef17859 into rust-lang:master Mar 6, 2021
@rustbot rustbot added this to the 1.52.0 milestone Mar 6, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants