Improvements for Substrate #8409's bugs

src/auth.rs

@@ -34,11 +34,11 @@ impl GithubUserAuthenticator {
 				Error::OrganizationMembership {
 					source: Box::new(e),
 				}
-				.map_issue(Some((
+				.map_issue((
 					self.org.clone(),
 					self.repo_name.clone(),
 					self.pr_number,
-				)))
+				))
 			})?;
 
 		if !is_member {
@@ -50,11 +50,11 @@ impl GithubUserAuthenticator {
 					),
 				}),
 			}
-			.map_issue(Some((
+			.map_issue((
 				self.org.clone(),
 				self.repo_name.clone(),
 				self.pr_number,
-			))))?;
+			)))?;
 		}
 		Ok(())
 	}

src/companion.rs

@@ -1,10 +1,15 @@
 use regex::Regex;
+use rocksdb::DB;
 use snafu::ResultExt;
 use std::path::Path;
 
-use crate::{cmd::*, error::*, github_bot::GithubBot, Result};
+use crate::{
+	cmd::*, error::*, github::*, github_bot::GithubBot, webhook::wait_to_merge,
+	Result, COMPANION_LONG_REGEX, COMPANION_PREFIX_REGEX,
+	COMPANION_SHORT_REGEX, PR_HTML_URL_REGEX,
+};
 
-pub async fn companion_update(
+async fn update_companion_repository(
 	github_bot: &GithubBot,
 	owner: &str,
 	owner_repo: &str,
@@ -231,15 +236,12 @@ pub async fn companion_update(
 	Ok(updated_sha)
 }
 
-pub fn companion_parse(body: &str) -> Option<(String, String, String, i64)> {
+fn companion_parse(body: &str) -> Option<(String, String, String, i64)> {
 	companion_parse_long(body).or(companion_parse_short(body))
 }
 
 fn companion_parse_long(body: &str) -> Option<(String, String, String, i64)> {
-	let re = Regex::new(
-		r"companion[^[[:alpha:]]\n]*(?P<html_url>https://github.com/(?P<owner>[^/\n]+)/(?P<repo>[^/\n]+)/pull/(?P<number>[[:digit:]]+))"
-	)
-	.unwrap();
+	let re = Regex::new(COMPANION_LONG_REGEX!()).unwrap();
 	let caps = re.captures(&body)?;
 	let html_url = caps.name("html_url")?.as_str().to_owned();
 	let owner = caps.name("owner")?.as_str().to_owned();
@@ -254,10 +256,7 @@ fn companion_parse_long(body: &str) -> Option<(String, String, String, i64)> {
 }
 
 fn companion_parse_short(body: &str) -> Option<(String, String, String, i64)> {
-	let re = Regex::new(
-		r"companion[^[[:alpha:]]\n]*(?P<owner>[^/\n]+)/(?P<repo>[^/\n]+)#(?P<number>[[:digit:]]+)",
-	)
-	.unwrap();
+	let re = Regex::new(COMPANION_SHORT_REGEX!()).unwrap();
 	let caps = re.captures(&body)?;
 	let owner = caps.name("owner")?.as_str().to_owned();
 	let repo = caps.name("repo")?.as_str().to_owned();
@@ -276,6 +275,117 @@ fn companion_parse_short(body: &str) -> Option<(String, String, String, i64)> {
 	Some((html_url, owner, repo, number))
 }
 
+async fn perform_companion_update(
+	github_bot: &GithubBot,
+	db: &DB,
+	html_url: &str,
+	owner: &str,
+	repo: &str,
+	number: i64,
+) -> Result<()> {
+	let comp_pr = github_bot.pull_request(&owner, &repo, number).await?;
+
+	if let PullRequest {
+		head:
+			Some(Head {
+				ref_field: Some(contributor_branch),
+				repo:
+					Some(HeadRepo {
+						name: contributor_repo,
+						owner:
+							Some(User {
+								login: contributor, ..
+							}),
+						..
+					}),
+				..
+			}),
+		..
+	} = comp_pr.clone()
+	{
+		log::info!("Updating companion {}", html_url);
+		let updated_sha = update_companion_repository(
+			github_bot,
+			&owner,
+			&repo,
+			&contributor,
+			&contributor_repo,
+			&contributor_branch,
+		)
+		.await?;
+
+		log::info!("Companion updated; waiting for checks on {}", html_url);
+		wait_to_merge(
+			github_bot,
+			&owner,
+			&repo,
+			comp_pr.number,
+			&comp_pr.html_url,
+			&format!("parity-processbot[bot]"),
+			&updated_sha,
+			db,
+		)
+		.await?;
+	} else {
+		return Err(Error::Message {
+			msg: "Companion PR is missing some API data.".to_string(),
+		});
+	}
+
+	Ok(())
+}
+
+async fn detect_then_update_companion(
+	github_bot: &GithubBot,
+	merge_done_in: &str,
+	pr: &PullRequest,
+	db: &DB,
+) -> Result<()> {
+	if merge_done_in == "substrate" {
+		log::info!("Checking for companion.");
+		if let Some((html_url, owner, repo, number)) =
+			pr.body.as_ref().map(|body| companion_parse(body)).flatten()
+		{
+			log::info!("Found companion {}", html_url);
+			perform_companion_update(
+				github_bot, db, &html_url, &owner, &repo, number,
+			)
+			.await
+			.map_err(|e| e.map_issue((owner, repo, number)))?;
+		} else {
+			log::info!("No companion found.");
+		}
+	}
+
+	Ok(())
+}
+
+/// Check for a Polkadot companion and update it if found.
+pub async fn update_companion(
+	github_bot: &GithubBot,
+	merge_done_in: &str,
+	pr: &PullRequest,
+	db: &DB,
+) -> Result<()> {
+	detect_then_update_companion(github_bot, merge_done_in, pr, db)
+		.await
+		.map_err(|e| match e {
+			Error::WithIssue { source, issue } => {
+				Error::CompanionUpdate { source }.map_issue(issue)
+			}
+			_ => {
+				let e = Error::CompanionUpdate {
+					source: Box::new(e),
+				};
+				if let Some(details) = pr.get_issue_details() {
+					e.map_issue(details)
+				} else {
+					e
+				}
+			}
+		})
+}
+
 #[cfg(test)]
 mod tests {
 	use super::*;

src/constants.rs

@@ -1,6 +1,17 @@
 pub const AUTO_MERGE_REQUEST: &str = "bot merge";
 pub const AUTO_MERGE_FORCE: &str = "bot merge force";
 pub const AUTO_MERGE_CANCEL: &str = "bot merge cancel";
+pub const REBASE: &str = "bot rebase";
+pub const BURNIN_REQUEST: &str = "bot burnin";
+pub const COMPARE_RELEASE_REQUEST: &str = "bot compare substrate";
+pub const BOT_COMMANDS: [&str; 6] = [
+	AUTO_MERGE_REQUEST,
+	AUTO_MERGE_FORCE,
+	AUTO_MERGE_CANCEL,
+	REBASE,
+	BURNIN_REQUEST,
+	COMPARE_RELEASE_REQUEST,
+];
 
 pub const AUTO_MERGE_FAILED: &str = "Cannot merge; please ensure the pull request is mergeable and has approval from the project owner or at least {min_reviewers} core devs.";
 pub const AUTO_MERGE_CHECKS_FAILED: &str = "Checks failed; cannot auto-merge.";
@@ -9,10 +20,6 @@ pub const AUTO_MERGE_CHECKS_ERROR: &str =
 pub const AUTO_MERGE_INVALIDATED: &str =
 	"Something has changed since auto-merge was requested; cancelling.";
 
-pub const COMPARE_RELEASE_REQUEST: &str = "bot compare substrate";
-pub const REBASE: &str = "bot rebase";
-pub const BURNIN_REQUEST: &str = "bot burnin";
-
 pub const FEATURES_KEY: &str = "features";
 
 pub const PROJECT_NEEDS_BACKLOG: &str =

src/error.rs

@@ -1,19 +1,20 @@
 use snafu::Snafu;
 
-type IssueDetails = Option<(String, String, i64)>;
+// TODO this really should be struct { repository, owner, number }
+pub type IssueDetails = (String, String, i64);
 
 #[derive(Debug, Snafu)]
 #[snafu(visibility = "pub")]
 pub enum Error {
-	#[snafu(display("Source: {}", source))]
+	#[snafu(display("WithIssue: {}", source))]
 	WithIssue {
 		source: Box<Error>,
 		issue: IssueDetails,
 	},
 
-	#[snafu(display("Error updating companion: {}", source))]
-	Companion {
-		source: Box<Error>,
+	#[snafu(display("Field is missing: {}", field))]
+	MissingField {
+		field: String,
 	},
 
 	#[snafu(display("Error merging: {}", source))]
@@ -22,14 +23,25 @@ pub enum Error {
 		commit_sha: String,
 	},
 
+	#[snafu(display("Companion update failed: {}", source))]
+	CompanionUpdate {
+		source: Box<Error>,
+	},
+
+	#[snafu(display("Rebase failed: {}", source))]
+	Rebase {
+		source: Box<Error>,
+	},
+
 	#[snafu(display("Checks failed for {}", commit_sha))]
 	ChecksFailed {
 		commit_sha: String,
 	},
 
-	#[snafu(display("Head SHA changed from {}", commit_sha))]
+	#[snafu(display("Head SHA changed from {} to {}", expected, actual))]
 	HeadChanged {
-		commit_sha: String,
+		expected: String,
+		actual: String,
 	},
 
 	#[snafu(display("Error getting organization membership: {}", source))]
@@ -48,7 +60,7 @@ pub enum Error {
 	#[snafu(display("Missing approval."))]
 	Approval {},
 
-	#[snafu(display("Error: {}", msg))]
+	#[snafu(display("{}", msg))]
 	Message {
 		msg: String,
 	},
@@ -62,13 +74,13 @@ pub enum Error {
 
 	/// An error occurred while sending or receiving a HTTP request or response
 	/// respectively.
-	#[snafu(display("Source: {}", source))]
+	#[snafu(display("Http: {}", source))]
 	Http {
 		source: reqwest::Error,
 	},
 
 	/// An error occurred in a Tokio call.
-	#[snafu(display("Source: {}", source))]
+	#[snafu(display("Tokio: {}", source))]
 	Tokio {
 		source: tokio::io::Error,
 	},
@@ -78,31 +90,25 @@ pub enum Error {
 	MissingData {},
 
 	/// An error occurred while retrieving or setting values in Rocks DB.
-	#[snafu(display("Source: {}", source))]
+	#[snafu(display("Db: {}", source))]
 	Db {
 		source: rocksdb::Error,
 	},
 
 	/// An error occurred while parsing or serializing JSON.
-	#[snafu(display("Source: {}", source))]
+	#[snafu(display("Utf8: {}", source))]
 	Utf8 {
 		source: std::string::FromUtf8Error,
 	},
 
 	/// An error occurred while parsing or serializing JSON.
-	#[snafu(display("Source: {}", source))]
+	#[snafu(display("Json: {}", source))]
 	Json {
 		source: serde_json::Error,
 	},
 
 	/// An error occurred while parsing TOML.
-	#[snafu(display("Source: {}", source))]
-	Toml {
-		source: toml::de::Error,
-	},
-
-	/// An error occurred while parsing TOML.
-	#[snafu(display("Source: {}", source))]
+	#[snafu(display("Base64: {}", source))]
 	Base64 {
 		source: base64::DecodeError,
 	},
@@ -118,7 +124,7 @@ pub enum Error {
 		source: jsonwebtoken::errors::Error,
 	},
 
-	#[snafu(display("Source: {}", source))]
+	#[snafu(display("Bincode: {}", source))]
 	Bincode {
 		source: bincode::Error,
 	},
@@ -166,7 +172,7 @@ pub enum Error {
 	},
 
 	#[snafu(display(
-		"Cmd '{}' failed with status {:?}; output: {}",
+		"Command '{}' failed with status {:?}; output: {}",
 		cmd,
 		status_code,
 		err
@@ -180,9 +186,12 @@ pub enum Error {
 
 impl Error {
 	pub fn map_issue(self, issue: IssueDetails) -> Self {
-		Self::WithIssue {
-			source: Box::new(self),
-			issue: issue,
+		match self {
+			Self::WithIssue { source, .. } => Self::WithIssue { source, issue },
+			_ => Self::WithIssue {
+				source: Box::new(self),
+				issue,
+			},
 		}
 	}
 }