Skip to content

Commit 004619f

Browse files
committed
unify git command preperation
Due to rust-lang#125954, we had to modify git invocations with certain flags in rust-lang#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. Signed-off-by: onur-ozkan <[email protected]>
1 parent 655600c commit 004619f

File tree

8 files changed

+85
-96
lines changed

8 files changed

+85
-96
lines changed

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

+4-8
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,7 @@
11
//! Runs rustfmt on the repository.
22
33
use crate::core::builder::Builder;
4-
use crate::utils::helpers::{output, program_out_of_date, t};
4+
use crate::utils::helpers::{self, output, program_out_of_date, t};
55
use build_helper::ci::CiEnv;
66
use build_helper::git::get_git_modified_files;
77
use ignore::WalkBuilder;
@@ -160,7 +160,7 @@ pub fn format(build: &Builder<'_>, check: bool, all: bool, paths: &[PathBuf]) {
160160
override_builder.add(&format!("!{ignore}")).expect(&ignore);
161161
}
162162
}
163-
let git_available = match Command::new("git")
163+
let git_available = match helpers::git(None)
164164
.arg("--version")
165165
.stdout(Stdio::null())
166166
.stderr(Stdio::null())
@@ -172,9 +172,7 @@ pub fn format(build: &Builder<'_>, check: bool, all: bool, paths: &[PathBuf]) {
172172

173173
let mut adjective = None;
174174
if git_available {
175-
let in_working_tree = match build
176-
.config
177-
.git()
175+
let in_working_tree = match helpers::git(Some(&build.src))
178176
.arg("rev-parse")
179177
.arg("--is-inside-work-tree")
180178
.stdout(Stdio::null())
@@ -186,9 +184,7 @@ pub fn format(build: &Builder<'_>, check: bool, all: bool, paths: &[PathBuf]) {
186184
};
187185
if in_working_tree {
188186
let untracked_paths_output = output(
189-
build
190-
.config
191-
.git()
187+
helpers::git(Some(&build.src))
192188
.arg("status")
193189
.arg("--porcelain")
194190
.arg("-z")

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

+2-2
Original file line numberDiff line numberDiff line change
@@ -159,7 +159,7 @@ pub(crate) fn detect_llvm_sha(config: &Config, is_git: bool) -> String {
159159
// in that case.
160160
let closest_upstream = get_git_merge_base(&config.git_config(), Some(&config.src))
161161
.unwrap_or_else(|_| "HEAD".into());
162-
let mut rev_list = config.git();
162+
let mut rev_list = helpers::git(Some(&config.src));
163163
rev_list.args(&[
164164
PathBuf::from("rev-list"),
165165
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 {
252252
// We assume we have access to git, so it's okay to unconditionally pass
253253
// `true` here.
254254
let llvm_sha = detect_llvm_sha(config, true);
255-
let head_sha = output(config.git().arg("rev-parse").arg("HEAD"));
255+
let head_sha = output(helpers::git(Some(&config.src)).arg("rev-parse").arg("HEAD"));
256256
let head_sha = head_sha.trim();
257257
llvm_sha == head_sha
258258
}

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

+8-5
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,7 @@
88
use crate::core::builder::{Builder, RunConfig, ShouldRun, Step};
99
use crate::t;
1010
use crate::utils::change_tracker::CONFIG_CHANGE_HISTORY;
11-
use crate::utils::helpers::hex_encode;
11+
use crate::utils::helpers::{self, hex_encode};
1212
use crate::Config;
1313
use sha2::Digest;
1414
use std::env::consts::EXE_SUFFIX;
@@ -482,10 +482,13 @@ impl Step for Hook {
482482

483483
// install a git hook to automatically run tidy, if they want
484484
fn install_git_hook_maybe(config: &Config) -> io::Result<()> {
485-
let git = config.git().args(["rev-parse", "--git-common-dir"]).output().map(|output| {
486-
assert!(output.status.success(), "failed to run `git`");
487-
PathBuf::from(t!(String::from_utf8(output.stdout)).trim())
488-
})?;
485+
let git = helpers::git(Some(&config.src))
486+
.args(["rev-parse", "--git-common-dir"])
487+
.output()
488+
.map(|output| {
489+
assert!(output.status.success(), "failed to run `git`");
490+
PathBuf::from(t!(String::from_utf8(output.stdout)).trim())
491+
})?;
489492
let hooks_dir = git.join("hooks");
490493
let dst = hooks_dir.join("pre-push");
491494
if dst.exists() {

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

+10-19
Original file line numberDiff line numberDiff line change
@@ -5,15 +5,14 @@
55
//! [Toolstate]: https://forge.rust-lang.org/infra/toolstate.html
66
77
use crate::core::builder::{Builder, RunConfig, ShouldRun, Step};
8-
use crate::utils::helpers::t;
8+
use crate::utils::helpers::{self, t};
99
use serde_derive::{Deserialize, Serialize};
1010
use std::collections::HashMap;
1111
use std::env;
1212
use std::fmt;
1313
use std::fs;
1414
use std::io::{Seek, SeekFrom};
1515
use std::path::{Path, PathBuf};
16-
use std::process::Command;
1716
use std::time;
1817

1918
// 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) {
102101

103102
fn check_changed_files(toolstates: &HashMap<Box<str>, ToolState>) {
104103
// Changed files
105-
let output = std::process::Command::new("git")
106-
.arg("diff")
107-
.arg("--name-status")
108-
.arg("HEAD")
109-
.arg("HEAD^")
110-
.output();
104+
let output =
105+
helpers::git(None).arg("diff").arg("--name-status").arg("HEAD").arg("HEAD^").output();
111106
let output = match output {
112107
Ok(o) => o,
113108
Err(e) => {
@@ -324,7 +319,7 @@ fn checkout_toolstate_repo() {
324319
t!(fs::remove_dir_all(TOOLSTATE_DIR));
325320
}
326321

327-
let status = Command::new("git")
322+
let status = helpers::git(None)
328323
.arg("clone")
329324
.arg("--depth=1")
330325
.arg(toolstate_repo())
@@ -342,7 +337,7 @@ fn checkout_toolstate_repo() {
342337
/// Sets up config and authentication for modifying the toolstate repo.
343338
fn prepare_toolstate_config(token: &str) {
344339
fn git_config(key: &str, value: &str) {
345-
let status = Command::new("git").arg("config").arg("--global").arg(key).arg(value).status();
340+
let status = helpers::git(None).arg("config").arg("--global").arg(key).arg(value).status();
346341
let success = match status {
347342
Ok(s) => s.success(),
348343
Err(_) => false,
@@ -406,8 +401,7 @@ fn commit_toolstate_change(current_toolstate: &ToolstateData) {
406401
publish_test_results(current_toolstate);
407402

408403
// `git commit` failing means nothing to commit.
409-
let status = t!(Command::new("git")
410-
.current_dir(TOOLSTATE_DIR)
404+
let status = t!(helpers::git(Some(Path::new(TOOLSTATE_DIR)))
411405
.arg("commit")
412406
.arg("-a")
413407
.arg("-m")
@@ -418,8 +412,7 @@ fn commit_toolstate_change(current_toolstate: &ToolstateData) {
418412
break;
419413
}
420414

421-
let status = t!(Command::new("git")
422-
.current_dir(TOOLSTATE_DIR)
415+
let status = t!(helpers::git(Some(Path::new(TOOLSTATE_DIR)))
423416
.arg("push")
424417
.arg("origin")
425418
.arg("master")
@@ -431,15 +424,13 @@ fn commit_toolstate_change(current_toolstate: &ToolstateData) {
431424
}
432425
eprintln!("Sleeping for 3 seconds before retrying push");
433426
std::thread::sleep(std::time::Duration::from_secs(3));
434-
let status = t!(Command::new("git")
435-
.current_dir(TOOLSTATE_DIR)
427+
let status = t!(helpers::git(Some(Path::new(TOOLSTATE_DIR)))
436428
.arg("fetch")
437429
.arg("origin")
438430
.arg("master")
439431
.status());
440432
assert!(status.success());
441-
let status = t!(Command::new("git")
442-
.current_dir(TOOLSTATE_DIR)
433+
let status = t!(helpers::git(Some(Path::new(TOOLSTATE_DIR)))
443434
.arg("reset")
444435
.arg("--hard")
445436
.arg("origin/master")
@@ -458,7 +449,7 @@ fn commit_toolstate_change(current_toolstate: &ToolstateData) {
458449
/// `publish_toolstate.py` script if the PR passes all tests and is merged to
459450
/// master.
460451
fn publish_test_results(current_toolstate: &ToolstateData) {
461-
let commit = t!(std::process::Command::new("git").arg("rev-parse").arg("HEAD").output());
452+
let commit = t!(helpers::git(None).arg("rev-parse").arg("HEAD").output());
462453
let commit = t!(String::from_utf8(commit.stdout));
463454

464455
let toolstate_serialized = t!(serde_json::to_string(&current_toolstate));

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

+12-20
Original file line numberDiff line numberDiff line change
@@ -20,7 +20,7 @@ use crate::core::build_steps::llvm;
2020
use crate::core::config::flags::{Color, Flags, Warnings};
2121
use crate::utils::cache::{Interned, INTERNER};
2222
use crate::utils::channel::{self, GitInfo};
23-
use crate::utils::helpers::{exe, output, t};
23+
use crate::utils::helpers::{self, exe, output, t};
2424
use build_helper::exit;
2525
use serde::{Deserialize, Deserializer};
2626
use serde_derive::Deserialize;
@@ -1248,7 +1248,7 @@ impl Config {
12481248

12491249
// Infer the source directory. This is non-trivial because we want to support a downloaded bootstrap binary,
12501250
// running on a completely different machine from where it was compiled.
1251-
let mut cmd = Command::new("git");
1251+
let mut cmd = helpers::git(None);
12521252
// NOTE: we cannot support running from outside the repository because the only other path we have available
12531253
// is set at compile time, which can be wrong if bootstrap was downloaded rather than compiled locally.
12541254
// We still support running outside the repository if we find we aren't in a git directory.
@@ -2101,15 +2101,6 @@ impl Config {
21012101
build_helper::util::try_run(cmd, self.is_verbose())
21022102
}
21032103

2104-
/// A git invocation which runs inside the source directory.
2105-
///
2106-
/// Use this rather than `Command::new("git")` in order to support out-of-tree builds.
2107-
pub(crate) fn git(&self) -> Command {
2108-
let mut git = Command::new("git");
2109-
git.current_dir(&self.src);
2110-
git
2111-
}
2112-
21132104
pub(crate) fn test_args(&self) -> Vec<&str> {
21142105
let mut test_args = match self.cmd {
21152106
Subcommand::Test { ref test_args, .. }
@@ -2138,10 +2129,10 @@ impl Config {
21382129
/// Return the version it would have used for the given commit.
21392130
pub(crate) fn artifact_version_part(&self, commit: &str) -> String {
21402131
let (channel, version) = if self.rust_info.is_managed_git_subrepository() {
2141-
let mut channel = self.git();
2132+
let mut channel = helpers::git(Some(&self.src));
21422133
channel.arg("show").arg(format!("{commit}:src/ci/channel"));
21432134
let channel = output(&mut channel);
2144-
let mut version = self.git();
2135+
let mut version = helpers::git(Some(&self.src));
21452136
version.arg("show").arg(format!("{commit}:src/version"));
21462137
let version = output(&mut version);
21472138
(channel.trim().to_owned(), version.trim().to_owned())
@@ -2435,15 +2426,16 @@ impl Config {
24352426
};
24362427

24372428
// Handle running from a directory other than the top level
2438-
let top_level = output(self.git().args(["rev-parse", "--show-toplevel"]));
2429+
let top_level =
2430+
output(helpers::git(Some(&self.src)).args(["rev-parse", "--show-toplevel"]));
24392431
let top_level = top_level.trim_end();
24402432
let compiler = format!("{top_level}/compiler/");
24412433
let library = format!("{top_level}/library/");
24422434

24432435
// Look for a version to compare to based on the current commit.
24442436
// Only commits merged by bors will have CI artifacts.
24452437
let merge_base = output(
2446-
self.git()
2438+
helpers::git(Some(&self.src))
24472439
.arg("rev-list")
24482440
.arg(format!("--author={}", self.stage0_metadata.config.git_merge_commit_email))
24492441
.args(["-n1", "--first-parent", "HEAD"]),
@@ -2458,8 +2450,7 @@ impl Config {
24582450
}
24592451

24602452
// Warn if there were changes to the compiler or standard library since the ancestor commit.
2461-
let has_changes = !t!(self
2462-
.git()
2453+
let has_changes = !t!(helpers::git(Some(&self.src))
24632454
.args(["diff-index", "--quiet", commit, "--", &compiler, &library])
24642455
.status())
24652456
.success();
@@ -2532,13 +2523,14 @@ impl Config {
25322523
if_unchanged: bool,
25332524
) -> Option<String> {
25342525
// Handle running from a directory other than the top level
2535-
let top_level = output(self.git().args(["rev-parse", "--show-toplevel"]));
2526+
let top_level =
2527+
output(helpers::git(Some(&self.src)).args(["rev-parse", "--show-toplevel"]));
25362528
let top_level = top_level.trim_end();
25372529

25382530
// Look for a version to compare to based on the current commit.
25392531
// Only commits merged by bors will have CI artifacts.
25402532
let merge_base = output(
2541-
self.git()
2533+
helpers::git(Some(&self.src))
25422534
.arg("rev-list")
25432535
.arg(format!("--author={}", self.stage0_metadata.config.git_merge_commit_email))
25442536
.args(["-n1", "--first-parent", "HEAD"]),
@@ -2553,7 +2545,7 @@ impl Config {
25532545
}
25542546

25552547
// Warn if there were changes to the compiler or standard library since the ancestor commit.
2556-
let mut git = self.git();
2548+
let mut git = helpers::git(Some(&self.src));
25572549
git.args(["diff-index", "--quiet", commit, "--"]);
25582550

25592551
for path in modified_paths {

0 commit comments

Comments
 (0)