Skip to content

Commit fd7d71b

Browse files
committed
Suggest update the branch when based on a too old commit in upstream
Signed-off-by: xizheyin <[email protected]>
1 parent e3c95db commit fd7d71b

File tree

5 files changed

+152
-2
lines changed

5 files changed

+152
-2
lines changed

src/config.rs

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -49,6 +49,7 @@ pub(crate) struct Config {
4949
#[serde(alias = "canonicalize-issue-links")]
5050
pub(crate) issue_links: Option<IssueLinksConfig>,
5151
pub(crate) no_mentions: Option<NoMentionsConfig>,
52+
pub(crate) behind_upstream: Option<BehindUpstreamConfig>,
5253
}
5354

5455
#[derive(PartialEq, Eq, Debug, serde::Deserialize)]
@@ -427,6 +428,15 @@ pub(crate) struct IssueLinksConfig {}
427428
#[serde(deny_unknown_fields)]
428429
pub(crate) struct NoMentionsConfig {}
429430

431+
/// Configuration for PR behind commits checks
432+
#[derive(PartialEq, Eq, Debug, serde::Deserialize)]
433+
#[serde(deny_unknown_fields)]
434+
pub(crate) struct BehindUpstreamConfig {
435+
/// The threshold of days for parent commit age to trigger a warning.
436+
/// Default is 14 days if not specified.
437+
pub(crate) parent_age_threshold: Option<usize>,
438+
}
439+
430440
fn get_cached_config(repo: &str) -> Option<Result<Arc<Config>, ConfigurationError>> {
431441
let cache = CONFIG_CACHE.read().unwrap();
432442
cache.get(repo).and_then(|(config, fetch_time)| {
@@ -618,6 +628,7 @@ mod tests {
618628
}),
619629
issue_links: Some(IssueLinksConfig {}),
620630
no_mentions: Some(NoMentionsConfig {}),
631+
behind_upstream: None,
621632
}
622633
);
623634
}
@@ -685,6 +696,7 @@ mod tests {
685696
rendered_link: None,
686697
issue_links: Some(IssueLinksConfig {}),
687698
no_mentions: None,
699+
behind_upstream: None,
688700
}
689701
);
690702
}

src/github.rs

Lines changed: 13 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1458,7 +1458,7 @@ impl Repository {
14581458

14591459
/// Returns a list of commits between the SHA ranges of start (exclusive)
14601460
/// and end (inclusive).
1461-
pub async fn commits_in_range(
1461+
pub async fn github_commits_in_range(
14621462
&self,
14631463
client: &GithubClient,
14641464
start: &str,
@@ -1499,6 +1499,18 @@ impl Repository {
14991499
}
15001500
}
15011501

1502+
pub async fn github_commit(
1503+
&self,
1504+
client: &GithubClient,
1505+
sha: &str,
1506+
) -> anyhow::Result<GithubCommit> {
1507+
let url = format!("{}/commits/{}", self.url(client), sha);
1508+
client
1509+
.json(client.get(&url))
1510+
.await
1511+
.with_context(|| format!("{} failed to get github commit {sha}", self.full_name))
1512+
}
1513+
15021514
/// Retrieves a git commit for the given SHA.
15031515
pub async fn git_commit(&self, client: &GithubClient, sha: &str) -> anyhow::Result<GitCommit> {
15041516
let url = format!("{}/git/commits/{sha}", self.url(client));

src/handlers/check_commits.rs

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,7 @@ use crate::{
1313
#[cfg(test)]
1414
use crate::github::GithubCommit;
1515

16+
mod behind_upstream;
1617
mod issue_links;
1718
mod modified_submodule;
1819
mod no_mentions;
@@ -93,6 +94,19 @@ pub(super) async fn handle(ctx: &Context, event: &Event, config: &Config) -> any
9394
}
9495
}
9596

97+
// Check if PR is behind upstream branch by a significant number of commits
98+
if let Some(behind_upstream) = &config.behind_upstream {
99+
let age_threshold = behind_upstream
100+
.parent_age_threshold
101+
.unwrap_or(behind_upstream::DEFAULT_PARENT_AGE_THRESHOLD);
102+
103+
if let Some(warning) =
104+
behind_upstream::behind_upstream(age_threshold, event, &ctx.github).await
105+
{
106+
warnings.push(warning);
107+
}
108+
}
109+
96110
handle_warnings_and_labels(ctx, event, warnings, labels).await
97111
}
98112

Lines changed: 112 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,112 @@
1+
use crate::github::{GithubClient, GithubCommit, Issue, IssuesEvent, Repository};
2+
use tracing as log;
3+
4+
/// Default threshold for parent commit age in days to trigger a warning
5+
pub const DEFAULT_PARENT_AGE_THRESHOLD: usize = 14;
6+
7+
/// Check if the PR is based on an old parent commit
8+
pub async fn behind_upstream(
9+
age_threshold: usize,
10+
event: &IssuesEvent,
11+
client: &GithubClient,
12+
) -> Option<String> {
13+
if !event.issue.is_pr() {
14+
return None;
15+
}
16+
17+
log::debug!("Checking if PR #{} is behind upstream", event.issue.number);
18+
19+
// First try the parent commit age check as it's more accurate
20+
match is_parent_commit_too_old(&event.issue, client, age_threshold).await {
21+
Ok(Some(days_old)) => {
22+
log::info!(
23+
"PR #{} has a parent commit that is {} days old",
24+
event.issue.number,
25+
days_old
26+
);
27+
28+
return Some(format!(
29+
r"This PR is based on an upstream commit that is {} days old.
30+
It's recommended to update your branch according to the
31+
[Rustc Dev Guide](https://rustc-dev-guide.rust-lang.org/contributing.html#keeping-your-branch-up-to-date).",
32+
days_old
33+
));
34+
}
35+
Ok(None) => {
36+
// Parent commit is not too old, log and do nothing
37+
log::debug!("PR #{} parent commit is not too old", event.issue.number);
38+
}
39+
Err(e) => {
40+
// Error checking parent commit age, log and do nothing
41+
log::error!(
42+
"Error checking parent commit age for PR #{}: {}",
43+
event.issue.number,
44+
e
45+
);
46+
}
47+
}
48+
49+
None
50+
}
51+
52+
/// Checks if the PR's parent commit is too old.
53+
///
54+
/// This determines if a PR needs updating by examining the first parent of the PR's head commit,
55+
/// which typically represents the base branch commit that the PR is based on.
56+
///
57+
/// If this parent commit is older than the specified threshold, it suggests the PR
58+
/// should be updated/rebased to a more recent version of the base branch.
59+
///
60+
/// Returns:
61+
/// - Ok(Some(days_old)) - If parent commit is older than the threshold
62+
/// - Ok(None)
63+
/// - If not a PR
64+
/// - If can't get commit details (both for the head and parent)
65+
/// - If parent is within threshold
66+
/// - Err(...) - If an error occurred during processing
67+
pub async fn is_parent_commit_too_old(
68+
pull_request: &Issue,
69+
client: &GithubClient,
70+
max_days_old: usize,
71+
) -> anyhow::Result<Option<usize>> {
72+
if !pull_request.is_pr() {
73+
return Ok(None);
74+
}
75+
76+
// Get the head commit SHA
77+
let Some(head) = &pull_request.head else {
78+
return Ok(None);
79+
};
80+
// Get the commit details
81+
let commit: GithubCommit = head.repo.github_commit(client, &head.sha).await?;
82+
83+
// Get the first parent (it should be from the base branch)
84+
let Some(parent_sha) = commit.parents.get(0).map(|c| c.sha.clone()) else {
85+
return Ok(None);
86+
};
87+
88+
let days_old = commit_days_old(&parent_sha, &head.repo, client).await?;
89+
90+
if days_old > max_days_old {
91+
Ok(Some(days_old))
92+
} else {
93+
Ok(None)
94+
}
95+
}
96+
97+
/// Returns the number of days old the commit is
98+
pub async fn commit_days_old(
99+
sha: &str,
100+
repo: &Repository,
101+
client: &GithubClient,
102+
) -> anyhow::Result<usize> {
103+
// Get the commit details to check its date
104+
let commit: GithubCommit = repo.github_commit(client, &sha).await?;
105+
106+
// compute the number of days old the commit is
107+
let commit_date = commit.commit.author.date;
108+
let now = chrono::Utc::now().with_timezone(&commit_date.timezone());
109+
let days_old = (now - commit_date).num_days() as usize;
110+
111+
Ok(days_old)
112+
}

src/handlers/milestone_prs.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -134,7 +134,7 @@ async fn milestone_cargo(
134134
let cargo_repo = gh.repository("rust-lang/cargo").await?;
135135
log::info!("loading cargo changes {cargo_start_hash}...{cargo_end_hash}");
136136
let commits = cargo_repo
137-
.commits_in_range(gh, cargo_start_hash, cargo_end_hash)
137+
.github_commits_in_range(gh, cargo_start_hash, cargo_end_hash)
138138
.await?;
139139

140140
// For each commit, look for a message from bors that indicates which

0 commit comments

Comments
 (0)