Skip to content

Commit

Permalink
Auto merge of rust-lang#10258 - joshtriplett:unconditional-pipelining…
Browse files Browse the repository at this point in the history
…, r=alexcrichton

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.
-->
  • Loading branch information
bors committed Jan 5, 2022
2 parents c094313 + 65ddbbd commit dc6d847
Show file tree
Hide file tree
Showing 6 changed files with 9 additions and 33 deletions.
20 changes: 4 additions & 16 deletions src/cargo/core/compiler/context/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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)?,
Expand All @@ -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(),
})
Expand Down Expand Up @@ -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
Expand Down
2 changes: 1 addition & 1 deletion src/cargo/core/compiler/job_queue.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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:
//
Expand Down
1 change: 1 addition & 0 deletions src/cargo/util/config/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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>,
Expand Down
7 changes: 1 addition & 6 deletions src/doc/src/reference/config.md
Original file line number Diff line number Diff line change
Expand Up @@ -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`,
Expand Down Expand Up @@ -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]`

Expand Down
2 changes: 0 additions & 2 deletions src/doc/src/reference/environment-variables.md
Original file line number Diff line number Diff line change
Expand Up @@ -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`].
Expand Down Expand Up @@ -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
Expand Down
10 changes: 2 additions & 8 deletions tests/testsuite/build.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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]
Expand All @@ -5301,7 +5297,6 @@ fn pipelining_works() {
.build();

foo.cargo("build")
.env("CARGO_BUILD_PIPELINING", "true")
.with_stdout("")
.with_stderr(
"\
Expand Down Expand Up @@ -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();
Expand Down

0 comments on commit dc6d847

Please sign in to comment.