about summary refs log tree commit diff
path: root/src/bootstrap
diff options
context:
space:
mode:
authorJoshua Nelson <joshua@yottadb.com>2021-03-01 11:25:06 -0500
committerGitHub <noreply@github.com>2021-03-01 11:25:06 -0500
commit4f14f174859da3394f2014a4dcab59446c77b008 (patch)
treebc2a01401102e5bef6a9353a89dbdf2198499356 /src/bootstrap
parent57d7c7e3d98dd5d1216545f78caa3080edfec80f (diff)
parent8fb272c8e34cc3b2f7426743c6db398daba7645d (diff)
downloadrust-4f14f174859da3394f2014a4dcab59446c77b008.tar.gz
rust-4f14f174859da3394f2014a4dcab59446c77b008.zip
Rollup merge of #82480 - jyn514:no-enable-constant, r=Mark-Simulacrum
Remove `ENABLE_DOWNLOAD_RUSTC` constant

`ENABLE_DOWNLOAD_RUSTC` was introduced as part of the MVP for `download-rustc` as a way not to rebuild artifacts that have already been downloaded. Unfortunately, it doesn't work very well:

- Steps are ignored by default, which makes it easy to leave out a step
that should be built. For example, the MVP forgot to enable any tests,
so it was only possible to *build* locally.
- It didn't work correctly even when it was enabled: calling
  `builder.ensure()` would completely ignore the constant and rebuild the
  step anyway. This has no obvious fix since `ensure()` has to return a
  `Step::Output`.

Instead, this handles `download-rustc` in `impl Step for Rustc` and
`impl Step for Std`, which to my knowledge are the only build steps that
don't first go through `impl Step for Sysroot` (`Rustc` is used for
the `rustc-dev` component).

See https://github.com/rust-lang/rust/pull/79540#discussion_r563350075 and https://github.com/rust-lang/rust/issues/81930 for further context.

Here are some example runs with these changes and `download-rustc`
enabled:

```
$ x.py build src/tools/clippy
Building stage1 tool clippy-driver (x86_64-unknown-linux-gnu)
    Finished release [optimized] target(s) in 1m 09s
Building stage1 tool cargo-clippy (x86_64-unknown-linux-gnu)
    Finished release [optimized] target(s) in 0.11s
$ x.py test src/tools/clippy
    Finished dev [unoptimized + debuginfo] target(s) in 0.09s
Building stage1 tool clippy-driver (x86_64-unknown-linux-gnu)
    Finished release [optimized] target(s) in 0.09s
Building rustdoc for stage1 (x86_64-unknown-linux-gnu)
    Finished release [optimized] target(s) in 0.28s
    Finished release [optimized] target(s) in 15.26s
     Running build/x86_64-unknown-linux-gnu/stage1-tools/x86_64-unknown-linux-gnu/release/deps/clippy_driver-8b407b140e0aa91c
test result: ok. 592 passed; 0 failed; 3 ignored; 0 measured; 0 filtered out
$ x.py build src/tools/rustdoc
Building rustdoc for stage1 (x86_64-unknown-linux-gnu)
    Finished release [optimized] target(s) in 41.28s
Build completed successfully in 0:00:41
$ x.py test src/test/rustdoc-ui
Building stage0 tool compiletest (x86_64-unknown-linux-gnu)
    Finished release [optimized] target(s) in 0.12s
Building rustdoc for stage1 (x86_64-unknown-linux-gnu)
    Finished release [optimized] target(s) in 0.10s
test result: ok. 105 passed; 0 failed; 0 ignored; 0 measured; 0 filtered out; finished in 8.15s
$ x.py build compiler/rustc
    Finished dev [unoptimized + debuginfo] target(s) in 0.09s
Build completed successfully in 0:00:00
```

Note a few things:

- Clippy depends on stage1 rustc-dev artifacts, but rustc didn't have to
  be recompiled. Instead, the artifacts were copied automatically.
- All steps are always enabled. There is no danger of forgetting a step,
  since only the entrypoints have to handle `download-rustc`.
- Building the compiler (`compiler/rustc`) automatically does no work.

Helps with https://github.com/rust-lang/rust/issues/81930.

r? `@Mark-Simulacrum`
Diffstat (limited to 'src/bootstrap')
-rw-r--r--src/bootstrap/builder.rs18
-rw-r--r--src/bootstrap/check.rs4
-rw-r--r--src/bootstrap/compile.rs13
-rw-r--r--src/bootstrap/tool.rs1
4 files changed, 13 insertions, 23 deletions
diff --git a/src/bootstrap/builder.rs b/src/bootstrap/builder.rs
index 4a7c1850dd3..2008348ea8d 100644
--- a/src/bootstrap/builder.rs
+++ b/src/bootstrap/builder.rs
@@ -57,14 +57,6 @@ pub trait Step: 'static + Clone + Debug + PartialEq + Eq + Hash {
     /// `true` here can still be overwritten by `should_run` calling `default_condition`.
     const DEFAULT: bool = false;
 
-    /// Whether this step should be run even when `download-rustc` is set.
-    ///
-    /// Most steps are not important when the compiler is downloaded, since they will be included in
-    /// the pre-compiled sysroot. Steps can set this to `true` to be built anyway.
-    ///
-    /// When in doubt, set this to `false`.
-    const ENABLE_DOWNLOAD_RUSTC: bool = false;
-
     /// If true, then this rule should be skipped if --target was specified, but --host was not
     const ONLY_HOSTS: bool = false;
 
@@ -107,7 +99,6 @@ impl RunConfig<'_> {
 
 struct StepDescription {
     default: bool,
-    enable_download_rustc: bool,
     only_hosts: bool,
     should_run: fn(ShouldRun<'_>) -> ShouldRun<'_>,
     make_run: fn(RunConfig<'_>),
@@ -162,7 +153,6 @@ impl StepDescription {
     fn from<S: Step>() -> StepDescription {
         StepDescription {
             default: S::DEFAULT,
-            enable_download_rustc: S::ENABLE_DOWNLOAD_RUSTC,
             only_hosts: S::ONLY_HOSTS,
             should_run: S::should_run,
             make_run: S::make_run,
@@ -179,14 +169,6 @@ impl StepDescription {
                 "{:?} not skipped for {:?} -- not in {:?}",
                 pathset, self.name, builder.config.exclude
             );
-        } else if builder.config.download_rustc && !self.enable_download_rustc {
-            if !builder.config.dry_run {
-                eprintln!(
-                    "Not running {} because its artifacts have been downloaded from CI (`download-rustc` is set)",
-                    self.name
-                );
-            }
-            return;
         }
 
         // Determine the targets participating in this rule.
diff --git a/src/bootstrap/check.rs b/src/bootstrap/check.rs
index 9b80f1cf9fc..6626fead774 100644
--- a/src/bootstrap/check.rs
+++ b/src/bootstrap/check.rs
@@ -62,7 +62,6 @@ fn cargo_subcommand(kind: Kind) -> &'static str {
 impl Step for Std {
     type Output = ();
     const DEFAULT: bool = true;
-    const ENABLE_DOWNLOAD_RUSTC: bool = true;
 
     fn should_run(run: ShouldRun<'_>) -> ShouldRun<'_> {
         run.all_krates("test")
@@ -156,7 +155,6 @@ impl Step for Rustc {
     type Output = ();
     const ONLY_HOSTS: bool = true;
     const DEFAULT: bool = true;
-    const ENABLE_DOWNLOAD_RUSTC: bool = true;
 
     fn should_run(run: ShouldRun<'_>) -> ShouldRun<'_> {
         run.all_krates("rustc-main")
@@ -235,7 +233,6 @@ impl Step for CodegenBackend {
     type Output = ();
     const ONLY_HOSTS: bool = true;
     const DEFAULT: bool = true;
-    const ENABLE_DOWNLOAD_RUSTC: bool = true;
 
     fn should_run(run: ShouldRun<'_>) -> ShouldRun<'_> {
         run.paths(&["compiler/rustc_codegen_cranelift", "rustc_codegen_cranelift"])
@@ -293,7 +290,6 @@ macro_rules! tool_check_step {
             type Output = ();
             const ONLY_HOSTS: bool = true;
             const DEFAULT: bool = true;
-            const ENABLE_DOWNLOAD_RUSTC: bool = true;
 
             fn should_run(run: ShouldRun<'_>) -> ShouldRun<'_> {
                 run.path($path)
diff --git a/src/bootstrap/compile.rs b/src/bootstrap/compile.rs
index 7c8e02bdb8e..24800b7886d 100644
--- a/src/bootstrap/compile.rs
+++ b/src/bootstrap/compile.rs
@@ -63,6 +63,12 @@ impl Step for Std {
         let target = self.target;
         let compiler = self.compiler;
 
+        // These artifacts were already copied (in `impl Step for Sysroot`).
+        // Don't recompile them.
+        if builder.config.download_rustc {
+            return;
+        }
+
         if builder.config.keep_stage.contains(&compiler.stage)
             || builder.config.keep_stage_std.contains(&compiler.stage)
         {
@@ -507,6 +513,13 @@ impl Step for Rustc {
         let compiler = self.compiler;
         let target = self.target;
 
+        if builder.config.download_rustc {
+            // Copy the existing artifacts instead of rebuilding them.
+            // NOTE: this path is only taken for tools linking to rustc-dev.
+            builder.ensure(Sysroot { compiler });
+            return;
+        }
+
         builder.ensure(Std { compiler, target });
 
         if builder.config.keep_stage.contains(&compiler.stage) {
diff --git a/src/bootstrap/tool.rs b/src/bootstrap/tool.rs
index 71aa6bb150e..3fc3b68fd86 100644
--- a/src/bootstrap/tool.rs
+++ b/src/bootstrap/tool.rs
@@ -483,7 +483,6 @@ pub struct Rustdoc {
 impl Step for Rustdoc {
     type Output = PathBuf;
     const DEFAULT: bool = true;
-    const ENABLE_DOWNLOAD_RUSTC: bool = true;
     const ONLY_HOSTS: bool = true;
 
     fn should_run(run: ShouldRun<'_>) -> ShouldRun<'_> {