Skip to content

Commit

Permalink
Enable pipelined compilation by default
Browse files Browse the repository at this point in the history
This commit enables pipelined compilation by default in Cargo now that
the requisite support has been stablized in rust-lang/rust#62766. This
involved minor updates in a number of locations here and there, but
nothing of meat has changed from the original implementation (just
tweaks to how rustc is called).
  • Loading branch information
alexcrichton committed Jul 31, 2019
1 parent 1f74bdf commit daa1bce
Show file tree
Hide file tree
Showing 10 changed files with 114 additions and 77 deletions.
14 changes: 14 additions & 0 deletions src/cargo/core/compiler/build_context/target_info.rs
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,7 @@ pub struct TargetInfo {
pub rustflags: Vec<String>,
/// Extra flags to pass to `rustdoc`, see `env_args`.
pub rustdocflags: Vec<String>,
pub supports_pipelining: Option<bool>,
}

/// Kind of each file generated by a Unit, part of `FileType`.
Expand Down Expand Up @@ -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())
Expand Down Expand Up @@ -179,6 +192,7 @@ impl TargetInfo {
"RUSTDOCFLAGS",
)?,
cfg,
supports_pipelining,
})
}

Expand Down
2 changes: 1 addition & 1 deletion src/cargo/core/compiler/context/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
81 changes: 27 additions & 54 deletions src/cargo/core/compiler/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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 {
Expand All @@ -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);
Expand Down Expand Up @@ -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
Expand All @@ -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 => (),
Expand All @@ -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(())
}
Expand Down Expand Up @@ -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() {
Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -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,
Expand Down
2 changes: 1 addition & 1 deletion src/cargo/ops/fix.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
7 changes: 6 additions & 1 deletion tests/testsuite/build_script.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand Down Expand Up @@ -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 [..]
",
)
Expand Down
58 changes: 46 additions & 12 deletions tests/testsuite/cache_messages.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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() {
Expand Down Expand Up @@ -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();
}
2 changes: 1 addition & 1 deletion tests/testsuite/dep_info.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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")],
);
}
14 changes: 7 additions & 7 deletions tests/testsuite/profile_overrides.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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] [..]
",
)
Expand Down
5 changes: 5 additions & 0 deletions tests/testsuite/rustc_info_cache.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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();
Expand Down
6 changes: 6 additions & 0 deletions tests/testsuite/rustdoc.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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 [..]
",
Expand All @@ -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 [..]
Expand Down Expand Up @@ -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 [..]`
Expand Down Expand Up @@ -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 [..]
Expand All @@ -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 [..]
Expand Down Expand Up @@ -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 [..]",
Expand Down

0 comments on commit daa1bce

Please sign in to comment.