Remove build_helper

The majority of the code is only used by either rustbuild or rustc_llvm's build script. Rust_build is compiled once for rustbuild and once for every stage. This means that the majority of the code in this crate is needlessly compiled multiple times. By moving only the code actually used by the respective crates to rustbuild and rustc_llvm's build script, this needless duplicate compilation is avoided.
rust-lang · Mar 5, 2022 · 0cfc3e1 · 0cfc3e1
1 parent c8a49fc
commit 0cfc3e1
Show file tree

Hide file tree

Showing 27 changed files with 95 additions and 115 deletions.
diff --git a/Cargo.lock b/Cargo.lock
@@ -214,7 +214,6 @@ dependencies = [
 name = "bootstrap"
 version = "0.0.0"
 dependencies = [
- "build_helper",
  "cc",
  "cmake",
  "filetime",
@@ -256,10 +255,6 @@ dependencies = [
  "toml",
 ]
 
-[[package]]
-name = "build_helper"
-version = "0.1.0"
-
 [[package]]
 name = "bump-stage0"
 version = "0.1.0"
@@ -3891,7 +3886,6 @@ dependencies = [
 name = "rustc_llvm"
 version = "0.0.0"
 dependencies = [
- "build_helper",
  "cc",
  "libc",
 ]

diff --git a/compiler/rustc_llvm/Cargo.toml b/compiler/rustc_llvm/Cargo.toml
@@ -11,5 +11,4 @@ emscripten = []
 libc = "0.2.73"
 
 [build-dependencies]
-build_helper = { path = "../../src/build_helper" }
 cc = "1.0.69"
diff --git a/compiler/rustc_llvm/build.rs b/compiler/rustc_llvm/build.rs
@@ -1,8 +1,8 @@
 use std::env;
+use std::ffi::{OsStr, OsString};
+use std::fmt::Display;
 use std::path::{Path, PathBuf};
-use std::process::Command;
-
-use build_helper::{output, tracked_env_var_os};
+use std::process::{Command, Stdio};
 
 fn detect_llvm_link() -> (&'static str, &'static str) {
     // Force the link mode we want, preferring static by default, but
@@ -14,13 +14,74 @@ fn detect_llvm_link() -> (&'static str, &'static str) {
     }
 }
 
+// Because Cargo adds the compiler's dylib path to our library search path, llvm-config may
+// break: the dylib path for the compiler, as of this writing, contains a copy of the LLVM
+// shared library, which means that when our freshly built llvm-config goes to load it's
+// associated LLVM, it actually loads the compiler's LLVM. In particular when building the first
+// compiler (i.e., in stage 0) that's a problem, as the compiler's LLVM is likely different from
+// the one we want to use. As such, we restore the environment to what bootstrap saw. This isn't
+// perfect -- we might actually want to see something from Cargo's added library paths -- but
+// for now it works.
+fn restore_library_path() {
+    let key = tracked_env_var_os("REAL_LIBRARY_PATH_VAR").expect("REAL_LIBRARY_PATH_VAR");
+    if let Some(env) = tracked_env_var_os("REAL_LIBRARY_PATH") {
+        env::set_var(&key, &env);
+    } else {
+        env::remove_var(&key);
+    }
+}
+
+/// Reads an environment variable and adds it to dependencies.
+/// Supposed to be used for all variables except those set for build scripts by cargo
+/// <https://doc.rust-lang.org/cargo/reference/environment-variables.html#environment-variables-cargo-sets-for-build-scripts>
+fn tracked_env_var_os<K: AsRef<OsStr> + Display>(key: K) -> Option<OsString> {
+    println!("cargo:rerun-if-env-changed={}", key);
+    env::var_os(key)
+}
+
+fn rerun_if_changed_anything_in_dir(dir: &Path) {
+    let mut stack = dir
+        .read_dir()
+        .unwrap()
+        .map(|e| e.unwrap())
+        .filter(|e| &*e.file_name() != ".git")
+        .collect::<Vec<_>>();
+    while let Some(entry) = stack.pop() {
+        let path = entry.path();
+        if entry.file_type().unwrap().is_dir() {
+            stack.extend(path.read_dir().unwrap().map(|e| e.unwrap()));
+        } else {
+            println!("cargo:rerun-if-changed={}", path.display());
+        }
+    }
+}
+
+#[track_caller]
+fn output(cmd: &mut Command) -> String {
+    let output = match cmd.stderr(Stdio::inherit()).output() {
+        Ok(status) => status,
+        Err(e) => {
+            println!("\n\nfailed to execute command: {:?}\nerror: {}\n\n", cmd, e);
+            std::process::exit(1);
+        }
+    };
+    if !output.status.success() {
+        panic!(
+            "command did not execute successfully: {:?}\n\
+             expected success, got: {}",
+            cmd, output.status
+        );
+    }
+    String::from_utf8(output.stdout).unwrap()
+}
+
 fn main() {
     if tracked_env_var_os("RUST_CHECK").is_some() {
         // If we're just running `check`, there's no need for LLVM to be built.
         return;
     }
 
-    build_helper::restore_library_path();
+    restore_library_path();
 
     let target = env::var("TARGET").expect("TARGET was not set");
     let llvm_config =
@@ -160,7 +221,7 @@ fn main() {
         cfg.debug(false);
     }
 
-    build_helper::rerun_if_changed_anything_in_dir(Path::new("llvm-wrapper"));
+    rerun_if_changed_anything_in_dir(Path::new("llvm-wrapper"));
     cfg.file("llvm-wrapper/PassWrapper.cpp")
         .file("llvm-wrapper/RustWrapper.cpp")
         .file("llvm-wrapper/ArchiveWrapper.cpp")

diff --git a/src/bootstrap/Cargo.toml b/src/bootstrap/Cargo.toml
@@ -34,7 +34,6 @@ path = "bin/llvm-config-wrapper.rs"
 test = false
 
 [dependencies]
-build_helper = { path = "../build_helper" }
 cmake = "0.1.38"
 filetime = "0.2"
 getopts = "0.2.19"

diff --git a/src/build_helper/lib.rs → src/bootstrap/build_helper.rs b/src/build_helper/lib.rs → src/bootstrap/build_helper.rs
@@ -1,9 +1,7 @@
-use std::ffi::{OsStr, OsString};
-use std::fmt::Display;
+use std::fs;
 use std::path::{Path, PathBuf};
 use std::process::{Command, Stdio};
 use std::time::{SystemTime, UNIX_EPOCH};
-use std::{env, fs};
 
 /// A helper macro to `unwrap` a result except also print out details like:
 ///
@@ -13,7 +11,6 @@ use std::{env, fs};
 ///
 /// This is currently used judiciously throughout the build system rather than
 /// using a `Result` with `try!`, but this may change one day...
-#[macro_export]
 macro_rules! t {
     ($e:expr) => {
         match $e {
@@ -29,31 +26,7 @@ macro_rules! t {
         }
     };
 }
-
-/// Reads an environment variable and adds it to dependencies.
-/// Supposed to be used for all variables except those set for build scripts by cargo
-/// <https://doc.rust-lang.org/cargo/reference/environment-variables.html#environment-variables-cargo-sets-for-build-scripts>
-pub fn tracked_env_var_os<K: AsRef<OsStr> + Display>(key: K) -> Option<OsString> {
-    println!("cargo:rerun-if-env-changed={}", key);
-    env::var_os(key)
-}
-
-// Because Cargo adds the compiler's dylib path to our library search path, llvm-config may
-// break: the dylib path for the compiler, as of this writing, contains a copy of the LLVM
-// shared library, which means that when our freshly built llvm-config goes to load it's
-// associated LLVM, it actually loads the compiler's LLVM. In particular when building the first
-// compiler (i.e., in stage 0) that's a problem, as the compiler's LLVM is likely different from
-// the one we want to use. As such, we restore the environment to what bootstrap saw. This isn't
-// perfect -- we might actually want to see something from Cargo's added library paths -- but
-// for now it works.
-pub fn restore_library_path() {
-    let key = tracked_env_var_os("REAL_LIBRARY_PATH_VAR").expect("REAL_LIBRARY_PATH_VAR");
-    if let Some(env) = tracked_env_var_os("REAL_LIBRARY_PATH") {
-        env::set_var(&key, &env);
-    } else {
-        env::remove_var(&key);
-    }
-}
+pub(crate) use t;
 
 pub fn run(cmd: &mut Command, print_cmd_on_fail: bool) {
     if !try_run(cmd, print_cmd_on_fail) {
@@ -130,23 +103,6 @@ pub fn output(cmd: &mut Command) -> String {
     String::from_utf8(output.stdout).unwrap()
 }
 
-pub fn rerun_if_changed_anything_in_dir(dir: &Path) {
-    let mut stack = dir
-        .read_dir()
-        .unwrap()
-        .map(|e| e.unwrap())
-        .filter(|e| &*e.file_name() != ".git")
-        .collect::<Vec<_>>();
-    while let Some(entry) = stack.pop() {
-        let path = entry.path();
-        if entry.file_type().unwrap().is_dir() {
-            stack.extend(path.read_dir().unwrap().map(|e| e.unwrap()));
-        } else {
-            println!("cargo:rerun-if-changed={}", path.display());
-        }
-    }
-}
-
 /// Returns the last-modified time for `path`, or zero if it doesn't exist.
 pub fn mtime(path: &Path) -> SystemTime {
     fs::metadata(path).and_then(|f| f.modified()).unwrap_or(UNIX_EPOCH)

diff --git a/src/bootstrap/builder.rs b/src/bootstrap/builder.rs
@@ -11,8 +11,7 @@ use std::path::{Component, Path, PathBuf};
 use std::process::Command;
 use std::time::{Duration, Instant};
 
-use build_helper::{output, t};
-
+use crate::build_helper::{output, t};
 use crate::cache::{Cache, Interned, INTERNER};
 use crate::check;
 use crate::compile;

diff --git a/src/bootstrap/cc_detect.rs b/src/bootstrap/cc_detect.rs
@@ -26,8 +26,7 @@ use std::path::{Path, PathBuf};
 use std::process::Command;
 use std::{env, iter};
 
-use build_helper::output;
-
+use crate::build_helper::output;
 use crate::config::{Target, TargetSelection};
 use crate::{Build, CLang, GitRepo};
 

diff --git a/src/bootstrap/channel.rs b/src/bootstrap/channel.rs
@@ -8,8 +8,7 @@
 use std::path::Path;
 use std::process::Command;
 
-use build_helper::output;
-
+use crate::build_helper::output;
 use crate::Build;
 
 pub enum GitInfo {

diff --git a/src/bootstrap/clean.rs b/src/bootstrap/clean.rs
@@ -9,8 +9,7 @@ use std::fs;
 use std::io::{self, ErrorKind};
 use std::path::Path;
 
-use build_helper::t;
-
+use crate::build_helper::t;
 use crate::Build;
 
 pub fn clean(build: &Build, all: bool) {

diff --git a/src/bootstrap/compile.rs b/src/bootstrap/compile.rs
@@ -16,9 +16,9 @@ use std::path::{Path, PathBuf};
 use std::process::{exit, Command, Stdio};
 use std::str;
 
-use build_helper::{output, t, up_to_date};
 use serde::Deserialize;
 
+use crate::build_helper::{output, t, up_to_date};
 use crate::builder::Cargo;
 use crate::builder::{Builder, Kind, RunConfig, ShouldRun, Step};
 use crate::cache::{Interned, INTERNER};

diff --git a/src/bootstrap/config.rs b/src/bootstrap/config.rs
@@ -12,13 +12,13 @@ use std::fs;
 use std::path::{Path, PathBuf};
 use std::str::FromStr;
 
+use crate::build_helper::t;
 use crate::builder::TaskPath;
 use crate::cache::{Interned, INTERNER};
 use crate::channel::GitInfo;
 pub use crate::flags::Subcommand;
 use crate::flags::{Color, Flags};
 use crate::util::exe;
-use build_helper::t;
 use serde::Deserialize;
 
 macro_rules! check_ci_llvm {

diff --git a/src/bootstrap/dist.rs b/src/bootstrap/dist.rs
@@ -14,8 +14,7 @@ use std::fs;
 use std::path::{Path, PathBuf};
 use std::process::Command;
 
-use build_helper::{output, t};
-
+use crate::build_helper::{output, t};
 use crate::builder::{Builder, Kind, RunConfig, ShouldRun, Step};
 use crate::cache::{Interned, INTERNER};
 use crate::compile;
@@ -635,14 +634,6 @@ impl Step for RustcDev {
             &[],
             &tarball.image_dir().join("lib/rustlib/rustc-src/rust"),
         );
-        // This particular crate is used as a build dependency of the above.
-        copy_src_dirs(
-            builder,
-            &builder.src,
-            &["src/build_helper"],
-            &[],
-            &tarball.image_dir().join("lib/rustlib/rustc-src/rust"),
-        );
         for file in src_files {
             tarball.add_file(builder.src.join(file), "lib/rustlib/rustc-src/rust", 0o644);
         }

diff --git a/src/bootstrap/doc.rs b/src/bootstrap/doc.rs
@@ -12,15 +12,14 @@ use std::fs;
 use std::io;
 use std::path::{Path, PathBuf};
 
-use crate::Mode;
-use build_helper::{t, up_to_date};
-
+use crate::build_helper::{t, up_to_date};
 use crate::builder::{Builder, Compiler, Kind, RunConfig, ShouldRun, Step};
 use crate::cache::{Interned, INTERNER};
 use crate::compile;
 use crate::config::{Config, TargetSelection};
 use crate::tool::{self, prepare_tool_cargo, SourceType, Tool};
 use crate::util::symlink_dir;
+use crate::Mode;
 
 macro_rules! submodule_helper {
     ($path:expr, submodule) => {

diff --git a/src/bootstrap/flags.rs b/src/bootstrap/flags.rs
@@ -7,9 +7,9 @@ use std::env;
 use std::path::PathBuf;
 use std::process;
 
-use build_helper::t;
 use getopts::Options;
 
+use crate::build_helper::t;
 use crate::builder::Builder;
 use crate::config::{Config, TargetSelection};
 use crate::setup::Profile;

diff --git a/src/bootstrap/format.rs b/src/bootstrap/format.rs
@@ -1,7 +1,7 @@
 //! Runs rustfmt on the repository.
 
+use crate::build_helper::{output, t};
 use crate::Build;
-use build_helper::{output, t};
 use ignore::WalkBuilder;
 use std::collections::VecDeque;
 use std::path::{Path, PathBuf};

diff --git a/src/bootstrap/install.rs b/src/bootstrap/install.rs
@@ -8,7 +8,7 @@ use std::fs;
 use std::path::{Component, PathBuf};
 use std::process::Command;
 
-use build_helper::t;
+use crate::build_helper::t;
 
 use crate::dist::{self, sanitize_sh};
 use crate::tarball::GeneratedTarball;

diff --git a/src/bootstrap/lib.rs b/src/bootstrap/lib.rs
@@ -116,13 +116,14 @@ use std::os::unix::fs::symlink as symlink_file;
 #[cfg(windows)]
 use std::os::windows::fs::symlink_file;
 
-use build_helper::{mtime, output, run, run_suppressed, t, try_run, try_run_suppressed};
 use filetime::FileTime;
 
+use crate::build_helper::{mtime, output, run, run_suppressed, t, try_run, try_run_suppressed};
 use crate::builder::Kind;
 use crate::config::{LlvmLibunwind, TargetSelection};
 use crate::util::{exe, libdir, CiEnv};
 
+mod build_helper;
 mod builder;
 mod cache;
 mod cc_detect;
@@ -1301,13 +1302,10 @@ impl Build {
                 }
                 // Don't include optional deps if their features are not
                 // enabled. Ideally this would be computed from `cargo
-                // metadata --features …`, but that is somewhat slow. Just
-                // skip `build_helper` since there aren't any operations we
-                // want to perform on it. In the future, we may want to
-                // consider just filtering all build and dev dependencies in
-                // metadata::build.
+                // metadata --features …`, but that is somewhat slow. In
+                // the future, we may want to consider just filtering all
+                // build and dev dependencies in metadata::build.
                 if visited.insert(dep)
-                    && dep != "build_helper"
                     && (dep != "profiler_builtins"
                         || target
                             .map(|t| self.config.profiler_enabled(t))

diff --git a/src/bootstrap/metadata.rs b/src/bootstrap/metadata.rs
@@ -1,9 +1,9 @@
 use std::path::PathBuf;
 use std::process::Command;
 
-use build_helper::output;
 use serde::Deserialize;
 
+use crate::build_helper::output;
 use crate::cache::INTERNER;
 use crate::{Build, Crate};