Unrolled build for #154880

Rollup merge of #154880 - ferrocene:jyn/download-rustc-improvements, r=Kobzol,jieyouxu

bootstrap: minor improvements to download-rustc

Split out from #154243.

Author: rust-timer

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

@@ -78,6 +78,7 @@ pub const RUSTC_IF_UNCHANGED_ALLOWED_PATHS: &[&str] = &[
     ":!src/rustdoc-json-types",
     ":!tests",
     ":!triagebot.toml",
+    ":!src/bootstrap/defaults",
 ];
 
 /// Global configuration for the entire build and/or bootstrap.
@@ -2237,11 +2238,7 @@ pub fn download_ci_rustc_commit<'a>(
         });
         match freshness {
             PathFreshness::LastModifiedUpstream { upstream } => upstream,
-            PathFreshness::HasLocalModifications { upstream } => {
-                if if_unchanged {
-                    return None;
-                }
-
+            PathFreshness::HasLocalModifications { upstream, modifications } => {
                 if dwn_ctx.is_running_on_ci() {
                     eprintln!("CI rustc commit matches with HEAD and we are in CI.");
                     eprintln!(
@@ -2250,6 +2247,24 @@ pub fn download_ci_rustc_commit<'a>(
                     return None;
                 }
 
+                eprintln!(
+                    "NOTE: detected {} modifications that could affect a build of rustc",
+                    modifications.len()
+                );
+                for file in modifications.iter().take(10) {
+                    eprintln!("- {}", file.display());
+                }
+                if modifications.len() > 10 {
+                    eprintln!("- ... and {} more", modifications.len() - 10);
+                }
+
+                if if_unchanged {
+                    eprintln!("skipping rustc download due to `download-rustc = 'if-unchanged'`");
+                    return None;
+                } else {
+                    eprintln!("downloading unconditionally due to `download-rustc = true`");
+                }
+
                 upstream
             }
             PathFreshness::MissingUpstream => {

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

@@ -32,6 +32,13 @@ fn get_toml(file: &Path) -> Result<TomlConfig, toml::de::Error> {
     toml::from_str(&contents).and_then(|table: toml::Value| TomlConfig::deserialize(table))
 }
 
+fn modified(upstream: impl Into<String>, changes: &[&str]) -> PathFreshness {
+    PathFreshness::HasLocalModifications {
+        upstream: upstream.into(),
+        modifications: changes.iter().copied().map(PathBuf::from).collect(),
+    }
+}
+
 #[test]
 fn download_ci_llvm() {
     let config = TestCtx::new().config("check").create_config();
@@ -692,7 +699,7 @@ fn test_pr_ci_changed_in_pr() {
         let sha = ctx.create_upstream_merge(&["a"]);
         ctx.create_nonupstream_merge(&["b"]);
         let src = ctx.check_modifications(&["b"], CiEnv::GitHubActions);
-        assert_eq!(src, PathFreshness::HasLocalModifications { upstream: sha });
+        assert_eq!(src, modified(sha, &["b"]));
     });
 }
 
@@ -712,7 +719,7 @@ fn test_auto_ci_changed_in_pr() {
         let sha = ctx.create_upstream_merge(&["a"]);
         ctx.create_upstream_merge(&["b", "c"]);
         let src = ctx.check_modifications(&["c", "d"], CiEnv::GitHubActions);
-        assert_eq!(src, PathFreshness::HasLocalModifications { upstream: sha });
+        assert_eq!(src, modified(sha, &["c"]));
     });
 }
 
@@ -723,10 +730,7 @@ fn test_local_uncommitted_modifications() {
         ctx.create_branch("feature");
         ctx.modify("a");
 
-        assert_eq!(
-            ctx.check_modifications(&["a", "d"], CiEnv::None),
-            PathFreshness::HasLocalModifications { upstream: sha }
-        );
+        assert_eq!(ctx.check_modifications(&["a", "d"], CiEnv::None), modified(sha, &["a"]),);
     });
 }
 
@@ -741,10 +745,7 @@ fn test_local_committed_modifications() {
         ctx.modify("a");
         ctx.commit();
 
-        assert_eq!(
-            ctx.check_modifications(&["a", "d"], CiEnv::None),
-            PathFreshness::HasLocalModifications { upstream: sha }
-        );
+        assert_eq!(ctx.check_modifications(&["a", "d"], CiEnv::None), modified(sha, &["a"]),);
     });
 }
 
@@ -757,10 +758,7 @@ fn test_local_committed_modifications_subdirectory() {
         ctx.modify("a/b/d");
         ctx.commit();
 
-        assert_eq!(
-            ctx.check_modifications(&["a/b"], CiEnv::None),
-            PathFreshness::HasLocalModifications { upstream: sha }
-        );
+        assert_eq!(ctx.check_modifications(&["a/b"], CiEnv::None), modified(sha, &["a/b/d"]),);
     });
 }
 
@@ -836,11 +834,11 @@ fn test_local_changes_negative_path() {
         );
         assert_eq!(
             ctx.check_modifications(&[":!c"], CiEnv::None),
-            PathFreshness::HasLocalModifications { upstream: upstream.clone() }
+            modified(&upstream, &["b", "d"]),
         );
         assert_eq!(
             ctx.check_modifications(&[":!d", ":!x"], CiEnv::None),
-            PathFreshness::HasLocalModifications { upstream }
+            modified(&upstream, &["b"]),
         );
     });
 }

src/bootstrap/src/core/download.rs

@@ -266,7 +266,7 @@ impl Config {
         });
         let llvm_sha = match llvm_freshness {
             PathFreshness::LastModifiedUpstream { upstream } => upstream,
-            PathFreshness::HasLocalModifications { upstream } => upstream,
+            PathFreshness::HasLocalModifications { upstream, modifications: _ } => upstream,
             PathFreshness::MissingUpstream => {
                 eprintln!("error: could not find commit hash for downloading LLVM");
                 eprintln!("HELP: maybe your repository history is too shallow?");

src/build_helper/src/git.rs

@@ -1,4 +1,4 @@
-use std::path::Path;
+use std::path::{Path, PathBuf};
 use std::process::{Command, Stdio};
 
 use crate::ci::CiEnv;
@@ -38,7 +38,7 @@ pub enum PathFreshness {
     /// "Local" essentially means "not-upstream" here.
     /// `upstream` is the latest upstream merge commit that made modifications to the
     /// set of paths.
-    HasLocalModifications { upstream: String },
+    HasLocalModifications { upstream: String, modifications: Vec<PathBuf> },
     /// No upstream commit was found.
     /// This should not happen in most reasonable circumstances, but one never knows.
     MissingUpstream,
@@ -134,21 +134,34 @@ pub fn check_path_modifications(
     // However, that should be equivalent to checking if something has changed
     // from the latest upstream commit *that modified `target_paths`*, and
     // with this approach we do not need to invoke git an additional time.
-    if has_changed_since(git_dir, &upstream_sha, target_paths) {
-        Ok(PathFreshness::HasLocalModifications { upstream: upstream_sha })
+    let modifications = changes_since(git_dir, &upstream_sha, target_paths)?;
+    if !modifications.is_empty() {
+        Ok(PathFreshness::HasLocalModifications { upstream: upstream_sha, modifications })
     } else {
         Ok(PathFreshness::LastModifiedUpstream { upstream: upstream_sha })
     }
 }
 
 /// Returns true if any of the passed `paths` have changed since the `base` commit.
-pub fn has_changed_since(git_dir: &Path, base: &str, paths: &[&str]) -> bool {
+pub fn changes_since(git_dir: &Path, base: &str, paths: &[&str]) -> Result<Vec<PathBuf>, String> {
+    use std::io::BufRead;
+
     run_git_diff_index(Some(git_dir), |cmd| {
-        cmd.args(["--quiet", base, "--"]).args(paths);
+        cmd.args([base, "--name-only", "--"]).args(paths);
+
+        let output = cmd.stderr(Stdio::inherit()).output().expect("cannot run git diff-index");
+        if !output.status.success() {
+            return Err(format!("failed to run: {cmd:?}: {:?}", output.status));
+        }
 
-        // Exit code 0 => no changes
-        // Exit code 1 => some changes were detected
-        !cmd.status().expect("cannot run git diff-index").success()
+        output
+            .stdout
+            .lines()
+            .map(|res| match res {
+                Ok(line) => Ok(PathBuf::from(line)),
+                Err(e) => Err(format!("invalid UTF-8 in diff-index: {e:?}")),
+            })
+            .collect()
     })
 }