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

Add support for customizing JSON diagnostics from Cargo #7214

Merged
merged 1 commit into from
Aug 7, 2019
Merged

Add support for customizing JSON diagnostics from Cargo #7214

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 11 additions & 1 deletion CHANGELOG.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,10 +5,20 @@

### Added

- Cargo build pipelining has been enabled by default to leverage more idle CPU
parallelism during builds.
[#7143](https://github.com/rust-lang/cargo/pull/7143)
- The `--message-format` option to Cargo can now be specified multiple times and
accepts a comma-separated list of values. In addition to the previous values
it also now accepts `json-diagnostic-short` and
`json-diagnostic-rendered-ansi` which configures the output coming from rustc
in `json` message mode.
[#7214](https://github.com/rust-lang/cargo/pull/7214)

### Changed

### Fixed
- (Nightly only): Fixed exponential blowup when using CARGO_BUILD_PIPELINING.
- (Nightly only): Fixed exponential blowup when using `CARGO_BUILD_PIPELINING`.
[#7062](https://github.com/rust-lang/cargo/pull/7062)
- Fixed using the wrong directory when updating git repositories when using
the `git-fetch-with-cli` config option, and the `GIT_DIR` environment
Expand Down
17 changes: 15 additions & 2 deletions src/cargo/core/compiler/build_config.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -112,7 +112,10 @@ impl BuildConfig {
/// Whether or not the *user* wants JSON output. Whether or not rustc
/// actually uses JSON is decided in `add_error_format`.
pub fn emit_json(&self) -> bool {
self.message_format == MessageFormat::Json
match self.message_format {
MessageFormat::Json { .. } => true,
_ => false,
}
}

pub fn test(&self) -> bool {
Expand All @@ -123,7 +126,17 @@ impl BuildConfig {
#[derive(Clone, Copy, Debug, PartialEq, Eq)]
pub enum MessageFormat {
Human,
Json,
Json {
/// Whether rustc diagnostics are rendered by cargo or included into the
/// output stream.
render_diagnostics: bool,
/// Whether the `rendered` field of rustc diagnostics are using the
/// "short" rendering.
short: bool,
/// Whether the `rendered` field of rustc diagnostics embed ansi color
/// codes.
ansi: bool,
},
Short,
}

Expand Down
153 changes: 89 additions & 64 deletions src/cargo/core/compiler/mod.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -754,27 +754,47 @@ fn add_error_format_and_color(
if pipelined {
json.push_str(",artifacts");
}
if cx.bcx.build_config.message_format == MessageFormat::Short {
json.push_str(",diagnostic-short");
match cx.bcx.build_config.message_format {
MessageFormat::Short | MessageFormat::Json { short: true, .. } => {
json.push_str(",diagnostic-short");
}
_ => {}
}
cmd.arg(json);
} else {
let mut color = true;
match cx.bcx.build_config.message_format {
MessageFormat::Human => (),
MessageFormat::Json => {
MessageFormat::Json {
ansi,
short,
render_diagnostics,
} => {
cmd.arg("--error-format").arg("json");
// If ansi is explicitly requested, enable it. If we're
// rendering diagnostics ourselves then also enable it because
// we'll figure out what to do with the colors later.
if ansi || render_diagnostics {
cmd.arg("--json=diagnostic-rendered-ansi");
}
if short {
cmd.arg("--json=diagnostic-short");
}
color = false;
}
MessageFormat::Short => {
cmd.arg("--error-format").arg("short");
}
}

let color = if cx.bcx.config.shell().supports_color() {
"always"
} else {
"never"
};
cmd.args(&["--color", color]);
if color {
let color = if cx.bcx.config.shell().supports_color() {
"always"
} else {
"never"
};
cmd.args(&["--color", color]);
}
}
Ok(())
}
Expand Down Expand Up @@ -1094,9 +1114,8 @@ impl Kind {
}

struct OutputOptions {
/// Get the `"rendered"` field from the JSON output and display it on
/// stderr instead of the JSON message.
extract_rendered_messages: bool,
/// What format we're emitting from Cargo itself.
format: MessageFormat,
/// Look for JSON message that indicates .rmeta file is available for
/// pipelined compilation.
look_for_metadata_directive: bool,
Expand All @@ -1110,7 +1129,6 @@ struct OutputOptions {

impl OutputOptions {
fn new<'a>(cx: &Context<'a, '_>, unit: &Unit<'a>) -> OutputOptions {
let extract_rendered_messages = cx.bcx.build_config.message_format != MessageFormat::Json;
let look_for_metadata_directive = cx.rmeta_required(unit);
let color = cx.bcx.config.shell().supports_color();
let cache_cell = if cx.bcx.build_config.cache_messages() {
Expand All @@ -1122,7 +1140,7 @@ impl OutputOptions {
None
};
OutputOptions {
extract_rendered_messages,
format: cx.bcx.build_config.message_format,
look_for_metadata_directive,
color,
cache_cell,
Expand Down Expand Up @@ -1179,55 +1197,66 @@ fn on_stderr_line(
}
};

// In some modes of compilation Cargo switches the compiler to JSON mode
// but the user didn't request that so we still want to print pretty rustc
// colorized diagnostics. In those cases (`extract_rendered_messages`) we
// take a look at the JSON blob we go, see if it's a relevant diagnostics,
// and if so forward just that diagnostic for us to print.
if options.extract_rendered_messages {
#[derive(serde::Deserialize)]
struct CompilerMessage {
rendered: String,
// Depending on what we're emitting from Cargo itself, we figure out what to
// do with this JSON message.
match options.format {
// In the "human" output formats (human/short) or if diagnostic messages
// from rustc aren't being included in the output of Cargo's JSON
// messages then we extract the diagnostic (if present) here and handle
// it ourselves.
MessageFormat::Human
| MessageFormat::Short
| MessageFormat::Json {
render_diagnostics: true,
..
} => {
#[derive(serde::Deserialize)]
struct CompilerMessage {
rendered: String,
}
if let Ok(mut error) = serde_json::from_str::<CompilerMessage>(compiler_message.get()) {
// state.stderr will add a newline
if error.rendered.ends_with('\n') {
error.rendered.pop();
}
let rendered = if options.color {
error.rendered
} else {
// Strip only fails if the the Writer fails, which is Cursor
// on a Vec, which should never fail.
strip_ansi_escapes::strip(&error.rendered)
.map(|v| String::from_utf8(v).expect("utf8"))
.expect("strip should never fail")
};
state.stderr(rendered);
return Ok(());
}
}
if let Ok(mut error) = serde_json::from_str::<CompilerMessage>(compiler_message.get()) {
// state.stderr will add a newline
if error.rendered.ends_with('\n') {
error.rendered.pop();

// Remove color information from the rendered string. When pipelining is
// enabled and/or when cached messages are enabled we're always asking
// for ANSI colors from rustc, so unconditionally postprocess here and
// remove ansi color codes.
MessageFormat::Json { ansi: false, .. } => {
#[derive(serde::Deserialize, serde::Serialize)]
struct CompilerMessage {
rendered: String,
#[serde(flatten)]
other: std::collections::BTreeMap<String, serde_json::Value>,
}
let rendered = if options.color {
error.rendered
} else {
// Strip only fails if the the Writer fails, which is Cursor
// on a Vec, which should never fail.
strip_ansi_escapes::strip(&error.rendered)
if let Ok(mut error) = serde_json::from_str::<CompilerMessage>(compiler_message.get()) {
error.rendered = strip_ansi_escapes::strip(&error.rendered)
.map(|v| String::from_utf8(v).expect("utf8"))
.expect("strip should never fail")
};
state.stderr(rendered);
return Ok(());
}
} else {
// Remove color information from the rendered string. rustc has not
// included color in the past, so to avoid breaking anything, strip it
// out when --json=diagnostic-rendered-ansi is used. This runs
// unconditionally under the assumption that Cargo will eventually
// move to this as the default mode. Perhaps in the future, cargo
// could allow the user to enable/disable color (such as with a
// `--json` or `--color` or `--message-format` flag).
#[derive(serde::Deserialize, serde::Serialize)]
struct CompilerMessage {
rendered: String,
#[serde(flatten)]
other: std::collections::BTreeMap<String, serde_json::Value>,
}
if let Ok(mut error) = serde_json::from_str::<CompilerMessage>(compiler_message.get()) {
error.rendered = strip_ansi_escapes::strip(&error.rendered)
.map(|v| String::from_utf8(v).expect("utf8"))
.unwrap_or(error.rendered);
let new_line = serde_json::to_string(&error)?;
let new_msg: Box<serde_json::value::RawValue> = serde_json::from_str(&new_line)?;
compiler_message = new_msg;
.unwrap_or(error.rendered);
let new_line = serde_json::to_string(&error)?;
let new_msg: Box<serde_json::value::RawValue> = serde_json::from_str(&new_line)?;
compiler_message = new_msg;
}
}

// If ansi colors are desired then we should be good to go! We can just
// pass through this message as-is.
MessageFormat::Json { ansi: true, .. } => {}
}

// In some modes of execution we will execute rustc with `-Z
Expand Down Expand Up @@ -1278,12 +1307,8 @@ fn replay_output_cache(
color: bool,
) -> Work {
let target = target.clone();
let extract_rendered_messages = match format {
MessageFormat::Human | MessageFormat::Short => true,
MessageFormat::Json => false,
};
let mut options = OutputOptions {
extract_rendered_messages,
format,
look_for_metadata_directive: false,
color,
cache_cell: None,
Expand Down
89 changes: 65 additions & 24 deletions src/cargo/util/command_prelude.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,7 +1,3 @@
use std::ffi::{OsStr, OsString};
use std::fs;
use std::path::PathBuf;

use crate::core::compiler::{BuildConfig, MessageFormat};
use crate::core::Workspace;
use crate::ops::{CompileFilter, CompileOptions, NewOptions, Packages, VersionControl};
Expand All @@ -14,6 +10,10 @@ use crate::util::{
};
use crate::CargoResult;
use clap::{self, SubCommand};
use failure::bail;
use std::ffi::{OsStr, OsString};
use std::fs;
use std::path::PathBuf;

pub use crate::core::compiler::CompileMode;
pub use crate::{CliError, CliResult, Config};
Expand Down Expand Up @@ -134,13 +134,7 @@ pub trait AppExt: Sized {
}

fn arg_message_format(self) -> Self {
self._arg(
opt("message-format", "Error format")
.value_name("FMT")
.case_insensitive(true)
.possible_values(&["human", "json", "short"])
.default_value("human"),
)
self._arg(multi_opt("message-format", "FMT", "Error format"))
}

fn arg_build_plan(self) -> Self {
Expand Down Expand Up @@ -301,23 +295,70 @@ pub trait ArgMatchesExt {
self._values_of("package"),
)?;

let message_format = match self._value_of("message-format") {
None => MessageFormat::Human,
Some(f) => {
if f.eq_ignore_ascii_case("json") {
MessageFormat::Json
} else if f.eq_ignore_ascii_case("human") {
MessageFormat::Human
} else if f.eq_ignore_ascii_case("short") {
MessageFormat::Short
} else {
panic!("Impossible message format: {:?}", f)
let mut message_format = None;
let default_json = MessageFormat::Json {
short: false,
ansi: false,
render_diagnostics: false,
};
for fmt in self._values_of("message-format") {
for fmt in fmt.split(',') {
let fmt = fmt.to_ascii_lowercase();
match fmt.as_str() {
"json" => {
if message_format.is_some() {
bail!("cannot specify two kinds of `message-format` arguments");
}
message_format = Some(default_json);
}
"human" => {
if message_format.is_some() {
bail!("cannot specify two kinds of `message-format` arguments");
}
message_format = Some(MessageFormat::Human);
}
"short" => {
if message_format.is_some() {
bail!("cannot specify two kinds of `message-format` arguments");
}
message_format = Some(MessageFormat::Short);
}
"json-render-diagnostics" => {
if message_format.is_none() {
message_format = Some(default_json);
}
match &mut message_format {
Some(MessageFormat::Json {
render_diagnostics, ..
}) => *render_diagnostics = true,
_ => bail!("cannot specify two kinds of `message-format` arguments"),
}
}
"json-diagnostic-short" => {
if message_format.is_none() {
message_format = Some(default_json);
}
match &mut message_format {
Some(MessageFormat::Json { short, .. }) => *short = true,
_ => bail!("cannot specify two kinds of `message-format` arguments"),
}
}
"json-diagnostic-rendered-ansi" => {
if message_format.is_none() {
message_format = Some(default_json);
}
match &mut message_format {
Some(MessageFormat::Json { ansi, .. }) => *ansi = true,
_ => bail!("cannot specify two kinds of `message-format` arguments"),
}
}
s => bail!("invalid message format specifier: `{}`", s),
}
}
};
}

let mut build_config = BuildConfig::new(config, self.jobs()?, &self.target(), mode)?;
build_config.message_format = message_format;
build_config.message_format = message_format.unwrap_or(MessageFormat::Human);
build_config.release = self._is_present("release");
build_config.build_plan = self._is_present("build-plan");
if build_config.build_plan {
Expand Down
Loading