Rollup merge of #126309 - onur-ozkan:unify-git-utilization, r=Kobzol

unify git command preperation

Due to https://github.com/rust-lang/rust/issues/125954, we had to modify git invocations with certain flags in https://github.com/rust-lang/rust/pull/126255. However, because there are so many instances of `Command::new("git")` in bootstrap, it is difficult to apply these solutions to all of them.

This PR creates a helper function that unifies the git usage in bootstrap. Meaning, whenever special flags or hacks are needed, we can apply them to this single function which makes things much simpler for the bootstrap team.

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

@@ -1,7 +1,7 @@
 //! Runs rustfmt on the repository.
 
 use crate::core::builder::Builder;
-use crate::utils::helpers::{output, program_out_of_date, t};
+use crate::utils::helpers::{self, output, program_out_of_date, t};
 use build_helper::ci::CiEnv;
 use build_helper::git::get_git_modified_files;
 use ignore::WalkBuilder;
@@ -160,7 +160,7 @@ pub fn format(build: &Builder<'_>, check: bool, all: bool, paths: &[PathBuf]) {
             override_builder.add(&format!("!{ignore}")).expect(&ignore);
         }
     }
-    let git_available = match Command::new("git")
+    let git_available = match helpers::git(None)
         .arg("--version")
         .stdout(Stdio::null())
         .stderr(Stdio::null())
@@ -172,9 +172,7 @@ pub fn format(build: &Builder<'_>, check: bool, all: bool, paths: &[PathBuf]) {
 
     let mut adjective = None;
     if git_available {
-        let in_working_tree = match build
-            .config
-            .git()
+        let in_working_tree = match helpers::git(Some(&build.src))
             .arg("rev-parse")
             .arg("--is-inside-work-tree")
             .stdout(Stdio::null())
@@ -186,9 +184,7 @@ pub fn format(build: &Builder<'_>, check: bool, all: bool, paths: &[PathBuf]) {
         };
         if in_working_tree {
             let untracked_paths_output = output(
-                build
-                    .config
-                    .git()
+                helpers::git(Some(&build.src))
                     .arg("status")
                     .arg("--porcelain")
                     .arg("-z")

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

@@ -159,7 +159,7 @@ pub(crate) fn detect_llvm_sha(config: &Config, is_git: bool) -> String {
         // in that case.
         let closest_upstream = get_git_merge_base(&config.git_config(), Some(&config.src))
             .unwrap_or_else(|_| "HEAD".into());
-        let mut rev_list = config.git();
+        let mut rev_list = helpers::git(Some(&config.src));
         rev_list.args(&[
             PathBuf::from("rev-list"),
             format!("--author={}", config.stage0_metadata.config.git_merge_commit_email).into(),
@@ -252,7 +252,7 @@ pub(crate) fn is_ci_llvm_modified(config: &Config) -> bool {
         // We assume we have access to git, so it's okay to unconditionally pass
         // `true` here.
         let llvm_sha = detect_llvm_sha(config, true);
-        let head_sha = output(config.git().arg("rev-parse").arg("HEAD"));
+        let head_sha = output(helpers::git(Some(&config.src)).arg("rev-parse").arg("HEAD"));
         let head_sha = head_sha.trim();
         llvm_sha == head_sha
     }

src/bootstrap/src/core/build_steps/setup.rs

@@ -8,7 +8,7 @@
 use crate::core::builder::{Builder, RunConfig, ShouldRun, Step};
 use crate::t;
 use crate::utils::change_tracker::CONFIG_CHANGE_HISTORY;
-use crate::utils::helpers::hex_encode;
+use crate::utils::helpers::{self, hex_encode};
 use crate::Config;
 use sha2::Digest;
 use std::env::consts::EXE_SUFFIX;
@@ -482,10 +482,13 @@ impl Step for Hook {
 
 // install a git hook to automatically run tidy, if they want
 fn install_git_hook_maybe(config: &Config) -> io::Result<()> {
-    let git = config.git().args(["rev-parse", "--git-common-dir"]).output().map(|output| {
-        assert!(output.status.success(), "failed to run `git`");
-        PathBuf::from(t!(String::from_utf8(output.stdout)).trim())
-    })?;
+    let git = helpers::git(Some(&config.src))
+        .args(["rev-parse", "--git-common-dir"])
+        .output()
+        .map(|output| {
+            assert!(output.status.success(), "failed to run `git`");
+            PathBuf::from(t!(String::from_utf8(output.stdout)).trim())
+        })?;
     let hooks_dir = git.join("hooks");
     let dst = hooks_dir.join("pre-push");
     if dst.exists() {

src/bootstrap/src/core/build_steps/toolstate.rs

@@ -5,7 +5,7 @@
 //! [Toolstate]: https://forge.rust-lang.org/infra/toolstate.html
 
 use crate::core::builder::{Builder, RunConfig, ShouldRun, Step};
-use crate::utils::helpers::t;
+use crate::utils::helpers::{self, t};
 use serde_derive::{Deserialize, Serialize};
 use std::collections::HashMap;
 use std::env;
@@ -13,7 +13,6 @@ use std::fmt;
 use std::fs;
 use std::io::{Seek, SeekFrom};
 use std::path::{Path, PathBuf};
-use std::process::Command;
 use std::time;
 
 // Each cycle is 42 days long (6 weeks); the last week is 35..=42 then.
@@ -102,12 +101,8 @@ fn print_error(tool: &str, submodule: &str) {
 
 fn check_changed_files(toolstates: &HashMap<Box<str>, ToolState>) {
     // Changed files
-    let output = std::process::Command::new("git")
-        .arg("diff")
-        .arg("--name-status")
-        .arg("HEAD")
-        .arg("HEAD^")
-        .output();
+    let output =
+        helpers::git(None).arg("diff").arg("--name-status").arg("HEAD").arg("HEAD^").output();
     let output = match output {
         Ok(o) => o,
         Err(e) => {
@@ -324,7 +319,7 @@ fn checkout_toolstate_repo() {
         t!(fs::remove_dir_all(TOOLSTATE_DIR));
     }
 
-    let status = Command::new("git")
+    let status = helpers::git(None)
         .arg("clone")
         .arg("--depth=1")
         .arg(toolstate_repo())
@@ -342,7 +337,7 @@ fn checkout_toolstate_repo() {
 /// Sets up config and authentication for modifying the toolstate repo.
 fn prepare_toolstate_config(token: &str) {
     fn git_config(key: &str, value: &str) {
-        let status = Command::new("git").arg("config").arg("--global").arg(key).arg(value).status();
+        let status = helpers::git(None).arg("config").arg("--global").arg(key).arg(value).status();
         let success = match status {
             Ok(s) => s.success(),
             Err(_) => false,
@@ -406,8 +401,7 @@ fn commit_toolstate_change(current_toolstate: &ToolstateData) {
         publish_test_results(current_toolstate);
 
         // `git commit` failing means nothing to commit.
-        let status = t!(Command::new("git")
-            .current_dir(TOOLSTATE_DIR)
+        let status = t!(helpers::git(Some(Path::new(TOOLSTATE_DIR)))
             .arg("commit")
             .arg("-a")
             .arg("-m")
@@ -418,8 +412,7 @@ fn commit_toolstate_change(current_toolstate: &ToolstateData) {
             break;
         }
 
-        let status = t!(Command::new("git")
-            .current_dir(TOOLSTATE_DIR)
+        let status = t!(helpers::git(Some(Path::new(TOOLSTATE_DIR)))
             .arg("push")
             .arg("origin")
             .arg("master")
@@ -431,15 +424,13 @@ fn commit_toolstate_change(current_toolstate: &ToolstateData) {
         }
         eprintln!("Sleeping for 3 seconds before retrying push");
         std::thread::sleep(std::time::Duration::from_secs(3));
-        let status = t!(Command::new("git")
-            .current_dir(TOOLSTATE_DIR)
+        let status = t!(helpers::git(Some(Path::new(TOOLSTATE_DIR)))
             .arg("fetch")
             .arg("origin")
             .arg("master")
             .status());
         assert!(status.success());
-        let status = t!(Command::new("git")
-            .current_dir(TOOLSTATE_DIR)
+        let status = t!(helpers::git(Some(Path::new(TOOLSTATE_DIR)))
             .arg("reset")
             .arg("--hard")
             .arg("origin/master")
@@ -458,7 +449,7 @@ fn commit_toolstate_change(current_toolstate: &ToolstateData) {
 /// `publish_toolstate.py` script if the PR passes all tests and is merged to
 /// master.
 fn publish_test_results(current_toolstate: &ToolstateData) {
-    let commit = t!(std::process::Command::new("git").arg("rev-parse").arg("HEAD").output());
+    let commit = t!(helpers::git(None).arg("rev-parse").arg("HEAD").output());
     let commit = t!(String::from_utf8(commit.stdout));
 
     let toolstate_serialized = t!(serde_json::to_string(&current_toolstate));

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

@@ -20,7 +20,7 @@ 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};
-use crate::utils::helpers::{exe, output, t};
+use crate::utils::helpers::{self, exe, output, t};
 use build_helper::exit;
 use serde::{Deserialize, Deserializer};
 use serde_derive::Deserialize;
@@ -1248,7 +1248,7 @@ impl Config {
 
         // 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 = Command::new("git");
+        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.
@@ -2090,15 +2090,6 @@ impl Config {
         build_helper::util::try_run(cmd, self.is_verbose())
     }
 
-    /// A git invocation which runs inside the source directory.
-    ///
-    /// Use this rather than `Command::new("git")` in order to support out-of-tree builds.
-    pub(crate) fn git(&self) -> Command {
-        let mut git = Command::new("git");
-        git.current_dir(&self.src);
-        git
-    }
-
     pub(crate) fn test_args(&self) -> Vec<&str> {
         let mut test_args = match self.cmd {
             Subcommand::Test { ref test_args, .. }
@@ -2130,7 +2121,7 @@ impl Config {
             "`Config::read_file_by_commit` is not supported in non-git sources."
         );
 
-        let mut git = self.git();
+        let mut git = helpers::git(Some(&self.src));
         git.arg("show").arg(format!("{commit}:{}", file.to_str().unwrap()));
         output(&mut git)
     }
@@ -2436,7 +2427,8 @@ impl Config {
         };
 
         // Handle running from a directory other than the top level
-        let top_level = output(self.git().args(["rev-parse", "--show-toplevel"]));
+        let top_level =
+            output(helpers::git(Some(&self.src)).args(["rev-parse", "--show-toplevel"]));
         let top_level = top_level.trim_end();
         let compiler = format!("{top_level}/compiler/");
         let library = format!("{top_level}/library/");
@@ -2444,7 +2436,7 @@ impl Config {
         // 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()
+            helpers::git(Some(&self.src))
                 .arg("rev-list")
                 .arg(format!("--author={}", self.stage0_metadata.config.git_merge_commit_email))
                 .args(["-n1", "--first-parent", "HEAD"]),
@@ -2459,8 +2451,7 @@ impl Config {
         }
 
         // Warn if there were changes to the compiler or standard library since the ancestor commit.
-        let has_changes = !t!(self
-            .git()
+        let has_changes = !t!(helpers::git(Some(&self.src))
             .args(["diff-index", "--quiet", commit, "--", &compiler, &library])
             .status())
         .success();
@@ -2533,13 +2524,14 @@ impl Config {
         if_unchanged: bool,
     ) -> Option<String> {
         // Handle running from a directory other than the top level
-        let top_level = output(self.git().args(["rev-parse", "--show-toplevel"]));
+        let top_level =
+            output(helpers::git(Some(&self.src)).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()
+            helpers::git(Some(&self.src))
                 .arg("rev-list")
                 .arg(format!("--author={}", self.stage0_metadata.config.git_merge_commit_email))
                 .args(["-n1", "--first-parent", "HEAD"]),
@@ -2554,7 +2546,7 @@ impl Config {
         }
 
         // Warn if there were changes to the compiler or standard library since the ancestor commit.
-        let mut git = self.git();
+        let mut git = helpers::git(Some(&self.src));
         git.args(["diff-index", "--quiet", commit, "--"]);
 
         for path in modified_paths {

src/bootstrap/src/lib.rs

@@ -522,14 +522,10 @@ impl Build {
 
         // check_submodule
         let checked_out_hash =
-            output(Command::new("git").args(["rev-parse", "HEAD"]).current_dir(&absolute_path));
+            output(helpers::git(Some(&absolute_path)).args(["rev-parse", "HEAD"]));
         // update_submodules
-        let recorded = output(
-            Command::new("git")
-                .args(["ls-tree", "HEAD"])
-                .arg(relative_path)
-                .current_dir(&self.config.src),
-        );
+        let recorded =
+            output(helpers::git(Some(&self.src)).args(["ls-tree", "HEAD"]).arg(relative_path));
         let actual_hash = recorded
             .split_whitespace()
             .nth(2)
@@ -543,10 +539,7 @@ impl Build {
 
         println!("Updating submodule {}", relative_path.display());
         self.run(
-            Command::new("git")
-                .args(["submodule", "-q", "sync"])
-                .arg(relative_path)
-                .current_dir(&self.config.src),
+            helpers::git(Some(&self.src)).args(["submodule", "-q", "sync"]).arg(relative_path),
         );
 
         // Try passing `--progress` to start, then run git again without if that fails.
@@ -554,9 +547,7 @@ impl Build {
             // 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 = {
-                let output = self
-                    .config
-                    .git()
+                let output = helpers::git(Some(&self.src))
                     .args(["symbolic-ref", "--short", "HEAD"])
                     .stderr(Stdio::inherit())
                     .output();
@@ -568,7 +559,7 @@ impl Build {
                 }
             };
 
-            let mut git = self.config.git();
+            let mut git = helpers::git(Some(&self.src));
             if let Some(branch) = current_branch {
                 // 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.
@@ -590,11 +581,11 @@ impl Build {
         // 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 = !self.run_cmd(
-            BootstrapCommand::from(
-                Command::new("git")
-                    .args(["diff-index", "--quiet", "HEAD"])
-                    .current_dir(&absolute_path),
-            )
+            BootstrapCommand::from(helpers::git(Some(&absolute_path)).args([
+                "diff-index",
+                "--quiet",
+                "HEAD",
+            ]))
             .allow_failure()
             .output_mode(match self.is_verbose() {
                 true => OutputMode::PrintAll,
@@ -602,14 +593,14 @@ impl Build {
             }),
         );
         if has_local_modifications {
-            self.run(Command::new("git").args(["stash", "push"]).current_dir(&absolute_path));
+            self.run(helpers::git(Some(&absolute_path)).args(["stash", "push"]));
         }
 
-        self.run(Command::new("git").args(["reset", "-q", "--hard"]).current_dir(&absolute_path));
-        self.run(Command::new("git").args(["clean", "-qdfx"]).current_dir(&absolute_path));
+        self.run(helpers::git(Some(&absolute_path)).args(["reset", "-q", "--hard"]));
+        self.run(helpers::git(Some(&absolute_path)).args(["clean", "-qdfx"]));
 
         if has_local_modifications {
-            self.run(Command::new("git").args(["stash", "pop"]).current_dir(absolute_path));
+            self.run(helpers::git(Some(&absolute_path)).args(["stash", "pop"]));
         }
     }
 
@@ -621,8 +612,7 @@ impl Build {
             return;
         }
         let output = output(
-            self.config
-                .git()
+            helpers::git(Some(&self.src))
                 .args(["config", "--file"])
                 .arg(self.config.src.join(".gitmodules"))
                 .args(["--get-regexp", "path"]),
@@ -1563,10 +1553,14 @@ impl Build {
             // Figure out how many merge commits happened since we branched off master.
             // That's our beta number!
             // (Note that we use a `..` range, not the `...` symmetric difference.)
-            output(self.config.git().arg("rev-list").arg("--count").arg("--merges").arg(format!(
-                "refs/remotes/origin/{}..HEAD",
-                self.config.stage0_metadata.config.nightly_branch
-            )))
+            output(
+                helpers::git(Some(&self.src)).arg("rev-list").arg("--count").arg("--merges").arg(
+                    format!(
+                        "refs/remotes/origin/{}..HEAD",
+                        self.config.stage0_metadata.config.nightly_branch
+                    ),
+                ),
+            )
         });
         let n = count.trim().parse().unwrap();
         self.prerelease_version.set(Some(n));
@@ -1984,15 +1978,13 @@ fn envify(s: &str) -> String {
 /// In case of errors during `git` command execution (e.g., in tarball sources), default values
 /// are used to prevent panics.
 pub fn generate_smart_stamp_hash(dir: &Path, additional_input: &str) -> String {
-    let diff = Command::new("git")
-        .current_dir(dir)
+    let diff = helpers::git(Some(dir))
         .arg("diff")
         .output()
         .map(|o| String::from_utf8(o.stdout).unwrap_or_default())
         .unwrap_or_default();
 
-    let status = Command::new("git")
-        .current_dir(dir)
+    let status = helpers::git(Some(dir))
         .arg("status")
         .arg("--porcelain")
         .arg("-z")

src/bootstrap/src/utils/channel.rs

@@ -7,11 +7,12 @@
 
 use std::fs;
 use std::path::Path;
-use std::process::Command;
 
 use crate::utils::helpers::{output, t};
 use crate::Build;
 
+use super::helpers;
+
 #[derive(Clone, Default)]
 pub enum GitInfo {
     /// This is not a git repository.
@@ -44,7 +45,7 @@ impl GitInfo {
         }
 
         // Make sure git commands work
-        match Command::new("git").arg("rev-parse").current_dir(dir).output() {
+        match helpers::git(Some(dir)).arg("rev-parse").output() {
             Ok(ref out) if out.status.success() => {}
             _ => return GitInfo::Absent,
         }
@@ -57,17 +58,15 @@ impl GitInfo {
 
         // Ok, let's scrape some info
         let ver_date = output(
-            Command::new("git")
-                .current_dir(dir)
+            helpers::git(Some(dir))
                 .arg("log")
                 .arg("-1")
                 .arg("--date=short")
                 .arg("--pretty=format:%cd"),
         );
-        let ver_hash = output(Command::new("git").current_dir(dir).arg("rev-parse").arg("HEAD"));
-        let short_ver_hash = output(
-            Command::new("git").current_dir(dir).arg("rev-parse").arg("--short=9").arg("HEAD"),
-        );
+        let ver_hash = output(helpers::git(Some(dir)).arg("rev-parse").arg("HEAD"));
+        let short_ver_hash =
+            output(helpers::git(Some(dir)).arg("rev-parse").arg("--short=9").arg("HEAD"));
         GitInfo::Present(Some(Info {
             commit_date: ver_date.trim().to_string(),
             sha: ver_hash.trim().to_string(),

src/bootstrap/src/utils/helpers.rs

@@ -489,3 +489,19 @@ pub fn check_cfg_arg(name: &str, values: Option<&[&str]>) -> String {
     };
     format!("--check-cfg=cfg({name}{next})")
 }
+
+/// Prepares `Command` that runs git inside the source directory if given.
+///
+/// Whenever a git invocation is needed, this function should be preferred over
+/// manually building a git `Command`. This approach allows us to manage bootstrap-specific
+/// needs/hacks from a single source, rather than applying them on next to every `Command::new("git")`,
+/// which is painful to ensure that the required change is applied on each one of them correctly.
+pub fn git(source_dir: Option<&Path>) -> Command {
+    let mut git = Command::new("git");
+
+    if let Some(source_dir) = source_dir {
+        git.current_dir(source_dir);
+    }
+
+    git
+}