Skip to content

Commit

Permalink
Auto merge of #3000 - matklad:error-format, r=alexcrichton
Browse files Browse the repository at this point in the history
Add --message-format flag.

Closes #2982

This adds a `--message-format` flag with values `human` or `json-v1` to commands that may trigger compilation.

After the discussion in the issue I am no longer sure that this is a way to go:

* Looks like it buys nothing compared to `RUST_FLAGS` approach: a flag is more useful on the command line, but from the tool point of view there should be no significant differences between a flag and an environmental variable.

* Looks like we really want to wrap compiler's messages into our own json to tie them to particular compilation.
  • Loading branch information
bors authored Oct 6, 2016
2 parents d3bad1a + b9ec2b1 commit 0873893
Show file tree
Hide file tree
Showing 22 changed files with 311 additions and 47 deletions.
6 changes: 4 additions & 2 deletions src/bin/bench.rs
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
use cargo::core::Workspace;
use cargo::ops;
use cargo::ops::{self, MessageFormat};
use cargo::util::{CliResult, CliError, Human, Config, human};
use cargo::util::important_paths::{find_root_manifest_for_wd};

Expand All @@ -16,6 +16,7 @@ pub struct Options {
flag_verbose: u32,
flag_quiet: Option<bool>,
flag_color: Option<String>,
flag_message_format: MessageFormat,
flag_lib: bool,
flag_bin: Vec<String>,
flag_example: Vec<String>,
Expand Down Expand Up @@ -50,6 +51,7 @@ Options:
-v, --verbose ... Use verbose output
-q, --quiet No output printed to stdout
--color WHEN Coloring: auto, always, never
--message-format FMT Error format: human, json [default: human]
--frozen Require Cargo.lock and cache are up to date
--locked Require Cargo.lock is up to date
Expand All @@ -75,7 +77,6 @@ pub fn execute(options: Options, config: &Config) -> CliResult<Option<()>> {
&options.flag_color,
options.flag_frozen,
options.flag_locked));

let ops = ops::TestOptions {
no_run: options.flag_no_run,
no_fail_fast: false,
Expand All @@ -96,6 +97,7 @@ pub fn execute(options: Options, config: &Config) -> CliResult<Option<()>> {
&options.flag_test,
&options.flag_example,
&options.flag_bench),
message_format: options.flag_message_format,
target_rustdoc_args: None,
target_rustc_args: None,
},
Expand Down
6 changes: 4 additions & 2 deletions src/bin/build.rs
Original file line number Diff line number Diff line change
@@ -1,8 +1,7 @@
use std::env;

use cargo::core::Workspace;
use cargo::ops::CompileOptions;
use cargo::ops;
use cargo::ops::{self, CompileOptions, MessageFormat};
use cargo::util::important_paths::{find_root_manifest_for_wd};
use cargo::util::{CliResult, Config};

Expand All @@ -18,6 +17,7 @@ pub struct Options {
flag_verbose: u32,
flag_quiet: Option<bool>,
flag_color: Option<String>,
flag_message_format: MessageFormat,
flag_release: bool,
flag_lib: bool,
flag_bin: Vec<String>,
Expand Down Expand Up @@ -52,6 +52,7 @@ Options:
-v, --verbose ... Use verbose output
-q, --quiet No output printed to stdout
--color WHEN Coloring: auto, always, never
--message-format FMT Error format: human, json [default: human]
--frozen Require Cargo.lock and cache are up to date
--locked Require Cargo.lock is up to date
Expand Down Expand Up @@ -92,6 +93,7 @@ pub fn execute(options: Options, config: &Config) -> CliResult<Option<()>> {
&options.flag_test,
&options.flag_example,
&options.flag_bench),
message_format: options.flag_message_format,
target_rustdoc_args: None,
target_rustc_args: None,
};
Expand Down
5 changes: 4 additions & 1 deletion src/bin/doc.rs
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
use cargo::core::Workspace;
use cargo::ops;
use cargo::ops::{self, MessageFormat};
use cargo::util::{CliResult, Config};
use cargo::util::important_paths::{find_root_manifest_for_wd};

Expand All @@ -17,6 +17,7 @@ pub struct Options {
flag_verbose: u32,
flag_quiet: Option<bool>,
flag_color: Option<String>,
flag_message_format: MessageFormat,
flag_package: Vec<String>,
flag_lib: bool,
flag_bin: Vec<String>,
Expand Down Expand Up @@ -47,6 +48,7 @@ Options:
-v, --verbose ... Use verbose output
-q, --quiet No output printed to stdout
--color WHEN Coloring: auto, always, never
--message-format FMT Error format: human, json [default: human]
--frozen Require Cargo.lock and cache are up to date
--locked Require Cargo.lock is up to date
Expand Down Expand Up @@ -85,6 +87,7 @@ pub fn execute(options: Options, config: &Config) -> CliResult<Option<()>> {
&empty,
&empty,
&empty),
message_format: options.flag_message_format,
release: options.flag_release,
mode: ops::CompileMode::Doc {
deps: !options.flag_no_deps,
Expand Down
1 change: 1 addition & 0 deletions src/bin/install.rs
Original file line number Diff line number Diff line change
Expand Up @@ -114,6 +114,7 @@ pub fn execute(options: Options, config: &Config) -> CliResult<Option<()>> {
release: !options.flag_debug,
filter: ops::CompileFilter::new(false, &options.flag_bin, &[],
&options.flag_example, &[]),
message_format: ops::MessageFormat::Human,
target_rustc_args: None,
target_rustdoc_args: None,
};
Expand Down
5 changes: 4 additions & 1 deletion src/bin/run.rs
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
use cargo::core::Workspace;
use cargo::ops;
use cargo::ops::{self, MessageFormat};
use cargo::util::{CliResult, CliError, Config, Human};
use cargo::util::important_paths::{find_root_manifest_for_wd};

Expand All @@ -16,6 +16,7 @@ pub struct Options {
flag_verbose: u32,
flag_quiet: Option<bool>,
flag_color: Option<String>,
flag_message_format: MessageFormat,
flag_release: bool,
flag_frozen: bool,
flag_locked: bool,
Expand All @@ -42,6 +43,7 @@ Options:
-v, --verbose ... Use verbose output
-q, --quiet No output printed to stdout
--color WHEN Coloring: auto, always, never
--message-format FMT Error format: human, json [default: human]
--frozen Require Cargo.lock and cache are up to date
--locked Require Cargo.lock is up to date
Expand Down Expand Up @@ -91,6 +93,7 @@ pub fn execute(options: Options, config: &Config) -> CliResult<Option<()>> {
bins: &bins, examples: &examples,
}
},
message_format: options.flag_message_format,
target_rustdoc_args: None,
target_rustc_args: None,
};
Expand Down
6 changes: 4 additions & 2 deletions src/bin/rustc.rs
Original file line number Diff line number Diff line change
@@ -1,8 +1,7 @@
use std::env;

use cargo::core::Workspace;
use cargo::ops::{CompileOptions, CompileMode};
use cargo::ops;
use cargo::ops::{self, CompileOptions, CompileMode, MessageFormat};
use cargo::util::important_paths::{find_root_manifest_for_wd};
use cargo::util::{CliResult, CliError, Config, human};

Expand All @@ -19,6 +18,7 @@ pub struct Options {
flag_verbose: u32,
flag_quiet: Option<bool>,
flag_color: Option<String>,
flag_message_format: MessageFormat,
flag_release: bool,
flag_lib: bool,
flag_bin: Vec<String>,
Expand Down Expand Up @@ -55,6 +55,7 @@ Options:
-v, --verbose ... Use verbose output
-q, --quiet No output printed to stdout
--color WHEN Coloring: auto, always, never
--message-format FMT Error format: human, json [default: human]
--frozen Require Cargo.lock and cache are up to date
--locked Require Cargo.lock is up to date
Expand Down Expand Up @@ -110,6 +111,7 @@ pub fn execute(options: Options, config: &Config) -> CliResult<Option<()>> {
&options.flag_test,
&options.flag_example,
&options.flag_bench),
message_format: options.flag_message_format,
target_rustdoc_args: None,
target_rustc_args: options.arg_opts.as_ref().map(|a| &a[..]),
};
Expand Down
5 changes: 4 additions & 1 deletion src/bin/rustdoc.rs
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
use cargo::core::Workspace;
use cargo::ops;
use cargo::ops::{self, MessageFormat};
use cargo::util::{CliResult, Config};
use cargo::util::important_paths::{find_root_manifest_for_wd};

Expand All @@ -17,6 +17,7 @@ pub struct Options {
flag_release: bool,
flag_quiet: Option<bool>,
flag_color: Option<String>,
flag_message_format: MessageFormat,
flag_package: Option<String>,
flag_lib: bool,
flag_bin: Vec<String>,
Expand Down Expand Up @@ -52,6 +53,7 @@ Options:
-v, --verbose ... Use verbose output
-q, --quiet No output printed to stdout
--color WHEN Coloring: auto, always, never
--message-format FMT Error format: human, json [default: human]
--frozen Require Cargo.lock and cache are up to date
--locked Require Cargo.lock is up to date
Expand Down Expand Up @@ -95,6 +97,7 @@ pub fn execute(options: Options, config: &Config) -> CliResult<Option<()>> {
&options.flag_test,
&options.flag_example,
&options.flag_bench),
message_format: options.flag_message_format,
mode: ops::CompileMode::Doc { deps: false },
target_rustdoc_args: Some(&options.arg_opts),
target_rustc_args: None,
Expand Down
6 changes: 5 additions & 1 deletion src/bin/test.rs
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
use cargo::core::Workspace;
use cargo::ops;
use cargo::ops::{self, MessageFormat};
use cargo::util::{CliResult, CliError, Human, human, Config};
use cargo::util::important_paths::{find_root_manifest_for_wd};

Expand All @@ -23,6 +23,7 @@ pub struct Options {
flag_verbose: u32,
flag_quiet: Option<bool>,
flag_color: Option<String>,
flag_message_format: MessageFormat,
flag_release: bool,
flag_no_fail_fast: bool,
flag_frozen: bool,
Expand Down Expand Up @@ -55,6 +56,7 @@ Options:
-v, --verbose ... Use verbose output
-q, --quiet No output printed to stdout
--color WHEN Coloring: auto, always, never
--message-format FMT Error format: human, json [default: human]
--no-fail-fast Run all tests regardless of failure
--frozen Require Cargo.lock and cache are up to date
--locked Require Cargo.lock is up to date
Expand Down Expand Up @@ -92,6 +94,7 @@ pub fn execute(options: Options, config: &Config) -> CliResult<Option<()>> {
&options.flag_color,
options.flag_frozen,
options.flag_locked));

let root = try!(find_root_manifest_for_wd(options.flag_manifest_path, config.cwd()));

let empty = Vec::new();
Expand Down Expand Up @@ -124,6 +127,7 @@ pub fn execute(options: Options, config: &Config) -> CliResult<Option<()>> {
release: options.flag_release,
mode: mode,
filter: filter,
message_format: options.flag_message_format,
target_rustdoc_args: None,
target_rustc_args: None,
},
Expand Down
3 changes: 1 addition & 2 deletions src/cargo/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -211,8 +211,7 @@ pub fn version() -> String {
})
}

fn flags_from_args<T>(usage: &str, args: &[String],
options_first: bool) -> CliResult<T>
fn flags_from_args<T>(usage: &str, args: &[String], options_first: bool) -> CliResult<T>
where T: Decodable
{
let docopt = Docopt::new(usage).unwrap()
Expand Down
11 changes: 10 additions & 1 deletion src/cargo/ops/cargo_compile.rs
Original file line number Diff line number Diff line change
Expand Up @@ -59,6 +59,8 @@ pub struct CompileOptions<'a> {
pub release: bool,
/// Mode for this compile.
pub mode: CompileMode,
/// `--error_format` flag for the compiler.
pub message_format: MessageFormat,
/// Extra arguments to be passed to rustdoc (for main crate and dependencies)
pub target_rustdoc_args: Option<&'a [String]>,
/// The specified target will be compiled with all the available arguments,
Expand All @@ -74,6 +76,12 @@ pub enum CompileMode {
Doc { deps: bool },
}

#[derive(Clone, Copy, PartialEq, Eq, RustcDecodable)]
pub enum MessageFormat {
Human,
Json
}

pub enum CompileFilter<'a> {
Everything,
Only {
Expand Down Expand Up @@ -150,7 +158,7 @@ pub fn compile_ws<'a>(ws: &Workspace<'a>,
let root_package = try!(ws.current());
let CompileOptions { config, jobs, target, spec, features,
all_features, no_default_features,
release, mode,
release, mode, message_format,
ref filter, ref exec_engine,
ref target_rustdoc_args,
ref target_rustc_args } = *options;
Expand Down Expand Up @@ -242,6 +250,7 @@ pub fn compile_ws<'a>(ws: &Workspace<'a>,
build_config.exec_engine = exec_engine.clone();
build_config.release = release;
build_config.test = mode == CompileMode::Test;
build_config.json_errors = message_format == MessageFormat::Json;
if let CompileMode::Doc { deps } = mode {
build_config.doc_all = deps;
}
Expand Down
1 change: 1 addition & 0 deletions src/cargo/ops/cargo_package.rs
Original file line number Diff line number Diff line change
Expand Up @@ -295,6 +295,7 @@ fn run_verify(ws: &Workspace, tar: &File, opts: &PackageOpts) -> CargoResult<()>
filter: ops::CompileFilter::Everything,
exec_engine: None,
release: false,
message_format: ops::MessageFormat::Human,
mode: ops::CompileMode::Build,
target_rustdoc_args: None,
target_rustc_args: None,
Expand Down
4 changes: 2 additions & 2 deletions src/cargo/ops/cargo_rustc/custom_build.rs
Original file line number Diff line number Diff line change
Expand Up @@ -206,8 +206,8 @@ fn build_work<'a, 'cfg>(cx: &mut Context<'a, 'cfg>, unit: &Unit<'a>)
state.running(&p);
let cmd = p.into_process_builder();
let output = try!(cmd.exec_with_streaming(
&mut |out_line| state.stdout(out_line),
&mut |err_line| state.stderr(err_line),
&mut |out_line| { state.stdout(out_line); Ok(()) },
&mut |err_line| { state.stderr(err_line); Ok(()) },
).map_err(|mut e| {
e.desc = format!("failed to run custom build command for `{}`\n{}",
pkg_name, e.desc);
Expand Down
40 changes: 36 additions & 4 deletions src/cargo/ops/cargo_rustc/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -5,10 +5,12 @@ use std::fs;
use std::path::{self, PathBuf};
use std::sync::Arc;

use rustc_serialize::json;

use core::{Package, PackageId, PackageSet, Target, Resolve};
use core::{Profile, Profiles, Workspace};
use core::shell::ColorConfig;
use util::{self, CargoResult, human};
use util::{self, CargoResult, human, machine_message};
use util::{Config, internal, ChainError, profile, join_paths, short_hash};

use self::job::{Job, Work};
Expand Down Expand Up @@ -44,6 +46,7 @@ pub struct BuildConfig {
pub release: bool,
pub test: bool,
pub doc_all: bool,
pub json_errors: bool,
}

#[derive(Clone, Default)]
Expand Down Expand Up @@ -213,7 +216,6 @@ fn rustc(cx: &mut Context, unit: &Unit) -> CargoResult<Work> {
}
}
let has_custom_args = unit.profile.rustc_args.is_some();
let exec_engine = cx.exec_engine.clone();

let filenames = try!(cx.target_filenames(unit));
let root = cx.out_dir(unit);
Expand Down Expand Up @@ -241,7 +243,9 @@ fn rustc(cx: &mut Context, unit: &Unit) -> CargoResult<Work> {
let cwd = cx.config.cwd().to_path_buf();

rustc.args(&try!(cx.rustflags_args(unit)));

let json_errors = cx.build_config.json_errors;
let package_id = unit.pkg.package_id().clone();
let target = unit.target.clone();
return Ok(Work::new(move |state| {
// Only at runtime have we discovered what the extra -L and -l
// arguments are for native libraries, so we process those here. We
Expand All @@ -267,7 +271,31 @@ fn rustc(cx: &mut Context, unit: &Unit) -> CargoResult<Work> {
}

state.running(&rustc);
try!(exec_engine.exec(rustc).chain_error(|| {
let process_builder = rustc.into_process_builder();
try!(if json_errors {
process_builder.exec_with_streaming(
&mut |line| if !line.is_empty() {
Err(internal(&format!("compiler stdout is not empty: `{}`", line)))
} else {
Ok(())
},
&mut |line| {
let compiler_message = try!(json::Json::from_str(line).map_err(|_| {
internal(&format!("compiler produced invalid json: `{}`", line))
}));

machine_message::FromCompiler::new(
&package_id,
&target,
compiler_message
).emit();

Ok(())
},
).map(|_| ())
} else {
process_builder.exec()
}.chain_error(|| {
human(format!("Could not compile `{}`.", name))
}));

Expand Down Expand Up @@ -496,6 +524,10 @@ fn build_base_args(cx: &Context,
cmd.arg("--color").arg(&color_config.to_string());
}

if cx.build_config.json_errors {
cmd.arg("--error-format").arg("json");
}

cmd.arg("--crate-name").arg(&unit.target.crate_name());

if !test {
Expand Down
Loading

0 comments on commit 0873893

Please sign in to comment.