From b4675dcac87378d1e936ae928fe06f38c4cf6a30 Mon Sep 17 00:00:00 2001 From: bit-aloo Date: Fri, 8 Aug 2025 22:44:48 +0530 Subject: [PATCH 1/9] add download_ci_rustc_commit function and invoke from parse_inner --- src/bootstrap/src/core/config/config.rs | 129 +++++++++++++++++++++++- 1 file changed, 128 insertions(+), 1 deletion(-) diff --git a/src/bootstrap/src/core/config/config.rs b/src/bootstrap/src/core/config/config.rs index 2c008f957d98b..e7b7ab4f4d4bc 100644 --- a/src/bootstrap/src/core/config/config.rs +++ b/src/bootstrap/src/core/config/config.rs @@ -947,7 +947,14 @@ impl Config { ); } - config.download_rustc_commit = config.download_ci_rustc_commit( + config.download_rustc_commit = download_ci_rustc_commit( + &config.stage0_metadata, + config.path_modification_cache.clone(), + &config.src, + config.is_running_on_ci, + &config.exec_ctx, + &config.rust_info, + &config.host_target, rust_download_rustc, debug_assertions_requested, config.llvm_assertions, @@ -2335,3 +2342,123 @@ pub fn check_stage0_version( )); } } + +pub fn download_ci_rustc_commit( + stage0_metadata: &build_helper::stage0_parser::Stage0, + path_modification_cache: Arc, PathFreshness>>>, + src: &Path, + is_running_on_ci: bool, + exec_ctx: &ExecutionContext, + rust_info: &channel::GitInfo, + host_target: &TargetSelection, + download_rustc: Option, + debug_assertions_requested: bool, + llvm_assertions: bool, +) -> Option { + if !is_download_ci_available(&host_target.triple, llvm_assertions) { + return None; + } + + // If `download-rustc` is not set, default to rebuilding. + let if_unchanged = match download_rustc { + // Globally default `download-rustc` to `false`, because some contributors don't use + // profiles for reasons such as: + // - They need to seamlessly switch between compiler/library work. + // - They don't want to use compiler profile because they need to override too many + // things and it's easier to not use a profile. + None | Some(StringOrBool::Bool(false)) => return None, + Some(StringOrBool::Bool(true)) => false, + Some(StringOrBool::String(s)) if s == "if-unchanged" => { + if !rust_info.is_managed_git_subrepository() { + println!( + "ERROR: `download-rustc=if-unchanged` is only compatible with Git managed sources." + ); + crate::exit!(1); + } + + true + } + Some(StringOrBool::String(other)) => { + panic!("unrecognized option for download-rustc: {other}") + } + }; + + let commit = if rust_info.is_managed_git_subrepository() { + // Look for a version to compare to based on the current commit. + // Only commits merged by bors will have CI artifacts. + let freshness = check_path_modifications_( + stage0_metadata, + src, + path_modification_cache, + RUSTC_IF_UNCHANGED_ALLOWED_PATHS, + ); + exec_ctx.verbose(|| { + eprintln!("rustc freshness: {freshness:?}"); + }); + match freshness { + PathFreshness::LastModifiedUpstream { upstream } => upstream, + PathFreshness::HasLocalModifications { upstream } => { + if if_unchanged { + return None; + } + + if is_running_on_ci { + eprintln!("CI rustc commit matches with HEAD and we are in CI."); + eprintln!( + "`rustc.download-ci` functionality will be skipped as artifacts are not available." + ); + return None; + } + + upstream + } + PathFreshness::MissingUpstream => { + eprintln!("No upstream commit found"); + return None; + } + } + } else { + channel::read_commit_info_file(src) + .map(|info| info.sha.trim().to_owned()) + .expect("git-commit-info is missing in the project root") + }; + + if debug_assertions_requested { + eprintln!( + "WARN: `rust.debug-assertions = true` will prevent downloading CI rustc as alt CI \ + rustc is not currently built with debug assertions." + ); + return None; + } + + Some(commit) +} + +pub fn check_path_modifications_( + stage0_metadata: &build_helper::stage0_parser::Stage0, + src: &Path, + path_modification_cache: Arc, PathFreshness>>>, + paths: &[&'static str], +) -> PathFreshness { + // Checking path modifications through git can be relatively expensive (>100ms). + // We do not assume that the sources would change during bootstrap's execution, + // so we can cache the results here. + // Note that we do not use a static variable for the cache, because it would cause problems + // in tests that create separate `Config` instsances. + path_modification_cache + .lock() + .unwrap() + .entry(paths.to_vec()) + .or_insert_with(|| { + check_path_modifications(src, &git_config(stage0_metadata), paths, CiEnv::current()) + .unwrap() + }) + .clone() +} + +pub fn git_config(stage0_metadata: &build_helper::stage0_parser::Stage0) -> GitConfig<'_> { + GitConfig { + nightly_branch: &stage0_metadata.config.nightly_branch, + git_merge_commit_email: &stage0_metadata.config.git_merge_commit_email, + } +} From 111a0e8f2347c7e5996fe1e63d9ade86fba81e01 Mon Sep 17 00:00:00 2001 From: bit-aloo Date: Fri, 8 Aug 2025 23:21:57 +0530 Subject: [PATCH 2/9] add parse_download_ci_llvm function and invoke from parse_inner --- src/bootstrap/src/core/build_steps/llvm.rs | 9 +- src/bootstrap/src/core/config/config.rs | 246 ++++++++++++++++++++- 2 files changed, 248 insertions(+), 7 deletions(-) diff --git a/src/bootstrap/src/core/build_steps/llvm.rs b/src/bootstrap/src/core/build_steps/llvm.rs index 721ba6ca459e4..513d7260a3ece 100644 --- a/src/bootstrap/src/core/build_steps/llvm.rs +++ b/src/bootstrap/src/core/build_steps/llvm.rs @@ -196,7 +196,10 @@ pub(crate) fn detect_llvm_freshness(config: &Config, is_git: bool) -> PathFreshn /// /// This checks the build triple platform to confirm we're usable at all, and if LLVM /// with/without assertions is available. -pub(crate) fn is_ci_llvm_available_for_target(config: &Config, asserts: bool) -> bool { +pub(crate) fn is_ci_llvm_available_for_target( + host_target: &TargetSelection, + asserts: bool, +) -> bool { // This is currently all tier 1 targets and tier 2 targets with host tools // (since others may not have CI artifacts) // https://doc.rust-lang.org/rustc/platform-support.html#tier-1 @@ -235,8 +238,8 @@ pub(crate) fn is_ci_llvm_available_for_target(config: &Config, asserts: bool) -> ("x86_64-unknown-netbsd", false), ]; - if !supported_platforms.contains(&(&*config.host_target.triple, asserts)) - && (asserts || !supported_platforms.contains(&(&*config.host_target.triple, true))) + if !supported_platforms.contains(&(&*host_target.triple, asserts)) + && (asserts || !supported_platforms.contains(&(&*host_target.triple, true))) { return false; } diff --git a/src/bootstrap/src/core/config/config.rs b/src/bootstrap/src/core/config/config.rs index e7b7ab4f4d4bc..bfee43148c0e9 100644 --- a/src/bootstrap/src/core/config/config.rs +++ b/src/bootstrap/src/core/config/config.rs @@ -1200,8 +1200,19 @@ impl Config { config.llvm_enable_warnings = llvm_enable_warnings.unwrap_or(false); config.llvm_build_config = llvm_build_config.clone().unwrap_or(Default::default()); - config.llvm_from_ci = - config.parse_download_ci_llvm(llvm_download_ci_llvm, config.llvm_assertions); + config.llvm_from_ci = parse_download_ci_llvm( + &config.exec_ctx, + &config.submodules, + &config.stage0_metadata, + &config.src, + config.path_modification_cache.clone(), + &config.host_target, + &config.download_rustc_commit, + &config.rust_info, + config.is_running_on_ci, + llvm_download_ci_llvm, + config.llvm_assertions, + ); if config.llvm_from_ci { let warn = |option: &str| { @@ -1902,7 +1913,11 @@ impl Config { let has_changes = self.has_changes_from_upstream(LLVM_INVALIDATION_PATHS); // Return false if there are untracked changes, otherwise check if CI LLVM is available. - if has_changes { false } else { llvm::is_ci_llvm_available_for_target(self, asserts) } + if has_changes { + false + } else { + llvm::is_ci_llvm_available_for_target(&self.host_target, asserts) + } }; match download_ci_llvm { @@ -1921,7 +1936,7 @@ impl Config { } // If download-ci-llvm=true we also want to check that CI llvm is available - b && llvm::is_ci_llvm_available_for_target(self, asserts) + b && llvm::is_ci_llvm_available_for_target(&self.host_target, asserts) } StringOrBool::String(s) if s == "if-unchanged" => if_unchanged(), StringOrBool::String(other) => { @@ -2462,3 +2477,226 @@ pub fn git_config(stage0_metadata: &build_helper::stage0_parser::Stage0) -> GitC git_merge_commit_email: &stage0_metadata.config.git_merge_commit_email, } } + +pub fn parse_download_ci_llvm( + exec_ctx: &ExecutionContext, + submodules: &Option, + stage0_metadata: &build_helper::stage0_parser::Stage0, + src: &Path, + path_modification_cache: Arc, PathFreshness>>>, + host_target: &TargetSelection, + download_rustc_commit: &Option, + rust_info: &channel::GitInfo, + is_running_on_ci: bool, + download_ci_llvm: Option, + asserts: bool, +) -> bool { + // We don't ever want to use `true` on CI, as we should not + // download upstream artifacts if there are any local modifications. + let default = if is_running_on_ci { + StringOrBool::String("if-unchanged".to_string()) + } else { + StringOrBool::Bool(true) + }; + let download_ci_llvm = download_ci_llvm.unwrap_or(default); + + let if_unchanged = || { + if rust_info.is_from_tarball() { + // Git is needed for running "if-unchanged" logic. + println!("ERROR: 'if-unchanged' is only compatible with Git managed sources."); + crate::exit!(1); + } + + // Fetching the LLVM submodule is unnecessary for self-tests. + #[cfg(not(test))] + update_submodule(submodules, exec_ctx, src, rust_info, "src/llvm-project"); + + // Check for untracked changes in `src/llvm-project` and other important places. + let has_changes = has_changes_from_upstream( + stage0_metadata, + src, + path_modification_cache, + LLVM_INVALIDATION_PATHS, + ); + + // Return false if there are untracked changes, otherwise check if CI LLVM is available. + if has_changes { + false + } else { + llvm::is_ci_llvm_available_for_target(host_target, asserts) + } + }; + + match download_ci_llvm { + StringOrBool::Bool(b) => { + if !b && download_rustc_commit.is_some() { + panic!( + "`llvm.download-ci-llvm` cannot be set to `false` if `rust.download-rustc` is set to `true` or `if-unchanged`." + ); + } + + if b && is_running_on_ci { + // On CI, we must always rebuild LLVM if there were any modifications to it + panic!( + "`llvm.download-ci-llvm` cannot be set to `true` on CI. Use `if-unchanged` instead." + ); + } + + // If download-ci-llvm=true we also want to check that CI llvm is available + b && llvm::is_ci_llvm_available_for_target(host_target, asserts) + } + StringOrBool::String(s) if s == "if-unchanged" => if_unchanged(), + StringOrBool::String(other) => { + panic!("unrecognized option for download-ci-llvm: {other:?}") + } + } +} + +pub fn has_changes_from_upstream( + stage0_metadata: &build_helper::stage0_parser::Stage0, + src: &Path, + path_modification_cache: Arc, PathFreshness>>>, + paths: &[&'static str], +) -> bool { + match check_path_modifications_(stage0_metadata, src, path_modification_cache, paths) { + PathFreshness::LastModifiedUpstream { .. } => false, + PathFreshness::HasLocalModifications { .. } | PathFreshness::MissingUpstream => true, + } +} + +#[cfg_attr( + feature = "tracing", + instrument( + level = "trace", + name = "Config::update_submodule", + skip_all, + fields(relative_path = ?relative_path), + ), +)] +pub(crate) fn update_submodule( + submodules: &Option, + exec_ctx: &ExecutionContext, + src: &Path, + rust_info: &channel::GitInfo, + relative_path: &str, +) { + if rust_info.is_from_tarball() || !submodules_(submodules, rust_info) { + return; + } + + let absolute_path = src.join(relative_path); + + // NOTE: This check is required because `jj git clone` doesn't create directories for + // submodules, they are completely ignored. The code below assumes this directory exists, + // so create it here. + if !absolute_path.exists() { + t!(fs::create_dir_all(&absolute_path)); + } + + // NOTE: The check for the empty directory is here because when running x.py the first time, + // the submodule won't be checked out. Check it out now so we can build it. + if !git_info(exec_ctx, false, &absolute_path).is_managed_git_subrepository() + && !helpers::dir_is_empty(&absolute_path) + { + return; + } + + // Submodule updating actually happens during in the dry run mode. We need to make sure that + // all the git commands below are actually executed, because some follow-up code + // in bootstrap might depend on the submodules being checked out. Furthermore, not all + // the command executions below work with an empty output (produced during dry run). + // Therefore, all commands below are marked with `run_in_dry_run()`, so that they also run in + // dry run mode. + let submodule_git = || { + let mut cmd = helpers::git(Some(&absolute_path)); + cmd.run_in_dry_run(); + cmd + }; + + // Determine commit checked out in submodule. + let checked_out_hash = + submodule_git().args(["rev-parse", "HEAD"]).run_capture_stdout(exec_ctx).stdout(); + let checked_out_hash = checked_out_hash.trim_end(); + // Determine commit that the submodule *should* have. + let recorded = helpers::git(Some(src)) + .run_in_dry_run() + .args(["ls-tree", "HEAD"]) + .arg(relative_path) + .run_capture_stdout(exec_ctx) + .stdout(); + + let actual_hash = recorded + .split_whitespace() + .nth(2) + .unwrap_or_else(|| panic!("unexpected output `{recorded}`")); + + if actual_hash == checked_out_hash { + // already checked out + return; + } + + println!("Updating submodule {relative_path}"); + + helpers::git(Some(src)) + .allow_failure() + .run_in_dry_run() + .args(["submodule", "-q", "sync"]) + .arg(relative_path) + .run(exec_ctx); + + // Try passing `--progress` to start, then run git again without if that fails. + let update = |progress: bool| { + // Git is buggy and will try to fetch submodules from the tracking branch for *this* repository, + // even though that has no relation to the upstream for the submodule. + let current_branch = helpers::git(Some(src)) + .allow_failure() + .run_in_dry_run() + .args(["symbolic-ref", "--short", "HEAD"]) + .run_capture(exec_ctx); + + let mut git = helpers::git(Some(&src)).allow_failure(); + git.run_in_dry_run(); + if current_branch.is_success() { + // If there is a tag named after the current branch, git will try to disambiguate by prepending `heads/` to the branch name. + // This syntax isn't accepted by `branch.{branch}`. Strip it. + let branch = current_branch.stdout(); + let branch = branch.trim(); + let branch = branch.strip_prefix("heads/").unwrap_or(branch); + git.arg("-c").arg(format!("branch.{branch}.remote=origin")); + } + git.args(["submodule", "update", "--init", "--recursive", "--depth=1"]); + if progress { + git.arg("--progress"); + } + git.arg(relative_path); + git + }; + if !update(true).allow_failure().run(exec_ctx) { + update(false).allow_failure().run(exec_ctx); + } + + // Save any local changes, but avoid running `git stash pop` if there are none (since it will exit with an error). + // diff-index reports the modifications through the exit status + let has_local_modifications = + !submodule_git().allow_failure().args(["diff-index", "--quiet", "HEAD"]).run(exec_ctx); + if has_local_modifications { + submodule_git().allow_failure().args(["stash", "push"]).run(exec_ctx); + } + + submodule_git().allow_failure().args(["reset", "-q", "--hard"]).run(exec_ctx); + submodule_git().allow_failure().args(["clean", "-qdfx"]).run(exec_ctx); + + if has_local_modifications { + submodule_git().allow_failure().args(["stash", "pop"]).run(exec_ctx); + } +} + +pub fn git_info(exec_ctx: &ExecutionContext, omit_git_hash: bool, dir: &Path) -> GitInfo { + GitInfo::new(omit_git_hash, dir, exec_ctx) +} + +pub fn submodules_(submodules: &Option, rust_info: &channel::GitInfo) -> bool { + // If not specified in config, the default is to only manage + // submodules if we're currently inside a git repository. + submodules.unwrap_or(rust_info.is_managed_git_subrepository()) +} From 7eb8f6001e528e811018824e71e36547ea9b7e50 Mon Sep 17 00:00:00 2001 From: bit-aloo Date: Sat, 9 Aug 2025 07:23:05 +0530 Subject: [PATCH 3/9] add is_system_llvm function and invoke from parse_inner --- src/bootstrap/src/core/config/config.rs | 34 ++++++++++++++++++++++++- 1 file changed, 33 insertions(+), 1 deletion(-) diff --git a/src/bootstrap/src/core/config/config.rs b/src/bootstrap/src/core/config/config.rs index bfee43148c0e9..56c549297e27b 100644 --- a/src/bootstrap/src/core/config/config.rs +++ b/src/bootstrap/src/core/config/config.rs @@ -1315,7 +1315,14 @@ impl Config { ); } - if config.lld_enabled && config.is_system_llvm(config.host_target) { + if config.lld_enabled + && is_system_llvm( + &config.host_target, + config.llvm_from_ci, + &config.target_config, + config.host_target, + ) + { panic!("Cannot enable LLD with `rust.lld = true` when using external llvm-config."); } @@ -2700,3 +2707,28 @@ pub fn submodules_(submodules: &Option, rust_info: &channel::GitInfo) -> b // submodules if we're currently inside a git repository. submodules.unwrap_or(rust_info.is_managed_git_subrepository()) } + +/// Returns `true` if this is an external version of LLVM not managed by bootstrap. +/// In particular, we expect llvm sources to be available when this is false. +/// +/// NOTE: this is not the same as `!is_rust_llvm` when `llvm_has_patches` is set. +pub fn is_system_llvm( + host_target: &TargetSelection, + llvm_from_ci: bool, + target_config: &HashMap, + target: TargetSelection, +) -> bool { + match target_config.get(&target) { + Some(Target { llvm_config: Some(_), .. }) => { + let ci_llvm = llvm_from_ci && is_host_target(host_target, &target); + !ci_llvm + } + // We're building from the in-tree src/llvm-project sources. + Some(Target { llvm_config: None, .. }) => false, + None => false, + } +} + +pub fn is_host_target(host_target: &TargetSelection, target: &TargetSelection) -> bool { + host_target == target +} From 134fcc8c66808bb781ae67e786e46d63fc48b4b7 Mon Sep 17 00:00:00 2001 From: bit-aloo Date: Sat, 9 Aug 2025 07:31:55 +0530 Subject: [PATCH 4/9] add git_info function and invoke from parse_inner --- src/bootstrap/src/core/config/config.rs | 24 ++++++++++++++---------- 1 file changed, 14 insertions(+), 10 deletions(-) diff --git a/src/bootstrap/src/core/config/config.rs b/src/bootstrap/src/core/config/config.rs index 56c549297e27b..e5bee0e7ce56a 100644 --- a/src/bootstrap/src/core/config/config.rs +++ b/src/bootstrap/src/core/config/config.rs @@ -893,21 +893,25 @@ impl Config { let default = config.channel == "dev"; config.omit_git_hash = rust_omit_git_hash.unwrap_or(default); - config.rust_info = config.git_info(config.omit_git_hash, &config.src); + config.rust_info = git_info(&config.exec_ctx, config.omit_git_hash, &config.src); config.cargo_info = - config.git_info(config.omit_git_hash, &config.src.join("src/tools/cargo")); - config.rust_analyzer_info = - config.git_info(config.omit_git_hash, &config.src.join("src/tools/rust-analyzer")); + git_info(&config.exec_ctx, config.omit_git_hash, &config.src.join("src/tools/cargo")); + config.rust_analyzer_info = git_info( + &config.exec_ctx, + config.omit_git_hash, + &config.src.join("src/tools/rust-analyzer"), + ); config.clippy_info = - config.git_info(config.omit_git_hash, &config.src.join("src/tools/clippy")); + git_info(&config.exec_ctx, config.omit_git_hash, &config.src.join("src/tools/clippy")); config.miri_info = - config.git_info(config.omit_git_hash, &config.src.join("src/tools/miri")); + git_info(&config.exec_ctx, config.omit_git_hash, &config.src.join("src/tools/miri")); config.rustfmt_info = - config.git_info(config.omit_git_hash, &config.src.join("src/tools/rustfmt")); + git_info(&config.exec_ctx, config.omit_git_hash, &config.src.join("src/tools/rustfmt")); config.enzyme_info = - config.git_info(config.omit_git_hash, &config.src.join("src/tools/enzyme")); - config.in_tree_llvm_info = config.git_info(false, &config.src.join("src/llvm-project")); - config.in_tree_gcc_info = config.git_info(false, &config.src.join("src/gcc")); + git_info(&config.exec_ctx, config.omit_git_hash, &config.src.join("src/tools/enzyme")); + config.in_tree_llvm_info = + git_info(&config.exec_ctx, false, &config.src.join("src/llvm-project")); + config.in_tree_gcc_info = git_info(&config.exec_ctx, false, &config.src.join("src/gcc")); config.vendor = build_vendor.unwrap_or( config.rust_info.is_from_tarball() From 2213b7a8f9c3e778a32cddfc19ec58efefcb902c Mon Sep 17 00:00:00 2001 From: bit-aloo Date: Sat, 9 Aug 2025 07:47:26 +0530 Subject: [PATCH 5/9] add ci_llvm_root function and invoke from parse_inner --- src/bootstrap/src/core/config/config.rs | 12 +++++++++++- 1 file changed, 11 insertions(+), 1 deletion(-) diff --git a/src/bootstrap/src/core/config/config.rs b/src/bootstrap/src/core/config/config.rs index e5bee0e7ce56a..ef8d541500f90 100644 --- a/src/bootstrap/src/core/config/config.rs +++ b/src/bootstrap/src/core/config/config.rs @@ -1278,7 +1278,8 @@ impl Config { if config.llvm_from_ci { let triple = &config.host_target.triple; - let ci_llvm_bin = config.ci_llvm_root().join("bin"); + let ci_llvm_bin = + ci_llvm_root(config.llvm_from_ci, &config.out, &config.host_target).join("bin"); let build_target = config .target_config .entry(config.host_target) @@ -2736,3 +2737,12 @@ pub fn is_system_llvm( pub fn is_host_target(host_target: &TargetSelection, target: &TargetSelection) -> bool { host_target == target } + +pub(crate) fn ci_llvm_root( + llvm_from_ci: bool, + out: &Path, + host_target: &TargetSelection, +) -> PathBuf { + assert!(llvm_from_ci); + out.join(host_target).join("ci-llvm") +} From 621af4a580436013e7af848f5abea9bc21b21c9b Mon Sep 17 00:00:00 2001 From: bit-aloo Date: Sat, 9 Aug 2025 07:53:56 +0530 Subject: [PATCH 6/9] add read_file_by_commit function and invoke from parse_inner --- src/bootstrap/src/core/config/config.rs | 29 +++++++++++++++++++++++-- 1 file changed, 27 insertions(+), 2 deletions(-) diff --git a/src/bootstrap/src/core/config/config.rs b/src/bootstrap/src/core/config/config.rs index ef8d541500f90..af6edb1242cc5 100644 --- a/src/bootstrap/src/core/config/config.rs +++ b/src/bootstrap/src/core/config/config.rs @@ -1168,8 +1168,15 @@ impl Config { "WARNING: `rust.download-rustc` is enabled. The `rust.channel` option will be overridden by the CI rustc's channel." ); - let channel = - config.read_file_by_commit(Path::new("src/ci/channel"), commit).trim().to_owned(); + let channel = read_file_by_commit( + &config.exec_ctx, + &config.src, + &config.rust_info, + Path::new("src/ci/channel"), + commit, + ) + .trim() + .to_owned(); config.channel = channel; } @@ -2746,3 +2753,21 @@ pub(crate) fn ci_llvm_root( assert!(llvm_from_ci); out.join(host_target).join("ci-llvm") } + +/// Returns the content of the given file at a specific commit. +pub(crate) fn read_file_by_commit( + exec_ctx: &ExecutionContext, + src: &Path, + rust_info: &channel::GitInfo, + file: &Path, + commit: &str, +) -> String { + assert!( + rust_info.is_managed_git_subrepository(), + "`Config::read_file_by_commit` is not supported in non-git sources." + ); + + let mut git = helpers::git(Some(src)); + git.arg("show").arg(format!("{commit}:{}", file.to_str().unwrap())); + git.run_capture_stdout(exec_ctx).stdout() +} From aad54a8ff4d25d4a20b25ccfcfbae01c9a2310ba Mon Sep 17 00:00:00 2001 From: bit-aloo Date: Sat, 9 Aug 2025 08:41:16 +0530 Subject: [PATCH 7/9] invoke functions from methods --- src/bootstrap/src/core/config/config.rs | 232 +++--------------------- 1 file changed, 28 insertions(+), 204 deletions(-) diff --git a/src/bootstrap/src/core/config/config.rs b/src/bootstrap/src/core/config/config.rs index af6edb1242cc5..2912b3f30c303 100644 --- a/src/bootstrap/src/core/config/config.rs +++ b/src/bootstrap/src/core/config/config.rs @@ -1469,14 +1469,7 @@ impl Config { /// Returns the content of the given file at a specific commit. pub(crate) fn read_file_by_commit(&self, file: &Path, commit: &str) -> String { - assert!( - self.rust_info.is_managed_git_subrepository(), - "`Config::read_file_by_commit` is not supported in non-git sources." - ); - - let mut git = helpers::git(Some(&self.src)); - git.arg("show").arg(format!("{commit}:{}", file.to_str().unwrap())); - git.run_capture_stdout(self).stdout() + read_file_by_commit(&self.exec_ctx, &self.src, &self.rust_info, file, commit) } /// Bootstrap embeds a version number into the name of shared libraries it uploads in CI. @@ -1547,8 +1540,7 @@ impl Config { /// The absolute path to the downloaded LLVM artifacts. pub(crate) fn ci_llvm_root(&self) -> PathBuf { - assert!(self.llvm_from_ci); - self.out.join(self.host_target).join("ci-llvm") + ci_llvm_root(self.llvm_from_ci, &self.out, &self.host_target) } /// Directory where the extracted `rustc-dev` component is stored. @@ -1711,115 +1703,13 @@ impl Config { ), )] pub(crate) fn update_submodule(&self, relative_path: &str) { - if self.rust_info.is_from_tarball() || !self.submodules() { - return; - } - - let absolute_path = self.src.join(relative_path); - - // NOTE: This check is required because `jj git clone` doesn't create directories for - // submodules, they are completely ignored. The code below assumes this directory exists, - // so create it here. - if !absolute_path.exists() { - t!(fs::create_dir_all(&absolute_path)); - } - - // NOTE: The check for the empty directory is here because when running x.py the first time, - // the submodule won't be checked out. Check it out now so we can build it. - if !self.git_info(false, &absolute_path).is_managed_git_subrepository() - && !helpers::dir_is_empty(&absolute_path) - { - return; - } - - // Submodule updating actually happens during in the dry run mode. We need to make sure that - // all the git commands below are actually executed, because some follow-up code - // in bootstrap might depend on the submodules being checked out. Furthermore, not all - // the command executions below work with an empty output (produced during dry run). - // Therefore, all commands below are marked with `run_in_dry_run()`, so that they also run in - // dry run mode. - let submodule_git = || { - let mut cmd = helpers::git(Some(&absolute_path)); - cmd.run_in_dry_run(); - cmd - }; - - // Determine commit checked out in submodule. - let checked_out_hash = - submodule_git().args(["rev-parse", "HEAD"]).run_capture_stdout(self).stdout(); - let checked_out_hash = checked_out_hash.trim_end(); - // Determine commit that the submodule *should* have. - let recorded = helpers::git(Some(&self.src)) - .run_in_dry_run() - .args(["ls-tree", "HEAD"]) - .arg(relative_path) - .run_capture_stdout(self) - .stdout(); - - let actual_hash = recorded - .split_whitespace() - .nth(2) - .unwrap_or_else(|| panic!("unexpected output `{recorded}`")); - - if actual_hash == checked_out_hash { - // already checked out - return; - } - - println!("Updating submodule {relative_path}"); - - helpers::git(Some(&self.src)) - .allow_failure() - .run_in_dry_run() - .args(["submodule", "-q", "sync"]) - .arg(relative_path) - .run(self); - - // Try passing `--progress` to start, then run git again without if that fails. - let update = |progress: bool| { - // Git is buggy and will try to fetch submodules from the tracking branch for *this* repository, - // even though that has no relation to the upstream for the submodule. - let current_branch = helpers::git(Some(&self.src)) - .allow_failure() - .run_in_dry_run() - .args(["symbolic-ref", "--short", "HEAD"]) - .run_capture(self); - - let mut git = helpers::git(Some(&self.src)).allow_failure(); - git.run_in_dry_run(); - if current_branch.is_success() { - // If there is a tag named after the current branch, git will try to disambiguate by prepending `heads/` to the branch name. - // This syntax isn't accepted by `branch.{branch}`. Strip it. - let branch = current_branch.stdout(); - let branch = branch.trim(); - let branch = branch.strip_prefix("heads/").unwrap_or(branch); - git.arg("-c").arg(format!("branch.{branch}.remote=origin")); - } - git.args(["submodule", "update", "--init", "--recursive", "--depth=1"]); - if progress { - git.arg("--progress"); - } - git.arg(relative_path); - git - }; - if !update(true).allow_failure().run(self) { - update(false).allow_failure().run(self); - } - - // Save any local changes, but avoid running `git stash pop` if there are none (since it will exit with an error). - // diff-index reports the modifications through the exit status - let has_local_modifications = - !submodule_git().allow_failure().args(["diff-index", "--quiet", "HEAD"]).run(self); - if has_local_modifications { - submodule_git().allow_failure().args(["stash", "push"]).run(self); - } - - submodule_git().allow_failure().args(["reset", "-q", "--hard"]).run(self); - submodule_git().allow_failure().args(["clean", "-qdfx"]).run(self); - - if has_local_modifications { - submodule_git().allow_failure().args(["stash", "pop"]).run(self); - } + update_submodule( + &self.submodules, + &self.exec_ctx, + &self.src, + &self.rust_info, + relative_path, + ); } /// Returns the commit to download, or `None` if we shouldn't download CI artifacts. @@ -1829,78 +1719,18 @@ impl Config { debug_assertions_requested: bool, llvm_assertions: bool, ) -> Option { - if !is_download_ci_available(&self.host_target.triple, llvm_assertions) { - return None; - } - - // If `download-rustc` is not set, default to rebuilding. - let if_unchanged = match download_rustc { - // Globally default `download-rustc` to `false`, because some contributors don't use - // profiles for reasons such as: - // - They need to seamlessly switch between compiler/library work. - // - They don't want to use compiler profile because they need to override too many - // things and it's easier to not use a profile. - None | Some(StringOrBool::Bool(false)) => return None, - Some(StringOrBool::Bool(true)) => false, - Some(StringOrBool::String(s)) if s == "if-unchanged" => { - if !self.rust_info.is_managed_git_subrepository() { - println!( - "ERROR: `download-rustc=if-unchanged` is only compatible with Git managed sources." - ); - crate::exit!(1); - } - - true - } - Some(StringOrBool::String(other)) => { - panic!("unrecognized option for download-rustc: {other}") - } - }; - - let commit = if self.rust_info.is_managed_git_subrepository() { - // Look for a version to compare to based on the current commit. - // Only commits merged by bors will have CI artifacts. - let freshness = self.check_path_modifications(RUSTC_IF_UNCHANGED_ALLOWED_PATHS); - self.verbose(|| { - eprintln!("rustc freshness: {freshness:?}"); - }); - match freshness { - PathFreshness::LastModifiedUpstream { upstream } => upstream, - PathFreshness::HasLocalModifications { upstream } => { - if if_unchanged { - return None; - } - - if self.is_running_on_ci { - eprintln!("CI rustc commit matches with HEAD and we are in CI."); - eprintln!( - "`rustc.download-ci` functionality will be skipped as artifacts are not available." - ); - return None; - } - - upstream - } - PathFreshness::MissingUpstream => { - eprintln!("No upstream commit found"); - return None; - } - } - } else { - channel::read_commit_info_file(&self.src) - .map(|info| info.sha.trim().to_owned()) - .expect("git-commit-info is missing in the project root") - }; - - if debug_assertions_requested { - eprintln!( - "WARN: `rust.debug-assertions = true` will prevent downloading CI rustc as alt CI \ - rustc is not currently built with debug assertions." - ); - return None; - } - - Some(commit) + download_ci_rustc_commit( + &self.stage0_metadata, + self.path_modification_cache.clone(), + &self.src, + self.is_running_on_ci, + &self.exec_ctx, + &self.rust_info, + &self.host_target, + download_rustc, + debug_assertions_requested, + llvm_assertions, + ) } pub fn parse_download_ci_llvm( @@ -1966,10 +1796,12 @@ impl Config { /// Returns true if any of the `paths` have been modified locally. pub fn has_changes_from_upstream(&self, paths: &[&'static str]) -> bool { - match self.check_path_modifications(paths) { - PathFreshness::LastModifiedUpstream { .. } => false, - PathFreshness::HasLocalModifications { .. } | PathFreshness::MissingUpstream => true, - } + has_changes_from_upstream( + &self.stage0_metadata, + &self.src, + self.path_modification_cache.clone(), + paths, + ) } /// Checks whether any of the given paths have been modified w.r.t. upstream. @@ -2077,15 +1909,7 @@ impl Config { /// /// NOTE: this is not the same as `!is_rust_llvm` when `llvm_has_patches` is set. pub fn is_system_llvm(&self, target: TargetSelection) -> bool { - match self.target_config.get(&target) { - Some(Target { llvm_config: Some(_), .. }) => { - let ci_llvm = self.llvm_from_ci && self.is_host_target(target); - !ci_llvm - } - // We're building from the in-tree src/llvm-project sources. - Some(Target { llvm_config: None, .. }) => false, - None => false, - } + is_system_llvm(&self.host_target, self.llvm_from_ci, &self.target_config, target) } /// Returns `true` if this is our custom, patched, version of LLVM. From 8612db2f3e5e85713e9aacfc794b328810e651f8 Mon Sep 17 00:00:00 2001 From: bit-aloo Date: Sat, 9 Aug 2025 09:52:13 +0530 Subject: [PATCH 8/9] extend download context and change functions to directly use that --- src/bootstrap/src/core/config/config.rs | 320 ++++++++---------------- src/bootstrap/src/core/download.rs | 37 ++- 2 files changed, 130 insertions(+), 227 deletions(-) diff --git a/src/bootstrap/src/core/config/config.rs b/src/bootstrap/src/core/config/config.rs index 2912b3f30c303..73a20df9b4e30 100644 --- a/src/bootstrap/src/core/config/config.rs +++ b/src/bootstrap/src/core/config/config.rs @@ -951,14 +951,9 @@ impl Config { ); } + let dwn_ctx = DownloadContext::from(&config); config.download_rustc_commit = download_ci_rustc_commit( - &config.stage0_metadata, - config.path_modification_cache.clone(), - &config.src, - config.is_running_on_ci, - &config.exec_ctx, - &config.rust_info, - &config.host_target, + dwn_ctx, rust_download_rustc, debug_assertions_requested, config.llvm_assertions, @@ -1168,15 +1163,9 @@ impl Config { "WARNING: `rust.download-rustc` is enabled. The `rust.channel` option will be overridden by the CI rustc's channel." ); - let channel = read_file_by_commit( - &config.exec_ctx, - &config.src, - &config.rust_info, - Path::new("src/ci/channel"), - commit, - ) - .trim() - .to_owned(); + let dwn_ctx = DownloadContext::from(&config); + let channel = + read_file_by_commit(dwn_ctx, Path::new("src/ci/channel"), commit).trim().to_owned(); config.channel = channel; } @@ -1211,19 +1200,9 @@ impl Config { config.llvm_enable_warnings = llvm_enable_warnings.unwrap_or(false); config.llvm_build_config = llvm_build_config.clone().unwrap_or(Default::default()); - config.llvm_from_ci = parse_download_ci_llvm( - &config.exec_ctx, - &config.submodules, - &config.stage0_metadata, - &config.src, - config.path_modification_cache.clone(), - &config.host_target, - &config.download_rustc_commit, - &config.rust_info, - config.is_running_on_ci, - llvm_download_ci_llvm, - config.llvm_assertions, - ); + let dwn_ctx = DownloadContext::from(&config); + config.llvm_from_ci = + parse_download_ci_llvm(dwn_ctx, llvm_download_ci_llvm, config.llvm_assertions); if config.llvm_from_ci { let warn = |option: &str| { @@ -1285,8 +1264,8 @@ impl Config { if config.llvm_from_ci { let triple = &config.host_target.triple; - let ci_llvm_bin = - ci_llvm_root(config.llvm_from_ci, &config.out, &config.host_target).join("bin"); + let dwn_ctx = DownloadContext::from(&config); + let ci_llvm_bin = ci_llvm_root(dwn_ctx).join("bin"); let build_target = config .target_config .entry(config.host_target) @@ -1327,14 +1306,8 @@ impl Config { ); } - if config.lld_enabled - && is_system_llvm( - &config.host_target, - config.llvm_from_ci, - &config.target_config, - config.host_target, - ) - { + let dwn_ctx = DownloadContext::from(&config); + if config.lld_enabled && is_system_llvm(dwn_ctx, config.host_target) { panic!("Cannot enable LLD with `rust.lld = true` when using external llvm-config."); } @@ -1469,7 +1442,8 @@ impl Config { /// Returns the content of the given file at a specific commit. pub(crate) fn read_file_by_commit(&self, file: &Path, commit: &str) -> String { - read_file_by_commit(&self.exec_ctx, &self.src, &self.rust_info, file, commit) + let dwn_ctx = DownloadContext::from(self); + read_file_by_commit(dwn_ctx, file, commit) } /// Bootstrap embeds a version number into the name of shared libraries it uploads in CI. @@ -1540,7 +1514,8 @@ impl Config { /// The absolute path to the downloaded LLVM artifacts. pub(crate) fn ci_llvm_root(&self) -> PathBuf { - ci_llvm_root(self.llvm_from_ci, &self.out, &self.host_target) + let dwn_ctx = DownloadContext::from(self); + ci_llvm_root(dwn_ctx) } /// Directory where the extracted `rustc-dev` component is stored. @@ -1703,13 +1678,8 @@ impl Config { ), )] pub(crate) fn update_submodule(&self, relative_path: &str) { - update_submodule( - &self.submodules, - &self.exec_ctx, - &self.src, - &self.rust_info, - relative_path, - ); + let dwn_ctx = DownloadContext::from(self); + update_submodule(dwn_ctx, relative_path); } /// Returns the commit to download, or `None` if we shouldn't download CI artifacts. @@ -1719,14 +1689,9 @@ impl Config { debug_assertions_requested: bool, llvm_assertions: bool, ) -> Option { + let dwn_ctx = DownloadContext::from(self); download_ci_rustc_commit( - &self.stage0_metadata, - self.path_modification_cache.clone(), - &self.src, - self.is_running_on_ci, - &self.exec_ctx, - &self.rust_info, - &self.host_target, + dwn_ctx, download_rustc, debug_assertions_requested, llvm_assertions, @@ -1738,70 +1703,14 @@ impl Config { download_ci_llvm: Option, asserts: bool, ) -> bool { - // We don't ever want to use `true` on CI, as we should not - // download upstream artifacts if there are any local modifications. - let default = if self.is_running_on_ci { - StringOrBool::String("if-unchanged".to_string()) - } else { - StringOrBool::Bool(true) - }; - let download_ci_llvm = download_ci_llvm.unwrap_or(default); - - let if_unchanged = || { - if self.rust_info.is_from_tarball() { - // Git is needed for running "if-unchanged" logic. - println!("ERROR: 'if-unchanged' is only compatible with Git managed sources."); - crate::exit!(1); - } - - // Fetching the LLVM submodule is unnecessary for self-tests. - #[cfg(not(test))] - self.update_submodule("src/llvm-project"); - - // Check for untracked changes in `src/llvm-project` and other important places. - let has_changes = self.has_changes_from_upstream(LLVM_INVALIDATION_PATHS); - - // Return false if there are untracked changes, otherwise check if CI LLVM is available. - if has_changes { - false - } else { - llvm::is_ci_llvm_available_for_target(&self.host_target, asserts) - } - }; - - match download_ci_llvm { - StringOrBool::Bool(b) => { - if !b && self.download_rustc_commit.is_some() { - panic!( - "`llvm.download-ci-llvm` cannot be set to `false` if `rust.download-rustc` is set to `true` or `if-unchanged`." - ); - } - - if b && self.is_running_on_ci { - // On CI, we must always rebuild LLVM if there were any modifications to it - panic!( - "`llvm.download-ci-llvm` cannot be set to `true` on CI. Use `if-unchanged` instead." - ); - } - - // If download-ci-llvm=true we also want to check that CI llvm is available - b && llvm::is_ci_llvm_available_for_target(&self.host_target, asserts) - } - StringOrBool::String(s) if s == "if-unchanged" => if_unchanged(), - StringOrBool::String(other) => { - panic!("unrecognized option for download-ci-llvm: {other:?}") - } - } + let dwn_ctx = DownloadContext::from(self); + parse_download_ci_llvm(dwn_ctx, download_ci_llvm, asserts) } /// Returns true if any of the `paths` have been modified locally. pub fn has_changes_from_upstream(&self, paths: &[&'static str]) -> bool { - has_changes_from_upstream( - &self.stage0_metadata, - &self.src, - self.path_modification_cache.clone(), - paths, - ) + let dwn_ctx = DownloadContext::from(self); + has_changes_from_upstream(dwn_ctx, paths) } /// Checks whether any of the given paths have been modified w.r.t. upstream. @@ -1909,7 +1818,8 @@ impl Config { /// /// NOTE: this is not the same as `!is_rust_llvm` when `llvm_has_patches` is set. pub fn is_system_llvm(&self, target: TargetSelection) -> bool { - is_system_llvm(&self.host_target, self.llvm_from_ci, &self.target_config, target) + let dwn_ctx = DownloadContext::from(self); + is_system_llvm(dwn_ctx, target) } /// Returns `true` if this is our custom, patched, version of LLVM. @@ -2201,19 +2111,15 @@ pub fn check_stage0_version( } } -pub fn download_ci_rustc_commit( - stage0_metadata: &build_helper::stage0_parser::Stage0, - path_modification_cache: Arc, PathFreshness>>>, - src: &Path, - is_running_on_ci: bool, - exec_ctx: &ExecutionContext, - rust_info: &channel::GitInfo, - host_target: &TargetSelection, +pub fn download_ci_rustc_commit<'a>( + dwn_ctx: impl AsRef>, download_rustc: Option, debug_assertions_requested: bool, llvm_assertions: bool, ) -> Option { - if !is_download_ci_available(&host_target.triple, llvm_assertions) { + let dwn_ctx = dwn_ctx.as_ref(); + + if !is_download_ci_available(&dwn_ctx.host_target.triple, llvm_assertions) { return None; } @@ -2227,7 +2133,7 @@ pub fn download_ci_rustc_commit( None | Some(StringOrBool::Bool(false)) => return None, Some(StringOrBool::Bool(true)) => false, Some(StringOrBool::String(s)) if s == "if-unchanged" => { - if !rust_info.is_managed_git_subrepository() { + if !dwn_ctx.rust_info.is_managed_git_subrepository() { println!( "ERROR: `download-rustc=if-unchanged` is only compatible with Git managed sources." ); @@ -2241,16 +2147,11 @@ pub fn download_ci_rustc_commit( } }; - let commit = if rust_info.is_managed_git_subrepository() { + let commit = if dwn_ctx.rust_info.is_managed_git_subrepository() { // Look for a version to compare to based on the current commit. // Only commits merged by bors will have CI artifacts. - let freshness = check_path_modifications_( - stage0_metadata, - src, - path_modification_cache, - RUSTC_IF_UNCHANGED_ALLOWED_PATHS, - ); - exec_ctx.verbose(|| { + let freshness = check_path_modifications_(dwn_ctx, RUSTC_IF_UNCHANGED_ALLOWED_PATHS); + dwn_ctx.exec_ctx.verbose(|| { eprintln!("rustc freshness: {freshness:?}"); }); match freshness { @@ -2260,7 +2161,7 @@ pub fn download_ci_rustc_commit( return None; } - if is_running_on_ci { + if dwn_ctx.is_running_on_ci { eprintln!("CI rustc commit matches with HEAD and we are in CI."); eprintln!( "`rustc.download-ci` functionality will be skipped as artifacts are not available." @@ -2276,7 +2177,7 @@ pub fn download_ci_rustc_commit( } } } else { - channel::read_commit_info_file(src) + channel::read_commit_info_file(dwn_ctx.src) .map(|info| info.sha.trim().to_owned()) .expect("git-commit-info is missing in the project root") }; @@ -2292,24 +2193,29 @@ pub fn download_ci_rustc_commit( Some(commit) } -pub fn check_path_modifications_( - stage0_metadata: &build_helper::stage0_parser::Stage0, - src: &Path, - path_modification_cache: Arc, PathFreshness>>>, +pub fn check_path_modifications_<'a>( + dwn_ctx: impl AsRef>, paths: &[&'static str], ) -> PathFreshness { + let dwn_ctx = dwn_ctx.as_ref(); // Checking path modifications through git can be relatively expensive (>100ms). // We do not assume that the sources would change during bootstrap's execution, // so we can cache the results here. // Note that we do not use a static variable for the cache, because it would cause problems // in tests that create separate `Config` instsances. - path_modification_cache + dwn_ctx + .path_modification_cache .lock() .unwrap() .entry(paths.to_vec()) .or_insert_with(|| { - check_path_modifications(src, &git_config(stage0_metadata), paths, CiEnv::current()) - .unwrap() + check_path_modifications( + dwn_ctx.src, + &git_config(dwn_ctx.stage0_metadata), + paths, + CiEnv::current(), + ) + .unwrap() }) .clone() } @@ -2321,22 +2227,16 @@ pub fn git_config(stage0_metadata: &build_helper::stage0_parser::Stage0) -> GitC } } -pub fn parse_download_ci_llvm( - exec_ctx: &ExecutionContext, - submodules: &Option, - stage0_metadata: &build_helper::stage0_parser::Stage0, - src: &Path, - path_modification_cache: Arc, PathFreshness>>>, - host_target: &TargetSelection, - download_rustc_commit: &Option, - rust_info: &channel::GitInfo, - is_running_on_ci: bool, +pub fn parse_download_ci_llvm<'a>( + dwn_ctx: impl AsRef>, download_ci_llvm: Option, asserts: bool, ) -> bool { + let dwn_ctx = dwn_ctx.as_ref(); + // We don't ever want to use `true` on CI, as we should not // download upstream artifacts if there are any local modifications. - let default = if is_running_on_ci { + let default = if dwn_ctx.is_running_on_ci { StringOrBool::String("if-unchanged".to_string()) } else { StringOrBool::Bool(true) @@ -2344,7 +2244,7 @@ pub fn parse_download_ci_llvm( let download_ci_llvm = download_ci_llvm.unwrap_or(default); let if_unchanged = || { - if rust_info.is_from_tarball() { + if dwn_ctx.rust_info.is_from_tarball() { // Git is needed for running "if-unchanged" logic. println!("ERROR: 'if-unchanged' is only compatible with Git managed sources."); crate::exit!(1); @@ -2352,33 +2252,28 @@ pub fn parse_download_ci_llvm( // Fetching the LLVM submodule is unnecessary for self-tests. #[cfg(not(test))] - update_submodule(submodules, exec_ctx, src, rust_info, "src/llvm-project"); + update_submodule(dwn_ctx, "src/llvm-project"); // Check for untracked changes in `src/llvm-project` and other important places. - let has_changes = has_changes_from_upstream( - stage0_metadata, - src, - path_modification_cache, - LLVM_INVALIDATION_PATHS, - ); + let has_changes = has_changes_from_upstream(dwn_ctx, LLVM_INVALIDATION_PATHS); // Return false if there are untracked changes, otherwise check if CI LLVM is available. if has_changes { false } else { - llvm::is_ci_llvm_available_for_target(host_target, asserts) + llvm::is_ci_llvm_available_for_target(&dwn_ctx.host_target, asserts) } }; match download_ci_llvm { StringOrBool::Bool(b) => { - if !b && download_rustc_commit.is_some() { + if !b && dwn_ctx.download_rustc_commit.is_some() { panic!( "`llvm.download-ci-llvm` cannot be set to `false` if `rust.download-rustc` is set to `true` or `if-unchanged`." ); } - if b && is_running_on_ci { + if b && dwn_ctx.is_running_on_ci { // On CI, we must always rebuild LLVM if there were any modifications to it panic!( "`llvm.download-ci-llvm` cannot be set to `true` on CI. Use `if-unchanged` instead." @@ -2386,7 +2281,7 @@ pub fn parse_download_ci_llvm( } // If download-ci-llvm=true we also want to check that CI llvm is available - b && llvm::is_ci_llvm_available_for_target(host_target, asserts) + b && llvm::is_ci_llvm_available_for_target(&dwn_ctx.host_target, asserts) } StringOrBool::String(s) if s == "if-unchanged" => if_unchanged(), StringOrBool::String(other) => { @@ -2395,13 +2290,12 @@ pub fn parse_download_ci_llvm( } } -pub fn has_changes_from_upstream( - stage0_metadata: &build_helper::stage0_parser::Stage0, - src: &Path, - path_modification_cache: Arc, PathFreshness>>>, +pub fn has_changes_from_upstream<'a>( + dwn_ctx: impl AsRef>, paths: &[&'static str], ) -> bool { - match check_path_modifications_(stage0_metadata, src, path_modification_cache, paths) { + let dwn_ctx = dwn_ctx.as_ref(); + match check_path_modifications_(dwn_ctx, paths) { PathFreshness::LastModifiedUpstream { .. } => false, PathFreshness::HasLocalModifications { .. } | PathFreshness::MissingUpstream => true, } @@ -2416,18 +2310,13 @@ pub fn has_changes_from_upstream( fields(relative_path = ?relative_path), ), )] -pub(crate) fn update_submodule( - submodules: &Option, - exec_ctx: &ExecutionContext, - src: &Path, - rust_info: &channel::GitInfo, - relative_path: &str, -) { - if rust_info.is_from_tarball() || !submodules_(submodules, rust_info) { +pub(crate) fn update_submodule<'a>(dwn_ctx: impl AsRef>, relative_path: &str) { + let dwn_ctx = dwn_ctx.as_ref(); + if dwn_ctx.rust_info.is_from_tarball() || !submodules_(dwn_ctx.submodules, dwn_ctx.rust_info) { return; } - let absolute_path = src.join(relative_path); + let absolute_path = dwn_ctx.src.join(relative_path); // NOTE: This check is required because `jj git clone` doesn't create directories for // submodules, they are completely ignored. The code below assumes this directory exists, @@ -2438,7 +2327,7 @@ pub(crate) fn update_submodule( // NOTE: The check for the empty directory is here because when running x.py the first time, // the submodule won't be checked out. Check it out now so we can build it. - if !git_info(exec_ctx, false, &absolute_path).is_managed_git_subrepository() + if !git_info(dwn_ctx.exec_ctx, false, &absolute_path).is_managed_git_subrepository() && !helpers::dir_is_empty(&absolute_path) { return; @@ -2458,14 +2347,14 @@ pub(crate) fn update_submodule( // Determine commit checked out in submodule. let checked_out_hash = - submodule_git().args(["rev-parse", "HEAD"]).run_capture_stdout(exec_ctx).stdout(); + submodule_git().args(["rev-parse", "HEAD"]).run_capture_stdout(dwn_ctx.exec_ctx).stdout(); let checked_out_hash = checked_out_hash.trim_end(); // Determine commit that the submodule *should* have. - let recorded = helpers::git(Some(src)) + let recorded = helpers::git(Some(dwn_ctx.src)) .run_in_dry_run() .args(["ls-tree", "HEAD"]) .arg(relative_path) - .run_capture_stdout(exec_ctx) + .run_capture_stdout(dwn_ctx.exec_ctx) .stdout(); let actual_hash = recorded @@ -2480,24 +2369,24 @@ pub(crate) fn update_submodule( println!("Updating submodule {relative_path}"); - helpers::git(Some(src)) + helpers::git(Some(dwn_ctx.src)) .allow_failure() .run_in_dry_run() .args(["submodule", "-q", "sync"]) .arg(relative_path) - .run(exec_ctx); + .run(dwn_ctx.exec_ctx); // Try passing `--progress` to start, then run git again without if that fails. let update = |progress: bool| { // Git is buggy and will try to fetch submodules from the tracking branch for *this* repository, // even though that has no relation to the upstream for the submodule. - let current_branch = helpers::git(Some(src)) + let current_branch = helpers::git(Some(dwn_ctx.src)) .allow_failure() .run_in_dry_run() .args(["symbolic-ref", "--short", "HEAD"]) - .run_capture(exec_ctx); + .run_capture(dwn_ctx.exec_ctx); - let mut git = helpers::git(Some(&src)).allow_failure(); + let mut git = helpers::git(Some(dwn_ctx.src)).allow_failure(); git.run_in_dry_run(); if current_branch.is_success() { // If there is a tag named after the current branch, git will try to disambiguate by prepending `heads/` to the branch name. @@ -2514,23 +2403,25 @@ pub(crate) fn update_submodule( git.arg(relative_path); git }; - if !update(true).allow_failure().run(exec_ctx) { - update(false).allow_failure().run(exec_ctx); + if !update(true).allow_failure().run(dwn_ctx.exec_ctx) { + update(false).allow_failure().run(dwn_ctx.exec_ctx); } // Save any local changes, but avoid running `git stash pop` if there are none (since it will exit with an error). // diff-index reports the modifications through the exit status - let has_local_modifications = - !submodule_git().allow_failure().args(["diff-index", "--quiet", "HEAD"]).run(exec_ctx); + let has_local_modifications = !submodule_git() + .allow_failure() + .args(["diff-index", "--quiet", "HEAD"]) + .run(dwn_ctx.exec_ctx); if has_local_modifications { - submodule_git().allow_failure().args(["stash", "push"]).run(exec_ctx); + submodule_git().allow_failure().args(["stash", "push"]).run(dwn_ctx.exec_ctx); } - submodule_git().allow_failure().args(["reset", "-q", "--hard"]).run(exec_ctx); - submodule_git().allow_failure().args(["clean", "-qdfx"]).run(exec_ctx); + submodule_git().allow_failure().args(["reset", "-q", "--hard"]).run(dwn_ctx.exec_ctx); + submodule_git().allow_failure().args(["clean", "-qdfx"]).run(dwn_ctx.exec_ctx); if has_local_modifications { - submodule_git().allow_failure().args(["stash", "pop"]).run(exec_ctx); + submodule_git().allow_failure().args(["stash", "pop"]).run(dwn_ctx.exec_ctx); } } @@ -2548,15 +2439,14 @@ pub fn submodules_(submodules: &Option, rust_info: &channel::GitInfo) -> b /// In particular, we expect llvm sources to be available when this is false. /// /// NOTE: this is not the same as `!is_rust_llvm` when `llvm_has_patches` is set. -pub fn is_system_llvm( - host_target: &TargetSelection, - llvm_from_ci: bool, - target_config: &HashMap, +pub fn is_system_llvm<'a>( + dwn_ctx: impl AsRef>, target: TargetSelection, ) -> bool { - match target_config.get(&target) { + let dwn_ctx = dwn_ctx.as_ref(); + match dwn_ctx.target_config.get(&target) { Some(Target { llvm_config: Some(_), .. }) => { - let ci_llvm = llvm_from_ci && is_host_target(host_target, &target); + let ci_llvm = dwn_ctx.llvm_from_ci && is_host_target(&dwn_ctx.host_target, &target); !ci_llvm } // We're building from the in-tree src/llvm-project sources. @@ -2569,29 +2459,25 @@ pub fn is_host_target(host_target: &TargetSelection, target: &TargetSelection) - host_target == target } -pub(crate) fn ci_llvm_root( - llvm_from_ci: bool, - out: &Path, - host_target: &TargetSelection, -) -> PathBuf { - assert!(llvm_from_ci); - out.join(host_target).join("ci-llvm") +pub(crate) fn ci_llvm_root<'a>(dwn_ctx: impl AsRef>) -> PathBuf { + let dwn_ctx = dwn_ctx.as_ref(); + assert!(dwn_ctx.llvm_from_ci); + dwn_ctx.out.join(dwn_ctx.host_target).join("ci-llvm") } /// Returns the content of the given file at a specific commit. -pub(crate) fn read_file_by_commit( - exec_ctx: &ExecutionContext, - src: &Path, - rust_info: &channel::GitInfo, +pub(crate) fn read_file_by_commit<'a>( + dwn_ctx: impl AsRef>, file: &Path, commit: &str, ) -> String { + let dwn_ctx = dwn_ctx.as_ref(); assert!( - rust_info.is_managed_git_subrepository(), + dwn_ctx.rust_info.is_managed_git_subrepository(), "`Config::read_file_by_commit` is not supported in non-git sources." ); - let mut git = helpers::git(Some(src)); + let mut git = helpers::git(Some(dwn_ctx.src)); git.arg("show").arg(format!("{commit}:{}", file.to_str().unwrap())); - git.run_capture_stdout(exec_ctx).stdout() + git.run_capture_stdout(dwn_ctx.exec_ctx).stdout() } diff --git a/src/bootstrap/src/core/download.rs b/src/bootstrap/src/core/download.rs index 7ec6c62a07d02..5ded44cef1447 100644 --- a/src/bootstrap/src/core/download.rs +++ b/src/bootstrap/src/core/download.rs @@ -1,14 +1,17 @@ +use std::collections::HashMap; use std::env; use std::ffi::OsString; use std::fs::{self, File}; use std::io::{BufRead, BufReader, BufWriter, ErrorKind, Write}; use std::path::{Path, PathBuf}; -use std::sync::OnceLock; +use std::sync::{Arc, Mutex, OnceLock}; +use build_helper::git::PathFreshness; use xz2::bufread::XzDecoder; -use crate::core::config::{BUILDER_CONFIG_FILENAME, TargetSelection}; +use crate::core::config::{BUILDER_CONFIG_FILENAME, Target, TargetSelection}; use crate::utils::build_stamp::BuildStamp; +use crate::utils::channel; use crate::utils::exec::{ExecutionContext, command}; use crate::utils::helpers::{exe, hex_encode, move_file}; use crate::{Config, t}; @@ -398,14 +401,21 @@ impl Config { /// Only should be used for pre config initialization downloads. pub(crate) struct DownloadContext<'a> { - host_target: TargetSelection, - out: &'a Path, - patch_binaries_for_nix: Option, - exec_ctx: &'a ExecutionContext, - stage0_metadata: &'a build_helper::stage0_parser::Stage0, - llvm_assertions: bool, - bootstrap_cache_path: &'a Option, - is_running_on_ci: bool, + pub path_modification_cache: Arc, PathFreshness>>>, + pub src: &'a Path, + pub rust_info: &'a channel::GitInfo, + pub submodules: &'a Option, + pub download_rustc_commit: &'a Option, + pub host_target: TargetSelection, + pub llvm_from_ci: bool, + pub target_config: &'a HashMap, + pub out: &'a Path, + pub patch_binaries_for_nix: Option, + pub exec_ctx: &'a ExecutionContext, + pub stage0_metadata: &'a build_helper::stage0_parser::Stage0, + pub llvm_assertions: bool, + pub bootstrap_cache_path: &'a Option, + pub is_running_on_ci: bool, } impl<'a> AsRef> for DownloadContext<'a> { @@ -417,7 +427,14 @@ impl<'a> AsRef> for DownloadContext<'a> { impl<'a> From<&'a Config> for DownloadContext<'a> { fn from(value: &'a Config) -> Self { DownloadContext { + path_modification_cache: value.path_modification_cache.clone(), + src: &value.src, host_target: value.host_target, + rust_info: &value.rust_info, + download_rustc_commit: &value.download_rustc_commit, + submodules: &value.submodules, + llvm_from_ci: value.llvm_from_ci, + target_config: &value.target_config, out: &value.out, patch_binaries_for_nix: value.patch_binaries_for_nix, exec_ctx: &value.exec_ctx, From bfa6b56920d8eb736c271e2285b7bf4fb52bb90c Mon Sep 17 00:00:00 2001 From: bit-aloo Date: Sat, 9 Aug 2025 18:12:05 +0530 Subject: [PATCH 9/9] add review comments --- src/bootstrap/src/core/config/config.rs | 55 +++++-------------------- 1 file changed, 11 insertions(+), 44 deletions(-) diff --git a/src/bootstrap/src/core/config/config.rs b/src/bootstrap/src/core/config/config.rs index 73a20df9b4e30..25b41a642d6d6 100644 --- a/src/bootstrap/src/core/config/config.rs +++ b/src/bootstrap/src/core/config/config.rs @@ -952,12 +952,17 @@ impl Config { } let dwn_ctx = DownloadContext::from(&config); - config.download_rustc_commit = download_ci_rustc_commit( - dwn_ctx, - rust_download_rustc, - debug_assertions_requested, - config.llvm_assertions, - ); + config.download_rustc_commit = + download_ci_rustc_commit(dwn_ctx, rust_download_rustc, config.llvm_assertions); + + if debug_assertions_requested { + eprintln!( + "WARN: `rust.debug-assertions = true` will prevent downloading CI rustc as alt CI \ + rustc is not currently built with debug assertions." + ); + // We need to put this later down_ci_rustc_commit. + config.download_rustc_commit = None; + } if let Some(t) = toml.target { for (triple, cfg) in t { @@ -1682,31 +1687,6 @@ impl Config { update_submodule(dwn_ctx, relative_path); } - /// Returns the commit to download, or `None` if we shouldn't download CI artifacts. - pub fn download_ci_rustc_commit( - &self, - download_rustc: Option, - debug_assertions_requested: bool, - llvm_assertions: bool, - ) -> Option { - let dwn_ctx = DownloadContext::from(self); - download_ci_rustc_commit( - dwn_ctx, - download_rustc, - debug_assertions_requested, - llvm_assertions, - ) - } - - pub fn parse_download_ci_llvm( - &self, - download_ci_llvm: Option, - asserts: bool, - ) -> bool { - let dwn_ctx = DownloadContext::from(self); - parse_download_ci_llvm(dwn_ctx, download_ci_llvm, asserts) - } - /// Returns true if any of the `paths` have been modified locally. pub fn has_changes_from_upstream(&self, paths: &[&'static str]) -> bool { let dwn_ctx = DownloadContext::from(self); @@ -1731,10 +1711,6 @@ impl Config { .clone() } - pub fn ci_env(&self) -> CiEnv { - if self.is_running_on_ci { CiEnv::GitHubActions } else { CiEnv::None } - } - pub fn sanitizers_enabled(&self, target: TargetSelection) -> bool { self.target_config.get(&target).and_then(|t| t.sanitizers).unwrap_or(self.sanitizers) } @@ -2114,7 +2090,6 @@ pub fn check_stage0_version( pub fn download_ci_rustc_commit<'a>( dwn_ctx: impl AsRef>, download_rustc: Option, - debug_assertions_requested: bool, llvm_assertions: bool, ) -> Option { let dwn_ctx = dwn_ctx.as_ref(); @@ -2182,14 +2157,6 @@ pub fn download_ci_rustc_commit<'a>( .expect("git-commit-info is missing in the project root") }; - if debug_assertions_requested { - eprintln!( - "WARN: `rust.debug-assertions = true` will prevent downloading CI rustc as alt CI \ - rustc is not currently built with debug assertions." - ); - return None; - } - Some(commit) }