Skip to content

Commit

Permalink
Auto merge of #117122 - ferrocene:pa-configure-git-diff, r=albertlars…
Browse files Browse the repository at this point in the history
…an68

Allow configuring the parent GitHub repository

The git integration in build_helper hardcoded `rust-lang/rust` as the parent GitHub repository, and `master` as the branch name. This works great for `rust-lang/rust`, but causes problems in downstream forks like Ferrocene whenever those functions are invoked (like `./x fmt`).

In `src/stage0.json` there was already a configuration key for the name of the nightly branch, but it wasn't used by build_helper. This PR adds the `github_repository` key to the file, and requires both values to be passed to build_helper whenever a git function is called. This will allow downstream forks to tweak the values.
  • Loading branch information
bors committed Nov 9, 2023
2 parents b7583d3 + 488dd9b commit 4c8862b
Show file tree
Hide file tree
Showing 11 changed files with 93 additions and 21 deletions.
2 changes: 1 addition & 1 deletion src/bootstrap/src/core/build_steps/format.rs
Original file line number Diff line number Diff line change
Expand Up @@ -89,7 +89,7 @@ fn get_modified_rs_files(build: &Builder<'_>) -> Result<Option<Vec<String>>, Str
return Ok(None);
}

get_git_modified_files(Some(&build.config.src), &vec!["rs"])
get_git_modified_files(&build.config.git_config(), Some(&build.config.src), &vec!["rs"])
}

#[derive(serde_derive::Deserialize)]
Expand Down
4 changes: 2 additions & 2 deletions src/bootstrap/src/core/build_steps/llvm.rs
Original file line number Diff line number Diff line change
Expand Up @@ -132,8 +132,8 @@ pub(crate) fn detect_llvm_sha(config: &Config, is_git: bool) -> String {
// walk back further to the last bors merge commit that actually changed LLVM. The first
// step will fail on CI because only the `auto` branch exists; we just fall back to `HEAD`
// in that case.
let closest_upstream =
get_git_merge_base(Some(&config.src)).unwrap_or_else(|_| "HEAD".into());
let closest_upstream = get_git_merge_base(&config.git_config(), Some(&config.src))
.unwrap_or_else(|_| "HEAD".into());
let mut rev_list = config.git();
rev_list.args(&[
PathBuf::from("rev-list"),
Expand Down
9 changes: 7 additions & 2 deletions src/bootstrap/src/core/build_steps/suggest.rs
Original file line number Diff line number Diff line change
Expand Up @@ -9,8 +9,13 @@ use crate::core::builder::Builder;

/// Suggests a list of possible `x.py` commands to run based on modified files in branch.
pub fn suggest(builder: &Builder<'_>, run: bool) {
let suggestions =
builder.tool_cmd(Tool::SuggestTests).output().expect("failed to run `suggest-tests` tool");
let git_config = builder.config.git_config();
let suggestions = builder
.tool_cmd(Tool::SuggestTests)
.env("SUGGEST_TESTS_GIT_REPOSITORY", git_config.git_repository)
.env("SUGGEST_TESTS_NIGHTLY_BRANCH", git_config.nightly_branch)
.output()
.expect("failed to run `suggest-tests` tool");

if !suggestions.status.success() {
println!("failed to run `suggest-tests` tool ({})", suggestions.status);
Expand Down
4 changes: 4 additions & 0 deletions src/bootstrap/src/core/build_steps/test.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1971,6 +1971,10 @@ note: if you're sure you want to do this, please open an issue as to why. In the
cmd.arg("--git-hash");
}

let git_config = builder.config.git_config();
cmd.arg("--git-repository").arg(git_config.git_repository);
cmd.arg("--nightly-branch").arg(git_config.nightly_branch);

builder.ci_env.force_coloring_in_ci(&mut cmd);

#[cfg(feature = "build-metrics")]
Expand Down
9 changes: 9 additions & 0 deletions src/bootstrap/src/core/config/config.rs
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,7 @@ use serde::{Deserialize, Deserializer};
use serde_derive::Deserialize;

pub use crate::core::config::flags::Subcommand;
use build_helper::git::GitConfig;

macro_rules! check_ci_llvm {
($name:expr) => {
Expand Down Expand Up @@ -319,6 +320,7 @@ pub struct Stage0Config {
pub artifacts_server: String,
pub artifacts_with_llvm_assertions_server: String,
pub git_merge_commit_email: String,
pub git_repository: String,
pub nightly_branch: String,
}
#[derive(Default, Deserialize, Clone)]
Expand Down Expand Up @@ -2000,6 +2002,13 @@ impl Config {
self.rust_codegen_backends.get(0).cloned()
}

pub fn git_config(&self) -> GitConfig<'_> {
GitConfig {
git_repository: &self.stage0_metadata.config.git_repository,
nightly_branch: &self.stage0_metadata.config.nightly_branch,
}
}

pub fn check_build_rustc_version(&self, rustc_path: &str) {
if self.dry_run() {
return;
Expand Down
1 change: 1 addition & 0 deletions src/stage0.json
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@
"artifacts_server": "https://ci-artifacts.rust-lang.org/rustc-builds",
"artifacts_with_llvm_assertions_server": "https://ci-artifacts.rust-lang.org/rustc-builds-alt",
"git_merge_commit_email": "bors@rust-lang.org",
"git_repository": "rust-lang/rust",
"nightly_branch": "master"
},
"__comments": [
Expand Down
41 changes: 30 additions & 11 deletions src/tools/build_helper/src/git.rs
Original file line number Diff line number Diff line change
@@ -1,6 +1,11 @@
use std::process::Stdio;
use std::{path::Path, process::Command};

pub struct GitConfig<'a> {
pub git_repository: &'a str,
pub nightly_branch: &'a str,
}

/// Runs a command and returns the output
fn output_result(cmd: &mut Command) -> Result<String, String> {
let output = match cmd.stderr(Stdio::inherit()).output() {
Expand All @@ -27,7 +32,10 @@ fn output_result(cmd: &mut Command) -> Result<String, String> {
/// upstream https://github.com/rust-lang/rust (fetch)
/// upstream https://github.com/rust-lang/rust (push)
/// ```
pub fn get_rust_lang_rust_remote(git_dir: Option<&Path>) -> Result<String, String> {
pub fn get_rust_lang_rust_remote(
config: &GitConfig<'_>,
git_dir: Option<&Path>,
) -> Result<String, String> {
let mut git = Command::new("git");
if let Some(git_dir) = git_dir {
git.current_dir(git_dir);
Expand All @@ -37,8 +45,8 @@ pub fn get_rust_lang_rust_remote(git_dir: Option<&Path>) -> Result<String, Strin

let rust_lang_remote = stdout
.lines()
.find(|remote| remote.contains("rust-lang"))
.ok_or_else(|| "rust-lang/rust remote not found".to_owned())?;
.find(|remote| remote.contains(config.git_repository))
.ok_or_else(|| format!("{} remote not found", config.git_repository))?;

let remote_name =
rust_lang_remote.split('.').nth(1).ok_or_else(|| "remote name not found".to_owned())?;
Expand Down Expand Up @@ -76,9 +84,13 @@ pub fn rev_exists(rev: &str, git_dir: Option<&Path>) -> Result<bool, String> {
/// This could be because the user is updating their forked master branch using the GitHub UI
/// and therefore doesn't need an upstream master branch checked out.
/// We will then fall back to origin/master in the hope that at least this exists.
pub fn updated_master_branch(git_dir: Option<&Path>) -> Result<String, String> {
let upstream_remote = get_rust_lang_rust_remote(git_dir)?;
for upstream_master in [format!("{upstream_remote}/master"), format!("origin/master")] {
pub fn updated_master_branch(
config: &GitConfig<'_>,
git_dir: Option<&Path>,
) -> Result<String, String> {
let upstream_remote = get_rust_lang_rust_remote(config, git_dir)?;
let branch = config.nightly_branch;
for upstream_master in [format!("{upstream_remote}/{branch}"), format!("origin/{branch}")] {
if rev_exists(&upstream_master, git_dir)? {
return Ok(upstream_master);
}
Expand All @@ -87,8 +99,11 @@ pub fn updated_master_branch(git_dir: Option<&Path>) -> Result<String, String> {
Err(format!("Cannot find any suitable upstream master branch"))
}

pub fn get_git_merge_base(git_dir: Option<&Path>) -> Result<String, String> {
let updated_master = updated_master_branch(git_dir)?;
pub fn get_git_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);
Expand All @@ -100,10 +115,11 @@ pub fn get_git_merge_base(git_dir: Option<&Path>) -> Result<String, String> {
/// The `extensions` parameter can be used to filter the files by their extension.
/// If `extensions` is empty, all files will be returned.
pub fn get_git_modified_files(
config: &GitConfig<'_>,
git_dir: Option<&Path>,
extensions: &Vec<&str>,
) -> Result<Option<Vec<String>>, String> {
let merge_base = get_git_merge_base(git_dir)?;
let merge_base = get_git_merge_base(config, git_dir)?;

let mut git = Command::new("git");
if let Some(git_dir) = git_dir {
Expand All @@ -122,8 +138,11 @@ pub fn get_git_modified_files(
}

/// Returns the files that haven't been added to git yet.
pub fn get_git_untracked_files(git_dir: Option<&Path>) -> Result<Option<Vec<String>>, String> {
let Ok(_updated_master) = updated_master_branch(git_dir) else {
pub fn get_git_untracked_files(
config: &GitConfig<'_>,
git_dir: Option<&Path>,
) -> Result<Option<Vec<String>>, String> {
let Ok(_updated_master) = updated_master_branch(config, git_dir) else {
return Ok(None);
};
let mut git = Command::new("git");
Expand Down
9 changes: 9 additions & 0 deletions src/tools/compiletest/src/common.rs
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ use std::process::Command;
use std::str::FromStr;

use crate::util::{add_dylib_path, PathBufExt};
use build_helper::git::GitConfig;
use lazycell::AtomicLazyCell;
use serde::de::{Deserialize, Deserializer, Error as _};
use std::collections::{HashMap, HashSet};
Expand Down Expand Up @@ -379,6 +380,10 @@ pub struct Config {
pub target_cfgs: AtomicLazyCell<TargetCfgs>,

pub nocapture: bool,

// Needed both to construct build_helper::git::GitConfig
pub git_repository: String,
pub nightly_branch: String,
}

impl Config {
Expand Down Expand Up @@ -450,6 +455,10 @@ impl Config {
];
ASM_SUPPORTED_ARCHS.contains(&self.target_cfg().arch.as_str())
}

pub fn git_config(&self) -> GitConfig<'_> {
GitConfig { git_repository: &self.git_repository, nightly_branch: &self.nightly_branch }
}
}

#[derive(Debug, Clone)]
Expand Down
2 changes: 2 additions & 0 deletions src/tools/compiletest/src/header/tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -126,6 +126,8 @@ impl ConfigBuilder {
self.host.as_deref().unwrap_or("x86_64-unknown-linux-gnu"),
"--target",
self.target.as_deref().unwrap_or("x86_64-unknown-linux-gnu"),
"--git-repository=",
"--nightly-branch=",
];
let mut args: Vec<String> = args.iter().map(ToString::to_string).collect();

Expand Down
12 changes: 9 additions & 3 deletions src/tools/compiletest/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -144,7 +144,9 @@ pub fn parse_config(args: Vec<String>) -> Config {
.optflag("h", "help", "show this message")
.reqopt("", "channel", "current Rust channel", "CHANNEL")
.optflag("", "git-hash", "run tests which rely on commit version being compiled into the binaries")
.optopt("", "edition", "default Rust edition", "EDITION");
.optopt("", "edition", "default Rust edition", "EDITION")
.reqopt("", "git-repository", "name of the git repository", "ORG/REPO")
.reqopt("", "nightly-branch", "name of the git branch for nightly", "BRANCH");

let (argv0, args_) = args.split_first().unwrap();
if args.len() == 1 || args[1] == "-h" || args[1] == "--help" {
Expand Down Expand Up @@ -307,6 +309,9 @@ pub fn parse_config(args: Vec<String>) -> Config {
target_cfgs: AtomicLazyCell::new(),

nocapture: matches.opt_present("nocapture"),

git_repository: matches.opt_str("git-repository").unwrap(),
nightly_branch: matches.opt_str("nightly-branch").unwrap(),
}
}

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

let all_paths = [&files[..], &untracked_files[..]].concat();
let full_paths = {
Expand Down
21 changes: 19 additions & 2 deletions src/tools/suggest-tests/src/main.rs
Original file line number Diff line number Diff line change
@@ -1,10 +1,17 @@
use std::process::ExitCode;

use build_helper::git::get_git_modified_files;
use build_helper::git::{get_git_modified_files, GitConfig};
use suggest_tests::get_suggestions;

fn main() -> ExitCode {
let modified_files = get_git_modified_files(None, &Vec::new());
let modified_files = get_git_modified_files(
&GitConfig {
git_repository: &env("SUGGEST_TESTS_GIT_REPOSITORY"),
nightly_branch: &env("SUGGEST_TESTS_NIGHTLY_BRANCH"),
},
None,
&Vec::new(),
);
let modified_files = match modified_files {
Ok(Some(files)) => files,
Ok(None) => {
Expand All @@ -25,3 +32,13 @@ fn main() -> ExitCode {

ExitCode::SUCCESS
}

fn env(key: &str) -> String {
match std::env::var(key) {
Ok(var) => var,
Err(err) => {
eprintln!("suggest-tests: failed to read environment variable {key}: {err}");
std::process::exit(1);
}
}
}

0 comments on commit 4c8862b

Please sign in to comment.