diff --git a/site/src/api.rs b/site/src/api.rs index 86fe3bb43..31b1b0c74 100644 --- a/site/src/api.rs +++ b/site/src/api.rs @@ -619,33 +619,6 @@ pub mod github { issue: Issue, comment: Comment, }, - Push(Push), - } - - #[derive(Debug, Clone, Serialize, Deserialize)] - pub struct Push { - pub r#ref: String, - pub head_commit: HeadCommit, - pub before: String, - pub commits: Vec, - pub repository: Repository, - } - - #[derive(Debug, Clone, Serialize, Deserialize)] - pub struct Repository { - pub default_branch: String, - } - - #[derive(Debug, Clone, Serialize, Deserialize)] - pub struct Commit { - #[serde(rename = "id")] - pub sha: String, - pub message: String, - } - - #[derive(Debug, Clone, Serialize, Deserialize)] - pub struct HeadCommit { - pub message: String, } #[derive(Debug, Clone, Serialize, Deserialize)] diff --git a/site/src/github.rs b/site/src/github.rs index 773d73e1d..225223f88 100644 --- a/site/src/github.rs +++ b/site/src/github.rs @@ -1,12 +1,10 @@ pub mod client; pub mod comparison_summary; -use crate::api::github::Commit; use crate::job_queue::build_queue; use crate::load::{SiteCtxt, TryCommit}; use chrono::Utc; use serde::Deserialize; -use std::sync::LazyLock; use std::time::Duration; type BoxedError = Box; @@ -19,213 +17,8 @@ pub const RUST_REPO_GITHUB_API_URL: &str = "https://api.github.com/repos/rust-la /// They are removed once a perf. run comparison summary is posted on a PR. pub const COMMENT_MARK_TEMPORARY: &str = ""; -/// Used for comment that contains unrolled commits for merged rolled-up PRs. -pub const COMMENT_MARK_ROLLUP: &str = ""; - use database::{BenchmarkJobStatus, BenchmarkRequestStatus, Connection}; -/// Enqueues try build artifacts and posts a message about them on the original rollup PR -pub async fn unroll_rollup( - gh_client: client::Client, - rollup_merges: impl Iterator, - previous_master: &str, - rollup_pr_number: u32, -) -> Result<(), String> { - let commit_link = |sha: &str| format!("https://github.com/rust-lang/rust/commit/{sha}"); - - let format_commit = |s: &str, truncate: bool| { - let display = if truncate { s.split_at(10).0 } else { s }; - format!("[{display}]({})", commit_link(s)) - }; - - let mapping = enqueue_unrolled_try_builds(&gh_client, rollup_merges, previous_master) - .await? - .into_iter() - .fold(String::new(), |mut string, c| { - use std::fmt::Write; - let commit = c - .sha - .as_deref() - .map(|s| { - // Format the SHA as a code block to make it easy to copy-paste verbatim - let link = commit_link(s); - format!("`{s}` ([link]({link}))") - }) - .unwrap_or_else(|| { - let head = format_commit(&c.rolled_up_head, true); - format!("❌ conflicts merging '{head}' into previous master ❌") - }); - let message = c - .rollup_merge - .message - .split('\n') - // Skip over "Rollup merge of ..." and an empty line - .nth(2) - .map(|m| { - if m.len() <= 60 { - m.to_string() - } else { - format!("{}…", m.split_at(59).0) - } - }) - .unwrap_or_else(|| format!("#{}", c.original_pr_number)) - .replace('|', "\\|"); - writeln!( - &mut string, - "|#{pr}|{message}|{commit}|", - 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# | Message | 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`\n\ - {COMMENT_MARK_ROLLUP}"); - gh_client.post_comment(rollup_pr_number, msg).await; - Ok(()) -} - -/// Enqueues try builds on the try-perf branch for every rollup merge in `rollup_merges`. -/// Returns a mapping between the rollup merge commit and the try build sha. -async fn enqueue_unrolled_try_builds<'a>( - client: &client::Client, - rollup_merges: impl Iterator, - previous_master: &str, -) -> Result>, String> { - let mut mapping = Vec::new(); - for rollup_merge in rollup_merges { - // Grab the number of the rolled up PR from its commit message - let original_pr_number = ROLLEDUP_PR_NUMBER - .captures(&rollup_merge.message) - .and_then(|c| c.get(1)) - .map(|m| m.as_str()) - .ok_or_else(|| { - format!( - "Could not get PR number from message: '{}'", - rollup_merge.message - ) - })?; - - // Fetch the rollup merge commit which should have two parents. - // The first parent is in the chain of rollup merge commits all the way back to `previous_master`. - // The second parent is the head of the PR that was rolled up. We want the second parent. - let commit = client.get_commit(&rollup_merge.sha).await.map_err(|e| { - format!( - "Error getting rollup merge commit '{}': {e:?}", - rollup_merge.sha - ) - })?; - assert!( - commit.parents.len() == 2, - "What we thought was a merge commit was not a merge commit. sha: {}", - rollup_merge.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:?}"))?; - - // 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}\n{}", rollup_merge.message), - ) - .await - .map_err(|e| { - format!("Error merging #{original_pr_number}'s commit '{rolled_up_head}' into perf-tmp: {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 - tokio::time::sleep(std::time::Duration::from_secs(15)).await - } - - Ok(mapping) -} - -/// A commit representing a rolled up PR as if it had been merged into master directly -pub struct UnrolledCommit<'a> { - /// The PR number that was rolled up - pub original_pr_number: &'a str, - /// The original rollup merge commit - pub rollup_merge: &'a Commit, - /// 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, -} - -static ROLLUP_PR_NUMBER: LazyLock = - LazyLock::new(|| regex::Regex::new(r"^Auto merge of #(\d+)").unwrap()); -static ROLLEDUP_PR_NUMBER: LazyLock = - LazyLock::new(|| regex::Regex::new(r"^Rollup merge of #(\d+)").unwrap()); - -// Gets the pr number for the associated rollup PR message. Returns None if this is not a rollup PR -pub async fn rollup_pr_number( - client: &client::Client, - message: &str, -) -> Result, String> { - if !message.starts_with("Auto merge of") { - return Ok(None); - } - - let number = ROLLUP_PR_NUMBER - .captures(message) - .and_then(|c| c.get(1)) - .map(|m| m.as_str().parse::()) - .transpose() - .map_err(|e| format!("Error parsing PR number from '{message}': {e:?}"))?; - - let number = match number { - Some(n) => n, - None => return Ok(None), - }; - - let issue = client - .get_issue(number) - .await - .map_err(|e| format!("Error fetching PR #{number} {e:?}"))?; - - Ok(issue - .labels - .iter() - .any(|l| l.name == "rollup") - .then_some(issue.number)) -} - /// Enqueues the given SHA and returns a message that should be sent as a comment to the corresponding PR. /// If not benchmark reques was found to which the commit SHA could be attached, returns `Ok(None)`. pub async fn enqueue_sha( diff --git a/site/src/github/client.rs b/site/src/github/client.rs index 6d6554594..817233722 100644 --- a/site/src/github/client.rs +++ b/site/src/github/client.rs @@ -4,7 +4,7 @@ use http::header; use http::header::USER_AGENT; use serde::de::DeserializeOwned; -use crate::{api::github::Issue, load::SiteCtxt}; +use crate::load::SiteCtxt; const BOT_USER_AGENT: &str = "perf-rust-lang-org-server"; @@ -36,162 +36,6 @@ impl Client { Self::new(repository_url, token) } - pub async fn create_ref(&self, ref_: &str, sha: &str) -> anyhow::Result<()> { - #[derive(serde::Serialize)] - struct CreateRefRequest<'a> { - // Must start with `refs/` and have at least two slashes. - // e.g. `refs/heads/master`. - #[serde(rename = "ref")] - ref_: &'a str, - sha: &'a str, - } - let url = format!("{}/git/refs", self.repository_url); - let req = self.inner.post(&url).json(&CreateRefRequest { ref_, sha }); - let response = self.send(req).await.context("POST git/refs failed")?; - if response.status() != reqwest::StatusCode::CREATED { - anyhow::bail!("{:?} != 201 CREATED", response.status()); - } - - Ok(()) - } - - pub async fn create_pr( - &self, - title: &str, - head: &str, - base: &str, - description: &str, - draft: bool, - ) -> anyhow::Result { - #[derive(serde::Serialize)] - struct CreatePrRequest<'a> { - title: &'a str, - // username:branch if cross-repo - head: &'a str, - // branch to pull into (e.g, master) - base: &'a str, - #[serde(rename = "body")] - description: &'a str, - draft: bool, - } - - let url = format!("{}/pulls", self.repository_url); - let req = self.inner.post(&url).json(&CreatePrRequest { - title, - head, - base, - description, - draft, - }); - let response = self.send(req).await.context("POST pulls failed")?; - if response.status() != reqwest::StatusCode::CREATED { - anyhow::bail!("{:?} != 201 CREATED", response.status()); - } - - response.json().await.context("deserializing failed") - } - - pub async fn update_branch(&self, branch: &str, sha: &str) -> anyhow::Result<()> { - #[derive(serde::Serialize)] - struct UpdateBranchRequest<'a> { - sha: &'a str, - force: bool, - } - let url = format!("{}/git/refs/heads/{}", self.repository_url, branch); - let req = self - .inner - .patch(&url) - .json(&UpdateBranchRequest { sha, force: true }); - - let response = self.send(req).await.context("PATCH git/refs failed")?; - if response.status() != reqwest::StatusCode::OK { - anyhow::bail!("{:?} != 200 OK", response.status()); - } - - 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> { - #[derive(serde::Serialize)] - struct MergeBranchRequest<'a> { - base: &'a str, - head: &'a str, - commit_message: &'a str, - } - let url = format!("{}/merges", self.repository_url); - let req = self.inner.post(&url).json(&MergeBranchRequest { - base: branch, - head: sha, - commit_message, - }); - let response = self - .send(req) - .await - .context("POST /merges failed to send")?; - - 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::().await?.sha)) - } - } - - pub async fn create_commit( - &self, - message: &str, - tree: &str, - parents: &[&str], - ) -> anyhow::Result { - #[derive(serde::Serialize)] - struct CreateCommitRequest<'a> { - message: &'a str, - tree: &'a str, - parents: &'a [&'a str], - } - let url = format!("{}/git/commits", self.repository_url); - let req = self.inner.post(&url).json(&CreateCommitRequest { - message, - tree, - parents, - }); - - let response = self.send(req).await.context("POST git/commits failed")?; - if response.status() != reqwest::StatusCode::CREATED { - anyhow::bail!("{:?} != 201 CREATED", response.status()); - } - - Ok(response - .json::() - .await - .context("deserializing failed")? - .sha) - } - - pub async fn get_issue(&self, number: u64) -> anyhow::Result { - let url = format!("{}/issues/{}", self.repository_url, number); - let req = self.inner.get(&url); - let response = self.send(req).await.context("cannot get issue")?; - if !response.status().is_success() { - anyhow::bail!("{:?} != 200 OK", response.status()); - } - - Ok(response.json().await?) - } - pub async fn get_commit(&self, sha: &str) -> anyhow::Result { let url = format!("{}/commits/{}", self.repository_url, sha); let req = self.inner.get(&url); @@ -427,16 +271,6 @@ pub struct CreatePrResponse { pub comments_url: String, } -#[derive(serde::Deserialize)] -struct MergeBranchResponse { - sha: String, -} - -#[derive(serde::Deserialize)] -struct CreateCommitResponse { - sha: String, -} - #[derive(Debug, Clone, serde::Deserialize)] pub struct Commit { pub sha: String, diff --git a/site/src/github/comparison_summary.rs b/site/src/github/comparison_summary.rs index fa920db77..2c3f8a761 100644 --- a/site/src/github/comparison_summary.rs +++ b/site/src/github/comparison_summary.rs @@ -6,7 +6,7 @@ use crate::load::SiteCtxt; use database::{metric::Metric, QueuedCommit}; -use crate::github::{COMMENT_MARK_ROLLUP, COMMENT_MARK_TEMPORARY, RUST_REPO_GITHUB_API_URL}; +use crate::github::{COMMENT_MARK_TEMPORARY, RUST_REPO_GITHUB_API_URL}; use humansize::BINARY; use std::fmt::Write; @@ -21,9 +21,6 @@ pub async fn post_comparison_comment( let client = super::client::Client::from_ctxt(ctxt, RUST_REPO_GITHUB_API_URL.to_owned()); let pr = commit.pr; - // Was this perf. run triggered from a PR that was already merged and is a rollup? - let mut is_rollup = false; - // Scan comments to hide outdated ones and gather context let graph_client = super::client::GraphQLClient::from_ctxt(ctxt); for comment in graph_client.get_comments(pr).await? { @@ -36,24 +33,17 @@ pub async fn post_comparison_comment( log::debug!("Hiding comment {}", comment.id); graph_client.hide_comment(&comment.id, "OUTDATED").await?; } - - if comment.viewer_did_author && comment.body.contains(COMMENT_MARK_ROLLUP) { - is_rollup = true; - } } let source = if is_master_commit { PerfRunSource::MasterCommit - } else if is_rollup { - PerfRunSource::TryBuildRollup } else { PerfRunSource::TryBuild }; - let body = match summarize_run(ctxt, commit, source).await { - Ok(message) => message, - Err(error) => error, - }; + let body = summarize_run(ctxt, commit, source) + .await + .unwrap_or_else(|error| error); client.post_comment(pr, body).await; @@ -98,8 +88,6 @@ enum PerfRunSource { MasterCommit, // Manual try build on a PR TryBuild, - // Manual try build on a merged rollup PR - TryBuildRollup, } // Should the metric be shown by default in the summary? @@ -172,7 +160,6 @@ async fn summarize_run( let next_steps = match source { PerfRunSource::TryBuild => try_run_body(is_regression, deserves_attention), - PerfRunSource::TryBuildRollup => "".to_string(), PerfRunSource::MasterCommit => master_run_body(is_regression), }; writeln!(&mut message, "{next_steps}\n").unwrap(); diff --git a/site/src/request_handlers/github.rs b/site/src/request_handlers/github.rs index 2f432e578..6909fea83 100644 --- a/site/src/request_handlers/github.rs +++ b/site/src/request_handlers/github.rs @@ -1,7 +1,6 @@ use crate::api::{github, ServerResult}; use crate::github::{ - client, enqueue_sha, parse_homu_comment, rollup_pr_number, unroll_rollup, - COMMENT_MARK_TEMPORARY, RUST_REPO_GITHUB_API_URL, + client, enqueue_sha, parse_homu_comment, COMMENT_MARK_TEMPORARY, RUST_REPO_GITHUB_API_URL, }; use crate::load::SiteCtxt; use std::fmt::Write; @@ -32,36 +31,9 @@ pub async fn handle_github_webhook( } handle_issue(ctxt, issue, comment).await } - github::Request::Push(p) => handle_push(ctxt, p).await, } } -async fn handle_push(ctxt: Arc, push: github::Push) -> ServerResult { - let gh_client = client::Client::from_ctxt(&ctxt, RUST_REPO_GITHUB_API_URL.to_owned()); - if push.r#ref != format!("refs/heads/{}", push.repository.default_branch) { - return Ok(github::Response); - } - let rollup_pr_number = match rollup_pr_number(&gh_client, &push.head_commit.message).await? { - Some(pr) => pr, - None => return Ok(github::Response), - }; - - let previous_master = push.before; - let commits = push.commits; - - // GitHub webhooks have a timeout of 10 seconds, so we process this - // in the background. - tokio::spawn(async move { - let rollup_merges = commits - .iter() - .filter(|c| c.message.starts_with("Rollup merge of #")); - let result = - unroll_rollup(gh_client, rollup_merges, &previous_master, rollup_pr_number).await; - log::info!("Processing of rollup merge finished: {:#?}", result); - }); - Ok(github::Response) -} - const RUST_TIMER_PREFIX: &str = "@rust-timer"; async fn handle_issue(