Auto merge of #10258 - joshtriplett:unconditional-pipelining, r=alexc…

…richton

Remove the option to disable pipelining

Cargo has had pipelining enabled by default for a long time, without
issue. Remove support for `build.pipelining = false`. (Continue parsing
the option from the config file, but ignore it.)

<!--
Thanks for submitting a pull request 🎉! Here are some tips for you:

* If this is your first contribution, read "Cargo Contribution Guide":
  https://doc.crates.io/contrib/
* Run `cargo fmt --all` to format your code changes.
* Small commits and pull requests are always preferable and easy to review.
* If your idea is large and needs feedback from the community, read how:
  https://doc.crates.io/contrib/process/#working-on-large-features
* Cargo takes care of compatibility. Read our design principles:
  https://doc.crates.io/contrib/design.html
* When changing help text of cargo commands, follow the steps to generate docs:
  https://github.com/rust-lang/cargo/tree/master/src/doc#building-the-man-pages
* If your PR is not finished, set it as "draft" PR or add "WIP" in its title.
* It's ok to use the CI resources to test your PR, but please don't abuse them.

### What does this PR try to resolve?

Explain the motivation behind this change.
A clear overview along with an in-depth explanation are helpful.

You can use `Fixes #<issue number>` to associate this PR to an existing issue.

### How should we test and review this PR?

Demonstrate how you test this change and guide reviewers through your PR.
With a smooth review process, a pull request usually gets reviewed quicker.

If you don't know how to write and run your tests, please read the guide:
https://doc.crates.io/contrib/tests

### Additional information

Other information you want to mention in this PR, such as prior arts,
future extensions, an unresolved problem, or a TODO list.
-->

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

@@ -60,15 +60,8 @@ pub struct Context<'a, 'cfg> {
     /// been computed.
     files: Option<CompilationFiles<'a, 'cfg>>,
 
-    /// A flag indicating whether pipelining is enabled for this compilation
-    /// session. Pipelining largely only affects the edges of the dependency
-    /// graph that we generate at the end, and otherwise it's pretty
-    /// straightforward.
-    pipelining: bool,
-
     /// A set of units which are compiling rlibs and are expected to produce
-    /// metadata files in addition to the rlib itself. This is only filled in
-    /// when `pipelining` above is enabled.
+    /// metadata files in addition to the rlib itself.
     rmeta_required: HashSet<Unit>,
 
     /// When we're in jobserver-per-rustc process mode, this keeps those
@@ -106,8 +99,6 @@ impl<'a, 'cfg> Context<'a, 'cfg> {
             }
         };
 
-        let pipelining = bcx.config.build_config()?.pipelining.unwrap_or(true);
-
         Ok(Self {
             bcx,
             compilation: Compilation::new(bcx)?,
@@ -122,7 +113,6 @@ impl<'a, 'cfg> Context<'a, 'cfg> {
             files: None,
             rmeta_required: HashSet::new(),
             rustc_clients: HashMap::new(),
-            pipelining,
             lto: HashMap::new(),
             metadata_for_doc_units: HashMap::new(),
         })
@@ -589,11 +579,9 @@ impl<'a, 'cfg> Context<'a, 'cfg> {
     /// Returns whether when `parent` depends on `dep` if it only requires the
     /// metadata file from `dep`.
     pub fn only_requires_rmeta(&self, parent: &Unit, dep: &Unit) -> bool {
-        // this is only enabled when pipelining is enabled
-        self.pipelining
-            // We're only a candidate for requiring an `rmeta` file if we
-            // ourselves are building an rlib,
-            && !parent.requires_upstream_objects()
+        // We're only a candidate for requiring an `rmeta` file if we
+        // ourselves are building an rlib,
+        !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

src/cargo/core/compiler/job_queue.rs

@@ -413,7 +413,7 @@ impl<'cfg> JobQueue<'cfg> {
 
         // This is somewhat tricky, but we may need to synthesize some
         // dependencies for this target if it requires full upstream
-        // compilations to have completed. If we're in pipelining mode then some
+        // compilations to have completed. Because of pipelining, some
         // dependency edges may be `Metadata` due to the above clause (as
         // opposed to everything being `All`). For example consider:
         //

src/cargo/util/config/mod.rs

@@ -2108,6 +2108,7 @@ pub struct CargoNetConfig {
 #[derive(Debug, Deserialize)]
 #[serde(rename_all = "kebab-case")]
 pub struct CargoBuildConfig {
+    // deprecated, but preserved for compatibility
     pub pipelining: Option<bool>,
     pub dep_info_basedir: Option<ConfigRelativePath>,
     pub target_dir: Option<ConfigRelativePath>,

src/doc/src/reference/config.md

@@ -73,7 +73,6 @@ rustflags = ["…", "…"]        # custom flags to pass to all compiler invocat
 rustdocflags = ["…", "…"]     # custom flags to pass to rustdoc
 incremental = true            # whether or not to enable incremental compilation
 dep-info-basedir = "…"        # path for the base directory for targets in depfiles
-pipelining = true             # rustc pipelining
 
 [doc]
 browser = "chromium"          # browser to use with `cargo doc --open`,
@@ -441,12 +440,8 @@ The setting itself is a config-relative path. So, for example, a value of
 directory.
 
 ##### `build.pipelining`
-* Type: boolean
-* Default: true
-* Environment: `CARGO_BUILD_PIPELINING`
 
-Controls whether or not build pipelining is used. This allows Cargo to
-schedule overlapping invocations of `rustc` in parallel when possible.
+This option is deprecated and unused. Cargo always has pipelining enabled.
 
 #### `[doc]`
 

src/doc/src/reference/environment-variables.md

@@ -81,7 +81,6 @@ supported environment variables are:
 * `CARGO_BUILD_RUSTDOCFLAGS` — Extra `rustdoc` flags, see [`build.rustdocflags`].
 * `CARGO_BUILD_INCREMENTAL` — Incremental compilation, see [`build.incremental`].
 * `CARGO_BUILD_DEP_INFO_BASEDIR` — Dep-info relative directory, see [`build.dep-info-basedir`].
-* `CARGO_BUILD_PIPELINING` — Whether or not to use `rustc` pipelining, see [`build.pipelining`].
 * `CARGO_CARGO_NEW_VCS` — The default source control system with [`cargo new`], see [`cargo-new.vcs`].
 * `CARGO_FUTURE_INCOMPAT_REPORT_FREQUENCY` - How often we should generate a future incompat report notifcation, see [`future-incompat-report.frequency`].
 * `CARGO_HTTP_DEBUG` — Enables HTTP debugging, see [`http.debug`].
@@ -141,7 +140,6 @@ supported environment variables are:
 [`build.rustdocflags`]: config.md#buildrustdocflags
 [`build.incremental`]: config.md#buildincremental
 [`build.dep-info-basedir`]: config.md#builddep-info-basedir
-[`build.pipelining`]: config.md#buildpipelining
 [`doc.browser`]: config.md#docbrowser
 [`cargo-new.name`]: config.md#cargo-newname
 [`cargo-new.email`]: config.md#cargo-newemail

tests/testsuite/build.rs

@@ -5274,12 +5274,8 @@ fn tricky_pipelining() {
         .file("bar/src/lib.rs", "")
         .build();
 
-    foo.cargo("build -p bar")
-        .env("CARGO_BUILD_PIPELINING", "true")
-        .run();
-    foo.cargo("build -p foo")
-        .env("CARGO_BUILD_PIPELINING", "true")
-        .run();
+    foo.cargo("build -p bar").run();
+    foo.cargo("build -p foo").run();
 }
 
 #[cargo_test]
@@ -5301,7 +5297,6 @@ fn pipelining_works() {
         .build();
 
     foo.cargo("build")
-        .env("CARGO_BUILD_PIPELINING", "true")
         .with_stdout("")
         .with_stderr(
             "\
@@ -5364,7 +5359,6 @@ fn pipelining_big_graph() {
         .file("b30/src/lib.rs", "")
         .build();
     foo.cargo("build -p foo")
-        .env("CARGO_BUILD_PIPELINING", "true")
         .with_status(101)
         .with_stderr_contains("[ERROR] could not compile `a30`[..]")
         .run();