From b2ae2d2fc0913125b1d599ea179bcb917e4c79ea Mon Sep 17 00:00:00 2001 From: Jorge Aparicio Date: Mon, 5 May 2025 17:16:18 +0200 Subject: [PATCH] build_helper: handle emails containing square brackets The `git` API shells out to `git rev-list` and was directly passing an email string to the command's `--author` flag. That flag interprets its arguments as a regular expression, meaning that characters like square brackets (`[]`) in the email string made the command fail to find a commit matching the author e-mail This commit escapes the email string prior to passing it to `git rev-list`. The change is tested in the `bootstrap` crate because it contains the testing infrastructure to create temporary git repositories. fixes rust-lang/rust#140669 --- src/bootstrap/src/core/builder/tests.rs | 31 +++++++++++++++++++++++++ src/build_helper/src/git.rs | 20 +++++++++------- 2 files changed, 43 insertions(+), 8 deletions(-) diff --git a/src/bootstrap/src/core/builder/tests.rs b/src/bootstrap/src/core/builder/tests.rs index 51852099dc398..4998aba658e01 100644 --- a/src/bootstrap/src/core/builder/tests.rs +++ b/src/bootstrap/src/core/builder/tests.rs @@ -1,6 +1,8 @@ use std::env::VarError; use std::{panic, thread}; +use build_helper::ci::CiEnv; +use build_helper::git::GitConfig; use build_helper::stage0_parser::parse_stage0_file; use llvm::prebuilt_llvm_config; @@ -267,6 +269,35 @@ fn ci_rustc_if_unchanged_invalidate_on_library_changes_in_ci() { }); } +// this test is similar to the other two `ci_rustc_if_unchanged_` tests below but overrides +// `merge_bot_email`. it uses the lower level `build_helper::git` API because +// `parse_config_download_rustc_at` is hard-coded to use the `git_merge_commit_email` value in +// the git-tracked file `/src/stage0` +#[test] +fn ci_rustc_with_square_bracket_in_author_email() { + git_test(|ctx| { + let author_email = "bors[bot]@example.com"; + ctx.merge_bot_email = format!("Merge bot <{}>", author_email); + ctx.write("src/ci/channel", "nightly"); + ctx.commit(); + + let sha = ctx.create_upstream_merge(&["compiler/bar"]); + + let git_config = GitConfig { + git_repository: &ctx.git_repo, + nightly_branch: &ctx.nightly_branch, + git_merge_commit_email: author_email, + }; + let got = build_helper::git::get_closest_upstream_commit( + Some(ctx.get_path()), + &git_config, + CiEnv::None, + ); + + assert_eq!(got, Ok(Some(sha))); + }); +} + #[test] fn ci_rustc_if_unchanged_do_not_invalidate_on_library_changes_outside_ci() { git_test(|ctx| { diff --git a/src/build_helper/src/git.rs b/src/build_helper/src/git.rs index 438cd14389c1c..449fcfb9a88eb 100644 --- a/src/build_helper/src/git.rs +++ b/src/build_helper/src/git.rs @@ -9,6 +9,16 @@ pub struct GitConfig<'a> { pub git_merge_commit_email: &'a str, } +impl GitConfig<'_> { + // prepares `git_merge_commit_email` before it gets passed to `git rev-list`'s `--author` flag + // + // the `--author` flag takes a "pattern" (regular expression) not a substring so any + // square bracket in the original email needs to be escaped + fn author_email(&self) -> String { + self.git_merge_commit_email.replace('[', "\\[").replace(']', "\\]") + } +} + /// Runs a command and returns the output pub fn output_result(cmd: &mut Command) -> Result { let output = match cmd.stderr(Stdio::inherit()).output() { @@ -183,7 +193,7 @@ fn get_latest_upstream_commit_that_modified_files( "-n1", &upstream, "--author", - git_config.git_merge_commit_email, + &git_config.author_email(), ]); if !target_paths.is_empty() { @@ -227,13 +237,7 @@ fn get_closest_upstream_commit( // chronologically recent bors commit. // Here we assume that none of our subtrees use bors anymore, and that all their old bors // commits are way older than recent rustc bors commits! - git.args([ - "rev-list", - "--author-date-order", - &format!("--author={}", config.git_merge_commit_email), - "-n1", - &base, - ]); + git.args(["rev-list", "--author-date-order", "--author", &config.author_email(), "-n1", &base]); let output = output_result(&mut git)?.trim().to_owned(); if output.is_empty() { Ok(None) } else { Ok(Some(output)) }