Merge pull request #1942 from xizheyin/pr-behind-commits

Suggest update the branch when behind too many commits

Author: Kobzol

src/config.rs

@@ -49,6 +49,7 @@ pub(crate) struct Config {
     #[serde(alias = "canonicalize-issue-links")]
     pub(crate) issue_links: Option<IssueLinksConfig>,
     pub(crate) no_mentions: Option<NoMentionsConfig>,
+    pub(crate) behind_upstream: Option<BehindUpstreamConfig>,
 }
 
 #[derive(PartialEq, Eq, Debug, serde::Deserialize)]
@@ -427,6 +428,16 @@ pub(crate) struct IssueLinksConfig {}
 #[serde(deny_unknown_fields)]
 pub(crate) struct NoMentionsConfig {}
 
+/// Configuration for PR behind commits checks
+#[derive(PartialEq, Eq, Debug, serde::Deserialize)]
+#[serde(rename_all = "kebab-case")]
+#[serde(deny_unknown_fields)]
+pub(crate) struct BehindUpstreamConfig {
+    /// The threshold of days for parent commit age to trigger a warning.
+    /// Default is 7 days if not specified.
+    pub(crate) days_threshold: Option<usize>,
+}
+
 fn get_cached_config(repo: &str) -> Option<Result<Arc<Config>, ConfigurationError>> {
     let cache = CONFIG_CACHE.read().unwrap();
     cache.get(repo).and_then(|(config, fetch_time)| {
@@ -554,6 +565,9 @@ mod tests {
             trigger-files = ["posts/"]
 
             [no-mentions]
+
+            [behind-upstream]
+            days-threshold = 14
         "#;
         let config = toml::from_str::<Config>(&config).unwrap();
         let mut ping_teams = HashMap::new();
@@ -618,6 +632,9 @@ mod tests {
                 }),
                 issue_links: Some(IssueLinksConfig {}),
                 no_mentions: Some(NoMentionsConfig {}),
+                behind_upstream: Some(BehindUpstreamConfig {
+                    days_threshold: Some(14),
+                }),
             }
         );
     }
@@ -637,6 +654,9 @@ mod tests {
             branch = "stable"
 
             [canonicalize-issue-links]
+
+            [behind-upstream]
+            days-threshold = 7
         "#;
         let config = toml::from_str::<Config>(&config).unwrap();
         assert_eq!(
@@ -685,6 +705,9 @@ mod tests {
                 rendered_link: None,
                 issue_links: Some(IssueLinksConfig {}),
                 no_mentions: None,
+                behind_upstream: Some(BehindUpstreamConfig {
+                    days_threshold: Some(7),
+                }),
             }
         );
     }

src/github.rs

@@ -1459,7 +1459,7 @@ impl Repository {
 
     /// Returns a list of commits between the SHA ranges of start (exclusive)
     /// and end (inclusive).
-    pub async fn commits_in_range(
+    pub async fn github_commits_in_range(
         &self,
         client: &GithubClient,
         start: &str,
@@ -1500,6 +1500,18 @@ impl Repository {
         }
     }
 
+    pub async fn github_commit(
+        &self,
+        client: &GithubClient,
+        sha: &str,
+    ) -> anyhow::Result<GithubCommit> {
+        let url = format!("{}/commits/{}", self.url(client), sha);
+        client
+            .json(client.get(&url))
+            .await
+            .with_context(|| format!("{} failed to get github commit {sha}", self.full_name))
+    }
+
     /// Retrieves a git commit for the given SHA.
     pub async fn git_commit(&self, client: &GithubClient, sha: &str) -> anyhow::Result<GitCommit> {
         let url = format!("{}/git/commits/{sha}", self.url(client));

src/handlers/check_commits.rs

@@ -13,6 +13,7 @@ use crate::{
 #[cfg(test)]
 use crate::github::GithubCommit;
 
+mod behind_upstream;
 mod issue_links;
 mod modified_submodule;
 mod no_mentions;
@@ -93,6 +94,19 @@ pub(super) async fn handle(ctx: &Context, event: &Event, config: &Config) -> any
         }
     }
 
+    // Check if PR is behind upstream branch by a significant number of days
+    if let Some(behind_upstream) = &config.behind_upstream {
+        let age_threshold = behind_upstream
+            .days_threshold
+            .unwrap_or(behind_upstream::DEFAULT_DAYS_THRESHOLD);
+
+        if let Some(warning) =
+            behind_upstream::behind_upstream(age_threshold, event, &ctx.github, &commits).await
+        {
+            warnings.push(warning);
+        }
+    }
+
     handle_warnings_and_labels(ctx, event, warnings, labels).await
 }
 

src/handlers/check_commits/behind_upstream.rs

@@ -0,0 +1,102 @@
+use crate::github::{GithubClient, GithubCommit, IssuesEvent, Repository};
+use tracing as log;
+
+/// Default threshold for parent commit age in days to trigger a warning
+pub const DEFAULT_DAYS_THRESHOLD: usize = 7;
+
+/// Check if the PR is based on an old parent commit
+pub async fn behind_upstream(
+    age_threshold: usize,
+    event: &IssuesEvent,
+    client: &GithubClient,
+    commits: &Vec<GithubCommit>,
+) -> Option<String> {
+    log::debug!("Checking if PR #{} is behind upstream", event.issue.number);
+
+    let Some(head_commit) = commits.first() else {
+        return None;
+    };
+
+    // First try the parent commit age check as it's more accurate
+    match is_parent_commit_too_old(head_commit, &event.repository, client, age_threshold).await {
+        Ok(Some(days_old)) => {
+            log::info!(
+                "PR #{} has a parent commit that is {} days old",
+                event.issue.number,
+                days_old
+            );
+
+            return Some(format!(
+                r"This PR is based on an upstream commit that is {} days old.
+
+    *It's recommended to update your branch according to the [rustc_dev_guide](https://rustc-dev-guide.rust-lang.org/contributing.html#keeping-your-branch-up-to-date).*",
+                days_old
+            ));
+        }
+        Ok(None) => {
+            // Parent commit is not too old, log and do nothing
+            log::debug!("PR #{} parent commit is not too old", event.issue.number);
+        }
+        Err(e) => {
+            // Error checking parent commit age, log and do nothing
+            log::error!(
+                "Error checking parent commit age for PR #{}: {}",
+                event.issue.number,
+                e
+            );
+        }
+    }
+
+    None
+}
+
+/// Checks if the PR's parent commit is too old.
+///
+/// This determines if a PR needs updating by examining the first parent of the PR's head commit,
+/// which typically represents the base branch commit that the PR is based on.
+///
+/// If this parent commit is older than the specified threshold, it suggests the PR
+/// should be updated/rebased to a more recent version of the base branch.
+///
+/// Returns:
+/// - Ok(Some(days_old)) - If parent commit is older than the threshold
+/// - Ok(None)
+///     - If there is no parent commit
+///     - If parent is within threshold
+/// - Err(...) - If an error occurred during processing
+pub async fn is_parent_commit_too_old(
+    commit: &GithubCommit,
+    repo: &Repository,
+    client: &GithubClient,
+    max_days_old: usize,
+) -> anyhow::Result<Option<usize>> {
+    // Get the first parent (it should be from the base branch)
+    let Some(parent_sha) = commit.parents.get(0).map(|c| c.sha.clone()) else {
+        return Ok(None);
+    };
+
+    let days_old = commit_days_old(&parent_sha, repo, client).await?;
+
+    if days_old > max_days_old {
+        Ok(Some(days_old))
+    } else {
+        Ok(None)
+    }
+}
+
+/// Returns the number of days old the commit is
+pub async fn commit_days_old(
+    sha: &str,
+    repo: &Repository,
+    client: &GithubClient,
+) -> anyhow::Result<usize> {
+    // Get the commit details to check its date
+    let commit: GithubCommit = repo.github_commit(client, &sha).await?;
+
+    // compute the number of days old the commit is
+    let commit_date = commit.commit.author.date;
+    let now = chrono::Utc::now().with_timezone(&commit_date.timezone());
+    let days_old = (now - commit_date).num_days() as usize;
+
+    Ok(days_old)
+}

src/handlers/milestone_prs.rs

@@ -134,7 +134,7 @@ async fn milestone_cargo(
     let cargo_repo = gh.repository("rust-lang/cargo").await?;
     log::info!("loading cargo changes {cargo_start_hash}...{cargo_end_hash}");
     let commits = cargo_repo
-        .commits_in_range(gh, cargo_start_hash, cargo_end_hash)
+        .github_commits_in_range(gh, cargo_start_hash, cargo_end_hash)
         .await?;
 
     // For each commit, look for a message from bors that indicates which