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

driver: Only output ANSI logging if connected to a terminal #78548

Merged
merged 3 commits into from
Nov 24, 2020

Conversation

camelid
Copy link
Member

@camelid camelid commented Oct 29, 2020

Fixes #78435.

See #78435 for more.

Cc @RalfJung @oli-obk

@camelid camelid added the A-driver Area: rustc_driver that ties everything together into the `rustc` compiler label Oct 29, 2020
@rust-highfive
Copy link
Collaborator

r? @lcnr

(rust_highfive has picked a reviewer for you, use r? to override)

@rust-highfive rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Oct 29, 2020
@camelid
Copy link
Member Author

camelid commented Oct 30, 2020

This is the change in the contents of log.txt from the command MIRI_LOG=debug ./miri test > log.txt.

Note: I only showed the first 30 or so lines for brevity.

Before

A libstd for Miri is now available in `/Users/user/Library/Caches/org.rust-lang.miri/HOST`.

running 3 tests
test range_map::tests::basic_insert ... ok
test intptrcast::tests::test_align_addr ... ok
test range_map::tests::gaps ... ok

test result: ok. 3 passed; 0 failed; 0 ignored; 0 measured; 0 filtered out


running 200 tests
normalized stdout:
2:rustc�[2mrustc_mir::interpret::eval_context�[0m::�[1;32mframe�[0m main
2:rustc�[2mrustc_mir::interpret::eval_context�[0m::�[1;32mframe�[0m main
2:rustc�[1;34mDEBUG�[0m �[2mrustc_middle::mir::interpret�[0m creating alloc Function(Instance { def: Item(WithOptConstParam { did: DefId(0:8 ~ align[317d]::main), const_param_did: None }), substs: [] }) with id alloc5
2:rustc�[2mrustc_mir::interpret::eval_context�[0m::�[1;32mframe�[0m std::rt::lang_start::<()>
2:rustc├─�[2m12�[0m�[2mms�[0m �[1;32m INFO�[0m �[2mrustc_mir::interpret::step�[0m StorageLive(_4)
2:rustc├─�[2m12�[0m�[2mms�[0m �[1;32m INFO�[0m �[2mrustc_mir::interpret::step�[0m StorageLive(_5)
2:rustc├─�[2m12�[0m�[2mms�[0m �[1;32m INFO�[0m �[2mrustc_mir::interpret::step�[0m StorageLive(_6)
2:rustc├─�[2m12�[0m�[2mms�[0m �[1;32m INFO�[0m �[2mrustc_mir::interpret::step�[0m StorageLive(_7)
2:rustc├─�[2m12�[0m�[2mms�[0m �[1;32m INFO�[0m �[2mrustc_mir::interpret::step�[0m (_7.0: fn() -> T) = _1
2:rustc├─�[2m12�[0m�[2mms�[0m �[1;32m INFO�[0m �[2mrustc_mir::interpret::step�[0m Retag(_7)
2:rustc├─�[2m12�[0m�[2mms�[0m �[1;32m INFO�[0m �[2mrustc_mir::interpret::step�[0m _6 = &_7
2:rustc├─�[2m12�[0m�[2mms�[0m �[1;32m INFO�[0m �[2mrustc_mir::interpret::step�[0m Retag(_6)
2:rustc├─�[2m12�[0m�[2mms�[0m �[1;32m INFO�[0m �[2mrustc_mir::interpret::step�[0m _5 = &(*_6)
2:rustc├─�[2m12�[0m�[2mms�[0m �[1;32m INFO�[0m �[2mrustc_mir::interpret::step�[0m Retag(_5)
2:rustc├─�[2m12�[0m�[2mms�[0m �[1;32m INFO�[0m �[2mrustc_mir::interpret::step�[0m _4 = move _5 as &dyn std::ops::Fn() -> i32 + std::marker::Sync + std::panic::RefUnwindSafe (Pointer(Unsize))
2:rustc├─�[2m12�[0m�[2mms�[0m �[1;34mDEBUG�[0m �[2mrustc_mir::interpret::util�[0m ensure_monomorphic_enough: ty=[closure@std::rt::lang_start<()>::{closure#0}]
2:rustc├─�[2m12�[0m�[2mms�[0m �[1;34mDEBUG�[0m �[2mrustc_mir::interpret::util�[0m ensure_monomorphic_enough: ty=Some(Binder(std::ops::Fn<()>))
2:rustc├─�[2m16�[0m�[2mms�[0m �[1;32m INFO�[0m �[2mrustc_mir::interpret::step�[0m Retag(_4)
2:rustc├─�[2m16�[0m�[2mms�[0m �[1;32m INFO�[0m �[2mrustc_mir::interpret::step�[0m StorageDead(_5)
2:rustc├─�[2m16�[0m�[2mms�[0m �[1;32m INFO�[0m �[2mrustc_mir::interpret::step�[0m StorageLive(_8)
2:rustc├─�[2m16�[0m�[2mms�[0m �[1;32m INFO�[0m �[2mrustc_mir::interpret::step�[0m _8 = _2
2:rustc├─�[2m16�[0m�[2mms�[0m �[1;32m INFO�[0m �[2mrustc_mir::interpret::step�[0m StorageLive(_9)
2:rustc├─�[2m16�[0m�[2mms�[0m �[1;32m INFO�[0m �[2mrustc_mir::interpret::step�[0m _9 = _3
2:rustc├─�[2m16�[0m�[2mms�[0m �[1;32m INFO�[0m �[2mrustc_mir::interpret::step�[0m _0 = std::rt::lang_start_internal(move _4, move _8, move _9) -> bb1
2:rustc├┐�[2mrustc_mir::interpret::eval_context�[0m::�[1;32mframe�[0m std::rt::lang_start::<()>
2

After

A libstd for Miri is now available in `/Users/user/Library/Caches/org.rust-lang.miri/HOST`.

running 3 tests
test intptrcast::tests::test_align_addr ... ok
test range_map::tests::gaps ... ok
test range_map::tests::basic_insert ... ok

test result: ok. 3 passed; 0 failed; 0 ignored; 0 measured; 0 filtered out


running 200 tests
normalized stdout:
2:rustcrustc_mir::interpret::eval_context::frame main
2:rustcrustc_mir::interpret::eval_context::frame main
2:rustcDEBUG rustc_middle::mir::interpret creating alloc Function(Instance { def: Item(WithOptConstParam { did: DefId(0:8 ~ align[317d]::main), const_param_did: None }), substs: [] }) with id alloc5
2:rustcrustc_mir::interpret::eval_context::frame std::rt::lang_start::<()>
2:rustc├─1395ms INFO rustc_mir::interpret::step StorageLive(_4)
2:rustc├─1395ms INFO rustc_mir::interpret::step StorageLive(_5)
2:rustc├─1395ms INFO rustc_mir::interpret::step StorageLive(_6)
2:rustc├─1395ms INFO rustc_mir::interpret::step StorageLive(_7)
2:rustc├─1395ms INFO rustc_mir::interpret::step (_7.0: fn() -> T) = _1
2:rustc├─1396ms INFO rustc_mir::interpret::step Retag(_7)
2:rustc├─1396ms INFO rustc_mir::interpret::step _6 = &_7
2:rustc├─1396ms INFO rustc_mir::interpret::step Retag(_6)
2:rustc├─1452ms INFO rustc_mir::interpret::step _5 = &(*_6)
2:rustc├─1452ms INFO rustc_mir::interpret::step Retag(_5)
2:rustc├─1452ms INFO rustc_mir::interpret::step _4 = move _5 as &dyn std::ops::Fn() -> i32 + std::marker::Sync + std::panic::RefUnwindSafe (Pointer(Unsize))
2:rustc├─1454ms DEBUG rustc_mir::interpret::util ensure_monomorphic_enough: ty=[closure@std::rt::lang_start<()>::{closure#0}]
2:rustc├─1454ms DEBUG rustc_mir::interpret::util ensure_monomorphic_enough: ty=Some(Binder(std::ops::Fn<()>))
2:rustc├─1614ms INFO rustc_mir::interpret::step Retag(_4)
2:rustc├─1614ms INFO rustc_mir::interpret::step StorageDead(_5)
2:rustc├─1614ms INFO rustc_mir::interpret::step StorageLive(_8)
2:rustc├─1614ms INFO rustc_mir::interpret::step _8 = _2
2:rustc├─1614ms INFO rustc_mir::interpret::step StorageLive(_9)
2:rustc├─1614ms INFO rustc_mir::interpret::step _9 = _3
2:rustc├─1614ms INFO rustc_mir::interpret::step _0 = std::rt::lang_start_internal(move _4, move _8, move _9) -> bb1
2:rustc├┐rustc_mir::interpret::eval_context::frame std::rt::lang_start::<()>

@camelid camelid marked this pull request as draft October 30, 2020 00:06
@camelid
Copy link
Member Author

camelid commented Oct 30, 2020

Hmm, it looks like log colors aren't working when connected to a terminal...

EDIT: Never mind, it's working correctly. Not sure what happened before 🤷

@camelid camelid marked this pull request as ready for review October 30, 2020 00:37
@jyn514 jyn514 changed the title driver: Only output ANSI if connected to a terminal driver: Only output ANSI logging if connected to a terminal Oct 30, 2020
@jyn514
Copy link
Member

jyn514 commented Oct 30, 2020

Would it be possible to add an option to always enable color logging? I like to view logs with RUSTDOC_LOG=rustdoc::passes::collect rustdoc 2>&1 | less -R, which I think tricks rustc into thinking it's not being viewed interactively.

@jyn514 jyn514 added the T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. label Oct 30, 2020
@camelid
Copy link
Member Author

camelid commented Oct 30, 2020

I've never added a driver CLI option before, so I would appreciate it if you could give me general instructions on how to do it! I looked at the "Command-line arguments" chapter in the rustc-dev-guide, but it doesn't seem to give instructions on how to add an option.

@jyn514
Copy link
Member

jyn514 commented Oct 30, 2020

Stable options go in

pub fn rustc_short_optgroups() -> Vec<RustcOptGroup> {
, unstable options go in
options! {DebuggingOptions, DebuggingSetter, basic_debugging_options,
. I think this should be an unstable option since it's only for developers.

@camelid
Copy link
Member Author

camelid commented Oct 30, 2020

@jyn514 Okay, I finally got the new driver option working. Unfortunately, I think all the other tools that use the driver (rustdoc, miri, etc.) will have to be updated :/

Or do rustc options get passed through to rustdoc and the rest, and I can just access the option there? Not sure how to proceed.

@jyn514
Copy link
Member

jyn514 commented Oct 30, 2020

@camelid I know rustc options get passed through to rustdoc. Not sure about other tools, maybe hard-code them for now? Or ask in T-clippy on zulip.

src/librustdoc/lib.rs Outdated Show resolved Hide resolved
@camelid
Copy link
Member Author

camelid commented Oct 30, 2020

Hm, I wonder why the cranelift error was not caught when I ran ./x.py build...

@oli-obk
Copy link
Contributor

oli-obk commented Oct 30, 2020

Hm, I wonder why the cranelift error was not caught when I ran ./x.py build...

you need to enable cranelift by adding it to your config.toml,

rust/config.toml.example

Lines 479 to 482 in 0d33ab7

# This is an array of the codegen backends that will be compiled for the rustc
# that's being compiled. The default is to only build the LLVM codegen backend,
# and currently the only standard options supported are `"llvm"` and `"cranelift"`.
#codegen-backends = ["llvm"]

@lcnr
Copy link
Contributor

lcnr commented Oct 30, 2020

❤️

don't have the capacity to review this myself rn

r? @oli-obk

@rust-highfive rust-highfive assigned oli-obk and unassigned lcnr Oct 30, 2020
The log color variable is whatever the log variable is (`RUSTC_LOG`,
`RUSTDOC_LOG`, `MIRI_LOG`, etc.) + `_COLOR`. So `RUSTC_LOG_COLOR`,
`RUSTDOC_LOG_COLOR`, `MIRI_LOG_COLOR`, etc.

Thanks to @RalfJung for suggesting this! It was much easier to implement
than adding a new unstable argument, which is what I tried before.
},
Err(std::env::VarError::NotPresent) => stdout_isatty(),
Err(std::env::VarError::NotUnicode(_value)) => {
panic!("non-unicode log color value: expected one of always, never, or auto")
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This should use early_error I think.

@camelid camelid marked this pull request as draft October 30, 2020 23:09
@camelid
Copy link
Member Author

camelid commented Oct 30, 2020

Marking as draft since there are a few kinks to work out.

@oli-obk oli-obk added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Nov 3, 2020
@bors
Copy link
Contributor

bors commented Nov 24, 2020

⌛ Testing commit 173a7db with merge 0c05693eb54fd636c3b429610b558246685af361...

@bors
Copy link
Contributor

bors commented Nov 24, 2020

💔 Test failed - checks-actions

@bors bors added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. labels Nov 24, 2020
@jyn514
Copy link
Member

jyn514 commented Nov 24, 2020

Seems spurious? Not sure what's going wrong.

   = note: LINK : fatal error LNK1201: error writing to program database 'D:\a\rust\rust\build\i686-pc-windows-msvc\test\incremental\issue-42602\issue-42602.pdb'; check for insufficient disk space, invalid path, or insufficient privilege

@bors retry

@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 Nov 24, 2020
@bors
Copy link
Contributor

bors commented Nov 24, 2020

⌛ Testing commit 173a7db with merge 6cd99ee574cfcda812ce847b25134a49371f2bf1...

@bors
Copy link
Contributor

bors commented Nov 24, 2020

💔 Test failed - checks-actions

@bors bors added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. labels Nov 24, 2020
@RalfJung
Copy link
Member

2nd time we got that same strange error?

= note: LINK : fatal error LNK1201: error writing to program database 'D:\a\rust\rust\build\i686-pc-windows-msvc\test\run-make-fulldeps\override-aliased-flags\override-aliased-flags\main.pdb'; check for insufficient disk space, invalid path, or insufficient privilege

@pietroalbini @Mark-Simulacrum any idea what might be happening?
@bors retry

@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 Nov 24, 2020
@pietroalbini
Copy link
Member

Is this happening only on this PR? That partition has 52GB of free disk space...

@bors
Copy link
Contributor

bors commented Nov 24, 2020

⌛ Testing commit 173a7db with merge 1c389ff...

@jyn514
Copy link
Member

jyn514 commented Nov 24, 2020

@pietroalbini is there a way to see where the disk usage is coming from on the CI builder?

@camelid
Copy link
Member Author

camelid commented Nov 24, 2020

Looks like the Windows builder succeeded this time! Still waiting on aarch64-apple though.

@bors
Copy link
Contributor

bors commented Nov 24, 2020

☀️ Test successful - checks-actions
Approved by: oli-obk
Pushing 1c389ff to master...

@bors bors added the merged-by-bors This PR was explicitly merged by bors. label Nov 24, 2020
@bors bors merged commit 1c389ff into rust-lang:master Nov 24, 2020
@rustbot rustbot added this to the 1.50.0 milestone Nov 24, 2020
@camelid camelid deleted the driver-tty branch November 24, 2020 23:27
JohnTitor added a commit to JohnTitor/rust that referenced this pull request Jan 16, 2021
…crum

Make `x.py --color always` apply to logging too

Follow-up to rust-lang#78548, rust-lang#79004.

r? `@Mark-Simulacrum`
m-ou-se added a commit to m-ou-se/rust that referenced this pull request Jan 16, 2021
…crum

Make `x.py --color always` apply to logging too

Follow-up to rust-lang#78548, rust-lang#79004.

r? ``@Mark-Simulacrum``
m-ou-se added a commit to m-ou-se/rust that referenced this pull request Jan 16, 2021
…crum

Make `x.py --color always` apply to logging too

Follow-up to rust-lang#78548, rust-lang#79004.

r? ```@Mark-Simulacrum```
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-driver Area: rustc_driver that ties everything together into the `rustc` compiler merged-by-bors This PR was explicitly merged by bors. S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Driver logging: disable colors when logging to file
10 participants