Merge pull request #1426 from rylev/handle-merge-conflicts

Handle merge conflicts when unrolling up PR

Author: rylev

site/src/github.rs

@@ -18,24 +18,31 @@ pub async fn unroll_rollup(
     previous_master: &str,
     rollup_pr_number: u32,
 ) -> Result<(), String> {
+    let format_commit = |s: &str, truncate: bool| {
+        let display = truncate.then(|| s.split_at(10).0).unwrap_or(s);
+        format!("[{display}](https://github.com/rust-lang-ci/rust/commit/{s})")
+    };
     let mapping = enqueue_unrolled_try_builds(ci_client, rollup_merges, previous_master)
         .await?
         .into_iter()
         .fold(String::new(), |mut string, c| {
             use std::fmt::Write;
-            write!(
-                &mut string,
-                "|#{pr}|[{commit}](https://github.com/rust-lang-ci/rust/commit/{commit})|\n",
-                pr = c.original_pr_number,
-                commit = c.sha
-            )
-            .unwrap();
+            let commit = c
+                .sha
+                .as_deref()
+                .map(|s| format_commit(s, false))
+                .unwrap_or_else(|| {
+                    let head = format_commit(&c.rolled_up_head, true);
+                    format!("❌ conflicts merging '{head}' into previous master ❌")
+                });
+            write!(&mut string, "|#{pr}|{commit}|\n", pr = c.original_pr_number).unwrap();
             string
         });
+    let previous_master = format_commit(previous_master, true);
     let msg =
         format!("📌 Perf builds for each rolled up PR:\n\n\
-        |PR# | Perf Build Sha|\n|----|-----|\n\
-        {mapping}\nIn the case of a perf regression, \
+        |PR# | Perf Build Sha|\n|----|:-----:|\n\
+        {mapping}\n\n*previous master*: {previous_master}\n\nIn the case of a perf regression, \
         run the following command for each PR you suspect might be the cause: `@rust-timer build $SHA`");
     main_repo_client.post_comment(rollup_pr_number, msg).await;
     Ok(())
@@ -76,35 +83,49 @@ async fn enqueue_unrolled_try_builds<'a>(
             "What we thought was a merge commit was not a merge commit. sha: {}",
             rollup_merge.sha
         );
-        let rolled_up_head = &commit.parents[1].sha;
+        let rolled_up_head = commit.parents[1].sha.clone();
 
         // Reset perf-tmp to the previous master
         client
             .update_branch("perf-tmp", previous_master)
             .await
             .map_err(|e| format!("Error updating perf-tmp with previous master: {e:?}"))?;
 
-        // Merge in the rolled up PR's head commit into the previous master
+        // Try to merge in the rolled up PR's head commit into the previous master
         let sha = client
             .merge_branch(
                 "perf-tmp",
-                rolled_up_head,
-                &format!("Unrolled build for #{}", original_pr_number),
+                &rolled_up_head,
+                &format!("Unrolled build for #{original_pr_number}"),
             )
             .await
             .map_err(|e| {
                 format!("Error merging #{original_pr_number}'s commit '{rolled_up_head}' into perf-tmp: {e:?}")
             })?;
 
-        // Force the `try-perf` branch to point to what the perf-tmp branch points to
-        client
-            .update_branch("try-perf", &sha)
-            .await
-            .map_err(|e| format!("Error updating the try-perf branch: {e:?}"))?;
+        // Handle success and merge conflicts
+        match &sha {
+            Some(s) => {
+                // Force the `try-perf` branch to point to what the perf-tmp branch points to
+                client
+                    .update_branch("try-perf", s)
+                    .await
+                    .map_err(|e| format!("Error updating the try-perf branch: {e:?}"))?;
+            }
+            None => {
+                // Merge conflict
+                log::debug!(
+                    "Could not create unrolled commit for #{original_pr_number}. \
+                Merging the rolled up HEAD '{rolled_up_head}' into the previous master \
+                '{previous_master}' leads to a merge conflict."
+                );
+            }
+        };
 
         mapping.push(UnrolledCommit {
             original_pr_number,
             rollup_merge,
+            rolled_up_head,
             sha,
         });
         // Wait to ensure there's enough time for GitHub to checkout these changes before they are overwritten
@@ -120,8 +141,10 @@ pub struct UnrolledCommit<'a> {
     pub original_pr_number: &'a str,
     /// The original rollup merge commit
     pub rollup_merge: &'a Commit,
-    /// The sha of the new unrolled merge commit
-    pub sha: String,
+    /// The HEAD commit for the rolled up PR
+    pub rolled_up_head: String,
+    /// The sha of the new unrolled merge commit. `None` when creation failed due to merge conflicts.
+    pub sha: Option<String>,
 }
 
 lazy_static::lazy_static! {

site/src/github/client.rs

@@ -107,12 +107,15 @@ impl Client {
         Ok(())
     }
 
+    /// Merge the given sha into the given branch with the given commit message
+    ///
+    /// Returns `None` if the sha cannot be merged due to a merge conflict.
     pub async fn merge_branch(
         &self,
         branch: &str,
         sha: &str,
         commit_message: &str,
-    ) -> anyhow::Result<String> {
+    ) -> anyhow::Result<Option<String>> {
         #[derive(serde::Serialize)]
         struct MergeBranchRequest<'a> {
             base: &'a str,
@@ -125,12 +128,22 @@ impl Client {
             head: sha,
             commit_message,
         });
-        let response = self.send(req).await.context("PATCH /merges failed")?;
-        if !response.status().is_success() {
-            anyhow::bail!("{:?} != 201 CREATED", response.status());
-        }
+        let response = self
+            .send(req)
+            .await
+            .context("POST /merges failed to send")?;
 
-        Ok(response.json::<MergeBranchResponse>().await?.sha)
+        if response.status() == 409 {
+            // Return `None` on merge conflicts which are signaled by 409s
+            Ok(None)
+        } else if !response.status().is_success() {
+            Err(anyhow::format_err!(
+                "response has non-successful status: {:?} ",
+                response.status()
+            ))
+        } else {
+            Ok(Some(response.json::<MergeBranchResponse>().await?.sha))
+        }
     }
 
     pub async fn create_commit(