Port download-ci-rustc to check_path_modifications

And get rid of `get_closest_merge_commit`.

Author: Kobzol

library/stdarch

@@ -31,7 +31,7 @@ use crate::utils::exec::command;
 use crate::utils::helpers::{
     exe, get_clang_cl_resource_dir, is_debug_info, is_dylib, symlink_dir, t, up_to_date,
 };
-use crate::{CLang, Compiler, DependencyType, GitRepo, LLVM_TOOLS, Mode};
+use crate::{CLang, Compiler, DependencyType, GitRepo, LLVM_TOOLS, Mode, trace};
 
 #[derive(Debug, Clone, PartialEq, Eq, PartialOrd, Ord, Hash)]
 pub struct Std {
@@ -108,7 +108,11 @@ impl Step for Std {
         // the `rust.download-rustc=true` option.
         let force_recompile = builder.rust_info().is_managed_git_subrepository()
             && builder.download_rustc()
-            && builder.config.last_modified_commit(&["library"], "download-rustc", true).is_none();
+            && builder.config.has_changes_from_upstream(&["library"]);
+
+        trace!("is managed git repo: {}", builder.rust_info().is_managed_git_subrepository());
+        trace!("download_rustc: {}", builder.download_rustc());
+        trace!(force_recompile);
 
         run.builder.ensure(Std {
             compiler: run.builder.compiler(run.builder.top_stage, run.build_triple()),

src/bootstrap/src/core/build_steps/compile.rs

@@ -94,7 +94,7 @@ fn get_modified_rs_files(build: &Builder<'_>) -> Result<Option<Vec<String>>, Str
         return Ok(None);
     }
 
-    get_git_modified_files(&build.config.git_config(), Some(&build.config.src), &["rs"])
+    get_git_modified_files(&build.config.git_config(), Some(&build.config.src), &["rs"]).map(Some)
 }
 
 #[derive(serde_derive::Deserialize)]

src/bootstrap/src/core/build_steps/format.rs

@@ -520,7 +520,7 @@ impl Step for Llvm {
         }
 
         // https://llvm.org/docs/HowToCrossCompileLLVM.html
-        if !builder.is_builder_target(target) {
+        if !builder.is_builder_target(&target) {
             let LlvmResult { llvm_config, .. } =
                 builder.ensure(Llvm { target: builder.config.build });
             if !builder.config.dry_run() {
@@ -672,7 +672,7 @@ fn configure_cmake(
     }
     cfg.target(&target.triple).host(&builder.config.build.triple);
 
-    if !builder.is_builder_target(target) {
+    if !builder.is_builder_target(&target) {
         cfg.define("CMAKE_CROSSCOMPILING", "True");
 
         if target.contains("netbsd") {
@@ -1097,7 +1097,7 @@ impl Step for Lld {
             .define("LLVM_CMAKE_DIR", llvm_cmake_dir)
             .define("LLVM_INCLUDE_TESTS", "OFF");
 
-        if !builder.is_builder_target(target) {
+        if !builder.is_builder_target(&target) {
             // Use the host llvm-tblgen binary.
             cfg.define(
                 "LLVM_TABLEGEN_EXE",

src/bootstrap/src/core/build_steps/llvm.rs

@@ -604,8 +604,7 @@ impl Step for Rustdoc {
             let files_to_track = &["src/librustdoc", "src/tools/rustdoc"];
 
             // Check if unchanged
-            if builder.config.last_modified_commit(files_to_track, "download-rustc", true).is_some()
-            {
+            if !builder.config.has_changes_from_upstream(files_to_track) {
                 let precompiled_rustdoc = builder
                     .config
                     .ci_rustc_dir()

src/bootstrap/src/core/build_steps/tool.rs

@@ -261,8 +261,14 @@ fn ci_rustc_if_unchanged_logic() {
     // Make sure "if-unchanged" logic doesn't try to use CI rustc while there are changes
     // in compiler and/or library.
     if config.download_rustc_commit.is_some() {
-        let has_changes =
-            config.last_modified_commit(&["compiler", "library"], "download-rustc", true).is_none();
+        let mut paths = vec!["compiler"];
+
+        // Handle library tree the same way as in `Config::download_ci_rustc_commit`.
+        if build_helper::ci::CiEnv::is_ci() {
+            paths.push("library");
+        }
+
+        let has_changes = config.has_changes_from_upstream(&paths);
 
         assert!(
             !has_changes,

src/bootstrap/src/core/builder/tests.rs

@@ -15,9 +15,7 @@ use std::{cmp, env, fs};
 
 use build_helper::ci::CiEnv;
 use build_helper::exit;
-use build_helper::git::{
-    GitConfig, PathFreshness, check_path_modifications, get_closest_merge_commit, output_result,
-};
+use build_helper::git::{GitConfig, PathFreshness, check_path_modifications, output_result};
 use serde::{Deserialize, Deserializer};
 use serde_derive::Deserialize;
 #[cfg(feature = "tracing")]
@@ -2981,17 +2979,22 @@ impl Config {
         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.
-            match self.last_modified_commit(&allowed_paths, "download-rustc", if_unchanged) {
-                Some(commit) => commit,
-                None => {
+            match self.check_modifications(&allowed_paths) {
+                PathFreshness::LastModifiedUpstream { upstream } => upstream,
+                PathFreshness::HasLocalModifications { upstream } => {
                     if if_unchanged {
                         return None;
                     }
-                    println!("ERROR: could not find commit hash for downloading rustc");
-                    println!("HELP: maybe your repository history is too shallow?");
-                    println!("HELP: consider setting `rust.download-rustc=false` in config.toml");
-                    println!("HELP: or fetch enough history to include one upstream commit");
-                    crate::exit!(1);
+
+                    if CiEnv::is_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
                 }
             }
         } else {
@@ -3000,19 +3003,6 @@ impl Config {
                 .expect("git-commit-info is missing in the project root")
         };
 
-        if CiEnv::is_ci() && {
-            let head_sha =
-                output(helpers::git(Some(&self.src)).arg("rev-parse").arg("HEAD").as_command_mut());
-            let head_sha = head_sha.trim();
-            commit == head_sha
-        } {
-            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;
-        }
-
         if debug_assertions_requested {
             eprintln!(
                 "WARN: `rust.debug-assertions = true` will prevent downloading CI rustc as alt CI \
@@ -3068,61 +3058,16 @@ impl Config {
     }
 
     /// Returns true if any of the `paths` have been modified locally.
-    fn has_changes_from_upstream(&self, paths: &[&str]) -> bool {
-        let freshness =
-            check_path_modifications(Some(&self.src), &self.git_config(), paths, CiEnv::current())
-                .unwrap();
-        match freshness {
+    pub fn has_changes_from_upstream(&self, paths: &[&str]) -> bool {
+        match self.check_modifications(paths) {
             PathFreshness::LastModifiedUpstream { .. } => false,
             PathFreshness::HasLocalModifications { .. } => true,
         }
     }
 
-    /// 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<String> {
-        assert!(
-            self.rust_info.is_managed_git_subrepository(),
-            "Can't run `Config::last_modified_commit` on a non-git source."
-        );
-
-        // Look for a version to compare to based on the current commit.
-        // Only commits merged by bors will have CI artifacts.
-        let commit = get_closest_merge_commit(Some(&self.src), &self.git_config(), &[]).unwrap();
-        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 = helpers::git(Some(&self.src));
-        git.args(["diff-index", "--quiet", &commit, "--"]).args(modified_paths);
-
-        let has_changes = !t!(git.as_command_mut().status()).success();
-        if has_changes {
-            if if_unchanged {
-                if self.is_verbose() {
-                    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 check_modifications(&self, paths: &[&str]) -> PathFreshness {
+        check_path_modifications(Some(&self.src), &self.git_config(), paths, CiEnv::current())
+            .unwrap()
     }
 }
 

src/bootstrap/src/core/config/config.rs

@@ -39,9 +39,7 @@ fn download_ci_llvm() {
 
     let if_unchanged_config = parse("llvm.download-ci-llvm = \"if-unchanged\"");
     if if_unchanged_config.llvm_from_ci {
-        let has_changes = if_unchanged_config
-            .last_modified_commit(&["src/llvm-project"], "download-ci-llvm", true)
-            .is_none();
+        let has_changes = if_unchanged_config.has_changes_from_upstream(&["src/llvm-project"]);
 
         assert!(
             !has_changes,

src/bootstrap/src/core/config/tests.rs

@@ -1,7 +1,7 @@
 #[cfg(test)]
 mod tests;
 
-use std::path::{Path, PathBuf};
+use std::path::Path;
 use std::process::{Command, Stdio};
 
 use crate::ci::CiEnv;
@@ -101,73 +101,6 @@ pub fn updated_master_branch(
     Err("Cannot find any suitable upstream master branch".to_owned())
 }
 
-/// Finds the nearest merge commit by comparing the local `HEAD` with the upstream branch's state.
-/// To work correctly, the upstream remote must be properly configured using `git remote add <name> <url>`.
-/// In most cases `get_closest_merge_commit` is the function you are looking for as it doesn't require remote
-/// to be configured.
-fn git_upstream_merge_base(
-    config: &GitConfig<'_>,
-    git_dir: Option<&Path>,
-) -> Result<String, String> {
-    let updated_master = updated_master_branch(config, git_dir)?;
-    let mut git = Command::new("git");
-    if let Some(git_dir) = git_dir {
-        git.current_dir(git_dir);
-    }
-    Ok(output_result(git.arg("merge-base").arg(&updated_master).arg("HEAD"))?.trim().to_owned())
-}
-
-/// Searches for the nearest merge commit in the repository that also exists upstream.
-///
-/// It looks for the most recent commit made by the merge bot by matching the author's email
-/// address with the merge bot's email.
-pub fn get_closest_merge_commit(
-    git_dir: Option<&Path>,
-    config: &GitConfig<'_>,
-    target_paths: &[PathBuf],
-) -> Result<String, String> {
-    let mut git = Command::new("git");
-
-    if let Some(git_dir) = git_dir {
-        git.current_dir(git_dir);
-    }
-
-    let channel = include_str!("../../ci/channel").trim();
-
-    let merge_base = {
-        if CiEnv::is_ci() &&
-            // FIXME: When running on rust-lang managed CI and it's not a nightly build,
-            // `git_upstream_merge_base` fails with an error message similar to this:
-            // ```
-            //    called `Result::unwrap()` on an `Err` value: "command did not execute successfully:
-            //    cd \"/checkout\" && \"git\" \"merge-base\" \"origin/master\" \"HEAD\"\nexpected success, got: exit status: 1\n"
-            // ```
-            // Investigate and resolve this issue instead of skipping it like this.
-            (channel == "nightly" || !CiEnv::is_rust_lang_managed_ci_job())
-        {
-            git_upstream_merge_base(config, git_dir).unwrap()
-        } else {
-            // For non-CI environments, ignore rust-lang/rust upstream as it usually gets
-            // outdated very quickly.
-            "HEAD".to_string()
-        }
-    };
-
-    git.args([
-        "rev-list",
-        &format!("--author={}", config.git_merge_commit_email),
-        "-n1",
-        "--first-parent",
-        &merge_base,
-    ]);
-
-    if !target_paths.is_empty() {
-        git.arg("--").args(target_paths);
-    }
-
-    Ok(output_result(&mut git)?.trim().to_owned())
-}
-
 /// Represents the result of checking whether a set of paths
 /// have been modified locally or not.
 #[derive(PartialEq, Debug)]
@@ -184,10 +117,12 @@ pub enum PathFreshness {
 
 /// This function figures out if a set of paths was last modified upstream or
 /// if there are some local modifications made to them.
-///
 /// It can be used to figure out if we should download artifacts from CI or rather
 /// build them locally.
 ///
+/// The function assumes that at least a single upstream bors merge commit is in the
+/// local git history.
+///
 /// `target_paths` should be a non-empty slice of paths (relative to `git_dir` or the
 /// current working directory) whose modifications would invalidate the artifact.
 /// Each path can also be a negative match, i.e. `:!foo`. This matches changes outside