From 0c51d71c43b042810f3ed2ff7e1a69c8cf57e215 Mon Sep 17 00:00:00 2001 From: Alex Crichton Date: Tue, 4 Jun 2019 08:40:24 -0700 Subject: [PATCH] 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 --- .../compiler/context/compilation_files.rs | 2 +- src/cargo/core/compiler/context/mod.rs | 4 +-- src/cargo/core/compiler/job_queue.rs | 2 +- src/cargo/core/compiler/mod.rs | 2 +- src/cargo/core/compiler/unit.rs | 12 ++++++++ src/cargo/core/manifest.rs | 28 +++++++++---------- 6 files changed, 31 insertions(+), 19 deletions(-) diff --git a/src/cargo/core/compiler/context/compilation_files.rs b/src/cargo/core/compiler/context/compilation_files.rs index 6e408dfcc40..8f031844e6a 100644 --- a/src/cargo/core/compiler/context/compilation_files.rs +++ b/src/cargo/core/compiler/context/compilation_files.rs @@ -420,7 +420,7 @@ impl<'a, 'cfg: 'a> CompilationFiles<'a, 'cfg> { )?; } let path = out_dir.join(format!("lib{}.rmeta", file_stem)); - if !unit.target.requires_upstream_objects() { + if !unit.requires_upstream_objects() { ret.push(OutputFile { path, hardlink: None, diff --git a/src/cargo/core/compiler/context/mod.rs b/src/cargo/core/compiler/context/mod.rs index a38c0673125..96a544759a0 100644 --- a/src/cargo/core/compiler/context/mod.rs +++ b/src/cargo/core/compiler/context/mod.rs @@ -475,11 +475,11 @@ impl<'a, 'cfg> Context<'a, 'cfg> { self.pipelining // We're only a candidate for requiring an `rmeta` file if we // ourselves are building an rlib, - && !parent.target.requires_upstream_objects() + && !parent.requires_upstream_objects() && parent.mode == CompileMode::Build // Our dependency must also be built as an rlib, otherwise the // object code must be useful in some fashion - && !dep.target.requires_upstream_objects() + && !dep.requires_upstream_objects() && dep.mode == CompileMode::Build } diff --git a/src/cargo/core/compiler/job_queue.rs b/src/cargo/core/compiler/job_queue.rs index e26bb79cbf7..12a720b16c9 100644 --- a/src/cargo/core/compiler/job_queue.rs +++ b/src/cargo/core/compiler/job_queue.rs @@ -195,7 +195,7 @@ impl<'a, 'cfg> JobQueue<'a, 'cfg> { // the target as well. This should ensure that edges changed to // `Metadata` propagate upwards `All` dependencies to anything that // transitively contains the `Metadata` edge. - if unit.target.requires_upstream_objects() { + if unit.requires_upstream_objects() { for dep in dependencies.iter() { depend_on_deps_of_deps(cx, &mut queue_deps, dep); } diff --git a/src/cargo/core/compiler/mod.rs b/src/cargo/core/compiler/mod.rs index 1661889cd90..d64e9e69862 100644 --- a/src/cargo/core/compiler/mod.rs +++ b/src/cargo/core/compiler/mod.rs @@ -827,7 +827,7 @@ fn build_base_args<'a, 'cfg>( if unit.mode.is_check() { cmd.arg("--emit=dep-info,metadata"); - } else if !unit.target.requires_upstream_objects() { + } else if !unit.requires_upstream_objects() { // Always produce metdata files for rlib outputs. Metadata may be used // in this session for a pipelined compilation, or it may be used in a // future Cargo session as part of a pipelined compile. diff --git a/src/cargo/core/compiler/unit.rs b/src/cargo/core/compiler/unit.rs index 0451fc42a9e..00c9841cc14 100644 --- a/src/cargo/core/compiler/unit.rs +++ b/src/cargo/core/compiler/unit.rs @@ -50,6 +50,18 @@ pub struct UnitInner<'a> { pub mode: CompileMode, } +impl UnitInner<'_> { + /// Returns whether compilation of this unit requires all upstream artifacts + /// to be available. + /// + /// This effectively means that this unit is a synchronization point (if the + /// return value is `true`) that all previously pipelined units need to + /// finish in their entirety before this one is started. + pub fn requires_upstream_objects(&self) -> bool { + self.mode.is_any_test() || self.target.kind().requires_upstream_objects() + } +} + impl<'a> Unit<'a> { pub fn buildkey(&self) -> String { format!("{}-{}", self.pkg.name(), short_hash(self)) diff --git a/src/cargo/core/manifest.rs b/src/cargo/core/manifest.rs index 0f4c984f1de..37a78c82828 100644 --- a/src/cargo/core/manifest.rs +++ b/src/cargo/core/manifest.rs @@ -206,6 +206,20 @@ impl TargetKind { TargetKind::CustomBuild => "build-script", } } + + /// Returns whether production of this artifact requires the object files + /// from dependencies to be available. + /// + /// This only returns `false` when all we're producing is an rlib, otherwise + /// it will return `true`. + pub fn requires_upstream_objects(&self) -> bool { + match self { + TargetKind::Lib(kinds) | TargetKind::ExampleLib(kinds) => { + kinds.iter().any(|k| k.requires_upstream_objects()) + } + _ => true, + } + } } /// Information about a binary, a library, an example, etc. that is part of the @@ -821,20 +835,6 @@ impl Target { } } - /// Returns whether production of this artifact requires the object files - /// from dependencies to be available. - /// - /// This only returns `false` when all we're producing is an rlib, otherwise - /// it will return `true`. - pub fn requires_upstream_objects(&self) -> bool { - match &self.kind { - TargetKind::Lib(kinds) | TargetKind::ExampleLib(kinds) => { - kinds.iter().any(|k| k.requires_upstream_objects()) - } - _ => true, - } - } - pub fn is_bin(&self) -> bool { self.kind == TargetKind::Bin }