From e9691a062ee6f70aa958b74bf2a3080af9962c29 Mon Sep 17 00:00:00 2001 From: moxian Date: Sat, 3 May 2025 14:14:38 -0700 Subject: [PATCH 1/2] Use PR descriptions when presenting affected pr list. Also expand rollup description to show individual rolled up prs --- src/github.rs | 5 --- src/main.rs | 91 +++++++++++++++++++++++++++++++++++++++++++++++---- 2 files changed, 84 insertions(+), 12 deletions(-) diff --git a/src/github.rs b/src/github.rs index ca2a06f..5a0807f 100644 --- a/src/github.rs +++ b/src/github.rs @@ -177,11 +177,6 @@ impl CommitsQuery<'_> { } } - eprintln!( - "get_commits_between returning commits, len: {}", - commits.len() - ); - // reverse to obtain chronological order commits.reverse(); Ok(commits) diff --git a/src/main.rs b/src/main.rs index 65cab99..e3d31de 100644 --- a/src/main.rs +++ b/src/main.rs @@ -979,6 +979,41 @@ fn toolchains_between(cfg: &Config, a: ToolchainSpec, b: ToolchainSpec) -> Vec String { + let bors_re = regex::Regex::new( + r"Auto merge of #(?\d+) - (?[^:]+):.*\n\s*\n(?.*)", + ) + .unwrap(); + let Some(cap) = bors_re.captures(&commit_desc) else { + println!("failed"); + // If we can't parse the commit description - return the first line as is + let desc = commit_desc + .split('\n') + .next() + .unwrap_or(""); + return format!(" - {}", desc); + }; + let mut out = format!( + " - #{} ({}) by {}", + &cap["pr_num"], &cap["pr_summary"], &cap["author"] + ); + + // if it's a rollup, try to also add info on the sub-prs + let rollup_re = regex::Regex::new(r"Rollup of \d+ pull requests").unwrap(); + if rollup_re.is_match(&commit_desc) { + let ok_merges_end = commit_desc + .find("Failed merges") + .unwrap_or(commit_desc.len()); + let merge_list = &commit_desc[..ok_merges_end]; + + let merge_re = regex::Regex::new(r"- #\d+ \(.*\)").unwrap(); + for merge in merge_re.captures_iter(&merge_list) { + out.push_str(&format!("\n {}", &merge[0])); + } + } + out +} + impl Config { // CI branch of bisect execution fn bisect_ci(&self, start: &str, end: &str) -> anyhow::Result { @@ -1027,13 +1062,9 @@ impl Config { sorted_by_date }); - for (j, commit) in commits.iter().enumerate() { - eprintln!( - " commit[{}] {}: {}", - j, - commit.date, - commit.summary.split('\n').next().unwrap() - ) + eprintln!("PRs in range:"); + for (_j, commit) in commits.iter().enumerate() { + eprintln!("{}", format_commit_line(&commit.summary)) } self.bisect_ci_in_commits(start_sha, &end.sha, commits) @@ -1411,4 +1442,50 @@ In the case of a perf regression, run the following command for each PR you susp context.descriptions, ); } + + #[test] + fn test_rollup_description1() { + // check that the we correctly parse rollup commit message when trying to pretty print them + let desc = r#"Auto merge of #125028 - matthiaskrgr:rollup-3qk782d, r=matthiaskrgr + +Rollup of 6 pull requests + +Successful merges: + + - #124096 (Clean up users of rust_dbg_call) + - #124829 (Enable profiler for armv7-unknown-linux-gnueabihf.) + +r? `@ghost` +`@rustbot` modify labels: rollup"#; + let formatted = format_commit_line(desc); + let expected = r#" - #125028 (Rollup of 6 pull requests) by matthiaskrgr + - #124096 (Clean up users of rust_dbg_call) + - #124829 (Enable profiler for armv7-unknown-linux-gnueabihf.)"#; + assert_eq!(formatted, expected); + } + + #[test] + fn test_rollup_description2() { + // check that only successful merges get included + let desc = r#"Auto merge of #140520 - matthiaskrgr:rollup-7aoqcnp, r=matthiaskrgr + +Rollup of 9 pull requests + +Successful merges: + + - #134232 (Share the naked asm impl between cg_ssa and cg_clif) + - #139624 (Don't allow flattened format_args in const.) + +Failed merges: + + - #140374 (Resolve instance for SymFn in global/naked asm) + +r? `@ghost` +`@rustbot` modify labels: rollup"#; + let formatted = format_commit_line(desc); + let expected = r#" - #140520 (Rollup of 9 pull requests) by matthiaskrgr + - #134232 (Share the naked asm impl between cg_ssa and cg_clif) + - #139624 (Don't allow flattened format_args in const.)"#; + assert_eq!(formatted, expected); + } } From c4a426a6b5707314d9ddc6e3d67376aad66ca000 Mon Sep 17 00:00:00 2001 From: moxian Date: Mon, 12 May 2025 00:18:51 -0700 Subject: [PATCH 2/2] Add github collapsible formatting to the list-of-prs report --- src/main.rs | 138 +++++++++++++++++++++++++++++++--------------------- 1 file changed, 82 insertions(+), 56 deletions(-) diff --git a/src/main.rs b/src/main.rs index e3d31de..97ac0ee 100644 --- a/src/main.rs +++ b/src/main.rs @@ -477,7 +477,8 @@ impl Config { // bisection entry point fn bisect(&self) -> anyhow::Result<()> { if let Bounds::Commits { start, end } = &self.bounds { - let bisection_result = self.bisect_ci(start, end)?; + eprintln!("bisecting ci builds starting at {start}, ending at {end}"); + let bisection_result = self.bisect_ci_via(start, end, &format!("{start}..{end}"))?; self.print_results(&bisection_result); self.do_perf_search(&bisection_result); } else { @@ -511,7 +512,11 @@ impl Config { date.format(YYYY_MM_DD), ); - let ci_bisection_result = self.bisect_ci_via(&working_commit, &bad_commit)?; + let ci_bisection_result = self.bisect_ci_via( + &working_commit, + &bad_commit, + &nightly_regression.spec.to_string(), + )?; self.print_results(&ci_bisection_result); self.do_perf_search(&ci_bisection_result); @@ -646,6 +651,64 @@ fn remove_toolchain(cfg: &Config, toolchain: &Toolchain, dl_params: &DownloadPar } } +fn format_commit_line(commit_desc: &str, terse: bool) -> String { + let bors_summary_re = regex::Regex::new( + r"Auto merge of #(?\d+) - (?[^:]+):.*\n\s*\n(?.*)", + ) + .unwrap(); + + let Some(cap) = bors_summary_re.captures(&commit_desc) else { + // If we can't parse the commit description - return the first line as is + let desc = commit_desc + .split('\n') + .next() + .unwrap_or(""); + return format!("- {}", desc); + }; + + let mut out = format!( + " - #{} ({}) by {}", + &cap["pr_num"], &cap["pr_summary"], &cap["author"] + ); + + // if it's a rollup (and assuming we're not starved for screen space), try to also add info on the sub-prs + if !terse { + let rollup_summary_re = regex::Regex::new(r"Rollup of \d+ pull requests").unwrap(); + if rollup_summary_re.is_match(&commit_desc) { + // do not get confused by PRs that are listed in the description, but not actually part of the rollup PR + let ok_merges_end = commit_desc + .find("Failed merges") + .unwrap_or(commit_desc.len()); + let merge_list = &commit_desc[..ok_merges_end]; + + let merge_re = regex::Regex::new(r"- #(?\d+) \((?.*)\)").unwrap(); + for merge in merge_re.captures_iter(&merge_list) { + out.push_str(&format!( + "\n - #{} ({})", + &merge["pr_num"], &merge["pr_summary"] + )); + } + } + } + out +} + +fn format_commit_range_report(commits: &[Commit], range_desc: &str) -> String { + let mut report_lines: Vec = vec![]; + report_lines.push(format!( + "
\nRegression in {range_desc}. PRs in range: \n\n```" + )); + // normally we want to display a verbose report of contained PRs, + // but that might explode the terminal if the user explicitly set start/end commit SHAs + // spanning more than several days. + let terse = commits.len() > 50; + for (_j, commit) in commits.iter().enumerate() { + report_lines.push(format_commit_line(&commit.summary, terse)) + } + report_lines.push("```\n
".to_string()); + report_lines.join("\n") +} + fn print_final_report( cfg: &Config, nightly_bisection_result: &BisectionResult, @@ -979,49 +1042,13 @@ fn toolchains_between(cfg: &Config, a: ToolchainSpec, b: ToolchainSpec) -> Vec String { - let bors_re = regex::Regex::new( - r"Auto merge of #(?\d+) - (?[^:]+):.*\n\s*\n(?.*)", - ) - .unwrap(); - let Some(cap) = bors_re.captures(&commit_desc) else { - println!("failed"); - // If we can't parse the commit description - return the first line as is - let desc = commit_desc - .split('\n') - .next() - .unwrap_or(""); - return format!(" - {}", desc); - }; - let mut out = format!( - " - #{} ({}) by {}", - &cap["pr_num"], &cap["pr_summary"], &cap["author"] - ); - - // if it's a rollup, try to also add info on the sub-prs - let rollup_re = regex::Regex::new(r"Rollup of \d+ pull requests").unwrap(); - if rollup_re.is_match(&commit_desc) { - let ok_merges_end = commit_desc - .find("Failed merges") - .unwrap_or(commit_desc.len()); - let merge_list = &commit_desc[..ok_merges_end]; - - let merge_re = regex::Regex::new(r"- #\d+ \(.*\)").unwrap(); - for merge in merge_re.captures_iter(&merge_list) { - out.push_str(&format!("\n {}", &merge[0])); - } - } - out -} - impl Config { - // CI branch of bisect execution - fn bisect_ci(&self, start: &str, end: &str) -> anyhow::Result { - eprintln!("bisecting ci builds starting at {start}, ending at {end}"); - self.bisect_ci_via(start, end) - } - - fn bisect_ci_via(&self, start_sha: &str, end_sha: &str) -> anyhow::Result { + fn bisect_ci_via( + &self, + start_sha: &str, + end_sha: &str, + range_desc: &str, + ) -> anyhow::Result { let access = self.args.access.repo(); let start = access.commit(start_sha)?; let end = access.commit(end_sha)?; @@ -1062,10 +1089,9 @@ impl Config { sorted_by_date }); - eprintln!("PRs in range:"); - for (_j, commit) in commits.iter().enumerate() { - eprintln!("{}", format_commit_line(&commit.summary)) - } + let report = format_commit_range_report(&commits, range_desc); + let divider = "*".repeat(20); + eprintln!("{divider}\n\n{report}\n\n{divider}"); self.bisect_ci_in_commits(start_sha, &end.sha, commits) } @@ -1457,10 +1483,10 @@ Successful merges: r? `@ghost` `@rustbot` modify labels: rollup"#; - let formatted = format_commit_line(desc); - let expected = r#" - #125028 (Rollup of 6 pull requests) by matthiaskrgr - - #124096 (Clean up users of rust_dbg_call) - - #124829 (Enable profiler for armv7-unknown-linux-gnueabihf.)"#; + let formatted = format_commit_line(desc, false); + let expected = r#" - #125028 (Rollup of 6 pull requests) by matthiaskrgr + - #124096 (Clean up users of rust_dbg_call) + - #124829 (Enable profiler for armv7-unknown-linux-gnueabihf.)"#; assert_eq!(formatted, expected); } @@ -1482,10 +1508,10 @@ Failed merges: r? `@ghost` `@rustbot` modify labels: rollup"#; - let formatted = format_commit_line(desc); - let expected = r#" - #140520 (Rollup of 9 pull requests) by matthiaskrgr - - #134232 (Share the naked asm impl between cg_ssa and cg_clif) - - #139624 (Don't allow flattened format_args in const.)"#; + let formatted = format_commit_line(desc, false); + let expected = r#" - #140520 (Rollup of 9 pull requests) by matthiaskrgr + - #134232 (Share the naked asm impl between cg_ssa and cg_clif) + - #139624 (Don't allow flattened format_args in const.)"#; assert_eq!(formatted, expected); } }