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.

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

Target features reimplemented III #1197 #7203

Closed
wants to merge 21 commits into from
Closed
Changes from 1 commit
Commits
Show all changes
21 commits
Select commit Hold shift + click to select a range
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
Prev Previous commit
Next Next commit
Enable pipelined compilation by default
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 authored and Serhii Plyhun committed Aug 2, 2019
commit 9cde46cb2592f6744b8dd3f9f98c4e3458ee881f
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
@@ -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`.
@@ -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,
})
}

2 changes: 1 addition & 1 deletion src/cargo/core/compiler/context/mod.rs
Original file line number Diff line number Diff line change
@@ -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,
81 changes: 27 additions & 54 deletions src/cargo/core/compiler/mod.rs
Original file line number Diff line number Diff line change
@@ -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 {
@@ -643,7 +642,7 @@ fn rustdoc<'a, 'cfg>(cx: &mut Context<'a, 'cfg>, unit: &Unit<'a>) -> CargoResult
}
}

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);
@@ -730,39 +729,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
@@ -777,26 +757,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 => (),
@@ -807,6 +776,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(())
}
@@ -837,8 +813,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() {
@@ -1248,11 +1223,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,
@@ -1318,10 +1293,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,
2 changes: 1 addition & 1 deletion src/cargo/ops/fix.rs
Original file line number Diff line number Diff line change
@@ -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;
7 changes: 6 additions & 1 deletion tests/testsuite/build_script.rs
Original file line number Diff line number Diff line change
@@ -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 [..]
",
)
58 changes: 46 additions & 12 deletions tests/testsuite/cache_messages.rs
Original file line number Diff line number Diff line change
@@ -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();
}
2 changes: 1 addition & 1 deletion tests/testsuite/dep_info.rs
Original file line number Diff line number Diff line change
@@ -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
@@ -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] [..]
",
)
5 changes: 5 additions & 0 deletions tests/testsuite/rustc_info_cache.rs
Original file line number Diff line number Diff line change
@@ -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();
6 changes: 6 additions & 0 deletions tests/testsuite/rustdoc.rs
Original file line number Diff line number Diff line change
@@ -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 [..]",