Skip to content

Commit b21602c

Browse files
borsbors
committed
Auto merge of #7143 - alexcrichton:stabilize-pipelined-compilation, r=ehuss
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).
2 parents 1f74bdf + daa1bce commit b21602c

File tree

10 files changed

+114
-77
lines changed

10 files changed

+114
-77
lines changed

‎src/cargo/core/compiler/build_context/target_info.rs

+14
Original file line numberDiff line numberDiff line change
@@ -34,6 +34,7 @@ pub struct TargetInfo {
3434
pub rustflags: Vec<String>,
3535
/// Extra flags to pass to `rustdoc`, see `env_args`.
3636
pub rustdocflags: Vec<String>,
37+
pub supports_pipelining: Option<bool>,
3738
}
3839

3940
/// Kind of each file generated by a Unit, part of `FileType`.
@@ -98,6 +99,18 @@ impl TargetInfo {
9899
.args(&rustflags)
99100
.env_remove("RUSTC_LOG");
100101

102+
// NOTE: set this unconditionally to `true` once support for `--json`
103+
// rides to stable.
104+
//
105+
// Also note that we only learn about this functionality for the host
106+
// compiler since the host/target rustc are always the same.
107+
let mut pipelining_test = process.clone();
108+
pipelining_test.args(&["--error-format=json", "--json=artifacts"]);
109+
let supports_pipelining = match kind {
110+
Kind::Host => Some(rustc.cached_output(&pipelining_test).is_ok()),
111+
Kind::Target => None,
112+
};
113+
101114
let target_triple = requested_target
102115
.as_ref()
103116
.map(|s| s.as_str())
@@ -179,6 +192,7 @@ impl TargetInfo {
179192
"RUSTDOCFLAGS",
180193
)?,
181194
cfg,
195+
supports_pipelining,
182196
})
183197
}
184198

‎src/cargo/core/compiler/context/mod.rs

+1-1
Original file line numberDiff line numberDiff line change
@@ -77,7 +77,7 @@ impl<'a, 'cfg> Context<'a, 'cfg> {
7777
.config
7878
.get_bool("build.pipelining")?
7979
.map(|t| t.val)
80-
.unwrap_or(false);
80+
.unwrap_or(bcx.host_info.supports_pipelining.unwrap());
8181

8282
Ok(Self {
8383
bcx,

‎src/cargo/core/compiler/mod.rs

+27-54
Original file line numberDiff line numberDiff line change
@@ -18,7 +18,7 @@ use std::io::Write;
1818
use std::path::{Path, PathBuf};
1919
use std::sync::Arc;
2020

21-
use failure::{bail, Error};
21+
use failure::Error;
2222
use lazycell::LazyCell;
2323
use log::debug;
2424
use same_file::is_same_file;
@@ -614,7 +614,6 @@ fn rustdoc<'a, 'cfg>(cx: &mut Context<'a, 'cfg>, unit: &Unit<'a>) -> CargoResult
614614
rustdoc.arg("--crate-name").arg(&unit.target.crate_name());
615615
add_path_args(bcx, unit, &mut rustdoc);
616616
add_cap_lints(bcx, unit, &mut rustdoc);
617-
add_color(bcx, &mut rustdoc);
618617

619618
if unit.kind != Kind::Host {
620619
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
635634
rustdoc.arg("--cfg").arg(&format!("feature=\"{}\"", feat));
636635
}
637636

638-
add_error_format(cx, &mut rustdoc, false, false)?;
637+
add_error_format_and_color(cx, &mut rustdoc, false)?;
639638

640639
if let Some(args) = bcx.extra_args_for(unit) {
641640
rustdoc.args(args);
@@ -722,39 +721,20 @@ fn add_cap_lints(bcx: &BuildContext<'_, '_>, unit: &Unit<'_>, cmd: &mut ProcessB
722721
}
723722
}
724723

725-
fn add_color(bcx: &BuildContext<'_, '_>, cmd: &mut ProcessBuilder) {
726-
let shell = bcx.config.shell();
727-
let color = if shell.supports_color() {
728-
"always"
729-
} else {
730-
"never"
731-
};
732-
cmd.args(&["--color", color]);
733-
}
734-
735724
/// Add error-format flags to the command.
736725
///
737-
/// This is rather convoluted right now. The general overview is:
738-
/// - If -Zcache-messages or `build.pipelining` is enabled, Cargo always uses
739-
/// JSON output. This has several benefits, such as being easier to parse,
740-
/// handles changing formats (for replaying cached messages), ensures
741-
/// atomic output (so messages aren't interleaved), etc.
742-
/// - `supports_termcolor` is a temporary flag. rustdoc does not yet support
743-
/// the `--json-rendered` flag, but it is intended to fix that soon.
744-
/// - `short` output is not yet supported for JSON output. We haven't yet
745-
/// decided how this problem will be resolved. Probably either adding
746-
/// "short" to the JSON output, or more ambitiously moving diagnostic
747-
/// rendering to an external library that Cargo can share with rustc.
726+
/// This is somewhat odd right now, but the general overview is that if
727+
/// `-Zcache-messages` or `pipelined` is enabled then Cargo always uses JSON
728+
/// output. This has several benefits, such as being easier to parse, handles
729+
/// changing formats (for replaying cached messages), ensures atomic output (so
730+
/// messages aren't interleaved), etc.
748731
///
749-
/// It is intended in the future that Cargo *always* uses the JSON output, and
750-
/// this function can be simplified. The above issues need to be resolved, the
751-
/// flags need to be stabilized, and we need more testing to ensure there
752-
/// aren't any regressions.
753-
fn add_error_format(
732+
/// It is intended in the future that Cargo *always* uses the JSON output (by
733+
/// turning on cache-messages by default), and this function can be simplified.
734+
fn add_error_format_and_color(
754735
cx: &Context<'_, '_>,
755736
cmd: &mut ProcessBuilder,
756737
pipelined: bool,
757-
supports_termcolor: bool,
758738
) -> CargoResult<()> {
759739
// If this unit is producing a required rmeta file then we need to know
760740
// when the rmeta file is ready so we can signal to the rest of Cargo that
@@ -769,26 +749,15 @@ fn add_error_format(
769749
// internally understand that we should extract the `rendered` field and
770750
// present it if we can.
771751
if cx.bcx.build_config.cache_messages() || pipelined {
772-
cmd.arg("--error-format=json").arg("-Zunstable-options");
773-
if supports_termcolor {
774-
cmd.arg("--json-rendered=termcolor");
752+
cmd.arg("--error-format=json");
753+
let mut json = String::from("--json=diagnostic-rendered-ansi");
754+
if pipelined {
755+
json.push_str(",artifacts");
775756
}
776757
if cx.bcx.build_config.message_format == MessageFormat::Short {
777-
// FIXME(rust-lang/rust#60419): right now we have no way of
778-
// turning on JSON messages from the compiler and also asking
779-
// the rendered field to be in the `short` format.
780-
bail!(
781-
"currently `--message-format short` is incompatible with {}",
782-
if pipelined {
783-
"pipelined compilation"
784-
} else {
785-
"cached output"
786-
}
787-
);
788-
}
789-
if pipelined {
790-
cmd.arg("-Zemit-artifact-notifications");
758+
json.push_str(",diagnostic-short");
791759
}
760+
cmd.arg(json);
792761
} else {
793762
match cx.bcx.build_config.message_format {
794763
MessageFormat::Human => (),
@@ -799,6 +768,13 @@ fn add_error_format(
799768
cmd.arg("--error-format").arg("short");
800769
}
801770
}
771+
772+
let color = if cx.bcx.config.shell().supports_color() {
773+
"always"
774+
} else {
775+
"never"
776+
};
777+
cmd.args(&["--color", color]);
802778
}
803779
Ok(())
804780
}
@@ -829,8 +805,7 @@ fn build_base_args<'a, 'cfg>(
829805
cmd.arg("--crate-name").arg(&unit.target.crate_name());
830806

831807
add_path_args(bcx, unit, cmd);
832-
add_color(bcx, cmd);
833-
add_error_format(cx, cmd, cx.rmeta_required(unit), true)?;
808+
add_error_format_and_color(cx, cmd, cx.rmeta_required(unit))?;
834809

835810
if !test {
836811
for crate_type in crate_types.iter() {
@@ -1234,11 +1209,11 @@ fn on_stderr_line(
12341209
} else {
12351210
// Remove color information from the rendered string. rustc has not
12361211
// included color in the past, so to avoid breaking anything, strip it
1237-
// out when --json-rendered=termcolor is used. This runs
1212+
// out when --json=diagnostic-rendered-ansi is used. This runs
12381213
// unconditionally under the assumption that Cargo will eventually
12391214
// move to this as the default mode. Perhaps in the future, cargo
12401215
// could allow the user to enable/disable color (such as with a
1241-
// `--json-rendered` or `--color` or `--message-format` flag).
1216+
// `--json` or `--color` or `--message-format` flag).
12421217
#[derive(serde::Deserialize, serde::Serialize)]
12431218
struct CompilerMessage {
12441219
rendered: String,
@@ -1304,10 +1279,8 @@ fn replay_output_cache(
13041279
) -> Work {
13051280
let target = target.clone();
13061281
let extract_rendered_messages = match format {
1307-
MessageFormat::Human => true,
1282+
MessageFormat::Human | MessageFormat::Short => true,
13081283
MessageFormat::Json => false,
1309-
// FIXME: short not supported.
1310-
MessageFormat::Short => false,
13111284
};
13121285
let mut options = OutputOptions {
13131286
extract_rendered_messages,

‎src/cargo/ops/fix.rs

+1-1
Original file line numberDiff line numberDiff line change
@@ -621,7 +621,7 @@ impl FixArgs {
621621
ret.enabled_edition = Some(s[prefix.len()..].to_string());
622622
continue;
623623
}
624-
if s.starts_with("--error-format=") || s.starts_with("--json-rendered=") {
624+
if s.starts_with("--error-format=") || s.starts_with("--json=") {
625625
// Cargo may add error-format in some cases, but `cargo
626626
// fix` wants to add its own.
627627
continue;

‎tests/testsuite/build_script.rs

+6-1
Original file line numberDiff line numberDiff line change
@@ -2155,6 +2155,11 @@ fn flags_go_into_tests() {
21552155

21562156
#[cargo_test]
21572157
fn diamond_passes_args_only_once() {
2158+
// FIXME: when pipelining rides to stable, enable this test on all channels.
2159+
if !crate::support::is_nightly() {
2160+
return;
2161+
}
2162+
21582163
let p = project()
21592164
.file(
21602165
"Cargo.toml",
@@ -2229,7 +2234,7 @@ fn diamond_passes_args_only_once() {
22292234
[COMPILING] a v0.5.0 ([..]
22302235
[RUNNING] `rustc [..]`
22312236
[COMPILING] foo v0.5.0 ([..]
2232-
[RUNNING] `[..]rlib -L native=test`
2237+
[RUNNING] `[..]rmeta -L native=test`
22332238
[FINISHED] dev [unoptimized + debuginfo] target(s) in [..]
22342239
",
22352240
)

‎tests/testsuite/cache_messages.rs

+46-12
Original file line numberDiff line numberDiff line change
@@ -54,6 +54,52 @@ fn simple() {
5454
assert!(cargo_output2.stdout.is_empty());
5555
}
5656

57+
// same as `simple`, except everything is using the short format
58+
#[cargo_test]
59+
fn simple_short() {
60+
if !is_nightly() {
61+
// --json-rendered is unstable
62+
return;
63+
}
64+
let p = project()
65+
.file(
66+
"src/lib.rs",
67+
"
68+
fn a() {}
69+
fn b() {}
70+
",
71+
)
72+
.build();
73+
74+
let agnostic_path = Path::new("src").join("lib.rs");
75+
let agnostic_path_s = agnostic_path.to_str().unwrap();
76+
77+
let rustc_output = process("rustc")
78+
.cwd(p.root())
79+
.args(&["--crate-type=lib", agnostic_path_s, "--error-format=short"])
80+
.exec_with_output()
81+
.expect("rustc to run");
82+
83+
assert!(rustc_output.stdout.is_empty());
84+
assert!(rustc_output.status.success());
85+
86+
let cargo_output1 = p
87+
.cargo("check -Zcache-messages -q --color=never --message-format=short")
88+
.masquerade_as_nightly_cargo()
89+
.exec_with_output()
90+
.expect("cargo to run");
91+
assert_eq!(as_str(&rustc_output.stderr), as_str(&cargo_output1.stderr));
92+
// assert!(cargo_output1.stdout.is_empty());
93+
let cargo_output2 = p
94+
.cargo("check -Zcache-messages -q --message-format=short")
95+
.masquerade_as_nightly_cargo()
96+
.exec_with_output()
97+
.expect("cargo to run");
98+
println!("{}", String::from_utf8_lossy(&cargo_output2.stdout));
99+
assert_eq!(as_str(&rustc_output.stderr), as_str(&cargo_output2.stderr));
100+
assert!(cargo_output2.stdout.is_empty());
101+
}
102+
57103
#[cargo_test]
58104
fn color() {
59105
if !is_nightly() {
@@ -334,15 +380,3 @@ fn very_verbose() {
334380
.with_stderr_contains("[..]not_used[..]")
335381
.run();
336382
}
337-
338-
#[cargo_test]
339-
fn short_incompatible() {
340-
let p = project().file("src/lib.rs", "").build();
341-
p.cargo("check -Zcache-messages --message-format=short")
342-
.masquerade_as_nightly_cargo()
343-
.with_stderr(
344-
"[ERROR] currently `--message-format short` is incompatible with cached output",
345-
)
346-
.with_status(101)
347-
.run();
348-
}

‎tests/testsuite/dep_info.rs

+1-1
Original file line numberDiff line numberDiff line change
@@ -511,6 +511,6 @@ fn canonical_path() {
511511
assert_deps_contains(
512512
&p,
513513
"target/debug/.fingerprint/foo-*/dep-lib-foo-*",
514-
&[(1, "src/lib.rs"), (2, "debug/deps/libregdep-*.rlib")],
514+
&[(1, "src/lib.rs"), (2, "debug/deps/libregdep-*.rmeta")],
515515
);
516516
}

‎tests/testsuite/profile_overrides.rs

+7-7
Original file line numberDiff line numberDiff line change
@@ -321,17 +321,17 @@ fn profile_override_hierarchy() {
321321
p.cargo("build -v").masquerade_as_nightly_cargo().with_stderr_unordered("\
322322
[COMPILING] m3 [..]
323323
[COMPILING] dep [..]
324-
[RUNNING] `rustc --crate-name m3 m3/src/lib.rs --color never --crate-type lib --emit=[..]link -C codegen-units=4 [..]
325-
[RUNNING] `rustc --crate-name dep [..]dep/src/lib.rs --color never --crate-type lib --emit=[..]link -C codegen-units=3 [..]
326-
[RUNNING] `rustc --crate-name m3 m3/src/lib.rs --color never --crate-type lib --emit=[..]link -C codegen-units=1 [..]
327-
[RUNNING] `rustc --crate-name build_script_build m1/build.rs --color never --crate-type bin --emit=[..]link -C codegen-units=4 [..]
324+
[RUNNING] `rustc --crate-name m3 m3/src/lib.rs [..] --crate-type lib --emit=[..]link -C codegen-units=4 [..]
325+
[RUNNING] `rustc --crate-name dep [..]dep/src/lib.rs [..] --crate-type lib --emit=[..]link -C codegen-units=3 [..]
326+
[RUNNING] `rustc --crate-name m3 m3/src/lib.rs [..] --crate-type lib --emit=[..]link -C codegen-units=1 [..]
327+
[RUNNING] `rustc --crate-name build_script_build m1/build.rs [..] --crate-type bin --emit=[..]link -C codegen-units=4 [..]
328328
[COMPILING] m2 [..]
329-
[RUNNING] `rustc --crate-name build_script_build m2/build.rs --color never --crate-type bin --emit=[..]link -C codegen-units=2 [..]
329+
[RUNNING] `rustc --crate-name build_script_build m2/build.rs [..] --crate-type bin --emit=[..]link -C codegen-units=2 [..]
330330
[RUNNING] `[..]/m1-[..]/build-script-build`
331331
[RUNNING] `[..]/m2-[..]/build-script-build`
332-
[RUNNING] `rustc --crate-name m2 m2/src/lib.rs --color never --crate-type lib --emit=[..]link -C codegen-units=2 [..]
332+
[RUNNING] `rustc --crate-name m2 m2/src/lib.rs [..] --crate-type lib --emit=[..]link -C codegen-units=2 [..]
333333
[COMPILING] m1 [..]
334-
[RUNNING] `rustc --crate-name m1 m1/src/lib.rs --color never --crate-type lib --emit=[..]link -C codegen-units=1 [..]
334+
[RUNNING] `rustc --crate-name m1 m1/src/lib.rs [..] --crate-type lib --emit=[..]link -C codegen-units=1 [..]
335335
[FINISHED] dev [unoptimized + debuginfo] [..]
336336
",
337337
)

‎tests/testsuite/rustc_info_cache.rs

+5
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,11 @@ use std::env;
44

55
#[cargo_test]
66
fn rustc_info_cache() {
7+
// FIXME: when pipelining rides to stable, enable this test on all channels.
8+
if !crate::support::is_nightly() {
9+
return;
10+
}
11+
712
let p = project()
813
.file("src/main.rs", r#"fn main() { println!("hello"); }"#)
914
.build();

‎tests/testsuite/rustdoc.rs

+6
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,7 @@ fn rustdoc_simple() {
1010
[DOCUMENTING] foo v0.0.1 ([CWD])
1111
[RUNNING] `rustdoc --crate-name foo src/lib.rs [..]\
1212
-o [CWD]/target/doc \
13+
[..] \
1314
-L dependency=[CWD]/target/debug/deps`
1415
[FINISHED] dev [unoptimized + debuginfo] target(s) in [..]
1516
",
@@ -27,6 +28,7 @@ fn rustdoc_args() {
2728
[DOCUMENTING] foo v0.0.1 ([CWD])
2829
[RUNNING] `rustdoc --crate-name foo src/lib.rs [..]\
2930
-o [CWD]/target/doc \
31+
[..] \
3032
--cfg=foo \
3133
-L dependency=[CWD]/target/debug/deps`
3234
[FINISHED] dev [unoptimized + debuginfo] target(s) in [..]
@@ -66,6 +68,7 @@ fn rustdoc_foo_with_bar_dependency() {
6668
[DOCUMENTING] foo v0.0.1 ([CWD])
6769
[RUNNING] `rustdoc --crate-name foo src/lib.rs [..]\
6870
-o [CWD]/target/doc \
71+
[..] \
6972
--cfg=foo \
7073
-L dependency=[CWD]/target/debug/deps \
7174
--extern [..]`
@@ -104,6 +107,7 @@ fn rustdoc_only_bar_dependency() {
104107
[DOCUMENTING] bar v0.0.1 ([..])
105108
[RUNNING] `rustdoc --crate-name bar [..]bar/src/lib.rs [..]\
106109
-o [CWD]/target/doc \
110+
[..] \
107111
--cfg=foo \
108112
-L dependency=[CWD]/target/debug/deps`
109113
[FINISHED] dev [unoptimized + debuginfo] target(s) in [..]
@@ -125,6 +129,7 @@ fn rustdoc_same_name_documents_lib() {
125129
[DOCUMENTING] foo v0.0.1 ([..])
126130
[RUNNING] `rustdoc --crate-name foo src/lib.rs [..]\
127131
-o [CWD]/target/doc \
132+
[..] \
128133
--cfg=foo \
129134
-L dependency=[CWD]/target/debug/deps`
130135
[FINISHED] dev [unoptimized + debuginfo] target(s) in [..]
@@ -168,6 +173,7 @@ fn rustdoc_target() {
168173
[RUNNING] `rustdoc --crate-name foo src/lib.rs [..]\
169174
--target x86_64-unknown-linux-gnu \
170175
-o [CWD]/target/x86_64-unknown-linux-gnu/doc \
176+
[..] \
171177
-L dependency=[CWD]/target/x86_64-unknown-linux-gnu/debug/deps \
172178
-L dependency=[CWD]/target/debug/deps`
173179
[FINISHED] dev [unoptimized + debuginfo] target(s) in [..]",

0 commit comments

Comments
 (0)