cargo rustc --message-format=json fails to parse stderr of rustc

[fmdkdd]

Trying the following on a small test crate that contains one error:

$ cargo rustc --message-format=json --lib -- -Z no-trans
   Compiling test-crate v0.1.0 (file://<snip>/test-crate)
error: Could not compile `test-crate`.

To learn more, run the command again with --verbose.

I get no error message. Without -Z no-trans, I do get an error message:

$ cargo rustc --message-format=json --lib
   Compiling test-crate v0.1.0 (file://<snip>/test-crate)
{"reason":"compiler-message",<snip>,"message":{"children":[],"code":{"code":"E0499","explanation":"\nA variable was borrowed as mutable more than once. [...]

This is the expected output.

If I pass --verbose, we can see the error message is output by rustc:

$ cargo rustc --verbose --message-format=json --lib -- -Z no-trans
   Compiling test-crate v0.1.0 (file:<snip>/test-crate)
     Running `rustc src/lib.rs --error-format json --crate-name test_crate --crate-type lib -g -Z no-trans -C metadata=f713ecd4594c2f7f <snip>`
error: Could not compile `test-crate`.

Caused by:
  process didn't exit successfully: `rustc src/lib.rs --error-format json --crate-name test_crate --crate-type lib -g -Z no-trans -C metadata=f713ecd4594c2f7f <snip>` (exit code: 101)
--- stderr
warning: the option `Z` is unstable and should only be used on the nightly compiler, but it is currently accepted for backwards compatibility; this will soon change, see issue #31847 for more details

{"message":"cannot borrow `a` as mutable more than once at a time",<snip>}

But it's not picked up and reported by cargo.

Testing on a crate with no build errors, the causes are reported slightly differently:

$ cargo rustc --verbose --message-format=json -- -Z no-trans
   Compiling good-crate v0.1.0 (file://<snip>/good-crate)
     Running `rustc src/main.rs --error-format json [...]`
error: Could not compile `good-crate`.

Caused by:
  failed to parse process output: `rustc src/main.rs --error-format json [...]` (exit code: 0)
--- stderr
warning: the option `Z` is unstable and should only be used on the nightly compiler, but it is currently accepted for backwards compatibility; this will soon change, see issue #31847 for more details

Caused by:
  compiler produced invalid json: `warning: the option `Z` is unstable and should only be used on the nightly compiler, but it is currently accepted for backwards compatibility; this will soon change, see issue #31847 for more details`

So it seems cargo --message-format parses the JSON of rustc, but does not expect rustc to output anything else than JSON.

$ cargo --version
cargo 0.14.0 (eca9e15 2016-11-01)
$ rustc --version
rustc 1.13.0

I know I'm using an unstable flag on stable rust, so I should heed the warning and all would be fine. But I believe the behavior of --message-format may still be viewed as a bug, since it's not choking on non-JSON output from rustc.

[fmdkdd]

I went source diving, and found the place where this parsing happens.

Adding a check to look only at lines beginning with "{" seems to do the trick, but I don't know if it's a valid long-term solution.

Here's the patched closure:

                &mut |line| {
+                  if line.starts_with("{") {
                    let compiler_message = json::Json::from_str(line).map_err(|_| {
                        internal(&format!("compiler produced invalid json: `{}`", line))
                    })?;

                    machine_message::emit(machine_message::FromCompiler {
                        package_id: &package_id,
                        target: &target,
                        message: compiler_message,
                    });
+                  }
                    Ok(())
                },

[alexcrichton]

cc @matklad, mind taking a look at this?

[matklad]

So I think we have three options:

1. covert "warning: -Z ..." to JSON on the compiler's side.

2. change the compiler to emit JSON to the stderr, the way Cargo does.

3. use if !line.starts_with("{") { return }.

Regardless of a long term solution, we should implement 3 to fix the problem faster and to be compatible with all compilers. @fmdkdd can you send a pull request? :)

Option 1) does not seem right to me, because it is not an warning about the Rust code, it is a warning specific to one particular Rust compiler.

Option 2) I think is the Right Thing. Is it worth implementing? Perhaps. On the one hand, backward compatibility should not be a huge problem: I think there are only a few tools which parse compiler output directly (Cargo being a notable example). On the other hand, the gain from the change seems marginal, so we can just as well stick with option 3.

[matklad]

Ah, and for 3) we probably want to forward stderr if it's not JSON, like

if line.starts_with("{") {
  // handle JSON here
} else {
  writeln!(cx.config.shell().err(), "{}", line)?;
}

[alexcrichton]

Yeah (3) sounds good to me for now and I agree that (2) is the "best" way to go. I don't mind "hacks" in cargo though in the meantime because backwards compatibility may mean that we can't fix that :(.

Thanks for looking into it @matklad!