libtest output is not colored #2292
Comments
RalfJung
added
C-bug
|
With #2293 we can use |
|
Ah, got it -- in addition to forwarding I am not sure what the best solution is here, given the multi-second wait triggered by forwarding |
|
Is it sufficient to just lie about whether we are a TTY? I was honestly thinking of hacking up something like that for myself to induce colors to appear. |
|
As I said, we need both |
|
Argh I was reading your phrasing in the most desperately optimistic way possible. terminfo files are a mess, and I really wonder how much portability this is actually providing for libtest. Are people actually running |
|
Yeah they might be. However, is always forcing isatty even what we want? Playground was not happy last time we did that...
|
|
I was thinking of a flag so the user could pick, but it would be better to have the shim work correctly. |
|
If the host is also Unix (or maybe only for Linux), we could forward to libc. That will help at least in some cases?
|
|
Everyone goes through this crate https://crates.io/crates/atty |
|
Hm, not sure how I feel about adding external dependencies. We are quite conservative about that here. OTOH that sounds like exactly the API we want and it is even cross-platform... But anyway it seems like there are two orthogonal steps here:
|
Alternatively we could copy this file which is tiny. |
Improve isatty support Per #2292 (comment), this is an attempt at > do something more clever with Miri's `isatty` shim Since Unix -> Unix is very simple, I'm starting with a patch that just does that. Happy to augment/rewrite this based on feedback. The linked file in libtest specifically only supports stdout. If we're doing this to support terminal applications, I think it would be strange to support one but not all 3 of the standard streams. The `atty` crate contains a bunch of extra logic that libtest does not contain, in order to support MSYS terminals: softprops/atty@db8d55f so I think if we're going to do Windows support, we should probably access all that logic somehow. I think it's pretty clear that the implementation is not going to change, so I think if we want to, pasting the contents of the `atty` crate into Miri is on the table, instead of taking a dependency.
|
With #2349, the following now produces colored output: However it also takes forever to start, due to the terminfo parsing. |
|
(never mind I was running a Miri with debug assertions^^) |
test: skip terminfo parsing in Miri Terminfo parsing takes a significant amount of time in Miri, making libtest startup very slow. To work around that Miri in fact unsets the `TERM` variable. However, this means we don't get colors in `cargo miri test`. So I propose we add some logic in libtest that skips parsing terminfo files under Miri, and just uses the regular basic coloring commands (taken from the `colored` crate). As far as I can see, these two commands are all that libtest ever needs from terminfo, so Miri doesn't even lose any functionality through this. If you want I can entirely remove the terminfo parsing code and just use these commands instead. Cc rust-lang/miri#2292 `@saethlin`
test: skip terminfo parsing in Miri Terminfo parsing takes a significant amount of time in Miri, making libtest startup very slow. To work around that Miri in fact unsets the `TERM` variable. However, this means we don't get colors in `cargo miri test`. So I propose we add some logic in libtest that skips parsing terminfo files under Miri, and just uses the regular basic coloring commands (taken from the `colored` crate). As far as I can see, these two commands are all that libtest ever needs from terminfo, so Miri doesn't even lose any functionality through this. If you want I can entirely remove the terminfo parsing code and just use these commands instead. Cc rust-lang/miri#2292 `@saethlin`
cargo miri testoutput is not colored the same waycargo testis: the output that is generated inside Miri (the libtest runner) uses no colors. I think this is probably because of the hack where we remove theTERMenvironment variable even with isolation disabled -- that makes libtest think that the terminal does not support color so it falls back to no colors. (EDIT: looks like I am wrong, see below.)We remove the
TERMvariable because having it makes libtest startup super slow (that was #1702). Maybe we need another work-around for this problem, like adding acfg(miri)somewhere to skip the slow code -- or actually making Miri less slow on that code.^^@saethlin this is the bug you mentioned in one of your PRs; it has nothing to do with
--color=always(other than, even with--color=always, libtest still uses no color unlessTERMindicates that the terminal supports color).The text was updated successfully, but these errors were encountered: