Skip to content

Commit 7a37ed9

Browse files
committed
Auto-nominate for backport a pull request fixing a regression
1 parent 96e3ca8 commit 7a37ed9

File tree

3 files changed

+284
-0
lines changed

3 files changed

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

0 commit comments

Comments
 (0)