From 1b8dee19e8ddf560498e29a29b1efb62c080fa20 Mon Sep 17 00:00:00 2001 From: LuuuX Date: Wed, 18 Oct 2023 09:55:18 +0800 Subject: [PATCH] Fix issue #110087 Three tasks have been implemented here. Add a new `download-ci-llvm = if-unchange` option and enable if by default for `profile = codegen`. Include all build artifacts by traversing the llvm-project build output, Keep the downloadable llvm the same state as if you have just run a full source build. After selecting the codegen profile during ./x.py setup, the submodule will be automatically downloaded. --- config.example.toml | 10 +- src/bootstrap/defaults/config.codegen.toml | 2 +- src/bootstrap/download-ci-llvm-stamp | 2 +- src/bootstrap/src/core/build_steps/dist.rs | 26 +++-- src/bootstrap/src/core/build_steps/setup.rs | 9 ++ src/bootstrap/src/core/config/config.rs | 101 +++++++++++++++++--- src/bootstrap/src/lib.rs | 2 +- 7 files changed, 120 insertions(+), 32 deletions(-) diff --git a/config.example.toml b/config.example.toml index 4984cf8ba1e7c..e5df28a49af6c 100644 --- a/config.example.toml +++ b/config.example.toml @@ -30,7 +30,7 @@ # # If `change-id` does not match the version that is currently running, # `x.py` will prompt you to update it and check the related PR for more details. -change-id = 117435 +change-id = 116881 # ============================================================================= # Tweaking how LLVM is compiled @@ -42,11 +42,15 @@ change-id = 117435 # Unless you're developing for a target where Rust CI doesn't build a compiler # toolchain or changing LLVM locally, you probably want to leave this enabled. # -# All tier 1 targets are currently supported; set this to `"if-available"` if -# you are not sure whether you're on a tier 1 target. +# Set this to `"if-available"` if you are not sure whether you're on a tier 1 +# target. All tier 1 targets are currently supported; # # We also currently only support this when building LLVM for the build triple. # +# Set this to `"if-unchanged"` to only download if the llvm-project have not +# been modified. (If there are no changes or if built from tarball source, +# the logic is the same as "if-available") +# # Note that many of the LLVM options are not currently supported for # downloading. Currently only the "assertions" option can be toggled. #download-ci-llvm = if rust.channel == "dev" { "if-available" } else { false } diff --git a/src/bootstrap/defaults/config.codegen.toml b/src/bootstrap/defaults/config.codegen.toml index 113df88d7c349..7c33ce958c929 100644 --- a/src/bootstrap/defaults/config.codegen.toml +++ b/src/bootstrap/defaults/config.codegen.toml @@ -10,7 +10,7 @@ assertions = true # enable warnings during the llvm compilation enable-warnings = true # build llvm from source -download-ci-llvm = false +download-ci-llvm = "if-unchanged" [rust] # This enables `RUSTC_LOG=debug`, avoiding confusing situations diff --git a/src/bootstrap/download-ci-llvm-stamp b/src/bootstrap/download-ci-llvm-stamp index ffc38057900ad..9998fe2f5db8f 100644 --- a/src/bootstrap/download-ci-llvm-stamp +++ b/src/bootstrap/download-ci-llvm-stamp @@ -1,4 +1,4 @@ Change this file to make users of the `download-ci-llvm` configuration download a new version of LLVM from CI, even if the LLVM submodule hasn’t changed. -Last change is for: https://github.com/rust-lang/rust/pull/113996 +Last change is for: https://github.com/rust-lang/rust/pull/116881 diff --git a/src/bootstrap/src/core/build_steps/dist.rs b/src/bootstrap/src/core/build_steps/dist.rs index 6e80c55c8ce3c..950845b736f86 100644 --- a/src/bootstrap/src/core/build_steps/dist.rs +++ b/src/bootstrap/src/core/build_steps/dist.rs @@ -2219,23 +2219,19 @@ impl Step for RustDev { builder.ensure(crate::core::build_steps::llvm::Lld { target }); let src_bindir = builder.llvm_out(target).join("bin"); - // If updating this list, you likely want to change + // If updating this, you likely want to change // src/bootstrap/download-ci-llvm-stamp as well, otherwise local users // will not pick up the extra file until LLVM gets bumped. - for bin in &[ - "llvm-config", - "llvm-ar", - "llvm-objdump", - "llvm-profdata", - "llvm-bcanalyzer", - "llvm-cov", - "llvm-dwp", - "llvm-nm", - "llvm-dwarfdump", - "llvm-dis", - "llvm-tblgen", - ] { - tarball.add_file(src_bindir.join(exe(bin, target)), "bin", 0o755); + // We should include all the build artifacts obtained from a source build, + // so that you can use the downloadable LLVM as if you’ve just run a full source build. + if src_bindir.exists() { + for entry in walkdir::WalkDir::new(&src_bindir) { + let entry = t!(entry); + if entry.file_type().is_file() && !entry.path_is_symlink() { + let name = entry.file_name().to_str().unwrap(); + tarball.add_file(src_bindir.join(name), "bin", 0o755); + } + } } // We don't build LLD on some platforms, so only add it if it exists diff --git a/src/bootstrap/src/core/build_steps/setup.rs b/src/bootstrap/src/core/build_steps/setup.rs index b4540641bfa2a..04a1644377039 100644 --- a/src/bootstrap/src/core/build_steps/setup.rs +++ b/src/bootstrap/src/core/build_steps/setup.rs @@ -147,6 +147,15 @@ impl Step for Profile { } fn run(self, builder: &Builder<'_>) { + // During ./x.py setup once you select the codegen profile. + // The submodule will be downloaded. It does not work in the + // tarball case since they don't include Git and submodules + // are already included. + if !builder.rust_info().is_from_tarball() { + if self == Profile::Codegen { + builder.update_submodule(&Path::new("src/llvm-project")); + } + } setup(&builder.build.config, self) } } diff --git a/src/bootstrap/src/core/config/config.rs b/src/bootstrap/src/core/config/config.rs index a871399453e4f..2eb5d06bcb3bb 100644 --- a/src/bootstrap/src/core/config/config.rs +++ b/src/bootstrap/src/core/config/config.rs @@ -19,6 +19,7 @@ use std::process::Command; use std::str::FromStr; use crate::core::build_steps::compile::CODEGEN_BACKEND_PREFIX; +use crate::core::build_steps::llvm; use crate::core::config::flags::{Color, Flags, Warnings}; use crate::utils::cache::{Interned, INTERNER}; use crate::utils::channel::{self, GitInfo}; @@ -1530,17 +1531,7 @@ impl Config { config.llvm_build_config = llvm.build_config.clone().unwrap_or(Default::default()); let asserts = llvm_assertions.unwrap_or(false); - config.llvm_from_ci = match llvm.download_ci_llvm { - Some(StringOrBool::String(s)) => { - assert_eq!(s, "if-available", "unknown option `{s}` for download-ci-llvm"); - crate::core::build_steps::llvm::is_ci_llvm_available(&config, asserts) - } - Some(StringOrBool::Bool(b)) => b, - None => { - config.channel == "dev" - && crate::core::build_steps::llvm::is_ci_llvm_available(&config, asserts) - } - }; + config.llvm_from_ci = config.parse_download_ci_llvm(llvm.download_ci_llvm, asserts); if config.llvm_from_ci { // None of the LLVM options, except assertions, are supported @@ -2104,6 +2095,94 @@ impl Config { Some(commit.to_string()) } + + fn parse_download_ci_llvm( + &self, + download_ci_llvm: Option, + asserts: bool, + ) -> bool { + match download_ci_llvm { + None => self.channel == "dev" && llvm::is_ci_llvm_available(&self, asserts), + Some(StringOrBool::Bool(b)) => b, + Some(StringOrBool::String(s)) if s == "if-available" => { + llvm::is_ci_llvm_available(&self, asserts) + } + Some(StringOrBool::String(s)) if s == "if-unchanged" => { + // Git is needed to track modifications here, but tarball source is not available. + // If not modified here or built through tarball source, we maintain consistency + // with '"if available"'. + if !self.rust_info.is_from_tarball() + && self + .last_modified_commit(&["src/llvm-project"], "download-ci-llvm", true) + .is_none() + { + // there are some untracked changes in the the given paths. + false + } else { + llvm::is_ci_llvm_available(&self, asserts) + } + } + Some(StringOrBool::String(other)) => { + panic!("unrecognized option for download-ci-llvm: {:?}", other) + } + } + } + + /// Returns the last commit in which any of `modified_paths` were changed, + /// or `None` if there are untracked changes in the working directory and `if_unchanged` is true. + pub fn last_modified_commit( + &self, + modified_paths: &[&str], + option_name: &str, + if_unchanged: bool, + ) -> Option { + // Handle running from a directory other than the top level + let top_level = output(self.git().args(&["rev-parse", "--show-toplevel"])); + let top_level = top_level.trim_end(); + + // Look for a version to compare to based on the current commit. + // Only commits merged by bors will have CI artifacts. + let merge_base = output( + self.git() + .arg("rev-list") + .arg(format!("--author={}", self.stage0_metadata.config.git_merge_commit_email)) + .args(&["-n1", "--first-parent", "HEAD"]), + ); + let commit = merge_base.trim_end(); + if commit.is_empty() { + println!("error: could not find commit hash for downloading components from CI"); + println!("help: maybe your repository history is too shallow?"); + println!("help: consider disabling `{option_name}`"); + println!("help: or fetch enough history to include one upstream commit"); + crate::exit!(1); + } + + // Warn if there were changes to the compiler or standard library since the ancestor commit. + let mut git = self.git(); + git.args(&["diff-index", "--quiet", &commit, "--"]); + + for path in modified_paths { + git.arg(format!("{top_level}/{path}")); + } + + let has_changes = !t!(git.status()).success(); + if has_changes { + if if_unchanged { + if self.verbose > 0 { + println!( + "warning: saw changes to one of {modified_paths:?} since {commit}; \ + ignoring `{option_name}`" + ); + } + return None; + } + println!( + "warning: `{option_name}` is enabled, but there are changes to one of {modified_paths:?}" + ); + } + + Some(commit.to_string()) + } } fn set(field: &mut T, val: Option) { diff --git a/src/bootstrap/src/lib.rs b/src/bootstrap/src/lib.rs index 27922c9fbbe77..fc3413a3c8170 100644 --- a/src/bootstrap/src/lib.rs +++ b/src/bootstrap/src/lib.rs @@ -77,7 +77,7 @@ const LLD_FILE_NAMES: &[&str] = &["ld.lld", "ld64.lld", "lld-link", "wasm-ld"]; /// /// If you make any major changes (such as adding new values or changing default values), please /// ensure that the associated PR ID is added to the end of this list. -pub const CONFIG_CHANGE_HISTORY: &[usize] = &[115898, 116998, 117435]; +pub const CONFIG_CHANGE_HISTORY: &[usize] = &[115898, 116998, 117435, 116881]; /// Extra --check-cfg to add when building /// (Mode restriction, config name, config values (if any))