Merge pull request #2158 from Urgau/gh-range-diff_comment_on_rebase

Post comment to our range-diff a PR was rebased

Author: Urgau

src/config.rs

@@ -48,6 +48,7 @@ pub(crate) struct Config {
     pub(crate) no_mentions: Option<NoMentionsConfig>,
     pub(crate) behind_upstream: Option<BehindUpstreamConfig>,
     pub(crate) backport: Option<BackportConfig>,
+    pub(crate) range_diff: Option<RangeDiffConfig>,
 }
 
 #[derive(PartialEq, Eq, Debug, serde::Deserialize)]
@@ -551,6 +552,12 @@ pub(crate) struct BackportRuleConfig {
     pub(crate) add_labels: Vec<String>,
 }
 
+/// Configuration for rebase range-diff comment
+#[derive(Default, PartialEq, Eq, Debug, serde::Deserialize)]
+#[serde(rename_all = "kebab-case")]
+#[serde(deny_unknown_fields)]
+pub(crate) struct RangeDiffConfig {}
+
 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)| {
@@ -689,6 +696,8 @@ mod tests {
             required-pr-labels = ["T-libs", "T-libs-api"]
             required-issue-label = "regression-from-stable-to-stable"
             add-labels = ["stable-nominated"]
+
+            [range-diff]
         "#;
         let config = toml::from_str::<Config>(&config).unwrap();
         let mut ping_teams = HashMap::new();
@@ -776,7 +785,8 @@ mod tests {
                 concern: Some(ConcernConfig {
                     labels: vec!["has-concerns".to_string()],
                 }),
-                backport: Some(backport_team_config)
+                backport: Some(backport_team_config),
+                range_diff: Some(RangeDiffConfig {}),
             }
         );
     }
@@ -864,7 +874,8 @@ mod tests {
                 behind_upstream: Some(BehindUpstreamConfig {
                     days_threshold: Some(7),
                 }),
-                backport: None
+                backport: None,
+                range_diff: None,
             }
         );
     }

src/github.rs

@@ -1268,6 +1268,8 @@ pub struct IssuesEvent {
     #[serde(alias = "pull_request")]
     pub issue: Issue,
     pub changes: Option<Changes>,
+    pub before: Option<String>,
+    pub after: Option<String>,
     pub repository: Repository,
     /// The GitHub user that triggered the event.
     pub sender: User,

src/github/webhook.rs

@@ -1,6 +1,7 @@
 use std::{fmt, sync::Arc};
 
 use axum::{extract::State, response::IntoResponse};
+use axum_extra::extract::Host;
 use hmac::{Hmac, Mac};
 use hyper::HeaderMap;
 use sha2::Sha256;
@@ -106,6 +107,7 @@ pub fn deserialize_payload<T: serde::de::DeserializeOwned>(v: &str) -> anyhow::R
 pub async fn webhook(
     headers: HeaderMap,
     State(ctx): State<Arc<crate::handlers::Context>>,
+    Host(host): Host,
     body: Bytes,
 ) -> axum::response::Response {
     // Extract X-GitHub-Event header
@@ -156,7 +158,7 @@ pub async fn webhook(
         return (StatusCode::BAD_REQUEST, "Payload must be UTF-8").into_response();
     };
 
-    match process_payload(event, payload, &ctx).await {
+    match process_payload(event, payload, &ctx, &host).await {
         Ok(true) => ("processed request",).into_response(),
         Ok(false) => ("ignored request",).into_response(),
         Err(err) => {
@@ -171,6 +173,7 @@ async fn process_payload(
     event: EventName,
     payload: &str,
     ctx: &crate::handlers::Context,
+    host: &str,
 ) -> anyhow::Result<bool> {
     let event = match event {
         EventName::PullRequestReview => {
@@ -253,7 +256,7 @@ async fn process_payload(
             return Ok(false);
         }
     };
-    let errors = crate::handlers::handle(&ctx, &event).await;
+    let errors = crate::handlers::handle(&ctx, &host, &event).await;
     let mut other_error = false;
     let mut message = String::new();
     for err in errors {

src/handlers.rs

@@ -60,7 +60,7 @@ mod shortcut;
 mod transfer;
 pub mod types_planning_updates;
 
-pub async fn handle(ctx: &Context, event: &Event) -> Vec<HandlerError> {
+pub async fn handle(ctx: &Context, host: &str, event: &Event) -> Vec<HandlerError> {
     let config = config::get(&ctx.github, event.repo()).await;
     if let Err(e) = &config {
         log::warn!("configuration error {}: {e}", event.repo().full_name);
@@ -76,7 +76,7 @@ pub async fn handle(ctx: &Context, event: &Event) -> Vec<HandlerError> {
     }
 
     if let Ok(config) = &config {
-        if let Err(e) = check_commits::handle(ctx, event, &config).await {
+        if let Err(e) = check_commits::handle(ctx, host, event, &config).await {
             log::error!(
                 "failed to process event {:?} with `check_commits` handler: {:?}",
                 event,

src/handlers/check_commits.rs

@@ -15,6 +15,7 @@ use crate::{
 use crate::github::GithubCommit;
 
 mod behind_upstream;
+mod force_push_range_diff;
 mod issue_links;
 mod modified_submodule;
 mod no_mentions;
@@ -69,7 +70,12 @@ fn should_handle_event(event: &IssuesEvent) -> bool {
     false
 }
 
-pub(super) async fn handle(ctx: &Context, event: &Event, config: &Config) -> anyhow::Result<()> {
+pub(super) async fn handle(
+    ctx: &Context,
+    host: &str,
+    event: &Event,
+    config: &Config,
+) -> anyhow::Result<()> {
     let Event::Issue(event) = event else {
         return Ok(());
     };
@@ -135,6 +141,12 @@ pub(super) async fn handle(ctx: &Context, event: &Event, config: &Config) -> any
         }
     }
 
+    // Check if this is a force-push with rebase and if it is emit comment
+    // with link to our range-diff viewer.
+    if let Some(range_diff) = &config.range_diff {
+        force_push_range_diff::handle_event(ctx, host, range_diff, &event, &compare).await?;
+    }
+
     // Check if the `triagebot.toml` config is valid
     errors.extend(
         validate_config::validate_config(ctx, &event, diff)
@@ -408,6 +420,8 @@ r#":warning: **Warning** :warning:
                 author_association: octocrab::models::AuthorAssociation::Contributor,
             },
             changes: None,
+            before: None,
+            after: None,
             repository: crate::github::Repository {
                 full_name: "rust-lang/rust".to_string(),
                 default_branch: "master".to_string(),

src/handlers/check_commits/force_push_range_diff.rs

@@ -0,0 +1,61 @@
+use anyhow::Context as _;
+
+use crate::config::RangeDiffConfig;
+use crate::github::GithubCompare;
+use crate::github::IssueRepository;
+use crate::github::IssuesEvent;
+use crate::handlers::Context;
+
+pub(super) async fn handle_event(
+    ctx: &Context,
+    host: &str,
+    _config: &RangeDiffConfig,
+    event: &IssuesEvent,
+    compare_after: &GithubCompare,
+) -> anyhow::Result<()> {
+    if !matches!(event.action, crate::github::IssuesAction::Synchronize) {
+        return Ok(());
+    }
+
+    let (Some(before), Some(after)) = (event.before.as_ref(), event.after.as_ref()) else {
+        tracing::warn!("synchronize event but no before or after field");
+        return Ok(());
+    };
+
+    let base = event.issue.base.as_ref().context("no base ref")?;
+    let org = event.repository.owner();
+    let repo = event.repository.name();
+
+    let compare_before = ctx
+        .github
+        .compare(
+            &IssueRepository {
+                organization: org.to_string(),
+                repository: repo.to_string(),
+            },
+            &base.sha,
+            &before,
+        )
+        .await
+        .context("failed to get the before compare")?;
+
+    // Does the merge_base_commits differs? No, not a force-push with rebase.
+    if compare_before.merge_base_commit.sha == compare_after.merge_base_commit.sha {
+        return Ok(());
+    }
+
+    let protocol = if host.starts_with("localhost:") {
+        "http"
+    } else {
+        "https"
+    };
+
+    let branch = &base.git_ref;
+
+    // Rebase detected, post a comment to our range-diff.
+    event.issue.post_comment(&ctx.github,
+        &format!(r#"This PR was rebased onto a different {branch} commit! Check out the changes with our [`range-diff`]({protocol}://{host}/gh-range-diff/{org}/{repo}/{before}..{after})."#)
+    ).await.context("failed to post range-diff comment")?;
+
+    Ok(())
+}

src/handlers/pr_tracking.rs

@@ -571,6 +571,8 @@ mod tests {
             action,
             issue,
             changes: None,
+            before: None,
+            after: None,
             repository: Repository {
                 full_name: "rust-lang-test/triagebot-test".to_string(),
                 default_branch: "main".to_string(),