Skip to content

Commit afcd5ef

Browse files
committed
Auto merge of #7008 - alexcrichton:less-pipelining, r=ehuss
Handle pipelined tests of libraries The previous implementation of pipelining accidentally forgot to account for rlibs being compiled in `--test` mode. The compilations would be pipelined to where all the dependencies of an rlib might not be available yet, but `--test` actually performs linking in rustc! This commit fixes the issue by refactoring slightly and removing `Target::requires_upstream_objects` (moving it to `TargetKind`) and then making the source of truth a `Unit::requires_upstream_objects` method which takes into account the value from `TargetKind` as well as the `CompileMode`. Closes #6993
2 parents 0e38712 + 0c51d71 commit afcd5ef

File tree

6 files changed

+31
-19
lines changed

6 files changed

+31
-19
lines changed

src/cargo/core/compiler/context/compilation_files.rs

+1-1
Original file line numberDiff line numberDiff line change
@@ -420,7 +420,7 @@ impl<'a, 'cfg: 'a> CompilationFiles<'a, 'cfg> {
420420
)?;
421421
}
422422
let path = out_dir.join(format!("lib{}.rmeta", file_stem));
423-
if !unit.target.requires_upstream_objects() {
423+
if !unit.requires_upstream_objects() {
424424
ret.push(OutputFile {
425425
path,
426426
hardlink: None,

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

+2-2
Original file line numberDiff line numberDiff line change
@@ -475,11 +475,11 @@ impl<'a, 'cfg> Context<'a, 'cfg> {
475475
self.pipelining
476476
// We're only a candidate for requiring an `rmeta` file if we
477477
// ourselves are building an rlib,
478-
&& !parent.target.requires_upstream_objects()
478+
&& !parent.requires_upstream_objects()
479479
&& parent.mode == CompileMode::Build
480480
// Our dependency must also be built as an rlib, otherwise the
481481
// object code must be useful in some fashion
482-
&& !dep.target.requires_upstream_objects()
482+
&& !dep.requires_upstream_objects()
483483
&& dep.mode == CompileMode::Build
484484
}
485485

src/cargo/core/compiler/job_queue.rs

+1-1
Original file line numberDiff line numberDiff line change
@@ -195,7 +195,7 @@ impl<'a, 'cfg> JobQueue<'a, 'cfg> {
195195
// the target as well. This should ensure that edges changed to
196196
// `Metadata` propagate upwards `All` dependencies to anything that
197197
// transitively contains the `Metadata` edge.
198-
if unit.target.requires_upstream_objects() {
198+
if unit.requires_upstream_objects() {
199199
for dep in dependencies.iter() {
200200
depend_on_deps_of_deps(cx, &mut queue_deps, dep);
201201
}

src/cargo/core/compiler/mod.rs

+1-1
Original file line numberDiff line numberDiff line change
@@ -827,7 +827,7 @@ fn build_base_args<'a, 'cfg>(
827827

828828
if unit.mode.is_check() {
829829
cmd.arg("--emit=dep-info,metadata");
830-
} else if !unit.target.requires_upstream_objects() {
830+
} else if !unit.requires_upstream_objects() {
831831
// Always produce metdata files for rlib outputs. Metadata may be used
832832
// in this session for a pipelined compilation, or it may be used in a
833833
// future Cargo session as part of a pipelined compile.

src/cargo/core/compiler/unit.rs

+12
Original file line numberDiff line numberDiff line change
@@ -50,6 +50,18 @@ pub struct UnitInner<'a> {
5050
pub mode: CompileMode,
5151
}
5252

53+
impl UnitInner<'_> {
54+
/// Returns whether compilation of this unit requires all upstream artifacts
55+
/// to be available.
56+
///
57+
/// This effectively means that this unit is a synchronization point (if the
58+
/// return value is `true`) that all previously pipelined units need to
59+
/// finish in their entirety before this one is started.
60+
pub fn requires_upstream_objects(&self) -> bool {
61+
self.mode.is_any_test() || self.target.kind().requires_upstream_objects()
62+
}
63+
}
64+
5365
impl<'a> Unit<'a> {
5466
pub fn buildkey(&self) -> String {
5567
format!("{}-{}", self.pkg.name(), short_hash(self))

src/cargo/core/manifest.rs

+14-14
Original file line numberDiff line numberDiff line change
@@ -206,6 +206,20 @@ impl TargetKind {
206206
TargetKind::CustomBuild => "build-script",
207207
}
208208
}
209+
210+
/// Returns whether production of this artifact requires the object files
211+
/// from dependencies to be available.
212+
///
213+
/// This only returns `false` when all we're producing is an rlib, otherwise
214+
/// it will return `true`.
215+
pub fn requires_upstream_objects(&self) -> bool {
216+
match self {
217+
TargetKind::Lib(kinds) | TargetKind::ExampleLib(kinds) => {
218+
kinds.iter().any(|k| k.requires_upstream_objects())
219+
}
220+
_ => true,
221+
}
222+
}
209223
}
210224

211225
/// Information about a binary, a library, an example, etc. that is part of the
@@ -821,20 +835,6 @@ impl Target {
821835
}
822836
}
823837

824-
/// Returns whether production of this artifact requires the object files
825-
/// from dependencies to be available.
826-
///
827-
/// This only returns `false` when all we're producing is an rlib, otherwise
828-
/// it will return `true`.
829-
pub fn requires_upstream_objects(&self) -> bool {
830-
match &self.kind {
831-
TargetKind::Lib(kinds) | TargetKind::ExampleLib(kinds) => {
832-
kinds.iter().any(|k| k.requires_upstream_objects())
833-
}
834-
_ => true,
835-
}
836-
}
837-
838838
pub fn is_bin(&self) -> bool {
839839
self.kind == TargetKind::Bin
840840
}

0 commit comments

Comments
 (0)