Actually pass through the request for --color=always

[saethlin]

#2243 actually doesn't work 😂

The suggestion to split on , was good but arg is actually the whole --json=diagnostic-rendered-ansi,artifacts,future-incompat , and of course I didn't test that change locally and we have no test for this in CI.

Therefore, I would like some guidance on making a test for this because I'm going to rely on this working.

[oli-obk]

Basically create any test that emits a diagnostic. Then add // compile-flags: your flags go here to the file. Add only the json color flag, we already pass the flag for json diagnostics. Then bless and 🤞

[saethlin]

I did this:

$ cat tests/fail/color-always.rs 
// compile-flags: --color=always
// error-pattern: entering unreachable code
fn main() {
    unsafe { std::hint::unreachable_unchecked() }
}

But what I get out of ./miri test looks like this, no evidence of ANSI escapes:

tests/fail/color-always.rs FAILED
command: "/home/ben/miri/target/x86_64-unknown-linux-gnu/release/miri" "--edition" "2018" "-Dwarnings" "-Dunused" "--sysroot" "/home/ben/.cache/miri/HOST" "-Zui-testing" "tests/fail/color-always.rs" "--error-format=json" "--color=always"

actual output differed from expected tests/fail/color-always.stderr
Diff < left / right > :
>error: Undefined Behavior: entering unreachable code
>  --> RUSTLIB/core/src/hint.rs:LL:CC
>   |
>LL |     unsafe { intrinsics::unreachable() }
>   |              ^^^^^^^^^^^^^^^^^^^^^^^^^ entering unreachable code
>   |
>   = help: this indicates a bug in the program: it performed an invalid operation, and caused Undefined Behavior
>   = help: see https://doc.rust-lang.org/nightly/reference/behavior-considered-undefined.html for further information
>           
>   = note: inside `std::hint::unreachable_unchecked` at RUSTLIB/core/src/hint.rs:LL:CC
>note: inside `main` at $DIR/color-always.rs:LL:CC
>  --> $DIR/color-always.rs:LL:CC
>   |
>LL |     unsafe { std::hint::unreachable_unchecked() }
>   |              ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
>
>note: some details are omitted, run with `MIRIFLAGS=-Zmiri-backtrace=full` for a verbose backtrace
>
>error: aborting due to previous error
>
>

[saethlin]

Argh this PR doesn't cover doctests. I'll look into that tomorrow.

[RalfJung]

The change is in cargo-miri, which is literally not called by the compiletest suite. So it is impossible to test this from the compiletest suite.

[RalfJung]

FWIW I'd be fine with landing this as-is (assuming you tested it locally), and figuring out a way to test this later.

[oli-obk]

Yea, I have plans for testing cargo miri, just so that we have a reasonable way to test with dependencies

[RalfJung]

So @saethlin could you resolve the last comment and mark this as ready? We can do the test part later. And rustdoc, too -- this is still clearly an improvement.

[saethlin]

Just looked at this again and it seems like something is still missing. I'm running some tests, redirecting stdout and stderr to a file, and then cating the file. Those oks should be green.

[RalfJung]

That's not Miri/rustc output, that is output from the application running inside Miri.

--color=always should only affect the warning/error diagnostics, I think.

[saethlin]

Hunh. To get fully colored output from cargo test you need to do cargo test --color=always -- --color=always.

But for cargo-miri, cargo miri test --color=always -- --color=always doesn't seem to pass the argument to the test programs. I tried throwing in extra -- --color=always but still can't get any green ok.

[RalfJung]

Miri blocks the TERM env var, which might lead libtest to never use colors.

But anyway that seems orthogonal to this PR?

[saethlin]

🤷 perhaps it is. I'm just trying to get colored output from cargo-miri without lying to the process that it's connected to a TTY.

I'll poke around this later.

[RalfJung]

@bors r+

[bors]

📌 Commit fed0e16 has been approved by RalfJung

[bors]

⌛ Testing commit fed0e16 with merge 9e2dac4...

[bors]

☀️ Test successful - checks-actions
Approved by: RalfJung
Pushing 9e2dac4 to master...

[RalfJung]

Those oks should be green.

FWIW these are not green even when running this on a tty. So that is a different problem. And it's probably due to Miri removing the TERM env var.