Merge pull request #491 from Kobzol/mergeability-msg

Add conflict reporting on PRs

Author: Kobzol

.sqlx/query-05746d59bfab29431f55b85422e9edff63e8f6f97c748da8ca4fa8466350a829.json

@@ -7,6 +7,10 @@ timeout = 3600
 # (Optional, defaults to false)
 merge_queue_enabled = true
 
+# Whether merge conflicts should be reported on PRs.
+# (Optional, defaults to false)
+report_merge_conflicts = true
+
 # Labels that will block approval when present on a PR
 # (Optional)
 labels_blocking_approval = ["final-comment-period", "proposed-final-comment-period"]

…b8721d72dead28356fbbc69f84029d08132.json …7959d6e85012dba5e9d7159541ef317b691.json.sqlx/query-d16ca5631fcdd4216b1bf3a7b1672b8721d72dead28356fbbc69f84029d08132.json renamed to .sqlx/query-f697693a6cbccdf9b2557842a1ff87959d6e85012dba5e9d7159541ef317b691.json .sqlx/query-d16ca5631fcdd4216b1bf3a7b1672b8721d72dead28356fbbc69f84029d08132.json renamed to .sqlx/query-f697693a6cbccdf9b2557842a1ff87959d6e85012dba5e9d7159541ef317b691.json

@@ -5,7 +5,7 @@ use std::time::Duration;
 
 use crate::bors::FailedWorkflowRun;
 use crate::bors::command::CommandPrefix;
-use crate::github::GithubRepoName;
+use crate::github::{GithubRepoName, PullRequestNumber};
 use crate::utils::text::pluralize;
 use crate::{
     database::{WorkflowModel, WorkflowStatus},
@@ -368,3 +368,17 @@ pub fn auto_build_push_failed_comment(error: &str) -> Comment {
         ":eyes: Test was successful, but fast-forwarding failed: {error}"
     ))
 }
+
+pub fn conflict_comment(source: Option<PullRequestNumber>, was_unapproved: bool) -> Comment {
+    Comment::new(format!(
+        r#":umbrella: The latest upstream changes{} made this pull request unmergeable. Please [resolve the merge conflicts](https://rustc-dev-guide.rust-lang.org/git.html#rebasing-and-conflicts).{}"#,
+        source
+            .map(|n| format!(" (presumably #{n})"))
+            .unwrap_or_default(),
+        if was_unapproved {
+            "\n\nThis pull request was unapproved."
+        } else {
+            ""
+        }
+    ))
+}

rust-bors.example.toml

@@ -160,6 +160,7 @@ pub struct PullRequestUnassigned {
 pub struct PushToBranch {
     pub repository: GithubRepoName,
     pub branch: String,
+    pub sha: String,
 }
 
 #[derive(Debug)]

src/bors/comment.rs

@@ -5,14 +5,14 @@ use crate::bors::event::{
     PullRequestReadyForReview, PullRequestReopened, PullRequestUnassigned, PushToBranch,
 };
 
-use crate::bors::BorsContext;
 use crate::bors::handlers::handle_comment;
 use crate::bors::handlers::unapprove_pr;
 use crate::bors::handlers::workflow::{AutoBuildCancelReason, maybe_cancel_auto_build};
 use crate::bors::merge_queue::MergeQueueSender;
 use crate::bors::mergeability_queue::MergeabilityQueueSender;
+use crate::bors::{AUTO_BRANCH_NAME, BorsContext};
 use crate::bors::{Comment, PullRequestStatus, RepositoryState};
-use crate::database::{MergeableState, UpsertPullRequestParams};
+use crate::database::{MergeableState, PullRequestModel, UpsertPullRequestParams};
 use crate::github::{CommitSha, PullRequestNumber};
 use crate::utils::text::pluralize;
 use std::sync::Arc;
@@ -34,7 +34,7 @@ pub(super) async fn handle_pull_request_edited(
         return Ok(());
     };
 
-    mergeability_queue.enqueue_pr(&pr_model);
+    mergeability_queue.enqueue_pr(&pr_model, None);
 
     if !pr_model.is_approved() {
         return Ok(());
@@ -56,7 +56,7 @@ pub(super) async fn handle_push_to_pull_request(
         .upsert_pull_request(repo_state.repository(), pr.clone().into())
         .await?;
 
-    mergeability_queue.enqueue_pr(&pr_model);
+    mergeability_queue.enqueue_pr(&pr_model, None);
 
     let auto_build_cancel_message = maybe_cancel_auto_build(
         &repo_state.client,
@@ -119,15 +119,15 @@ pub(super) async fn handle_pull_request_opened(
                 author: payload.pull_request.author.username.clone(),
                 assignees,
                 base_branch: payload.pull_request.base.name.clone(),
-                mergeable_state: MergeableState::Unknown,
+                mergeable_state: payload.pull_request.mergeable_state.clone().into(),
                 pr_status,
             },
         )
         .await?;
 
     process_pr_description_commands(&payload, repo_state.clone(), db, ctx, merge_queue_tx).await?;
 
-    mergeability_queue.enqueue_pr(&pr);
+    mergeability_queue.enqueue_pr(&pr, None);
 
     Ok(())
 }
@@ -169,7 +169,7 @@ pub(super) async fn handle_pull_request_reopened(
         .upsert_pull_request(repo_state.repository(), pr.clone().into())
         .await?;
 
-    mergeability_queue.enqueue_pr(&pr);
+    mergeability_queue.enqueue_pr(&pr, None);
 
     Ok(())
 }
@@ -260,14 +260,44 @@ pub(super) async fn handle_push_to_branch(
             payload.branch
         );
 
+        // Try to find an auto build that matches this SHA
+        let merged_pr = find_pr_by_merged_commit(&repo_state, &db, CommitSha(payload.sha))
+            .await
+            .ok()
+            .flatten()
+            .map(|pr| pr.number);
+
         for pr in affected_prs {
-            mergeability_queue.enqueue_pr(&pr);
+            mergeability_queue.enqueue_pr(&pr, merged_pr);
         }
     }
 
     Ok(())
 }
 
+/// Try to find a merged PR that might have produced a build with the given commit SHA.
+async fn find_pr_by_merged_commit(
+    repo_state: &RepositoryState,
+    db: &PgDbClient,
+    sha: CommitSha,
+) -> anyhow::Result<Option<PullRequestModel>> {
+    let Some(build) = db
+        .find_build(repo_state.repository(), AUTO_BRANCH_NAME, sha)
+        .await?
+    else {
+        return Ok(None);
+    };
+
+    let Some(pr) = db.find_pr_by_build(&build).await? else {
+        return Ok(None);
+    };
+    if pr.auto_build.as_ref().map(|b| b.id) == Some(build.id) {
+        Ok(Some(pr))
+    } else {
+        Ok(None)
+    }
+}
+
 async fn process_pr_description_commands(
     payload: &PullRequestOpened,
     repo: Arc<RepositoryState>,
@@ -331,11 +361,12 @@ async fn notify_of_pushed_pr(
 
 #[cfg(test)]
 mod tests {
+    use insta::assert_snapshot;
     use octocrab::params::checks::{CheckRunConclusion, CheckRunStatus};
 
     use crate::bors::PullRequestStatus;
     use crate::bors::merge_queue::AUTO_BUILD_CHECK_RUN_NAME;
-    use crate::tests::{BorsTester, WorkflowRunData};
+    use crate::tests::{BorsBuilder, BorsTester, GitHubState, WorkflowRunData};
     use crate::{
         database::{MergeableState, OctocrabMergeableState},
         tests::{User, default_branch_name, default_repo_name, run_test},
@@ -409,7 +440,7 @@ mod tests {
             tester.get_pr_copy(()).await.expect_unapproved();
             Ok(())
         })
-        .await;
+            .await;
     }
 
     #[sqlx::test]
@@ -623,13 +654,13 @@ mod tests {
         run_test(pool, async |tester: &mut BorsTester| {
             let pr = tester
                 .open_pr(default_repo_name(), |pr| {
-                    pr.mergeable_state = OctocrabMergeableState::Unknown
+                    pr.mergeable_state = OctocrabMergeableState::Unknown;
                 })
                 .await?;
-            tester.push_to_branch(default_branch_name()).await?;
+            tester.push_to_branch(default_branch_name(), "sha").await?;
             tester
                 .modify_pr_state(pr.id(), |pr| {
-                    pr.mergeable_state = OctocrabMergeableState::Dirty
+                    pr.mergeable_state = OctocrabMergeableState::Dirty;
                 })
                 .await;
             tester
@@ -642,6 +673,124 @@ mod tests {
         .await;
     }
 
+    #[sqlx::test]
+    async fn conflict_message_disabled_in_config(pool: sqlx::PgPool) {
+        run_test(pool, async |tester: &mut BorsTester| {
+            let pr = tester
+                .open_pr(default_repo_name(), |pr| {
+                    pr.mergeable_state = OctocrabMergeableState::Clean;
+                })
+                .await?;
+            tester
+                .modify_pr_state(pr.id(), |pr| {
+                    pr.mergeable_state = OctocrabMergeableState::Dirty;
+                })
+                .await;
+            tester.push_to_branch(default_branch_name(), "sha").await?;
+
+            Ok(())
+        })
+        .await;
+    }
+
+    #[sqlx::test]
+    async fn conflict_message_unknown_sha(pool: sqlx::PgPool) {
+        BorsBuilder::new(pool)
+            .github(GitHubState::default().with_default_config(
+                r#"
+merge_queue_enabled = true
+report_merge_conflicts = true
+"#,
+            )).run_test(async |tester: &mut BorsTester| {
+            let pr = tester
+                .open_pr(default_repo_name(), |pr| {
+                    pr.mergeable_state = OctocrabMergeableState::Clean;
+                })
+                .await?;
+            tester
+                .modify_pr_state(pr.id(), |pr| {
+                    pr.mergeable_state = OctocrabMergeableState::Dirty;
+                })
+                .await;
+            tester.push_to_branch(default_branch_name(), "sha").await?;
+            assert_snapshot!(tester.get_next_comment_text(pr).await?, @":umbrella: The latest upstream changes made this pull request unmergeable. Please [resolve the merge conflicts](https://rustc-dev-guide.rust-lang.org/git.html#rebasing-and-conflicts).");
+
+            Ok(())
+        })
+            .await;
+    }
+
+    #[sqlx::test]
+    async fn conflict_message_unknown_sha_approved(pool: sqlx::PgPool) {
+        BorsBuilder::new(pool)
+            .github(GitHubState::default().with_default_config(
+                r#"
+merge_queue_enabled = true
+report_merge_conflicts = true
+"#,
+            )).run_test(async |tester: &mut BorsTester| {
+            let pr = tester
+                .open_pr(default_repo_name(), |pr| {
+                    pr.mergeable_state = OctocrabMergeableState::Clean;
+                })
+                .await?;
+            tester.approve(pr.id()).await?;
+            tester
+                .modify_pr_state(pr.id(), |pr| {
+                    pr.mergeable_state = OctocrabMergeableState::Dirty;
+                })
+                .await;
+            tester.push_to_branch(default_branch_name(), "sha").await?;
+            assert_snapshot!(tester.get_next_comment_text(pr.id()).await?, @r"
+            :umbrella: The latest upstream changes made this pull request unmergeable. Please [resolve the merge conflicts](https://rustc-dev-guide.rust-lang.org/git.html#rebasing-and-conflicts).
+
+            This pull request was unapproved.
+            ");
+            tester.get_pr_copy(pr.id()).await.expect_unapproved();
+
+            Ok(())
+        })
+            .await;
+    }
+
+    #[sqlx::test]
+    async fn conflict_message_known_sha(pool: sqlx::PgPool) {
+        BorsBuilder::new(pool)
+            .github(GitHubState::default().with_default_config(
+                r#"
+merge_queue_enabled = true
+report_merge_conflicts = true
+"#,
+            )).run_test(async |tester: &mut BorsTester| {
+            let pr2 = tester
+                .open_pr(default_repo_name(), |pr| {
+                    pr.mergeable_state = OctocrabMergeableState::Clean;
+                })
+                .await?;
+
+            let pr1 = tester
+                .open_pr(default_repo_name(), |_| {})
+                .await?;
+            tester.approve(pr1.id()).await?;
+            tester.start_auto_build(pr1.id()).await?;
+            tester.workflow_full_success(tester.auto_branch().await).await?;
+            tester.process_merge_queue().await;
+            tester.expect_comments(pr1.id(), 1).await;
+            let sha = tester.auto_branch().await.get_sha().to_string();
+
+            tester
+                .modify_pr_state(pr2.id(), |pr| {
+                    pr.mergeable_state = OctocrabMergeableState::Dirty;
+                })
+                .await;
+            tester.push_to_branch(default_branch_name(), &sha).await?;
+            assert_snapshot!(tester.get_next_comment_text(pr2.id()).await?, @":umbrella: The latest upstream changes (presumably #3) made this pull request unmergeable. Please [resolve the merge conflicts](https://rustc-dev-guide.rust-lang.org/git.html#rebasing-and-conflicts).");
+
+            Ok(())
+        })
+            .await;
+    }
+
     #[sqlx::test]
     async fn enqueue_prs_on_pr_opened(pool: sqlx::PgPool) {
         run_test(pool, async |tester: &mut BorsTester| {
@@ -722,7 +871,7 @@ mod tests {
             ");
             Ok(())
         })
-        .await;
+            .await;
         gh.check_cancelled_workflows(default_repo_name(), &[123]);
     }
 
@@ -747,7 +896,7 @@ mod tests {
             ");
             Ok(())
         })
-        .await;
+            .await;
     }
 
     #[sqlx::test]

src/bors/event.rs

@@ -115,7 +115,7 @@ pub async fn reload_mergeability_status(
     );
 
     for pr in prs {
-        mergeability_queue.enqueue_pr(&pr);
+        mergeability_queue.enqueue_pr(&pr, None);
     }
 
     Ok(())

src/bors/handlers/pr_events.rs

@@ -632,7 +632,7 @@ try-job: Bar
                     .db()
                     .find_build(
                         &default_repo_name(),
-                        TRY_BRANCH_NAME.to_string(),
+                        TRY_BRANCH_NAME,
                         CommitSha(tester.try_branch().await.get_sha().to_string()),
                     )
                     .await?
@@ -791,7 +791,7 @@ try-job: Bar
             insta::assert_snapshot!(tester.get_next_comment_text(()).await?, @"Try build was cancelled. It was not possible to cancel some workflows.");
             let build = tester.db().find_build(
                 &default_repo_name(),
-                tester.try_branch().await.get_name().to_string(),
+                tester.try_branch().await.get_name(),
                 tester.try_branch().await.get_sha().to_string().into()
             ).await?.expect("build not found");
             assert_eq!(build.status, BuildStatus::Cancelled);