diff --git a/src/cargo/core/compiler/build_context/target_info.rs b/src/cargo/core/compiler/build_context/target_info.rs index ecdb4239c65..81a2d622c63 100644 --- a/src/cargo/core/compiler/build_context/target_info.rs +++ b/src/cargo/core/compiler/build_context/target_info.rs @@ -34,6 +34,7 @@ pub struct TargetInfo { pub rustflags: Vec, /// Extra flags to pass to `rustdoc`, see `env_args`. pub rustdocflags: Vec, + pub supports_pipelining: Option, } /// Kind of each file generated by a Unit, part of `FileType`. @@ -98,6 +99,18 @@ impl TargetInfo { .args(&rustflags) .env_remove("RUSTC_LOG"); + // NOTE: set this unconditionally to `true` once support for `--json` + // rides to stable. + // + // Also note that we only learn about this functionality for the host + // compiler since the host/target rustc are always the same. + let mut pipelining_test = process.clone(); + pipelining_test.args(&["--error-format=json", "--json=artifacts"]); + let supports_pipelining = match kind { + Kind::Host => Some(rustc.cached_output(&pipelining_test).is_ok()), + Kind::Target => None, + }; + let target_triple = requested_target .as_ref() .map(|s| s.as_str()) @@ -179,6 +192,7 @@ impl TargetInfo { "RUSTDOCFLAGS", )?, cfg, + supports_pipelining, }) } diff --git a/src/cargo/core/compiler/context/mod.rs b/src/cargo/core/compiler/context/mod.rs index 9325c8c63e2..7ab5f5b74b1 100644 --- a/src/cargo/core/compiler/context/mod.rs +++ b/src/cargo/core/compiler/context/mod.rs @@ -77,7 +77,7 @@ impl<'a, 'cfg> Context<'a, 'cfg> { .config .get_bool("build.pipelining")? .map(|t| t.val) - .unwrap_or(false); + .unwrap_or(bcx.host_info.supports_pipelining.unwrap()); Ok(Self { bcx, diff --git a/src/cargo/core/compiler/mod.rs b/src/cargo/core/compiler/mod.rs index 4962e774506..efe332a93eb 100644 --- a/src/cargo/core/compiler/mod.rs +++ b/src/cargo/core/compiler/mod.rs @@ -18,7 +18,7 @@ use std::io::Write; use std::path::{Path, PathBuf}; use std::sync::Arc; -use failure::{bail, Error}; +use failure::Error; use lazycell::LazyCell; use log::debug; use same_file::is_same_file; @@ -614,7 +614,6 @@ fn rustdoc<'a, 'cfg>(cx: &mut Context<'a, 'cfg>, unit: &Unit<'a>) -> CargoResult rustdoc.arg("--crate-name").arg(&unit.target.crate_name()); add_path_args(bcx, unit, &mut rustdoc); add_cap_lints(bcx, unit, &mut rustdoc); - add_color(bcx, &mut rustdoc); if unit.kind != Kind::Host { if let Some(ref target) = bcx.build_config.requested_target { @@ -635,7 +634,7 @@ fn rustdoc<'a, 'cfg>(cx: &mut Context<'a, 'cfg>, unit: &Unit<'a>) -> CargoResult rustdoc.arg("--cfg").arg(&format!("feature=\"{}\"", feat)); } - add_error_format(cx, &mut rustdoc, false, false)?; + add_error_format_and_color(cx, &mut rustdoc, false)?; if let Some(args) = bcx.extra_args_for(unit) { rustdoc.args(args); @@ -722,39 +721,20 @@ fn add_cap_lints(bcx: &BuildContext<'_, '_>, unit: &Unit<'_>, cmd: &mut ProcessB } } -fn add_color(bcx: &BuildContext<'_, '_>, cmd: &mut ProcessBuilder) { - let shell = bcx.config.shell(); - let color = if shell.supports_color() { - "always" - } else { - "never" - }; - cmd.args(&["--color", color]); -} - /// Add error-format flags to the command. /// -/// This is rather convoluted right now. The general overview is: -/// - If -Zcache-messages or `build.pipelining` is enabled, Cargo always uses -/// JSON output. This has several benefits, such as being easier to parse, -/// handles changing formats (for replaying cached messages), ensures -/// atomic output (so messages aren't interleaved), etc. -/// - `supports_termcolor` is a temporary flag. rustdoc does not yet support -/// the `--json-rendered` flag, but it is intended to fix that soon. -/// - `short` output is not yet supported for JSON output. We haven't yet -/// decided how this problem will be resolved. Probably either adding -/// "short" to the JSON output, or more ambitiously moving diagnostic -/// rendering to an external library that Cargo can share with rustc. +/// This is somewhat odd right now, but the general overview is that if +/// `-Zcache-messages` or `pipelined` is enabled then Cargo always uses JSON +/// output. This has several benefits, such as being easier to parse, handles +/// changing formats (for replaying cached messages), ensures atomic output (so +/// messages aren't interleaved), etc. /// -/// It is intended in the future that Cargo *always* uses the JSON output, and -/// this function can be simplified. The above issues need to be resolved, the -/// flags need to be stabilized, and we need more testing to ensure there -/// aren't any regressions. -fn add_error_format( +/// It is intended in the future that Cargo *always* uses the JSON output (by +/// turning on cache-messages by default), and this function can be simplified. +fn add_error_format_and_color( cx: &Context<'_, '_>, cmd: &mut ProcessBuilder, pipelined: bool, - supports_termcolor: bool, ) -> CargoResult<()> { // If this unit is producing a required rmeta file then we need to know // when the rmeta file is ready so we can signal to the rest of Cargo that @@ -769,26 +749,15 @@ fn add_error_format( // internally understand that we should extract the `rendered` field and // present it if we can. if cx.bcx.build_config.cache_messages() || pipelined { - cmd.arg("--error-format=json").arg("-Zunstable-options"); - if supports_termcolor { - cmd.arg("--json-rendered=termcolor"); + cmd.arg("--error-format=json"); + let mut json = String::from("--json=diagnostic-rendered-ansi"); + if pipelined { + json.push_str(",artifacts"); } if cx.bcx.build_config.message_format == MessageFormat::Short { - // FIXME(rust-lang/rust#60419): right now we have no way of - // turning on JSON messages from the compiler and also asking - // the rendered field to be in the `short` format. - bail!( - "currently `--message-format short` is incompatible with {}", - if pipelined { - "pipelined compilation" - } else { - "cached output" - } - ); - } - if pipelined { - cmd.arg("-Zemit-artifact-notifications"); + json.push_str(",diagnostic-short"); } + cmd.arg(json); } else { match cx.bcx.build_config.message_format { MessageFormat::Human => (), @@ -799,6 +768,13 @@ fn add_error_format( cmd.arg("--error-format").arg("short"); } } + + let color = if cx.bcx.config.shell().supports_color() { + "always" + } else { + "never" + }; + cmd.args(&["--color", color]); } Ok(()) } @@ -829,8 +805,7 @@ fn build_base_args<'a, 'cfg>( cmd.arg("--crate-name").arg(&unit.target.crate_name()); add_path_args(bcx, unit, cmd); - add_color(bcx, cmd); - add_error_format(cx, cmd, cx.rmeta_required(unit), true)?; + add_error_format_and_color(cx, cmd, cx.rmeta_required(unit))?; if !test { for crate_type in crate_types.iter() { @@ -1234,11 +1209,11 @@ fn on_stderr_line( } 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-rendered=termcolor is used. This runs + // 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-rendered` or `--color` or `--message-format` flag). + // `--json` or `--color` or `--message-format` flag). #[derive(serde::Deserialize, serde::Serialize)] struct CompilerMessage { rendered: String, @@ -1304,10 +1279,8 @@ fn replay_output_cache( ) -> Work { let target = target.clone(); let extract_rendered_messages = match format { - MessageFormat::Human => true, + MessageFormat::Human | MessageFormat::Short => true, MessageFormat::Json => false, - // FIXME: short not supported. - MessageFormat::Short => false, }; let mut options = OutputOptions { extract_rendered_messages, diff --git a/src/cargo/ops/fix.rs b/src/cargo/ops/fix.rs index 1ac31974b72..a43c68687b7 100644 --- a/src/cargo/ops/fix.rs +++ b/src/cargo/ops/fix.rs @@ -621,7 +621,7 @@ impl FixArgs { ret.enabled_edition = Some(s[prefix.len()..].to_string()); continue; } - if s.starts_with("--error-format=") || s.starts_with("--json-rendered=") { + if s.starts_with("--error-format=") || s.starts_with("--json=") { // Cargo may add error-format in some cases, but `cargo // fix` wants to add its own. continue; diff --git a/tests/testsuite/build_script.rs b/tests/testsuite/build_script.rs index 0952033bacc..d26509e2954 100644 --- a/tests/testsuite/build_script.rs +++ b/tests/testsuite/build_script.rs @@ -2155,6 +2155,11 @@ fn flags_go_into_tests() { #[cargo_test] fn diamond_passes_args_only_once() { + // FIXME: when pipelining rides to stable, enable this test on all channels. + if !crate::support::is_nightly() { + return; + } + let p = project() .file( "Cargo.toml", @@ -2229,7 +2234,7 @@ fn diamond_passes_args_only_once() { [COMPILING] a v0.5.0 ([..] [RUNNING] `rustc [..]` [COMPILING] foo v0.5.0 ([..] -[RUNNING] `[..]rlib -L native=test` +[RUNNING] `[..]rmeta -L native=test` [FINISHED] dev [unoptimized + debuginfo] target(s) in [..] ", ) diff --git a/tests/testsuite/cache_messages.rs b/tests/testsuite/cache_messages.rs index b283a285f4e..2582d191810 100644 --- a/tests/testsuite/cache_messages.rs +++ b/tests/testsuite/cache_messages.rs @@ -54,6 +54,52 @@ fn simple() { assert!(cargo_output2.stdout.is_empty()); } +// same as `simple`, except everything is using the short format +#[cargo_test] +fn simple_short() { + if !is_nightly() { + // --json-rendered is unstable + return; + } + let p = project() + .file( + "src/lib.rs", + " + fn a() {} + fn b() {} + ", + ) + .build(); + + let agnostic_path = Path::new("src").join("lib.rs"); + let agnostic_path_s = agnostic_path.to_str().unwrap(); + + let rustc_output = process("rustc") + .cwd(p.root()) + .args(&["--crate-type=lib", agnostic_path_s, "--error-format=short"]) + .exec_with_output() + .expect("rustc to run"); + + assert!(rustc_output.stdout.is_empty()); + assert!(rustc_output.status.success()); + + let cargo_output1 = p + .cargo("check -Zcache-messages -q --color=never --message-format=short") + .masquerade_as_nightly_cargo() + .exec_with_output() + .expect("cargo to run"); + assert_eq!(as_str(&rustc_output.stderr), as_str(&cargo_output1.stderr)); + // assert!(cargo_output1.stdout.is_empty()); + let cargo_output2 = p + .cargo("check -Zcache-messages -q --message-format=short") + .masquerade_as_nightly_cargo() + .exec_with_output() + .expect("cargo to run"); + println!("{}", String::from_utf8_lossy(&cargo_output2.stdout)); + assert_eq!(as_str(&rustc_output.stderr), as_str(&cargo_output2.stderr)); + assert!(cargo_output2.stdout.is_empty()); +} + #[cargo_test] fn color() { if !is_nightly() { @@ -334,15 +380,3 @@ fn very_verbose() { .with_stderr_contains("[..]not_used[..]") .run(); } - -#[cargo_test] -fn short_incompatible() { - let p = project().file("src/lib.rs", "").build(); - p.cargo("check -Zcache-messages --message-format=short") - .masquerade_as_nightly_cargo() - .with_stderr( - "[ERROR] currently `--message-format short` is incompatible with cached output", - ) - .with_status(101) - .run(); -} diff --git a/tests/testsuite/dep_info.rs b/tests/testsuite/dep_info.rs index dce5c4025d1..439621e42e9 100644 --- a/tests/testsuite/dep_info.rs +++ b/tests/testsuite/dep_info.rs @@ -511,6 +511,6 @@ fn canonical_path() { assert_deps_contains( &p, "target/debug/.fingerprint/foo-*/dep-lib-foo-*", - &[(1, "src/lib.rs"), (2, "debug/deps/libregdep-*.rlib")], + &[(1, "src/lib.rs"), (2, "debug/deps/libregdep-*.rmeta")], ); } diff --git a/tests/testsuite/profile_overrides.rs b/tests/testsuite/profile_overrides.rs index 8445460b571..17b3c25c18b 100644 --- a/tests/testsuite/profile_overrides.rs +++ b/tests/testsuite/profile_overrides.rs @@ -321,17 +321,17 @@ fn profile_override_hierarchy() { p.cargo("build -v").masquerade_as_nightly_cargo().with_stderr_unordered("\ [COMPILING] m3 [..] [COMPILING] dep [..] -[RUNNING] `rustc --crate-name m3 m3/src/lib.rs --color never --crate-type lib --emit=[..]link -C codegen-units=4 [..] -[RUNNING] `rustc --crate-name dep [..]dep/src/lib.rs --color never --crate-type lib --emit=[..]link -C codegen-units=3 [..] -[RUNNING] `rustc --crate-name m3 m3/src/lib.rs --color never --crate-type lib --emit=[..]link -C codegen-units=1 [..] -[RUNNING] `rustc --crate-name build_script_build m1/build.rs --color never --crate-type bin --emit=[..]link -C codegen-units=4 [..] +[RUNNING] `rustc --crate-name m3 m3/src/lib.rs [..] --crate-type lib --emit=[..]link -C codegen-units=4 [..] +[RUNNING] `rustc --crate-name dep [..]dep/src/lib.rs [..] --crate-type lib --emit=[..]link -C codegen-units=3 [..] +[RUNNING] `rustc --crate-name m3 m3/src/lib.rs [..] --crate-type lib --emit=[..]link -C codegen-units=1 [..] +[RUNNING] `rustc --crate-name build_script_build m1/build.rs [..] --crate-type bin --emit=[..]link -C codegen-units=4 [..] [COMPILING] m2 [..] -[RUNNING] `rustc --crate-name build_script_build m2/build.rs --color never --crate-type bin --emit=[..]link -C codegen-units=2 [..] +[RUNNING] `rustc --crate-name build_script_build m2/build.rs [..] --crate-type bin --emit=[..]link -C codegen-units=2 [..] [RUNNING] `[..]/m1-[..]/build-script-build` [RUNNING] `[..]/m2-[..]/build-script-build` -[RUNNING] `rustc --crate-name m2 m2/src/lib.rs --color never --crate-type lib --emit=[..]link -C codegen-units=2 [..] +[RUNNING] `rustc --crate-name m2 m2/src/lib.rs [..] --crate-type lib --emit=[..]link -C codegen-units=2 [..] [COMPILING] m1 [..] -[RUNNING] `rustc --crate-name m1 m1/src/lib.rs --color never --crate-type lib --emit=[..]link -C codegen-units=1 [..] +[RUNNING] `rustc --crate-name m1 m1/src/lib.rs [..] --crate-type lib --emit=[..]link -C codegen-units=1 [..] [FINISHED] dev [unoptimized + debuginfo] [..] ", ) diff --git a/tests/testsuite/rustc_info_cache.rs b/tests/testsuite/rustc_info_cache.rs index 51dc4a42881..ceed53ee3a8 100644 --- a/tests/testsuite/rustc_info_cache.rs +++ b/tests/testsuite/rustc_info_cache.rs @@ -4,6 +4,11 @@ use std::env; #[cargo_test] fn rustc_info_cache() { + // FIXME: when pipelining rides to stable, enable this test on all channels. + if !crate::support::is_nightly() { + return; + } + let p = project() .file("src/main.rs", r#"fn main() { println!("hello"); }"#) .build(); diff --git a/tests/testsuite/rustdoc.rs b/tests/testsuite/rustdoc.rs index 6525054444f..195b47c0342 100644 --- a/tests/testsuite/rustdoc.rs +++ b/tests/testsuite/rustdoc.rs @@ -10,6 +10,7 @@ fn rustdoc_simple() { [DOCUMENTING] foo v0.0.1 ([CWD]) [RUNNING] `rustdoc --crate-name foo src/lib.rs [..]\ -o [CWD]/target/doc \ + [..] \ -L dependency=[CWD]/target/debug/deps` [FINISHED] dev [unoptimized + debuginfo] target(s) in [..] ", @@ -27,6 +28,7 @@ fn rustdoc_args() { [DOCUMENTING] foo v0.0.1 ([CWD]) [RUNNING] `rustdoc --crate-name foo src/lib.rs [..]\ -o [CWD]/target/doc \ + [..] \ --cfg=foo \ -L dependency=[CWD]/target/debug/deps` [FINISHED] dev [unoptimized + debuginfo] target(s) in [..] @@ -66,6 +68,7 @@ fn rustdoc_foo_with_bar_dependency() { [DOCUMENTING] foo v0.0.1 ([CWD]) [RUNNING] `rustdoc --crate-name foo src/lib.rs [..]\ -o [CWD]/target/doc \ + [..] \ --cfg=foo \ -L dependency=[CWD]/target/debug/deps \ --extern [..]` @@ -104,6 +107,7 @@ fn rustdoc_only_bar_dependency() { [DOCUMENTING] bar v0.0.1 ([..]) [RUNNING] `rustdoc --crate-name bar [..]bar/src/lib.rs [..]\ -o [CWD]/target/doc \ + [..] \ --cfg=foo \ -L dependency=[CWD]/target/debug/deps` [FINISHED] dev [unoptimized + debuginfo] target(s) in [..] @@ -125,6 +129,7 @@ fn rustdoc_same_name_documents_lib() { [DOCUMENTING] foo v0.0.1 ([..]) [RUNNING] `rustdoc --crate-name foo src/lib.rs [..]\ -o [CWD]/target/doc \ + [..] \ --cfg=foo \ -L dependency=[CWD]/target/debug/deps` [FINISHED] dev [unoptimized + debuginfo] target(s) in [..] @@ -168,6 +173,7 @@ fn rustdoc_target() { [RUNNING] `rustdoc --crate-name foo src/lib.rs [..]\ --target x86_64-unknown-linux-gnu \ -o [CWD]/target/x86_64-unknown-linux-gnu/doc \ + [..] \ -L dependency=[CWD]/target/x86_64-unknown-linux-gnu/debug/deps \ -L dependency=[CWD]/target/debug/deps` [FINISHED] dev [unoptimized + debuginfo] target(s) in [..]",