Auto merge of #404 - pietroalbini:recognize-try-builds, r=pietroalbini

Automatically detect and fill in try builds

This PR implements automatic detection of the latest try build in a PR, using those as the default when no try build is specified in the `run` command:

![demo](https://user-images.githubusercontent.com/2299951/53890036-bec51500-4027-11e9-8642-d8c6ab0a67ed.png)

docs/bot-usage.md

@@ -36,8 +36,16 @@ When you want to create an experiment to check if a PR is breaking some
 third-party code, the first thing is to request a **try build** to get a valid
 toolchain. You can do that with the `@bors try` GitHub comment.
 
-Once the try build is done, you can get the SHA of the start and end commits.
-Bors should have posted a comment like this in the thread:
+**After** the try build is done you need to choose the [experiment mode you want to
+use][h-experiment-moes] and type up the command in your GitHub PR:
+
+```
+@craterbot run mode=YOUR-MODE
+```
+
+If you don't want to do a Crater run with the last try build but with an older
+one, you need to get the SHA of the start and end commits. Bors should have
+posted a comment like this in the thread:
 
 > ⌛ Trying commit XXXXXXX with merge YYYYYYY...
 
@@ -49,11 +57,11 @@ parents (both should be merge commits by bors):
 You must prefix the start commit with `master#`, and the end commit with
 `try#`, and both of them should be written with the full 40-chars hash.
 
-Then, you choose the [experiment mode you want to use][h-experiment-modes], and
-type up the command in your GitHub PR:
+Then you need to choose the [experiment mode you want to
+use][h-experiment-mode] and type up the command in your GitHub PR:
 
 ```
-@craterbot run start=master#fullhash end=try#fullhash mode=check-only
+@craterbot run start=master#fullhash end=try#fullhash mode=YOUR-MODE
 ```
 
 [Go back to the TOC][h-toc]
@@ -110,9 +118,9 @@ beta run you can use:
 * `name`: name of the experiment; required only if Crater [can't determine it
  automatically][h-experiment-names]
 * `start`: name of the first toolchain; can be either a rustup name or
- `branch#sha` (required)
+ `branch#sha` (required if no try build is automatically detected)
 * `end`: name of the second toolchain; can be either a rustup name or
- `branch#sha` (required)
+ `branch#sha` (required if no try build is automatically detected)
 * `mode`: the experiment mode (default: `build-and-test`)
 * `crates`: the selection of crates to use (default: `full`)
 * `cap-lints`: the lints cap (default: `forbid`, which means no cap)

src/db/migrations.rs

@@ -262,6 +262,22 @@ fn migrations() -> Vec<(&'static str, MigrationKind)> {
  ),
  ));
 
+ migrations.push((
+ "add_try_builds_table",
+ MigrationKind::SQL(
+ "
+ CREATE TABLE try_builds (
+ pr INTEGER NOT NULL,
+ repo STRING NOT NULL,
+ merge_sha TEXT NOT NULL,
+ base_sha TEXT NOT NULL,
+
+ PRIMARY KEY (pr, repo)
+ );
+ ",
+ ),
+ ));
+
  migrations
 }
 

src/server/auth.rs

@@ -1,6 +1,6 @@
 use crate::config::Config;
 use crate::prelude::*;
-use crate::server::github::GitHubApi;
+use crate::server::github::{GitHub, GitHubApi};
 use crate::server::{Data, HttpError};
 use http::header::{HeaderMap, AUTHORIZATION, USER_AGENT};
 use regex::Regex;

src/server/github.rs

@@ -14,6 +14,17 @@ pub enum GitHubError {
  RequestFailed(StatusCode, String),
 }
 
+pub trait GitHub {
+ fn username(&self) -> Fallible<String>;
+ fn post_comment(&self, issue_url: &str, body: &str) -> Fallible<()>;
+ fn list_labels(&self, issue_url: &str) -> Fallible<Vec<Label>>;
+ fn add_label(&self, issue_url: &str, label: &str) -> Fallible<()>;
+ fn remove_label(&self, issue_url: &str, label: &str) -> Fallible<()>;
+ fn list_teams(&self, org: &str) -> Fallible<HashMap<String, usize>>;
+ fn team_members(&self, team: usize) -> Fallible<Vec<String>>;
+ fn get_commit(&self, repo: &str, sha: &str) -> Fallible<Commit>;
+}
+
 #[derive(Clone)]
 pub struct GitHubApi {
  token: String,
@@ -36,13 +47,15 @@ impl GitHubApi {
  utils::http::prepare_sync(method, &url)
  .header(AUTHORIZATION, format!("token {}", self.token))
  }
+}
 
- pub fn username(&self) -> Fallible<String> {
+impl GitHub for GitHubApi {
+ fn username(&self) -> Fallible<String> {
  let response: User = self.build_request(Method::GET, "user").send()?.json()?;
  Ok(response.login)
  }
 
- pub fn post_comment(&self, issue_url: &str, body: &str) -> Fallible<()> {
+ fn post_comment(&self, issue_url: &str, body: &str) -> Fallible<()> {
  let mut response = self
  .build_request(Method::POST, &format!("{}/comments", issue_url))
  .json(&json!({
@@ -58,7 +71,7 @@ impl GitHubApi {
  }
  }
 
- pub fn list_labels(&self, issue_url: &str) -> Fallible<Vec<Label>> {
+ fn list_labels(&self, issue_url: &str) -> Fallible<Vec<Label>> {
  let mut response = self
  .build_request(Method::GET, &format!("{}/labels", issue_url))
  .send()?;
@@ -71,7 +84,7 @@ impl GitHubApi {
  }
  }
 
- pub fn add_label(&self, issue_url: &str, label: &str) -> Fallible<()> {
+ fn add_label(&self, issue_url: &str, label: &str) -> Fallible<()> {
  let mut response = self
  .build_request(Method::POST, &format!("{}/labels", issue_url))
  .json(&json!([label]))
@@ -85,7 +98,7 @@ impl GitHubApi {
  }
  }
 
- pub fn remove_label(&self, issue_url: &str, label: &str) -> Fallible<()> {
+ fn remove_label(&self, issue_url: &str, label: &str) -> Fallible<()> {
  let mut response = self
  .build_request(Method::DELETE, &format!("{}/labels/{}", issue_url, label))
  .send()?;
@@ -98,7 +111,7 @@ impl GitHubApi {
  }
  }
 
- pub fn list_teams(&self, org: &str) -> Fallible<HashMap<String, usize>> {
+ fn list_teams(&self, org: &str) -> Fallible<HashMap<String, usize>> {
  let mut response = self
  .build_request(Method::GET, &format!("orgs/{}/teams", org))
  .send()?;
@@ -112,7 +125,7 @@ impl GitHubApi {
  }
  }
 
- pub fn team_members(&self, team: usize) -> Fallible<Vec<String>> {
+ fn team_members(&self, team: usize) -> Fallible<Vec<String>> {
  let mut response = self
  .build_request(Method::GET, &format!("teams/{}/members", team))
  .send()?;
@@ -125,6 +138,15 @@ impl GitHubApi {
  Err(GitHubError::RequestFailed(response.status(), error.message).into())
  }
  }
+
+ fn get_commit(&self, repo: &str, sha: &str) -> Fallible<Commit> {
+ let commit = self
+ .build_request(Method::GET, &format!("repos/{}/commits/{}", repo, sha))
+ .send()?
+ .error_for_status()?
+ .json()?;
+ Ok(commit)
+ }
 }
 
 #[derive(Deserialize)]
@@ -143,6 +165,7 @@ pub struct EventIssueComment {
  pub issue: Issue,
  pub comment: Comment,
  pub sender: User,
+ pub repository: Repository,
 }
 
 #[derive(Deserialize)]
@@ -159,6 +182,11 @@ pub struct PullRequest {
  pub html_url: String,
 }
 
+#[derive(Deserialize)]
+pub struct Repository {
+ pub full_name: String,
+}
+
 #[derive(Deserialize)]
 pub struct Label {
  pub name: String,
@@ -174,3 +202,14 @@ pub struct Team {
  pub id: usize,
  pub slug: String,
 }
+
+#[derive(Deserialize)]
+pub struct Commit {
+ pub sha: String,
+ pub parents: Vec<CommitParent>,
+}
+
+#[derive(Deserialize)]
+pub struct CommitParent {
+ pub sha: String,
+}

src/server/messages.rs

@@ -1,4 +1,5 @@
 use crate::prelude::*;
+use crate::server::github::GitHub;
 use crate::server::Data;
 
 pub enum Label {

src/server/mod.rs

@@ -6,13 +6,14 @@ mod messages;
 mod reports;
 mod routes;
 pub mod tokens;
+mod try_builds;
 
 use crate::config::Config;
 use crate::db::Database;
 use crate::prelude::*;
 use crate::server::agents::Agents;
 use crate::server::auth::ACL;
-use crate::server::github::GitHubApi;
+use crate::server::github::{GitHub, GitHubApi};
 use crate::server::tokens::Tokens;
 use http::{self, header::HeaderValue, Response};
 use hyper::Body;

src/server/routes/webhooks/commands.rs

@@ -2,12 +2,13 @@ use crate::actions::{self, Action, ActionsCtx};
 use crate::db::{Database, QueryUtils};
 use crate::experiments::{CapLints, CrateSelect, Experiment, GitHubIssue, Mode, Status};
 use crate::prelude::*;
-use crate::server::github::Issue;
+use crate::server::github::{Issue, Repository};
 use crate::server::messages::{Label, Message};
 use crate::server::routes::webhooks::args::{
  AbortArgs, EditArgs, RetryArgs, RetryReportArgs, RunArgs,
 };
 use crate::server::Data;
+use crate::toolchain::{Toolchain, ToolchainSource};
 
 pub fn ping(data: &Data, issue: &Issue) -> Fallible<()> {
  Message::new()
@@ -17,15 +18,48 @@ pub fn ping(data: &Data, issue: &Issue) -> Fallible<()> {
  Ok(())
 }
 
-pub fn run(host: &str, data: &Data, issue: &Issue, args: RunArgs) -> Fallible<()> {
+pub fn run(
+ host: &str,
+ data: &Data,
+ repo: &Repository,
+ issue: &Issue,
+ args: RunArgs,
+) -> Fallible<()> {
  let name = setup_run_name(&data.db, issue, args.name)?;
 
+ // Autodetect toolchains only if none of them was specified
+ let (mut detected_start, mut detected_end, mut try_build) = (None, None, None);
+ if args.start.is_none() && args.end.is_none() {
+ if let Some(build) =
+ crate::server::try_builds::get_sha(&data.db, &repo.full_name, issue.number)?
+ {
+ try_build = Some(build.merge_sha.clone());
+ detected_start = Some(Toolchain {
+ source: ToolchainSource::CI {
+ sha: build.base_sha.into(),
+ r#try: false,
+ },
+ rustflags: None,
+ });
+ detected_end = Some(Toolchain {
+ source: ToolchainSource::CI {
+ sha: build.merge_sha.into(),
+ r#try: true,
+ },
+ rustflags: None,
+ });
+ }
+ }
+
  actions::CreateExperiment {
  name: name.clone(),
  toolchains: [
  args.start
+ .or(detected_start)
  .ok_or_else(|| err_msg("missing start toolchain"))?,
- args.end.ok_or_else(|| err_msg("missing end toolchain"))?,
+ args.end
+ .or(detected_end)
+ .ok_or_else(|| err_msg("missing end toolchain"))?,
  ],
  mode: args.mode.unwrap_or(Mode::BuildAndTest),
  crates: args.crates.unwrap_or(CrateSelect::Full),
@@ -41,13 +75,14 @@ pub fn run(host: &str, data: &Data, issue: &Issue, args: RunArgs) -> Fallible<()
  }
  .apply(&ActionsCtx::new(&data.db, &data.config))?;
 
- Message::new()
- .line(
- "ok_hand",
- format!(
- "Experiment **`{}`** created and queued.", name
- ),
- )
+ let mut message = Message::new().line(
+ "ok_hand",
+ format!("Experiment **`{}`** created and queued.", name),
+ );
+ if let Some(sha) = try_build {
+ message = message.line("robot", format!("Automatically detected try build {}", sha));
+ }
+ message
  .line(
  "mag",
  format!(

src/server/routes/webhooks/mod.rs

@@ -2,7 +2,7 @@ mod args;
 mod commands;
 
 use crate::prelude::*;
-use crate::server::github::{EventIssueComment, Issue};
+use crate::server::github::{EventIssueComment, Issue, Repository};
 use crate::server::messages::Message;
 use crate::server::routes::webhooks::args::Command;
 use crate::server::Data;
@@ -36,8 +36,22 @@ fn process_webhook(
  return Ok(());
  }
 
- if let Err(e) = process_command(host, &p.sender.login, &p.comment.body, &p.issue, data)
- {
+ crate::server::try_builds::detect(
+ &data.db,
+ &data.github,
+ &p.repository.full_name,
+ p.issue.number,
+ &p.comment.body,
+ )?;
+
+ if let Err(e) = process_command(
+ host,
+ &p.sender.login,
+ &p.comment.body,
+ &p.repository,
+ &p.issue,
+ data,
+ ) {
  Message::new()
  .line("rotating_light", format!("**Error:** {}", e))
  .note(
@@ -57,6 +71,7 @@ fn process_command(
  host: &str,
  sender: &str,
  body: &str,
+ repo: &Repository,
  issue: &Issue,
  data: &Data,
 ) -> Fallible<()> {
@@ -100,7 +115,7 @@ fn process_command(
  }
 
  Command::Run(args) => {
- commands::run(host, data, issue, args)?;
+ commands::run(host, data, repo, issue, args)?;
  }
 
  Command::Edit(args) => {