From 05043a370a77f8647b5fa609488dbdfebb71d1a4 Mon Sep 17 00:00:00 2001 From: onur-ozkan Date: Mon, 9 Sep 2024 20:17:34 +0300 Subject: [PATCH 1/7] add `git_merge_commit_email` into `GitConfig` Signed-off-by: onur-ozkan --- src/bootstrap/src/core/config/config.rs | 1 + src/tools/build_helper/src/git.rs | 1 + 2 files changed, 2 insertions(+) diff --git a/src/bootstrap/src/core/config/config.rs b/src/bootstrap/src/core/config/config.rs index 68b42305fdcc8..646e76d46b680 100644 --- a/src/bootstrap/src/core/config/config.rs +++ b/src/bootstrap/src/core/config/config.rs @@ -2509,6 +2509,7 @@ impl Config { GitConfig { git_repository: &self.stage0_metadata.config.git_repository, nightly_branch: &self.stage0_metadata.config.nightly_branch, + git_merge_commit_email: &self.stage0_metadata.config.git_merge_commit_email, } } diff --git a/src/tools/build_helper/src/git.rs b/src/tools/build_helper/src/git.rs index cc48a8964a372..98e5b7a328eb9 100644 --- a/src/tools/build_helper/src/git.rs +++ b/src/tools/build_helper/src/git.rs @@ -4,6 +4,7 @@ use std::process::{Command, Stdio}; pub struct GitConfig<'a> { pub git_repository: &'a str, pub nightly_branch: &'a str, + pub git_merge_commit_email: &'a str, } /// Runs a command and returns the output From dc9c5f251c06c2c0193aff779a5074cff3d233ee Mon Sep 17 00:00:00 2001 From: onur-ozkan Date: Mon, 9 Sep 2024 20:18:06 +0300 Subject: [PATCH 2/7] implement `build_helper::git::get_closest_merge_commit` Compare to `get_git_merge_base`, this doesn't require configuring the upstream remote. Signed-off-by: onur-ozkan --- src/tools/build_helper/src/git.rs | 40 ++++++++++++++++++++++++++----- 1 file changed, 34 insertions(+), 6 deletions(-) diff --git a/src/tools/build_helper/src/git.rs b/src/tools/build_helper/src/git.rs index 98e5b7a328eb9..7da3c9de60c45 100644 --- a/src/tools/build_helper/src/git.rs +++ b/src/tools/build_helper/src/git.rs @@ -1,4 +1,4 @@ -use std::path::Path; +use std::path::{Path, PathBuf}; use std::process::{Command, Stdio}; pub struct GitConfig<'a> { @@ -96,10 +96,7 @@ pub fn updated_master_branch( Err("Cannot find any suitable upstream master branch".to_owned()) } -pub fn get_git_merge_base( - config: &GitConfig<'_>, - git_dir: Option<&Path>, -) -> Result { +fn get_git_merge_base(config: &GitConfig<'_>, git_dir: Option<&Path>) -> Result { let updated_master = updated_master_branch(config, git_dir)?; let mut git = Command::new("git"); if let Some(git_dir) = git_dir { @@ -108,6 +105,37 @@ pub fn get_git_merge_base( Ok(output_result(git.arg("merge-base").arg(&updated_master).arg("HEAD"))?.trim().to_owned()) } +/// Resolves the closest merge commit by the given `author` and `target_paths`. +/// +/// If it fails to find the commit from upstream using `git merge-base`, fallbacks to HEAD. +pub fn get_closest_merge_commit( + git_dir: Option<&Path>, + config: &GitConfig<'_>, + target_paths: &[PathBuf], +) -> Result { + let mut git = Command::new("git"); + + if let Some(git_dir) = git_dir { + git.current_dir(git_dir); + } + + let merge_base = get_git_merge_base(config, git_dir).unwrap_or_else(|_| "HEAD".into()); + + git.args([ + "rev-list", + &format!("--author={}", config.git_merge_commit_email), + "-n1", + "--first-parent", + &merge_base, + ]); + + if !target_paths.is_empty() { + git.arg("--").args(target_paths); + } + + Ok(output_result(&mut git)?.trim().to_owned()) +} + /// Returns the files that have been modified in the current branch compared to the master branch. /// The `extensions` parameter can be used to filter the files by their extension. /// Does not include removed files. @@ -117,7 +145,7 @@ pub fn get_git_modified_files( git_dir: Option<&Path>, extensions: &[&str], ) -> Result>, String> { - let merge_base = get_git_merge_base(config, git_dir)?; + let merge_base = get_closest_merge_commit(git_dir, config, &[])?; let mut git = Command::new("git"); if let Some(git_dir) = git_dir { From 9aa823cc6727fd4c68cf3fdc50478e47b5ef5989 Mon Sep 17 00:00:00 2001 From: onur-ozkan Date: Mon, 9 Sep 2024 20:19:29 +0300 Subject: [PATCH 3/7] replace `get_closest_merge_base_commit` with `get_closest_merge_commit` Signed-off-by: onur-ozkan --- src/bootstrap/src/core/build_steps/compile.rs | 14 ++++------- src/bootstrap/src/core/build_steps/llvm.rs | 4 ++-- src/bootstrap/src/core/build_steps/tool.rs | 7 +++--- src/bootstrap/src/core/config/config.rs | 20 ++++------------ src/bootstrap/src/utils/helpers.rs | 23 ------------------- 5 files changed, 15 insertions(+), 53 deletions(-) diff --git a/src/bootstrap/src/core/build_steps/compile.rs b/src/bootstrap/src/core/build_steps/compile.rs index 102c9fd255432..bb07d478f71aa 100644 --- a/src/bootstrap/src/core/build_steps/compile.rs +++ b/src/bootstrap/src/core/build_steps/compile.rs @@ -15,6 +15,7 @@ use std::path::{Path, PathBuf}; use std::process::Stdio; use std::{env, fs, str}; +use build_helper::git::get_closest_merge_commit; use serde_derive::Deserialize; use crate::core::build_steps::tool::SourceType; @@ -26,8 +27,7 @@ use crate::core::builder::{ use crate::core::config::{DebuginfoLevel, LlvmLibunwind, RustcLto, TargetSelection}; use crate::utils::exec::command; use crate::utils::helpers::{ - self, exe, get_clang_cl_resource_dir, get_closest_merge_base_commit, is_debug_info, is_dylib, - symlink_dir, t, up_to_date, + self, exe, get_clang_cl_resource_dir, is_debug_info, is_dylib, symlink_dir, t, up_to_date, }; use crate::{CLang, Compiler, DependencyType, GitRepo, Mode, LLVM_TOOLS}; @@ -127,13 +127,9 @@ impl Step for Std { // the `rust.download-rustc=true` option. let force_recompile = if builder.rust_info().is_managed_git_subrepository() && builder.download_rustc() { - let closest_merge_commit = get_closest_merge_base_commit( - Some(&builder.src), - &builder.config.git_config(), - &builder.config.stage0_metadata.config.git_merge_commit_email, - &[], - ) - .unwrap(); + let closest_merge_commit = + get_closest_merge_commit(Some(&builder.src), &builder.config.git_config(), &[]) + .unwrap(); // Check if `library` has changes (returns false otherwise) !t!(helpers::git(Some(&builder.src)) diff --git a/src/bootstrap/src/core/build_steps/llvm.rs b/src/bootstrap/src/core/build_steps/llvm.rs index 442638d32034b..94b03b1b1386b 100644 --- a/src/bootstrap/src/core/build_steps/llvm.rs +++ b/src/bootstrap/src/core/build_steps/llvm.rs @@ -16,6 +16,7 @@ use std::sync::OnceLock; use std::{env, io}; use build_helper::ci::CiEnv; +use build_helper::git::get_closest_merge_commit; use crate::core::builder::{Builder, RunConfig, ShouldRun, Step}; use crate::core::config::{Config, TargetSelection}; @@ -153,10 +154,9 @@ pub fn prebuilt_llvm_config(builder: &Builder<'_>, target: TargetSelection) -> L /// 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 { - helpers::get_closest_merge_base_commit( + get_closest_merge_commit( Some(&config.src), &config.git_config(), - &config.stage0_metadata.config.git_merge_commit_email, &[ config.src.join("src/llvm-project"), config.src.join("src/bootstrap/download-ci-llvm-stamp"), diff --git a/src/bootstrap/src/core/build_steps/tool.rs b/src/bootstrap/src/core/build_steps/tool.rs index 3c2d791c2090e..a437f829ba5a6 100644 --- a/src/bootstrap/src/core/build_steps/tool.rs +++ b/src/bootstrap/src/core/build_steps/tool.rs @@ -1,6 +1,8 @@ use std::path::PathBuf; use std::{env, fs}; +use build_helper::git::get_closest_merge_commit; + use crate::core::build_steps::compile; use crate::core::build_steps::toolstate::ToolState; use crate::core::builder; @@ -8,7 +10,7 @@ use crate::core::builder::{Builder, Cargo as CargoCommand, RunConfig, ShouldRun, use crate::core::config::TargetSelection; use crate::utils::channel::GitInfo; use crate::utils::exec::{command, BootstrapCommand}; -use crate::utils::helpers::{add_dylib_path, exe, get_closest_merge_base_commit, git, t}; +use crate::utils::helpers::{add_dylib_path, exe, git, t}; use crate::{gha, Compiler, Kind, Mode}; #[derive(Debug, Clone, Hash, PartialEq, Eq)] @@ -576,10 +578,9 @@ impl Step for Rustdoc { && target_compiler.stage > 0 && builder.rust_info().is_managed_git_subrepository() { - let commit = get_closest_merge_base_commit( + let commit = get_closest_merge_commit( Some(&builder.config.src), &builder.config.git_config(), - &builder.config.stage0_metadata.config.git_merge_commit_email, &[], ) .unwrap(); diff --git a/src/bootstrap/src/core/config/config.rs b/src/bootstrap/src/core/config/config.rs index 646e76d46b680..9be4f59bfbba4 100644 --- a/src/bootstrap/src/core/config/config.rs +++ b/src/bootstrap/src/core/config/config.rs @@ -14,7 +14,7 @@ use std::sync::OnceLock; use std::{cmp, env, fs}; use build_helper::exit; -use build_helper::git::{output_result, GitConfig}; +use build_helper::git::{get_closest_merge_commit, output_result, GitConfig}; use serde::{Deserialize, Deserializer}; use serde_derive::Deserialize; @@ -24,7 +24,7 @@ pub use crate::core::config::flags::Subcommand; use crate::core::config::flags::{Color, Flags, Warnings}; use crate::utils::cache::{Interned, INTERNER}; use crate::utils::channel::{self, GitInfo}; -use crate::utils::helpers::{self, exe, get_closest_merge_base_commit, output, t}; +use crate::utils::helpers::{self, exe, output, t}; macro_rules! check_ci_llvm { ($name:expr) => { @@ -2686,13 +2686,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 commit = get_closest_merge_base_commit( - Some(&self.src), - &self.git_config(), - &self.stage0_metadata.config.git_merge_commit_email, - &[], - ) - .unwrap(); + let commit = get_closest_merge_commit(Some(&self.src), &self.git_config(), &[]).unwrap(); if commit.is_empty() { println!("ERROR: could not find commit hash for downloading rustc"); println!("HELP: maybe your repository history is too shallow?"); @@ -2783,13 +2777,7 @@ impl Config { ) -> Option { // Look for a version to compare to based on the current commit. // Only commits merged by bors will have CI artifacts. - let commit = get_closest_merge_base_commit( - Some(&self.src), - &self.git_config(), - &self.stage0_metadata.config.git_merge_commit_email, - &[], - ) - .unwrap(); + let commit = get_closest_merge_commit(Some(&self.src), &self.git_config(), &[]).unwrap(); if commit.is_empty() { println!("error: could not find commit hash for downloading components from CI"); println!("help: maybe your repository history is too shallow?"); diff --git a/src/bootstrap/src/utils/helpers.rs b/src/bootstrap/src/utils/helpers.rs index a856c99ff55a5..beb3c5fb09842 100644 --- a/src/bootstrap/src/utils/helpers.rs +++ b/src/bootstrap/src/utils/helpers.rs @@ -10,7 +10,6 @@ use std::sync::OnceLock; use std::time::{Instant, SystemTime, UNIX_EPOCH}; use std::{env, fs, io, str}; -use build_helper::git::{get_git_merge_base, output_result, GitConfig}; use build_helper::util::fail; use crate::core::builder::Builder; @@ -523,28 +522,6 @@ pub fn git(source_dir: Option<&Path>) -> BootstrapCommand { git } -/// Returns the closest commit available from upstream for the given `author` and `target_paths`. -/// -/// If it fails to find the commit from upstream using `git merge-base`, fallbacks to HEAD. -pub fn get_closest_merge_base_commit( - source_dir: Option<&Path>, - config: &GitConfig<'_>, - author: &str, - target_paths: &[PathBuf], -) -> Result { - let mut git = git(source_dir); - - let merge_base = get_git_merge_base(config, source_dir).unwrap_or_else(|_| "HEAD".into()); - - git.args(["rev-list", &format!("--author={author}"), "-n1", "--first-parent", &merge_base]); - - if !target_paths.is_empty() { - git.arg("--").args(target_paths); - } - - Ok(output_result(git.as_command_mut())?.trim().to_owned()) -} - /// Sets the file times for a given file at `path`. pub fn set_file_times>(path: P, times: fs::FileTimes) -> io::Result<()> { // Windows requires file to be writable to modify file times. But on Linux CI the file does not From 12998c2e11a089e6c591e1704a63a7942826208e Mon Sep 17 00:00:00 2001 From: onur-ozkan Date: Mon, 9 Sep 2024 21:03:51 +0300 Subject: [PATCH 4/7] handle `GitConfig` for `tools/suggest-tests` Signed-off-by: onur-ozkan --- src/bootstrap/src/core/build_steps/suggest.rs | 1 + src/tools/suggest-tests/src/main.rs | 1 + 2 files changed, 2 insertions(+) diff --git a/src/bootstrap/src/core/build_steps/suggest.rs b/src/bootstrap/src/core/build_steps/suggest.rs index 8aaffab514d88..ba9b1b2fc3317 100644 --- a/src/bootstrap/src/core/build_steps/suggest.rs +++ b/src/bootstrap/src/core/build_steps/suggest.rs @@ -17,6 +17,7 @@ pub fn suggest(builder: &Builder<'_>, run: bool) { .tool_cmd(Tool::SuggestTests) .env("SUGGEST_TESTS_GIT_REPOSITORY", git_config.git_repository) .env("SUGGEST_TESTS_NIGHTLY_BRANCH", git_config.nightly_branch) + .env("SUGGEST_TESTS_MERGE_COMMIT_EMAIL", git_config.git_merge_commit_email) .run_capture_stdout(builder) .stdout(); diff --git a/src/tools/suggest-tests/src/main.rs b/src/tools/suggest-tests/src/main.rs index 8e3625c244916..6f09bddcf60b8 100644 --- a/src/tools/suggest-tests/src/main.rs +++ b/src/tools/suggest-tests/src/main.rs @@ -8,6 +8,7 @@ fn main() -> ExitCode { &GitConfig { git_repository: &env("SUGGEST_TESTS_GIT_REPOSITORY"), nightly_branch: &env("SUGGEST_TESTS_NIGHTLY_BRANCH"), + git_merge_commit_email: &env("SUGGEST_TESTS_MERGE_COMMIT_EMAIL"), }, None, &Vec::new(), From 35ce85e0fd6ae70ffb61139c1fcdb739e0ca1d95 Mon Sep 17 00:00:00 2001 From: onur-ozkan Date: Mon, 9 Sep 2024 21:09:30 +0300 Subject: [PATCH 5/7] handle `GitConfig` for `tools/compiletest` Signed-off-by: onur-ozkan --- src/bootstrap/src/core/build_steps/test.rs | 1 + src/tools/compiletest/src/common.rs | 7 ++++++- src/tools/compiletest/src/header/tests.rs | 1 + src/tools/compiletest/src/lib.rs | 4 +++- 4 files changed, 11 insertions(+), 2 deletions(-) diff --git a/src/bootstrap/src/core/build_steps/test.rs b/src/bootstrap/src/core/build_steps/test.rs index 83f65615c8ded..a7e9352bb1c4c 100644 --- a/src/bootstrap/src/core/build_steps/test.rs +++ b/src/bootstrap/src/core/build_steps/test.rs @@ -2098,6 +2098,7 @@ NOTE: if you're sure you want to do this, please open an issue as to why. In the 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); + cmd.arg("--git-merge-commit-email").arg(git_config.git_merge_commit_email); cmd.force_coloring_in_ci(); #[cfg(feature = "build-metrics")] diff --git a/src/tools/compiletest/src/common.rs b/src/tools/compiletest/src/common.rs index 773d795f75a72..5c18179b6fe2f 100644 --- a/src/tools/compiletest/src/common.rs +++ b/src/tools/compiletest/src/common.rs @@ -384,6 +384,7 @@ pub struct Config { // Needed both to construct build_helper::git::GitConfig pub git_repository: String, pub nightly_branch: String, + pub git_merge_commit_email: String, /// True if the profiler runtime is enabled for this target. /// Used by the "needs-profiler-support" header in test files. @@ -461,7 +462,11 @@ impl Config { } pub fn git_config(&self) -> GitConfig<'_> { - GitConfig { git_repository: &self.git_repository, nightly_branch: &self.nightly_branch } + GitConfig { + git_repository: &self.git_repository, + nightly_branch: &self.nightly_branch, + git_merge_commit_email: &self.git_merge_commit_email, + } } } diff --git a/src/tools/compiletest/src/header/tests.rs b/src/tools/compiletest/src/header/tests.rs index 29e11e77f1ce1..3a9a7eb911889 100644 --- a/src/tools/compiletest/src/header/tests.rs +++ b/src/tools/compiletest/src/header/tests.rs @@ -148,6 +148,7 @@ impl ConfigBuilder { self.target.as_deref().unwrap_or("x86_64-unknown-linux-gnu"), "--git-repository=", "--nightly-branch=", + "--git-merge-commit-email=", ]; let mut args: Vec = args.iter().map(ToString::to_string).collect(); diff --git a/src/tools/compiletest/src/lib.rs b/src/tools/compiletest/src/lib.rs index 5402e69bc664e..624111d48adfa 100644 --- a/src/tools/compiletest/src/lib.rs +++ b/src/tools/compiletest/src/lib.rs @@ -163,7 +163,8 @@ pub fn parse_config(args: Vec) -> Config { ) .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"); + .reqopt("", "nightly-branch", "name of the git branch for nightly", "BRANCH") + .reqopt("", "git-merge-commit-email", "email address used for the merge commits", "EMAIL"); let (argv0, args_) = args.split_first().unwrap(); if args.len() == 1 || args[1] == "-h" || args[1] == "--help" { @@ -346,6 +347,7 @@ pub fn parse_config(args: Vec) -> Config { git_repository: matches.opt_str("git-repository").unwrap(), nightly_branch: matches.opt_str("nightly-branch").unwrap(), + git_merge_commit_email: matches.opt_str("git-merge-commit-email").unwrap(), profiler_support: matches.opt_present("profiler-support"), } From 0a7f9e213448bfbe336bbb6d5f1f9a79961427f9 Mon Sep 17 00:00:00 2001 From: onur-ozkan Date: Mon, 9 Sep 2024 21:23:06 +0300 Subject: [PATCH 6/7] skip formatting if no files have been modified Signed-off-by: onur-ozkan --- src/bootstrap/src/core/build_steps/format.rs | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/src/bootstrap/src/core/build_steps/format.rs b/src/bootstrap/src/core/build_steps/format.rs index 91fbc57429a96..bbd81fb570bd5 100644 --- a/src/bootstrap/src/core/build_steps/format.rs +++ b/src/bootstrap/src/core/build_steps/format.rs @@ -200,6 +200,11 @@ pub fn format(build: &Builder<'_>, check: bool, all: bool, paths: &[PathBuf]) { adjective = Some("modified"); match get_modified_rs_files(build) { Ok(Some(files)) => { + if files.is_empty() { + println!("fmt info: No modified files detected for formatting."); + return; + } + for file in files { override_builder.add(&format!("/{file}")).expect(&file); } From 5f327176492e3ef66d8140fb771d77bc2b5eebac Mon Sep 17 00:00:00 2001 From: onur-ozkan Date: Tue, 10 Sep 2024 20:06:35 +0300 Subject: [PATCH 7/7] document the new git logic in more detail Signed-off-by: onur-ozkan --- src/tools/build_helper/src/git.rs | 16 ++++++++++++---- src/tools/compiletest/src/lib.rs | 7 ++++++- 2 files changed, 18 insertions(+), 5 deletions(-) diff --git a/src/tools/build_helper/src/git.rs b/src/tools/build_helper/src/git.rs index 7da3c9de60c45..15d863caf0c5f 100644 --- a/src/tools/build_helper/src/git.rs +++ b/src/tools/build_helper/src/git.rs @@ -96,7 +96,14 @@ pub fn updated_master_branch( Err("Cannot find any suitable upstream master branch".to_owned()) } -fn get_git_merge_base(config: &GitConfig<'_>, git_dir: Option<&Path>) -> Result { +/// Finds the nearest merge commit by comparing the local `HEAD` with the upstream branch's state. +/// To work correctly, the upstream remote must be properly configured using `git remote add `. +/// In most cases `get_closest_merge_commit` is the function you are looking for as it doesn't require remote +/// to be configured. +fn git_upstream_merge_base( + config: &GitConfig<'_>, + git_dir: Option<&Path>, +) -> Result { let updated_master = updated_master_branch(config, git_dir)?; let mut git = Command::new("git"); if let Some(git_dir) = git_dir { @@ -105,9 +112,10 @@ fn get_git_merge_base(config: &GitConfig<'_>, git_dir: Option<&Path>) -> Result< Ok(output_result(git.arg("merge-base").arg(&updated_master).arg("HEAD"))?.trim().to_owned()) } -/// Resolves the closest merge commit by the given `author` and `target_paths`. +/// Searches for the nearest merge commit in the repository that also exists upstream. /// -/// If it fails to find the commit from upstream using `git merge-base`, fallbacks to HEAD. +/// If it fails to find the upstream remote, it then looks for the most recent commit made +/// by the merge bot by matching the author's email address with the merge bot's email. pub fn get_closest_merge_commit( git_dir: Option<&Path>, config: &GitConfig<'_>, @@ -119,7 +127,7 @@ pub fn get_closest_merge_commit( git.current_dir(git_dir); } - let merge_base = get_git_merge_base(config, git_dir).unwrap_or_else(|_| "HEAD".into()); + let merge_base = git_upstream_merge_base(config, git_dir).unwrap_or_else(|_| "HEAD".into()); git.args([ "rev-list", diff --git a/src/tools/compiletest/src/lib.rs b/src/tools/compiletest/src/lib.rs index 624111d48adfa..5324c0a494ac0 100644 --- a/src/tools/compiletest/src/lib.rs +++ b/src/tools/compiletest/src/lib.rs @@ -164,7 +164,12 @@ pub fn parse_config(args: Vec) -> Config { .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") - .reqopt("", "git-merge-commit-email", "email address used for the merge commits", "EMAIL"); + .reqopt( + "", + "git-merge-commit-email", + "email address used for finding merge commits", + "EMAIL", + ); let (argv0, args_) = args.split_first().unwrap(); if args.len() == 1 || args[1] == "-h" || args[1] == "--help" {