Skip to content

Commit 4f7b659

Browse files
apirainomibac138
andcommitted
Auto-nominate for backport a pull request fixing a regression
Co-authored-by: mibac138 <[email protected]>
1 parent 96e3ca8 commit 4f7b659

File tree

3 files changed

+281
-0
lines changed

3 files changed

+281
-0
lines changed

src/config.rs

Lines changed: 41 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -47,6 +47,7 @@ pub(crate) struct Config {
4747
pub(crate) issue_links: Option<IssueLinksConfig>,
4848
pub(crate) no_mentions: Option<NoMentionsConfig>,
4949
pub(crate) behind_upstream: Option<BehindUpstreamConfig>,
50+
pub(crate) backport: Option<BackportTeamConfig>,
5051
}
5152

5253
#[derive(PartialEq, Eq, Debug, serde::Deserialize)]
@@ -522,6 +523,25 @@ fn default_true() -> bool {
522523
true
523524
}
524525

526+
#[derive(PartialEq, Eq, Debug, serde::Deserialize)]
527+
pub(crate) struct BackportTeamConfig {
528+
// Config identifier -> labels
529+
#[serde(flatten)]
530+
pub(crate) configs: HashMap<String, BackportConfig>,
531+
}
532+
533+
#[derive(Default, PartialEq, Eq, Debug, serde::Deserialize)]
534+
#[serde(rename_all = "kebab-case")]
535+
#[serde(deny_unknown_fields)]
536+
pub(crate) struct BackportConfig {
537+
/// Prerequisite label(s) (one of them) to trigger this handler for a specific team
538+
pub(crate) required_pr_labels: Vec<String>,
539+
/// Prerequisite label for an issue to qualify as regression
540+
pub(crate) required_issue_label: String,
541+
/// Labels to be added to a pull request closing the regression
542+
pub(crate) add_labels: Vec<String>,
543+
}
544+
525545
fn get_cached_config(repo: &str) -> Option<Result<Arc<Config>, ConfigurationError>> {
526546
let cache = CONFIG_CACHE.read().unwrap();
527547
cache.get(repo).and_then(|(config, fetch_time)| {
@@ -655,6 +675,11 @@ mod tests {
655675
656676
[behind-upstream]
657677
days-threshold = 14
678+
679+
[backport.teamRed]
680+
required-pr-labels = ["T-libs", "T-libs-api"]
681+
required-issue-label = "regression-from-stable-to-stable"
682+
add-labels = ["stable-nominated"]
658683
"#;
659684
let config = toml::from_str::<Config>(&config).unwrap();
660685
let mut ping_teams = HashMap::new();
@@ -679,6 +704,20 @@ mod tests {
679704
nominate_teams.insert("release".to_owned(), "T-release".to_owned());
680705
nominate_teams.insert("core".to_owned(), "T-core".to_owned());
681706
nominate_teams.insert("infra".to_owned(), "T-infra".to_owned());
707+
708+
let mut backport_configs = HashMap::new();
709+
backport_configs.insert(
710+
"teamRed".into(),
711+
BackportConfig {
712+
required_pr_labels: vec!["T-libs".into(), "T-libs-api".into()],
713+
required_issue_label: "regression-from-stable-to-stable".into(),
714+
add_labels: vec!["stable-nominated".into()],
715+
},
716+
);
717+
let backport_team_config = BackportTeamConfig {
718+
configs: backport_configs,
719+
};
720+
682721
assert_eq!(
683722
config,
684723
Config {
@@ -727,6 +766,7 @@ mod tests {
727766
concern: Some(ConcernConfig {
728767
labels: vec!["has-concerns".to_string()],
729768
}),
769+
backport: Some(backport_team_config)
730770
}
731771
);
732772
}
@@ -812,6 +852,7 @@ mod tests {
812852
behind_upstream: Some(BehindUpstreamConfig {
813853
days_threshold: Some(7),
814854
}),
855+
backport: None
815856
}
816857
);
817858
}

src/handlers.rs

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -28,6 +28,7 @@ impl fmt::Display for HandlerError {
2828

2929
mod assign;
3030
mod autolabel;
31+
mod backport;
3132
mod bot_pull_requests;
3233
mod check_commits;
3334
mod close;
@@ -225,6 +226,7 @@ macro_rules! issue_handlers {
225226
issue_handlers! {
226227
assign,
227228
autolabel,
229+
backport,
228230
issue_links,
229231
major_change,
230232
mentions,

src/handlers/backport.rs

Lines changed: 238 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,238 @@
1+
use std::collections::HashMap;
2+
use std::sync::LazyLock;
3+
4+
use crate::config::BackportTeamConfig;
5+
use crate::github::{IssuesAction, IssuesEvent, Label};
6+
use crate::handlers::Context;
7+
use anyhow::Context as AnyhowContext;
8+
use futures::future::join_all;
9+
use regex::Regex;
10+
use tracing as log;
11+
12+
// See https://docs.github.com/en/issues/tracking-your-work-with-issues/creating-issues/linking-a-pull-request-to-an-issue
13+
// See tests to see what matches
14+
static CLOSES_ISSUE_REGEXP: LazyLock<Regex> = LazyLock::new(|| {
15+
Regex::new("(?i)(?P<action>close[sd]*|fix([e]*[sd]*)?|resolve[sd]*)(?P<spaces>:? +)(?P<org_repo>[a-zA-Z0-9_-]*/[a-zA-Z0-9_-]*)?#(?P<issue_num>[0-9]+)").unwrap()
16+
});
17+
18+
const BACKPORT_LABELS: [&str; 4] = [
19+
"beta-nominated",
20+
"beta-accepted",
21+
"stable-nominated",
22+
"stable-accepted",
23+
];
24+
25+
const REGRESSION_LABELS: [&str; 3] = [
26+
"regression-from-stable-to-nightly",
27+
"regression-from-stable-to-beta",
28+
"regression-from-stable-to-stable",
29+
];
30+
31+
// auto-nominate for backport only patches fixing high/critical regressions
32+
// For `P-{medium,low}` regressions, let the author decide
33+
const PRIORITY_LABELS: [&str; 2] = ["P-high", "P-critical"];
34+
35+
#[derive(Default)]
36+
pub(crate) struct BackportInput {
37+
// Issue(s) fixed by this PR
38+
ids: Vec<u64>,
39+
// Handler configuration, it's a compound value of (required_issue_label -> add_labels)
40+
labels: HashMap<String, Vec<String>>,
41+
}
42+
43+
pub(super) async fn parse_input(
44+
_ctx: &Context,
45+
event: &IssuesEvent,
46+
config: Option<&BackportTeamConfig>,
47+
) -> Result<Option<BackportInput>, String> {
48+
let config = match config {
49+
Some(config) => config,
50+
None => return Ok(None),
51+
};
52+
53+
// Only handle events when the PR is opened or the first comment is edited
54+
if !matches!(event.action, IssuesAction::Opened | IssuesAction::Edited) || !event.issue.is_pr()
55+
{
56+
log::debug!(
57+
"Skipping backport event because: IssuesAction = {:?} issue.is_pr() {}",
58+
event.action,
59+
event.issue.is_pr()
60+
);
61+
return Ok(None);
62+
}
63+
let pr = &event.issue;
64+
65+
let pr_labels: Vec<&str> = pr.labels.iter().map(|l| l.name.as_str()).collect();
66+
if contains_any(&pr_labels, &BACKPORT_LABELS) {
67+
log::debug!("PR #{} already has a backport label", pr.number);
68+
return Ok(None);
69+
}
70+
71+
// Retrieve backport config for this PR, based on its team label(s)
72+
// If the PR has no team label matching any [backport.*.required-pr-labels] config, the backport labelling will be skipped
73+
let mut input = BackportInput::default();
74+
let valid_configs: Vec<_> = config
75+
.configs
76+
.iter()
77+
.clone()
78+
.filter(|(_cfg_name, cfg)| {
79+
let required_pr_labels: Vec<&str> =
80+
cfg.required_pr_labels.iter().map(|l| l.as_str()).collect();
81+
if !contains_any(&pr_labels, &required_pr_labels) {
82+
log::warn!(
83+
"Skipping backport nomination: PR is missing one required team label: {:?}",
84+
pr_labels
85+
);
86+
return false;
87+
}
88+
input
89+
.labels
90+
.insert(cfg.required_issue_label.clone(), cfg.add_labels.clone());
91+
true
92+
})
93+
.collect();
94+
if valid_configs.is_empty() {
95+
log::warn!(
96+
"Skipping backport nomination: could not find a suitable backport config. Please ensure the triagebot.toml has a `[backport.*.required-pr-labels]` section matching the team label(s) for PR #{}.",
97+
pr.number
98+
);
99+
return Ok(None);
100+
}
101+
102+
// Check marker text in the opening comment of the PR to retrieve the issue(s) being fixed
103+
for caps in CLOSES_ISSUE_REGEXP.captures_iter(&event.issue.body) {
104+
let id = caps
105+
.name("issue_num")
106+
.expect("Failed to cast issue_num to string")
107+
.as_str();
108+
let id = match id.parse::<u64>() {
109+
Ok(id) => id,
110+
Err(err) => {
111+
return Err(format!("Failed to parse issue id `{id}`, error: {err}"));
112+
}
113+
};
114+
input.ids.push(id);
115+
}
116+
117+
if input.ids.is_empty() || input.labels.is_empty() {
118+
return Ok(None);
119+
}
120+
121+
log::debug!(
122+
"Will handle event action {:?} in backport. Regression IDs found {:?}",
123+
event.action,
124+
input.ids
125+
);
126+
127+
Ok(Some(input))
128+
}
129+
130+
pub(super) async fn handle_input(
131+
ctx: &Context,
132+
_config: &BackportTeamConfig,
133+
event: &IssuesEvent,
134+
input: BackportInput,
135+
) -> anyhow::Result<()> {
136+
let pr = &event.issue;
137+
138+
// Retrieve the issue(s) this pull request closes
139+
let issues = input
140+
.ids
141+
.iter()
142+
.copied()
143+
.map(|id| async move { event.repository.get_issue(&ctx.github, id).await });
144+
let issues = join_all(issues).await;
145+
146+
// Add backport nomination label to the pull request
147+
for issue in issues {
148+
let issue = issue.expect("Failed to get issue from iterator");
149+
let issue_labels: Vec<&str> = issue.labels.iter().map(|l| l.name.as_str()).collect();
150+
151+
// Check issue for a prerequisite priority label
152+
// If none, skip this issue
153+
if !contains_any(&issue_labels, &PRIORITY_LABELS) {
154+
continue;
155+
}
156+
157+
// Get the labels to be added the PR according to the matching (required) regression label
158+
// that is found in the configuration that this handler has received
159+
// If no regression label is found, skip this issue
160+
let add_labels = issue_labels.iter().find_map(|l| input.labels.get(*l));
161+
if add_labels.is_none() {
162+
log::warn!(
163+
"Skipping backport nomination: nothing to do for issue #{}. No config found for regression label ({:?})",
164+
issue.number,
165+
REGRESSION_LABELS
166+
);
167+
continue;
168+
}
169+
170+
// Add backport nomination label(s) to PR
171+
let mut new_labels = pr.labels().to_owned();
172+
new_labels.extend(
173+
add_labels
174+
.expect("failed to unwrap add_labels")
175+
.iter()
176+
.cloned()
177+
.map(|name| Label { name }),
178+
);
179+
log::debug!(
180+
"PR#{} adding labels for backport {:?}",
181+
pr.number,
182+
add_labels
183+
);
184+
let _ = pr
185+
.add_labels(&ctx.github, new_labels)
186+
.await
187+
.context("failed to add backport labels to the PR");
188+
}
189+
190+
Ok(())
191+
}
192+
193+
fn contains_any(haystack: &[&str], needles: &[&str]) -> bool {
194+
needles.iter().any(|needle| haystack.contains(needle))
195+
}
196+
197+
#[cfg(test)]
198+
mod tests {
199+
use crate::handlers::backport::CLOSES_ISSUE_REGEXP;
200+
201+
#[tokio::test]
202+
async fn backport_match_comment() {
203+
let test_strings = vec![
204+
("close #10", vec![10]),
205+
("closes #10", vec![10]),
206+
("closed #10", vec![10]),
207+
("Closes #10", vec![10]),
208+
("close #10", vec![10]),
209+
("close rust-lang/rust#10", vec![10]),
210+
("cLose: rust-lang/rust#10", vec![10]),
211+
("fix #10", vec![10]),
212+
("fixes #10", vec![10]),
213+
("fixed #10", vec![10]),
214+
("resolve #10", vec![10]),
215+
("resolves #10", vec![10]),
216+
("resolved #10", vec![10]),
217+
(
218+
"Fixes #20, Resolves #21, closed #22, LOL #23",
219+
vec![20, 21, 22],
220+
),
221+
("Resolved #10", vec![10]),
222+
("Fixes #10", vec![10]),
223+
("Closes #10", vec![10]),
224+
];
225+
for test_case in test_strings {
226+
let mut ids: Vec<u64> = vec![];
227+
let test_str = test_case.0;
228+
let expected = test_case.1;
229+
for caps in CLOSES_ISSUE_REGEXP.captures_iter(test_str) {
230+
// eprintln!("caps {:?}", caps);
231+
let id = &caps["issue_num"];
232+
ids.push(id.parse::<u64>().unwrap());
233+
}
234+
// eprintln!("ids={:?}", ids);
235+
assert_eq!(ids, expected);
236+
}
237+
}
238+
}

0 commit comments

Comments
 (0)