From 0cfc3e101689d050432f600a35e953413b87147f Mon Sep 17 00:00:00 2001 From: bjorn3 Date: Thu, 24 Feb 2022 13:30:50 +0100 Subject: [PATCH] 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. --- Cargo.lock | 6 -- compiler/rustc_llvm/Cargo.toml | 1 - compiler/rustc_llvm/build.rs | 71 +++++++++++++++++-- src/bootstrap/Cargo.toml | 1 - .../lib.rs => bootstrap/build_helper.rs} | 48 +------------ src/bootstrap/builder.rs | 3 +- src/bootstrap/cc_detect.rs | 3 +- src/bootstrap/channel.rs | 3 +- src/bootstrap/clean.rs | 3 +- src/bootstrap/compile.rs | 2 +- src/bootstrap/config.rs | 2 +- src/bootstrap/dist.rs | 11 +-- src/bootstrap/doc.rs | 5 +- src/bootstrap/flags.rs | 2 +- src/bootstrap/format.rs | 2 +- src/bootstrap/install.rs | 2 +- src/bootstrap/lib.rs | 12 ++-- src/bootstrap/metadata.rs | 2 +- src/bootstrap/native.rs | 5 +- src/bootstrap/run.rs | 2 +- src/bootstrap/sanity.rs | 3 +- src/bootstrap/tarball.rs | 3 +- src/bootstrap/test.rs | 3 +- src/bootstrap/tool.rs | 3 +- src/bootstrap/toolstate.rs | 2 +- src/bootstrap/util.rs | 3 +- src/build_helper/Cargo.toml | 7 -- 27 files changed, 95 insertions(+), 115 deletions(-) rename src/{build_helper/lib.rs => bootstrap/build_helper.rs} (67%) delete mode 100644 src/build_helper/Cargo.toml diff --git a/Cargo.lock b/Cargo.lock index d7d221ee66f2a..2ee9a872d3ee2 100644 --- 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 index d8dfcc84e68da..34556df3c6d79 100644 --- 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 index 3b6808d693f21..ac758c15cca78 100644 --- 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 +/// +fn tracked_env_var_os + Display>(key: K) -> Option { + 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::>(); + 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 index 02efc08cc791f..fe9d6a727ed1e 100644 --- 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 b/src/bootstrap/build_helper.rs similarity index 67% rename from src/build_helper/lib.rs rename to src/bootstrap/build_helper.rs index 24aded547315e..320099102fc38 100644 --- a/src/build_helper/lib.rs +++ b/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 -/// -pub fn tracked_env_var_os + Display>(key: K) -> Option { - 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::>(); - 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 index 00aebc21f581a..531d9c27b4738 100644 --- 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 index 8c47f625d732b..9b19e22e0d01e 100644 --- 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 index 6478578c3c402..af5c2e1bcd616 100644 --- 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 index 3b73dc1c7df74..249ddf9c731ca 100644 --- 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 index 9971778034601..b9a80ce2d11ec 100644 --- 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 index 997e811e214df..f7c69e1161e05 100644 --- 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 index 8693e85e4742f..c0489de42de1d 100644 --- 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 index 23b5ddcd47a0e..710bf2493ec24 100644 --- 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 index 74528f2752f5f..d523171d5b1c2 100644 --- 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 index 530cc829320d1..5774a8ecf0916 100644 --- 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 index 06acf1a9a0083..5aa634ea3a75f 100644 --- 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 index 4089a63f881d1..81b54c852cd89 100644 --- 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 index 65e229697dc87..08e2b22fc1bdc 100644 --- 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}; diff --git a/src/bootstrap/native.rs b/src/bootstrap/native.rs index a751a6e3ece7f..2d7c1ebed6da3 100644 --- a/src/bootstrap/native.rs +++ b/src/bootstrap/native.rs @@ -16,13 +16,12 @@ use std::io; use std::path::{Path, PathBuf}; use std::process::Command; -use build_helper::{output, t}; - +use crate::build_helper::up_to_date; +use crate::build_helper::{output, t}; use crate::builder::{Builder, RunConfig, ShouldRun, Step}; use crate::config::TargetSelection; use crate::util::{self, exe}; use crate::{CLang, GitRepo}; -use build_helper::up_to_date; pub struct Meta { stamp: HashStamp, diff --git a/src/bootstrap/run.rs b/src/bootstrap/run.rs index 11b393857e74c..a9a784538b59a 100644 --- a/src/bootstrap/run.rs +++ b/src/bootstrap/run.rs @@ -1,7 +1,7 @@ +use crate::build_helper::output; use crate::builder::{Builder, RunConfig, ShouldRun, Step}; use crate::dist::distdir; use crate::tool::Tool; -use build_helper::output; use std::process::Command; #[derive(Debug, Copy, Clone, PartialEq, Eq, Hash)] diff --git a/src/bootstrap/sanity.rs b/src/bootstrap/sanity.rs index d7db2cef24f2b..36c9ba011c427 100644 --- a/src/bootstrap/sanity.rs +++ b/src/bootstrap/sanity.rs @@ -15,8 +15,7 @@ use std::fs; use std::path::PathBuf; use std::process::Command; -use build_helper::output; - +use crate::build_helper::output; use crate::cache::INTERNER; use crate::config::Target; use crate::Build; diff --git a/src/bootstrap/tarball.rs b/src/bootstrap/tarball.rs index 9ff5c2327e0f7..923522b90be04 100644 --- a/src/bootstrap/tarball.rs +++ b/src/bootstrap/tarball.rs @@ -3,8 +3,7 @@ use std::{ process::Command, }; -use build_helper::t; - +use crate::build_helper::t; use crate::builder::Builder; #[derive(Copy, Clone)] diff --git a/src/bootstrap/test.rs b/src/bootstrap/test.rs index 19d98df3ce902..714a2df34a741 100644 --- a/src/bootstrap/test.rs +++ b/src/bootstrap/test.rs @@ -11,8 +11,7 @@ use std::iter; use std::path::{Path, PathBuf}; use std::process::{Command, Stdio}; -use build_helper::{self, output, t}; - +use crate::build_helper::{self, output, t}; use crate::builder::{Builder, Compiler, Kind, RunConfig, ShouldRun, Step}; use crate::cache::Interned; use crate::compile; diff --git a/src/bootstrap/tool.rs b/src/bootstrap/tool.rs index 1317c3f983975..da2fbe4ef655a 100644 --- a/src/bootstrap/tool.rs +++ b/src/bootstrap/tool.rs @@ -4,8 +4,7 @@ use std::fs; use std::path::{Path, PathBuf}; use std::process::{exit, Command}; -use build_helper::t; - +use crate::build_helper::t; use crate::builder::{Builder, Cargo as CargoCommand, RunConfig, ShouldRun, Step}; use crate::channel::GitInfo; use crate::compile; diff --git a/src/bootstrap/toolstate.rs b/src/bootstrap/toolstate.rs index 08d0815806207..9880f413f04e6 100644 --- a/src/bootstrap/toolstate.rs +++ b/src/bootstrap/toolstate.rs @@ -1,5 +1,5 @@ +use crate::build_helper::t; use crate::builder::{Builder, RunConfig, ShouldRun, Step}; -use build_helper::t; use serde::{Deserialize, Serialize}; use std::collections::HashMap; use std::env; diff --git a/src/bootstrap/util.rs b/src/bootstrap/util.rs index 2c78ceb1e5bec..358bbd198106d 100644 --- a/src/bootstrap/util.rs +++ b/src/bootstrap/util.rs @@ -11,8 +11,7 @@ use std::process::Command; use std::str; use std::time::Instant; -use build_helper::t; - +use crate::build_helper::t; use crate::builder::Builder; use crate::config::{Config, TargetSelection}; diff --git a/src/build_helper/Cargo.toml b/src/build_helper/Cargo.toml deleted file mode 100644 index d88df0e08fab3..0000000000000 --- a/src/build_helper/Cargo.toml +++ /dev/null @@ -1,7 +0,0 @@ -[package] -name = "build_helper" -version = "0.1.0" -edition = "2021" - -[lib] -path = "lib.rs"