Merge pull request #2159 from Urgau/gh-range-diff-all-range

Add full ranges support for our range-diff feature

Author: Urgau

src/gh_range_diff.rs

@@ -20,6 +20,7 @@ use imara_diff::{
 use pulldown_cmark_escape::FmtWriter;
 use regex::Regex;
 
+use crate::github::GithubCompare;
 use crate::{github, handlers::Context, utils::AppError};
 
 static MARKER_RE: LazyLock<Regex> =
@@ -31,7 +32,7 @@ static MARKER_RE: LazyLock<Regex> =
 pub async fn gh_range_diff(
     Path((owner, repo, basehead)): Path<(String, String, String)>,
     State(ctx): State<Arc<Context>>,
-    Host(host): Host,
+    host: Host,
 ) -> axum::response::Result<impl IntoResponse, AppError> {
     let Some((oldhead, newhead)) = basehead.split_once("..") else {
         return Ok((
@@ -41,9 +42,6 @@ pub async fn gh_range_diff(
         ));
     };
 
-    // Configure unified diff
-    let config = UnifiedDiffConfig::default();
-
     let repos = ctx
         .team
         .repos()
@@ -96,18 +94,14 @@ pub async fn gh_range_diff(
             .sha;
 
         // Get the comparison between the oldbase..oldhead
-        let mut old = ctx
+        let old = ctx
             .github
             .compare(&issue_repo, &oldbase, oldhead)
             .await
             .with_context(|| {
                 format!("failed to retrive the comparison between {oldbase} and {oldhead}")
             })?;
 
-        // Sort by filename, so it's consistent with GitHub UI
-        old.files
-            .sort_unstable_by(|f1, f2| f1.filename.cmp(&f2.filename));
-
         anyhow::Result::<_>::Ok((oldbase, old))
     };
 
@@ -125,24 +119,128 @@ pub async fn gh_range_diff(
             .sha;
 
         // Get the comparison between the newbase..newhead
-        let mut new = ctx
+        let new = ctx
             .github
             .compare(&issue_repo, &newbase, newhead)
             .await
             .with_context(|| {
                 format!("failed to retrive the comparison between {newbase} and {newhead}")
             })?;
 
-        // Sort by filename, so it's consistent with GitHub UI
-        new.files
-            .sort_unstable_by(|f1, f2| f1.filename.cmp(&f2.filename));
-
         anyhow::Result::<_>::Ok((newbase, new))
     };
 
     // Wait for both futures and early exit if there is an error
     let ((oldbase, old), (newbase, new)) = futures::try_join!(old, new)?;
 
+    process_old_new(
+        host,
+        (&owner, &repo),
+        (&oldbase, oldhead, old),
+        (&newbase, newhead, new),
+    )
+}
+
+/// Compute and renders an emulated `git range-diff` between two pushes (old and new).
+///
+/// - `oldbasehead` is `OLDBASE..OLDHEAD`
+/// - `newbasehead` is `NEWBASE..NEWHEAD`
+pub async fn gh_ranges_diff(
+    Path((owner, repo, oldbasehead, newbasehead)): Path<(String, String, String, String)>,
+    State(ctx): State<Arc<Context>>,
+    host: Host,
+) -> axum::response::Result<impl IntoResponse, AppError> {
+    let Some((oldbase, oldhead)) = oldbasehead.split_once("..") else {
+        return Ok((
+            StatusCode::BAD_REQUEST,
+            HeaderMap::new(),
+            format!("`{oldbasehead}` is not in the form `base..head`"),
+        ));
+    };
+
+    let Some((newbase, newhead)) = newbasehead.split_once("..") else {
+        return Ok((
+            StatusCode::BAD_REQUEST,
+            HeaderMap::new(),
+            format!("`{newbasehead}` is not in the form `base..head`"),
+        ));
+    };
+
+    let repos = ctx
+        .team
+        .repos()
+        .await
+        .context("unable to retrieve team repos")?;
+
+    // Verify that the request org is part of the Rust project
+    let Some(repos) = repos.repos.get(&owner) else {
+        return Ok((
+            StatusCode::BAD_REQUEST,
+            HeaderMap::new(),
+            format!("organization `{owner}` is not part of the Rust Project team repos"),
+        ));
+    };
+
+    // Verify that the request repo is part of the Rust project
+    if !repos.iter().any(|r| r.name == repo) {
+        return Ok((
+            StatusCode::BAD_REQUEST,
+            HeaderMap::new(),
+            format!("repository `{owner}` is not part of the Rust Project team repos"),
+        ));
+    }
+
+    let issue_repo = github::IssueRepository {
+        organization: owner.to_string(),
+        repository: repo.to_string(),
+    };
+
+    // Get the comparison between the oldbase..oldhead
+    let old = async {
+        ctx.github
+            .compare(&issue_repo, &oldbase, oldhead)
+            .await
+            .with_context(|| {
+                format!("failed to retrive the comparison between {oldbase} and {oldhead}")
+            })
+    };
+
+    // Get the comparison between the newbase..newhead
+    let new = async {
+        ctx.github
+            .compare(&issue_repo, &newbase, newhead)
+            .await
+            .with_context(|| {
+                format!("failed to retrive the comparison between {newbase} and {newhead}")
+            })
+    };
+
+    // Wait for both futures and early exit if there is an error
+    let (old, new) = futures::try_join!(old, new)?;
+
+    process_old_new(
+        host,
+        (&owner, &repo),
+        (&oldbase, oldhead, old),
+        (&newbase, newhead, new),
+    )
+}
+
+fn process_old_new(
+    Host(host): Host,
+    (owner, repo): (&str, &str),
+    (oldbase, oldhead, mut old): (&str, &str, GithubCompare),
+    (newbase, newhead, mut new): (&str, &str, GithubCompare),
+) -> axum::response::Result<(StatusCode, HeaderMap, String), AppError> {
+    // Configure unified diff
+    let config = UnifiedDiffConfig::default();
+
+    // Sort by filename, so it's consistent with GitHub UI
+    old.files
+        .sort_unstable_by(|f1, f2| f1.filename.cmp(&f2.filename));
+    new.files
+        .sort_unstable_by(|f1, f2| f1.filename.cmp(&f2.filename));
+
     // Create the HTML buffer with a very rough approximation for the capacity
     let mut html: String = String::with_capacity(800 + old.files.len() * 100);
 

src/handlers/check_commits/force_push_range_diff.rs

@@ -51,10 +51,12 @@ pub(super) async fn handle_event(
     };
 
     let branch = &base.git_ref;
+    let (oldbase, oldhead) = (&compare_before.merge_base_commit.sha, before);
+    let (newbase, newhead) = (&compare_after.merge_base_commit.sha, after);
 
     // 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})."#)
+        &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}/{oldbase}..{oldhead}/{newbase}..{newhead})."#)
     ).await.context("failed to post range-diff comment")?;
 
     Ok(())

src/main.rs

@@ -189,6 +189,10 @@ async fn run_server(addr: SocketAddr) -> anyhow::Result<()> {
             "/gh-range-diff/{owner}/{repo}/{basehead}",
             get(triagebot::gh_range_diff::gh_range_diff),
         )
+        .route(
+            "/gh-range-diff/{owner}/{repo}/{oldbasehead}/{newbasehead}",
+            get(triagebot::gh_range_diff::gh_ranges_diff),
+        )
         .nest("/agenda", agenda)
         .route("/bors-commit-list", get(triagebot::bors::bors_commit_list))
         .route(