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.

Sign up for GitHub

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

Jump to bottom

rustc: Request ansi colors if stderr isn't a tty #55788

Merged
merged 1 commit into from
Nov 9, 2018
Merged

rustc: Request ansi colors if stderr isn't a tty #55788

merged 1 commit into from
Nov 9, 2018

Conversation

alexcrichton
Copy link
Member

Currently Cargo will always capture the output of rustc meaning that
rustc is never hooked up to a tty. To retain colors Cargo uses the
fwdansi crate to ensure that ansi color codes are translated to
windows terminal methods (and ansi codes otherwise just go their natural
route on Unix).

Cargo passes --color always to rustc to ensure that using a pipe
doesn't trick it into not emitting colors at all. It turns out, however,
that --color always ends up still accidentally using the native shell
api on native windows shells.

The fix here is to instead pass AlwaysAnsi to termcolor instead of
Always, ensuring that when --color always is passed to rustc and its
output isn't a terminal, we're always generating ansi colors regardless
of the platform.

Closes #55769

@alexcrichton
rustc: Request ansi colors if stderr isn't a tty
255cc1a 
Currently Cargo will always capture the output of rustc meaning that
rustc is never hooked up to a tty. To retain colors Cargo uses the
`fwdansi` crate to ensure that ansi color codes are translated to
windows terminal methods (and ansi codes otherwise just go their natural
route on Unix).

Cargo passes `--color always` to rustc to ensure that using a pipe
doesn't trick it into not emitting colors at all. It turns out, however,
that `--color always` ends up still accidentally using the native shell
api on native windows shells.

The fix here is to instead pass `AlwaysAnsi` to `termcolor` instead of
`Always`, ensuring that when `--color always` is passed to rustc and its
output isn't a terminal, we're always generating ansi colors regardless
of the platform.

Closes rust-lang#55769
@rust-highfive
Copy link
Collaborator

r? @petrochenkov

(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 Nov 8, 2018
@alexcrichton alexcrichton mentioned this pull request Nov 8, 2018
Closed
@petrochenkov
Copy link
Contributor

@bors r+ rollup

@bors
Copy link
Contributor

bors commented Nov 8, 2018

📌 Commit 255cc1a has been approved by petrochenkov

@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 8, 2018
kennytm added a commit to kennytm/rust that referenced this pull request Nov 8, 2018
@kennytm
Rollup merge of rust-lang#55788 - alexcrichton:wincolors, r=petrochenkov
de60b44 
rustc: Request ansi colors if stderr isn't a tty

Currently Cargo will always capture the output of rustc meaning that
rustc is never hooked up to a tty. To retain colors Cargo uses the
`fwdansi` crate to ensure that ansi color codes are translated to
windows terminal methods (and ansi codes otherwise just go their natural
route on Unix).

Cargo passes `--color always` to rustc to ensure that using a pipe
doesn't trick it into not emitting colors at all. It turns out, however,
that `--color always` ends up still accidentally using the native shell
api on native windows shells.

The fix here is to instead pass `AlwaysAnsi` to `termcolor` instead of
`Always`, ensuring that when `--color always` is passed to rustc and its
output isn't a terminal, we're always generating ansi colors regardless
of the platform.

Closes rust-lang#55769
@kennytm kennytm mentioned this pull request Nov 8, 2018
Closed
Mark-Simulacrum added a commit to Mark-Simulacrum/rust that referenced this pull request Nov 9, 2018
@Mark-Simulacrum
Rollup merge of rust-lang#55788 - alexcrichton:wincolors, r=petrochenkov
d293d1e 
rustc: Request ansi colors if stderr isn't a tty

Currently Cargo will always capture the output of rustc meaning that
rustc is never hooked up to a tty. To retain colors Cargo uses the
`fwdansi` crate to ensure that ansi color codes are translated to
windows terminal methods (and ansi codes otherwise just go their natural
route on Unix).

Cargo passes `--color always` to rustc to ensure that using a pipe
doesn't trick it into not emitting colors at all. It turns out, however,
that `--color always` ends up still accidentally using the native shell
api on native windows shells.

The fix here is to instead pass `AlwaysAnsi` to `termcolor` instead of
`Always`, ensuring that when `--color always` is passed to rustc and its
output isn't a terminal, we're always generating ansi colors regardless
of the platform.

Closes rust-lang#55769
@Mark-Simulacrum Mark-Simulacrum mentioned this pull request Nov 9, 2018
Merged
bors added a commit that referenced this pull request Nov 9, 2018
@bors
Auto merge of #55803 - Mark-Simulacrum:rollup, r=Mark-Simulacrum
36a50c2 
Rollup of 17 pull requests

Successful merges:

 - #55576 (Clarify error message for -C opt-level)
 - #55633 (Support memcpy/memmove with differing src/dst alignment)
 - #55638 (Fix ICE in msg_span_from_free_region on ReEmpty)
 - #55659 (rustc: Delete grouping logic from the musl target)
 - #55719 (Sidestep link error from rustfix'ed code by using a *defined* static.)
 - #55736 (Elide anon lifetimes in conflicting impl note)
 - #55739 (Consume optimization fuel from the MIR inliner)
 - #55742 (Avoid panic when matching function call)
 - #55753 (borrow_set: remove a helper function and a clone it uses)
 - #55755 (Improve creation of 3 IndexVecs)
 - #55758 ([regression - rust2018]: unused_mut lint false positives on nightly)
 - #55760 (Remove intermediate font specs)
 - #55761 (mir: remove a hacky recursive helper function)
 - #55774 (wasm32-unknown-emscripten expects the rust_eh_personality symbol)
 - #55777 (Use `Lit` rather than `P<Lit>` in `ast::ExprKind`.)
 - #55783 (Deprecate mpsc channel selection)
 - #55788 (rustc: Request ansi colors if stderr isn't a tty)

Failed merges:

r? @ghost
@bors bors merged commit 255cc1a into rust-lang:master Nov 9, 2018
@ehuss
Copy link
Contributor

ehuss commented Nov 11, 2018

@alexcrichton @kennytm This does not appear to be the correct fix for the problem (at least on my testing with Windows 8.1). It was already emitting ANSI codes. On Windows, only StandardStream is used (not BufferWriter), and it was already choosing the Ansi output here.

I did some debugging, and it looks like the issue is that rustc/termcolor is emitting ANSI codes with extended 256 colors, but older versions of Windows don't support those extended colors. For example, to display red, it is emitting ESC [38;5;9m. fwdansi is converting this to a Color::Ansi256 which cannot be displayed on old versions of windows.

I think there are a few options here. One is for fwdansi to interpret the first 16 colors of the 8-bit palette (0-7 normal and 8-15 high-intensity) into the standard colors (using set_fg or set_bg). Another option is for termcolor to translate these basic Ansi256 values into regular colors that old versions of windows can handle. Thoughts?

@kennytm
Copy link
Member

kennytm commented Nov 11, 2018

@ehuss ahh thanks! In that case the proper fix would be to update fwdansi to be compatible with these Ansi256 colors and revert this PR.

@kennytm kennytm mentioned this pull request Nov 11, 2018
Merged
@alexcrichton alexcrichton deleted the wincolors branch November 12, 2018 15:42
@alexcrichton
Copy link
Member Author

Thanks for the investigation here @ehuss! Looks like @kennytm is on the job in fixing this. (also incredibly appreciated!)

BurntSushi pushed a commit to BurntSushi/termcolor that referenced this pull request Jun 4, 2019
@kennytm @BurntSushi
windows: support the first 16 Ansi256
3beef8b 
While the native Windows console APIs have very limited support
for colors, we can at least support the first 16 colors of the ANSI
spectrum by translating them to the standard Windows 8-bit palette,
along with toggling the intensity to make 16 total colors.

This fixes a bug that was reported against rustc.
See: rust-lang/rust#55788 (comment)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees

@petrochenkov petrochenkov

Labels
S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

No console colors on Windows 7
6 participants
@alexcrichton @rust-highfive @petrochenkov @bors @ehuss @kennytm