Merge pull request #1399 from rust-lang/next-commit-real

Mark-Simulacrum · web-flow · commit dc9941c02ed1 · 2022-08-12T10:39:12.000-04:00
Store try commit dates into the DB properly
diff --git a/collector/src/api.rs b/collector/src/api.rs
@@ -1,14 +1,16 @@
 pub mod next_commit {
+    use database::Commit;
+
     #[derive(Debug, Clone, serde::Serialize, serde::Deserialize)]
-    pub struct Commit {
-        pub sha: String,
+    pub struct NextCommit {
+        pub commit: Commit,
         pub include: Option<String>,
         pub exclude: Option<String>,
         pub runs: Option<i32>,
     }
 
     #[derive(Debug, Clone, serde::Serialize, serde::Deserialize)]
     pub struct Response {
-        pub commit: Option<Commit>,
+        pub commit: Option<NextCommit>,
     }
 }
diff --git a/collector/src/main.rs b/collector/src/main.rs
@@ -1008,12 +1008,11 @@ fn main_result() -> anyhow::Result<i32> {
                 // no missing commits
                 return Ok(0);
             };
-            let commit = get_commit_or_fake_it(&next.sha)?;
 
             let pool = database::Pool::open(&db.db);
 
-            let sysroot = Sysroot::install(commit.sha.to_string(), &target_triple)
-                .with_context(|| format!("failed to install sysroot for {:?}", commit))?;
+            let sysroot = Sysroot::install(next.commit.sha.to_string(), &target_triple)
+                .with_context(|| format!("failed to install sysroot for {:?}", next.commit))?;
 
             let mut benchmarks = get_benchmarks(
                 &benchmark_dir,
@@ -1025,7 +1024,7 @@ fn main_result() -> anyhow::Result<i32> {
             let res = bench(
                 &mut rt,
                 pool,
-                &ArtifactId::Commit(commit),
+                &ArtifactId::Commit(next.commit),
                 &Profile::all(),
                 &Scenario::all(),
                 bench_rustc.bench_rustc,
diff --git a/database/src/lib.rs b/database/src/lib.rs
@@ -23,6 +23,7 @@ pub struct QueuedCommit {
     pub include: Option<String>,
     pub exclude: Option<String>,
     pub runs: Option<i32>,
+    pub commit_date: Option<Date>,
 }
 
 #[derive(Debug, Hash, Copy, Clone, PartialEq, Eq, PartialOrd, Ord)]
diff --git a/database/src/pool.rs b/database/src/pool.rs
@@ -102,7 +102,13 @@ pub trait Connection: Send + Sync {
         runs: Option<i32>,
     );
     /// Returns true if this PR was queued waiting for a commit
-    async fn pr_attach_commit(&self, pr: u32, sha: &str, parent_sha: &str) -> bool;
+    async fn pr_attach_commit(
+        &self,
+        pr: u32,
+        sha: &str,
+        parent_sha: &str,
+        commit_date: Option<DateTime<Utc>>,
+    ) -> bool;
     async fn queued_commits(&self) -> Vec<QueuedCommit>;
     async fn mark_complete(&self, sha: &str) -> Option<QueuedCommit>;
 
diff --git a/database/src/pool/postgres.rs b/database/src/pool/postgres.rs
@@ -212,6 +212,9 @@ static MIGRATIONS: &[&str] = &[
     r#"
     alter table benchmark add column category text not null DEFAULT 'secondary';
     "#,
+    r#"
+    alter table pull_request_build add column commit_date timestamptz;
+    "#,
 ];
 
 #[async_trait::async_trait]
@@ -623,12 +626,18 @@ where
                 log::error!("failed to queue_pr({}, {:?}, {:?}, {:?}): {:?}", pr, include, exclude, runs, e);
         }
     }
-    async fn pr_attach_commit(&self, pr: u32, sha: &str, parent_sha: &str) -> bool {
+    async fn pr_attach_commit(
+        &self,
+        pr: u32,
+        sha: &str,
+        parent_sha: &str,
+        commit_date: Option<DateTime<Utc>>,
+    ) -> bool {
         self.conn()
             .execute(
-                "update pull_request_build SET bors_sha = $1, parent_sha = $2
+                "update pull_request_build SET bors_sha = $1, parent_sha = $2, commit_date = $4
                 where pr = $3 and bors_sha is null",
-                &[&sha, &parent_sha, &(pr as i32)],
+                &[&sha, &parent_sha, &(pr as i32), &commit_date],
             )
             .await
             .unwrap()
@@ -638,7 +647,7 @@ where
         let rows = self
             .conn()
             .query(
-                "select pr, bors_sha, parent_sha, include, exclude, runs from pull_request_build
+                "select pr, bors_sha, parent_sha, include, exclude, runs, commit_date from pull_request_build
                 where complete is false and bors_sha is not null
                 order by requested asc",
                 &[],
@@ -653,6 +662,7 @@ where
                 include: row.get(3),
                 exclude: row.get(4),
                 runs: row.get(5),
+                commit_date: row.get::<_, Option<_>>(6).map(Date),
             })
             .collect()
     }
@@ -662,7 +672,7 @@ where
             .query_opt(
                 "update pull_request_build SET complete = true
                 where bors_sha = $1
-                returning pr, bors_sha, parent_sha, include, exclude, runs",
+                returning pr, bors_sha, parent_sha, include, exclude, runs, commit_date",
                 &[&sha],
             )
             .await
@@ -674,6 +684,7 @@ where
             include: row.get(3),
             exclude: row.get(4),
             runs: row.get(5),
+            commit_date: row.get::<_, Option<_>>(6).map(Date),
         })
     }
     async fn collection_id(&self, version: &str) -> CollectionId {
diff --git a/database/src/pool/sqlite.rs b/database/src/pool/sqlite.rs
@@ -3,7 +3,7 @@ use crate::{
     ArtifactId, Benchmark, BenchmarkData, CollectionId, Commit, CommitType, Date, Profile,
 };
 use crate::{ArtifactIdNumber, Index, QueryDatum, QueuedCommit};
-use chrono::{DateTime, TimeZone, Utc};
+use chrono::{DateTime, NaiveDateTime, TimeZone, Utc};
 use hashbrown::HashMap;
 use rusqlite::params;
 use rusqlite::OptionalExtension;
@@ -321,6 +321,7 @@ static MIGRATIONS: &[Migration] = &[
         "#,
     ),
     Migration::new("alter table benchmark add column category text not null default ''"),
+    Migration::new("alter table pull_request_build add column commit_date timestamp"),
 ];
 
 #[async_trait::async_trait]
@@ -564,21 +565,28 @@ impl Connection for SqliteConnection {
             .execute(params![pr, include, exclude, &runs])
             .unwrap();
     }
-    async fn pr_attach_commit(&self, pr: u32, sha: &str, parent_sha: &str) -> bool {
+    async fn pr_attach_commit(
+        &self,
+        pr: u32,
+        sha: &str,
+        parent_sha: &str,
+        commit_date: Option<DateTime<Utc>>,
+    ) -> bool {
+        let timestamp = commit_date.map(|d| d.timestamp());
         self.raw_ref()
             .prepare_cached(
-                "update pull_request_build SET bors_sha = ?, parent_sha = ?
+                "update pull_request_build SET bors_sha = ?, parent_sha = ?, commit_date = ?
                 where pr = ? and bors_sha is null",
             )
             .unwrap()
-            .execute(params![sha, parent_sha, pr])
+            .execute(params![sha, parent_sha, timestamp, pr])
             .unwrap()
             > 0
     }
     async fn queued_commits(&self) -> Vec<QueuedCommit> {
         self.raw_ref()
             .prepare_cached(
-                "select pr, bors_sha, parent_sha, include, exclude, runs from pull_request_build
+                "select pr, bors_sha, parent_sha, include, exclude, runs, commit_date from pull_request_build
                 where complete is false and bors_sha is not null
                 order by requested asc",
             )
@@ -593,6 +601,7 @@ impl Connection for SqliteConnection {
                     include: row.get(3).unwrap(),
                     exclude: row.get(4).unwrap(),
                     runs: row.get(5).unwrap(),
+                    commit_date: row.get::<_, Option<i64>>(6).unwrap().map(|timestamp| Date(DateTime::from_utc(NaiveDateTime::from_timestamp(timestamp, 0), Utc)))
                 })
             })
             .collect::<Result<Vec<_>, _>>()
@@ -612,7 +621,7 @@ impl Connection for SqliteConnection {
         assert_eq!(count, 1, "sha is unique column");
         self.raw_ref()
             .query_row(
-                "select pr, sha, parent_sha, include, exclude, runs from pull_request_build
+                "select pr, sha, parent_sha, include, exclude, runs, commit_date from pull_request_build
             where sha = ?",
                 params![sha],
                 |row| {
@@ -623,6 +632,7 @@ impl Connection for SqliteConnection {
                         include: row.get(3).unwrap(),
                         exclude: row.get(4).unwrap(),
                         runs: row.get(5).unwrap(),
+                        commit_date: row.get::<_, Option<i64>>(6).unwrap().map(|timestamp| Date(DateTime::from_utc(NaiveDateTime::from_timestamp(timestamp, 0), Utc)))
                     })
                 },
             )
diff --git a/site/src/github.rs b/site/src/github.rs
@@ -189,8 +189,13 @@ pub async fn enqueue_shas(
         };
         let queued = {
             let conn = ctxt.conn().await;
-            conn.pr_attach_commit(pr_number, &try_commit.sha, &try_commit.parent_sha)
-                .await
+            conn.pr_attach_commit(
+                pr_number,
+                &try_commit.sha,
+                &try_commit.parent_sha,
+                Some(commit_response.commit.committer.date),
+            )
+            .await
         };
         if queued {
             if !msg.is_empty() {
diff --git a/site/src/github/client.rs b/site/src/github/client.rs
@@ -1,4 +1,5 @@
 use anyhow::Context;
+use chrono::{DateTime, Utc};
 use http::header::USER_AGENT;
 
 use crate::{api::github::Issue, load::SiteCtxt};
@@ -253,6 +254,12 @@ pub struct InnerCommit {
     #[serde(default)]
     pub message: String,
     pub tree: CommitTree,
+    pub committer: Committer,
+}
+
+#[derive(Debug, Clone, serde::Deserialize)]
+pub struct Committer {
+    pub date: DateTime<Utc>,
 }
 
 #[derive(Debug, Clone, serde::Deserialize)]
diff --git a/site/src/load.rs b/site/src/load.rs
@@ -306,6 +306,7 @@ fn calculate_missing_from(
         include,
         exclude,
         runs,
+        commit_date,
     } in queued_pr_commits
         .into_iter()
         // filter out any queued PR master commits (leaving only try commits)
@@ -322,7 +323,7 @@ fn calculate_missing_from(
         queue.push((
             Commit {
                 sha: sha.to_string(),
-                date: Date::ymd_hms(2001, 01, 01, 0, 0, 0),
+                date: commit_date.unwrap_or(Date::empty()),
                 r#type: CommitType::Try,
             },
             MissingReason::Try {
@@ -499,6 +500,7 @@ mod tests {
                 include: None,
                 exclude: None,
                 runs: None,
+                commit_date: None,
             },
             QueuedCommit {
                 sha: "b".into(),
@@ -507,6 +509,7 @@ mod tests {
                 include: None,
                 exclude: None,
                 runs: None,
+                commit_date: None,
             },
             QueuedCommit {
                 sha: "a".into(),
@@ -515,6 +518,7 @@ mod tests {
                 include: None,
                 exclude: None,
                 runs: None,
+                commit_date: None,
             },
         ];
         let in_progress_artifacts = vec![];
@@ -606,6 +610,7 @@ mod tests {
                 include: None,
                 exclude: None,
                 runs: None,
+                commit_date: None,
             },
             // A try run
             QueuedCommit {
@@ -615,6 +620,7 @@ mod tests {
                 include: None,
                 exclude: None,
                 runs: None,
+                commit_date: None,
             },
         ];
         let in_progress_artifacts = vec![];
diff --git a/site/src/request_handlers/next_commit.rs b/site/src/request_handlers/next_commit.rs
@@ -18,7 +18,10 @@ pub async fn handle_next_commit(ctxt: Arc<SiteCtxt>) -> next_commit::Response {
             // TODO: add capability of doing the following in one step
             // to avoid possibile illegal inbetween states.
             conn.queue_pr(pr, None, None, None).await;
-            if !conn.pr_attach_commit(pr, &commit.sha, parent_sha).await {
+            if !conn
+                .pr_attach_commit(pr, &commit.sha, parent_sha, None)
+                .await
+            {
                 log::error!("failed to attach commit {} to PR queue", commit.sha);
             }
         }
@@ -49,8 +52,8 @@ pub async fn handle_next_commit(ctxt: Arc<SiteCtxt>) -> next_commit::Response {
             commit.sha,
             missing_reason_dbg
         );
-        Some(next_commit::Commit {
-            sha: commit.sha,
+        Some(next_commit::NextCommit {
+            commit,
             include,
             exclude,
             runs,

Original file line number	Diff line number	Diff line change
`@@ -1,14 +1,16 @@`
`1`	`1`	`pub mod next_commit {`
	`2`	`+ use database::Commit;`
	`3`	`+`
`2`	`4`	`#[derive(Debug, Clone, serde::Serialize, serde::Deserialize)]`
`3`		`- pub struct Commit {`
`4`		`- pub sha: String,`
	`5`	`+ pub struct NextCommit {`
	`6`	`+ pub commit: Commit,`
`5`	`7`	`pub include: Option<String>,`
`6`	`8`	`pub exclude: Option<String>,`
`7`	`9`	`pub runs: Option<i32>,`
`8`	`10`	`}`
`9`	`11`
`10`	`12`	`#[derive(Debug, Clone, serde::Serialize, serde::Deserialize)]`
`11`	`13`	`pub struct Response {`
`12`		`- pub commit: Option<Commit>,`
	`14`	`+ pub commit: Option<NextCommit>,`
`13`	`15`	`}`
`14`	`16`	`}`
Original file line number	Diff line number	Diff line change
`@@ -23,6 +23,7 @@ pub struct QueuedCommit {`
`23`	`23`	`pub include: Option<String>,`
`24`	`24`	`pub exclude: Option<String>,`
`25`	`25`	`pub runs: Option<i32>,`
	`26`	`+ pub commit_date: Option<Date>,`
`26`	`27`	`}`
`27`	`28`
`28`	`29`	`#[derive(Debug, Hash, Copy, Clone, PartialEq, Eq, PartialOrd, Ord)]`