Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

bootstrap: use git merge-base for LLVM CI download logic #113588

Merged
merged 2 commits into from
Aug 1, 2023
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 8 additions & 1 deletion src/bootstrap/llvm.rs
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@ use crate::util::{self, exe, output, t, up_to_date};
use crate::{CLang, GitRepo, Kind};

use build_helper::ci::CiEnv;
use build_helper::git::get_git_merge_base;

#[derive(Clone)]
pub struct LlvmResult {
Expand Down Expand Up @@ -128,13 +129,19 @@ pub fn prebuilt_llvm_config(
/// This retrieves the LLVM sha we *want* to use, according to git history.
pub(crate) fn detect_llvm_sha(config: &Config, is_git: bool) -> String {
let llvm_sha = if is_git {
// We proceed in 2 steps. First we get the closest commit that is actually upstream. Then we
// 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 mut rev_list = config.git();
rev_list.args(&[
PathBuf::from("rev-list"),
format!("--author={}", config.stage0_metadata.config.git_merge_commit_email).into(),
"-n1".into(),
"--first-parent".into(),
"HEAD".into(),
closest_upstream.into(),
"--".into(),
config.src.join("src/llvm-project"),
config.src.join("src/bootstrap/download-ci-llvm-stamp"),
Expand Down
38 changes: 20 additions & 18 deletions src/tools/build_helper/src/git.rs
Original file line number Diff line number Diff line change
Expand Up @@ -78,13 +78,22 @@ pub fn rev_exists(rev: &str, git_dir: Option<&Path>) -> Result<bool, String> {
/// 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)?;
let upstream_master = format!("{upstream_remote}/master");
if rev_exists(&upstream_master, git_dir)? {
return Ok(upstream_master);
for upstream_master in [format!("{upstream_remote}/master"), format!("origin/master")] {
if rev_exists(&upstream_master, git_dir)? {
return Ok(upstream_master);
}
}

// We could implement smarter logic here in the future.
Ok("origin/master".into())
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)?;
let mut git = Command::new("git");
if let Some(git_dir) = git_dir {
git.current_dir(git_dir);
}
Ok(output_result(git.arg("merge-base").arg(&updated_master).arg("HEAD"))?.trim().to_owned())
}

/// Returns the files that have been modified in the current branch compared to the master branch.
Expand All @@ -94,20 +103,13 @@ pub fn get_git_modified_files(
git_dir: Option<&Path>,
extensions: &Vec<&str>,
) -> Result<Option<Vec<String>>, String> {
let Ok(updated_master) = updated_master_branch(git_dir) else {
return Ok(None);
};

let git = || {
let mut git = Command::new("git");
if let Some(git_dir) = git_dir {
git.current_dir(git_dir);
}
git
};
let merge_base = get_git_merge_base(git_dir)?;

let merge_base = output_result(git().arg("merge-base").arg(&updated_master).arg("HEAD"))?;
let files = output_result(git().arg("diff-index").arg("--name-only").arg(merge_base.trim()))?
let mut git = Command::new("git");
if let Some(git_dir) = git_dir {
git.current_dir(git_dir);
}
let files = output_result(git.arg("diff-index").arg("--name-only").arg(merge_base.trim()))?
RalfJung marked this conversation as resolved.
Show resolved Hide resolved
.lines()
.map(|s| s.trim().to_owned())
.filter(|f| {
Expand Down