Skip to content

Commit c78f761

Browse files
authored
Merge pull request #2092 from apiraino/auto-nominate-backports
Auto-nominate for backport a pull request fixing a regression
2 parents cee576f + 2556583 commit c78f761

File tree

3 files changed

+295
-0
lines changed

3 files changed

+295
-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<BackportConfig>,
5051
}
5152

5253
#[derive(PartialEq, Eq, Debug, serde::Deserialize)]
@@ -531,6 +532,25 @@ fn default_true() -> bool {
531532
true
532533
}
533534

535+
#[derive(PartialEq, Eq, Debug, serde::Deserialize)]
536+
pub(crate) struct BackportConfig {
537+
// Config identifier -> labels
538+
#[serde(flatten)]
539+
pub(crate) configs: HashMap<String, BackportRuleConfig>,
540+
}
541+
542+
#[derive(Default, PartialEq, Eq, Debug, serde::Deserialize)]
543+
#[serde(rename_all = "kebab-case")]
544+
#[serde(deny_unknown_fields)]
545+
pub(crate) struct BackportRuleConfig {
546+
/// Prerequisite label(s) (one of them) to trigger this handler for a specific team
547+
pub(crate) required_pr_labels: Vec<String>,
548+
/// Prerequisite label for an issue to qualify as regression
549+
pub(crate) required_issue_label: String,
550+
/// Labels to be added to a pull request closing the regression
551+
pub(crate) add_labels: Vec<String>,
552+
}
553+
534554
fn get_cached_config(repo: &str) -> Option<Result<Arc<Config>, ConfigurationError>> {
535555
let cache = CONFIG_CACHE.read().unwrap();
536556
cache.get(repo).and_then(|(config, fetch_time)| {
@@ -664,6 +684,11 @@ mod tests {
664684
665685
[behind-upstream]
666686
days-threshold = 14
687+
688+
[backport.teamRed]
689+
required-pr-labels = ["T-libs", "T-libs-api"]
690+
required-issue-label = "regression-from-stable-to-stable"
691+
add-labels = ["stable-nominated"]
667692
"#;
668693
let config = toml::from_str::<Config>(&config).unwrap();
669694
let mut ping_teams = HashMap::new();
@@ -688,6 +713,20 @@ mod tests {
688713
nominate_teams.insert("release".to_owned(), "T-release".to_owned());
689714
nominate_teams.insert("core".to_owned(), "T-core".to_owned());
690715
nominate_teams.insert("infra".to_owned(), "T-infra".to_owned());
716+
717+
let mut backport_configs = HashMap::new();
718+
backport_configs.insert(
719+
"teamRed".into(),
720+
BackportRuleConfig {
721+
required_pr_labels: vec!["T-libs".into(), "T-libs-api".into()],
722+
required_issue_label: "regression-from-stable-to-stable".into(),
723+
add_labels: vec!["stable-nominated".into()],
724+
},
725+
);
726+
let backport_team_config = BackportConfig {
727+
configs: backport_configs,
728+
};
729+
691730
assert_eq!(
692731
config,
693732
Config {
@@ -737,6 +776,7 @@ mod tests {
737776
concern: Some(ConcernConfig {
738777
labels: vec!["has-concerns".to_string()],
739778
}),
779+
backport: Some(backport_team_config)
740780
}
741781
);
742782
}
@@ -824,6 +864,7 @@ mod tests {
824864
behind_upstream: Some(BehindUpstreamConfig {
825865
days_threshold: Some(7),
826866
}),
867+
backport: None
827868
}
828869
);
829870
}

src/handlers.rs

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

3030
mod assign;
3131
mod autolabel;
32+
mod backport;
3233
mod bot_pull_requests;
3334
mod check_commits;
3435
mod close;
@@ -226,6 +227,7 @@ macro_rules! issue_handlers {
226227
issue_handlers! {
227228
assign,
228229
autolabel,
230+
backport,
229231
issue_links,
230232
major_change,
231233
mentions,

src/handlers/backport.rs

Lines changed: 252 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,252 @@
1+
use std::collections::HashMap;
2+
use std::sync::LazyLock;
3+
4+
use crate::config::BackportConfig;
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<&BackportConfig>,
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+
let should_check = matches!(event.action, IssuesAction::Opened | IssuesAction::Edited);
55+
if !should_check || !event.issue.is_pr() {
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 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+
.ok_or_else(|| format!("failed to get issue_num from {caps:?}"))?
107+
.as_str();
108+
109+
let id = match id.parse::<u64>() {
110+
Ok(id) => id,
111+
Err(err) => {
112+
return Err(format!("Failed to parse issue id `{id}`, error: {err}"));
113+
}
114+
};
115+
if let Some(org_repo) = caps.name("org_repo")
116+
&& org_repo.as_str() != event.repository.full_name
117+
{
118+
log::info!(
119+
"Skipping backport nomination: Ignoring issue#{id} pointing to a different git repository: Expected {0}, found {org_repo:?}",
120+
event.repository.full_name
121+
);
122+
continue;
123+
}
124+
input.ids.push(id);
125+
}
126+
127+
if input.ids.is_empty() || input.labels.is_empty() {
128+
return Ok(None);
129+
}
130+
131+
log::debug!(
132+
"Will handle event action {:?} in backport. Regression IDs found {:?}",
133+
event.action,
134+
input.ids
135+
);
136+
137+
Ok(Some(input))
138+
}
139+
140+
pub(super) async fn handle_input(
141+
ctx: &Context,
142+
_config: &BackportConfig,
143+
event: &IssuesEvent,
144+
input: BackportInput,
145+
) -> anyhow::Result<()> {
146+
let pr = &event.issue;
147+
148+
// Retrieve the issue(s) this pull request closes
149+
let issues = input
150+
.ids
151+
.iter()
152+
.copied()
153+
.map(|id| async move { event.repository.get_issue(&ctx.github, id).await });
154+
let issues = join_all(issues).await;
155+
156+
// Add backport nomination label to the pull request
157+
for issue in issues {
158+
if let Err(ref err) = issue {
159+
log::warn!("Failed to get issue: {:?}", err);
160+
continue;
161+
}
162+
let issue = issue.context("failed to get issue")?;
163+
let issue_labels: Vec<&str> = issue.labels.iter().map(|l| l.name.as_str()).collect();
164+
165+
// Check issue for a prerequisite priority label
166+
// If none, skip this issue
167+
if !contains_any(&issue_labels, &PRIORITY_LABELS) {
168+
continue;
169+
}
170+
171+
// Get the labels to be added the PR according to the matching (required) regression label
172+
// that is found in the configuration that this handler has received
173+
// If no regression label is found, skip this issue
174+
let add_labels = issue_labels.iter().find_map(|l| input.labels.get(*l));
175+
if add_labels.is_none() {
176+
log::warn!(
177+
"Skipping backport nomination: nothing to do for issue #{}. No config found for regression label ({:?})",
178+
issue.number,
179+
REGRESSION_LABELS
180+
);
181+
continue;
182+
}
183+
184+
// Add backport nomination label(s) to PR
185+
let mut new_labels = pr.labels().to_owned();
186+
new_labels.extend(
187+
add_labels
188+
.expect("failed to unwrap add_labels")
189+
.iter()
190+
.cloned()
191+
.map(|name| Label { name }),
192+
);
193+
log::debug!(
194+
"PR#{} adding labels for backport {:?}",
195+
pr.number,
196+
add_labels
197+
);
198+
let _ = pr
199+
.add_labels(&ctx.github, new_labels)
200+
.await
201+
.context("failed to add backport labels to the PR");
202+
}
203+
204+
Ok(())
205+
}
206+
207+
fn contains_any(haystack: &[&str], needles: &[&str]) -> bool {
208+
needles.iter().any(|needle| haystack.contains(needle))
209+
}
210+
211+
#[cfg(test)]
212+
mod tests {
213+
use crate::handlers::backport::CLOSES_ISSUE_REGEXP;
214+
215+
#[tokio::test]
216+
async fn backport_match_comment() {
217+
let test_strings = vec![
218+
("close #10", vec![10]),
219+
("closes #10", vec![10]),
220+
("closed #10", vec![10]),
221+
("Closes #10", vec![10]),
222+
("close #10", vec![10]),
223+
("close rust-lang/rust#10", vec![10]),
224+
("cLose: rust-lang/rust#10", vec![10]),
225+
("fix #10", vec![10]),
226+
("fixes #10", vec![10]),
227+
("fixed #10", vec![10]),
228+
("resolve #10", vec![10]),
229+
("resolves #10", vec![10]),
230+
("resolved #10", vec![10]),
231+
(
232+
"Fixes #20, Resolves #21, closed #22, LOL #23",
233+
vec![20, 21, 22],
234+
),
235+
("Resolved #10", vec![10]),
236+
("Fixes #10", vec![10]),
237+
("Closes #10", vec![10]),
238+
];
239+
for test_case in test_strings {
240+
let mut ids: Vec<u64> = vec![];
241+
let test_str = test_case.0;
242+
let expected = test_case.1;
243+
for caps in CLOSES_ISSUE_REGEXP.captures_iter(test_str) {
244+
// eprintln!("caps {:?}", caps);
245+
let id = &caps["issue_num"];
246+
ids.push(id.parse::<u64>().unwrap());
247+
}
248+
// eprintln!("ids={:?}", ids);
249+
assert_eq!(ids, expected);
250+
}
251+
}
252+
}

0 commit comments

Comments
 (0)