Rollup merge of #138706 - Kobzol:bootstrap-git-refactor-1, r=onur-ozkan

Improve bootstrap git modified path handling

Drive-by improvements extracted out of #138591.

r? ``@onur-ozkan``

Author: matthiaskrgr

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

@@ -93,7 +93,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/config/config.rs

@@ -1428,52 +1428,56 @@ impl Config {
 
         // Infer the rest of the configuration.
 
-        // Infer the source directory. This is non-trivial because we want to support a downloaded bootstrap binary,
-        // running on a completely different machine from where it was compiled.
-        let mut cmd = helpers::git(None);
-        // NOTE: we cannot support running from outside the repository because the only other path we have available
-        // is set at compile time, which can be wrong if bootstrap was downloaded rather than compiled locally.
-        // We still support running outside the repository if we find we aren't in a git directory.
-
-        // NOTE: We get a relative path from git to work around an issue on MSYS/mingw. If we used an absolute path,
-        // and end up using MSYS's git rather than git-for-windows, we would get a unix-y MSYS path. But as bootstrap
-        // has already been (kinda-cross-)compiled to Windows land, we require a normal Windows path.
-        cmd.arg("rev-parse").arg("--show-cdup");
-        // Discard stderr because we expect this to fail when building from a tarball.
-        let output = cmd
-            .as_command_mut()
-            .stderr(std::process::Stdio::null())
-            .output()
-            .ok()
-            .and_then(|output| if output.status.success() { Some(output) } else { None });
-        if let Some(output) = output {
-            let git_root_relative = String::from_utf8(output.stdout).unwrap();
-            // We need to canonicalize this path to make sure it uses backslashes instead of forward slashes,
-            // and to resolve any relative components.
-            let git_root = env::current_dir()
-                .unwrap()
-                .join(PathBuf::from(git_root_relative.trim()))
-                .canonicalize()
-                .unwrap();
-            let s = git_root.to_str().unwrap();
-
-            // Bootstrap is quite bad at handling /? in front of paths
-            let git_root = match s.strip_prefix("\\\\?\\") {
-                Some(p) => PathBuf::from(p),
-                None => git_root,
-            };
-            // If this doesn't have at least `stage0`, we guessed wrong. This can happen when,
-            // for example, the build directory is inside of another unrelated git directory.
-            // In that case keep the original `CARGO_MANIFEST_DIR` handling.
-            //
-            // NOTE: this implies that downloadable bootstrap isn't supported when the build directory is outside
-            // the source directory. We could fix that by setting a variable from all three of python, ./x, and x.ps1.
-            if git_root.join("src").join("stage0").exists() {
-                config.src = git_root;
-            }
+        if let Some(src) = flags.src {
+            config.src = src
         } else {
-            // We're building from a tarball, not git sources.
-            // We don't support pre-downloaded bootstrap in this case.
+            // Infer the source directory. This is non-trivial because we want to support a downloaded bootstrap binary,
+            // running on a completely different machine from where it was compiled.
+            let mut cmd = helpers::git(None);
+            // NOTE: we cannot support running from outside the repository because the only other path we have available
+            // is set at compile time, which can be wrong if bootstrap was downloaded rather than compiled locally.
+            // We still support running outside the repository if we find we aren't in a git directory.
+
+            // NOTE: We get a relative path from git to work around an issue on MSYS/mingw. If we used an absolute path,
+            // and end up using MSYS's git rather than git-for-windows, we would get a unix-y MSYS path. But as bootstrap
+            // has already been (kinda-cross-)compiled to Windows land, we require a normal Windows path.
+            cmd.arg("rev-parse").arg("--show-cdup");
+            // Discard stderr because we expect this to fail when building from a tarball.
+            let output = cmd
+                .as_command_mut()
+                .stderr(std::process::Stdio::null())
+                .output()
+                .ok()
+                .and_then(|output| if output.status.success() { Some(output) } else { None });
+            if let Some(output) = output {
+                let git_root_relative = String::from_utf8(output.stdout).unwrap();
+                // We need to canonicalize this path to make sure it uses backslashes instead of forward slashes,
+                // and to resolve any relative components.
+                let git_root = env::current_dir()
+                    .unwrap()
+                    .join(PathBuf::from(git_root_relative.trim()))
+                    .canonicalize()
+                    .unwrap();
+                let s = git_root.to_str().unwrap();
+
+                // Bootstrap is quite bad at handling /? in front of paths
+                let git_root = match s.strip_prefix("\\\\?\\") {
+                    Some(p) => PathBuf::from(p),
+                    None => git_root,
+                };
+                // If this doesn't have at least `stage0`, we guessed wrong. This can happen when,
+                // for example, the build directory is inside of another unrelated git directory.
+                // In that case keep the original `CARGO_MANIFEST_DIR` handling.
+                //
+                // NOTE: this implies that downloadable bootstrap isn't supported when the build directory is outside
+                // the source directory. We could fix that by setting a variable from all three of python, ./x, and x.ps1.
+                if git_root.join("src").join("stage0").exists() {
+                    config.src = git_root;
+                }
+            } else {
+                // We're building from a tarball, not git sources.
+                // We don't support pre-downloaded bootstrap in this case.
+            }
         }
 
         if cfg!(test) {

src/build_helper/src/git.rs

@@ -173,7 +173,7 @@ pub fn get_git_modified_files(
     config: &GitConfig<'_>,
     git_dir: Option<&Path>,
     extensions: &[&str],
-) -> Result<Option<Vec<String>>, String> {
+) -> Result<Vec<String>, String> {
     let merge_base = get_closest_merge_commit(git_dir, config, &[])?;
 
     let mut git = Command::new("git");
@@ -186,7 +186,10 @@ pub fn get_git_modified_files(
             let (status, name) = f.trim().split_once(char::is_whitespace).unwrap();
             if status == "D" {
                 None
-            } else if Path::new(name).extension().map_or(false, |ext| {
+            } else if Path::new(name).extension().map_or(extensions.is_empty(), |ext| {
+                // If there is no extension, we allow the path if `extensions` is empty
+                // If there is an extension, we allow it if `extension` is empty or it contains the
+                // extension.
                 extensions.is_empty() || extensions.contains(&ext.to_str().unwrap())
             }) {
                 Some(name.to_owned())
@@ -195,7 +198,7 @@ pub fn get_git_modified_files(
             }
         })
         .collect();
-    Ok(Some(files))
+    Ok(files)
 }
 
 /// Returns the files that haven't been added to git yet.

src/tools/compiletest/src/lib.rs

@@ -747,8 +747,7 @@ fn modified_tests(config: &Config, dir: &Path) -> Result<Vec<PathBuf>, String> {
     }
 
     let files =
-        get_git_modified_files(&config.git_config(), Some(dir), &vec!["rs", "stderr", "fixed"])?
-            .unwrap_or(vec![]);
+        get_git_modified_files(&config.git_config(), Some(dir), &vec!["rs", "stderr", "fixed"])?;
     // Add new test cases to the list, it will be convenient in daily development.
     let untracked_files = get_git_untracked_files(&config.git_config(), None)?.unwrap_or(vec![]);
 

src/tools/suggest-tests/src/main.rs

@@ -14,11 +14,7 @@ fn main() -> ExitCode {
         &Vec::new(),
     );
     let modified_files = match modified_files {
-        Ok(Some(files)) => files,
-        Ok(None) => {
-            eprintln!("git error");
-            return ExitCode::FAILURE;
-        }
+        Ok(files) => files,
         Err(err) => {
             eprintln!("Could not get modified files from git: \"{err}\"");
             return ExitCode::FAILURE;