Merge pull request #318 from ehuss/bors-check

Check if commit is from bors

Author: ehuss

src/git.rs

@@ -13,22 +13,29 @@ use git2::build::RepoBuilder;
 use git2::{Commit as Git2Commit, Repository};
 use log::debug;
 
-use crate::Commit;
+use crate::{Author, Commit, GitDate, BORS_AUTHOR};
 
 impl Commit {
     // Takes &mut because libgit2 internally caches summaries
     fn from_git2_commit(commit: &mut Git2Commit<'_>) -> Self {
+        let committer = commit.committer();
         Commit {
             sha: commit.id().to_string(),
-            date: Utc
-                .timestamp_opt(commit.time().seconds(), 0)
-                .unwrap()
-                .date_naive(),
+            date: time_to_date(&commit.time()),
             summary: String::from_utf8_lossy(commit.summary_bytes().unwrap()).to_string(),
+            committer: Author {
+                name: committer.name().unwrap_or("").to_string(),
+                email: committer.email().unwrap_or("").to_string(),
+                date: time_to_date(&committer.when()),
+            },
         }
     }
 }
 
+fn time_to_date(time: &git2::Time) -> GitDate {
+    Utc.timestamp_opt(time.seconds(), 0).unwrap().date_naive()
+}
+
 struct RustcRepo {
     repository: Repository,
     origin_remote: String,
@@ -145,8 +152,12 @@ pub fn get_commits_between(first_commit: &str, last_commit: &str) -> anyhow::Res
     // two commits are merge commits made by bors
     let assert_by_bors = |c: &Git2Commit<'_>| -> anyhow::Result<()> {
         match c.author().name() {
-            Some("bors") => Ok(()),
-            Some(author) => bail!("Expected author {} to be bors for {}.\n Make sure specified commits are on the master branch!", author, c.id()),
+            Some(author) if author == BORS_AUTHOR => Ok(()),
+            Some(author) => bail!(
+                "Expected author {author} to be {BORS_AUTHOR} for {}.\n \
+                Make sure specified commits are on the master branch!",
+                c.id()
+            ),
             None => bail!("No author for {}", c.id()),
         }
     };
@@ -167,7 +178,7 @@ pub fn get_commits_between(first_commit: &str, last_commit: &str) -> anyhow::Res
         res.push(Commit::from_git2_commit(&mut current));
         match current.parents().next() {
             Some(c) => {
-                if c.author().name() != Some("bors") {
+                if c.author().name() != Some(BORS_AUTHOR) {
                     debug!(
                         "{:?} has non-bors author: {:?}, skipping",
                         c.id(),

src/github.rs

@@ -3,7 +3,7 @@ use reqwest::header::{HeaderMap, HeaderValue, InvalidHeaderValue, AUTHORIZATION,
 use reqwest::{blocking::Client, blocking::Response};
 use serde::{Deserialize, Serialize};
 
-use crate::{parse_to_naive_date, Commit, GitDate};
+use crate::{parse_to_naive_date, Author, Commit, GitDate, BORS_AUTHOR};
 
 #[derive(Serialize, Deserialize, Debug)]
 struct GithubCommitComparison {
@@ -16,8 +16,8 @@ struct GithubCommitElem {
 }
 #[derive(Serialize, Deserialize, Debug)]
 struct GithubCommit {
-    author: GithubAuthor,
-    committer: GithubAuthor,
+    author: Option<GithubAuthor>,
+    committer: Option<GithubAuthor>,
     message: String,
 }
 #[derive(Serialize, Deserialize, Debug)]
@@ -38,19 +38,33 @@ pub(crate) struct GithubComment {
 
 impl GithubCommitElem {
     fn date(&self) -> anyhow::Result<GitDate> {
-        let (date_str, _) =
-            self.commit.committer.date.split_once('T').context(
-                "commit date should folllow the ISO 8061 format, eg: 2022-05-04T09:55:51Z",
-            )?;
+        let (date_str, _) = self
+            .commit
+            .committer
+            .as_ref()
+            .ok_or_else(|| anyhow::anyhow!("commit should have committer"))?
+            .date
+            .split_once('T')
+            .context("commit date should folllow the ISO 8061 format, eg: 2022-05-04T09:55:51Z")?;
         Ok(parse_to_naive_date(date_str)?)
     }
 
     fn git_commit(self) -> anyhow::Result<Commit> {
         let date = self.date()?;
+        let committer = self
+            .commit
+            .committer
+            .ok_or_else(|| anyhow::anyhow!("commit should have committer"))?;
+        let committer = Author {
+            name: committer.name,
+            email: committer.email,
+            date,
+        };
         Ok(Commit {
             sha: self.sha,
             date,
             summary: self.commit.message,
+            committer,
         })
     }
 }
@@ -123,13 +137,11 @@ impl CommitsQuery<'_> {
         let mut commits = Vec::new();
 
         // focus on Pull Request merges, all authored and committed by bors.
-        let author = "bors";
-
         let client = Client::builder().default_headers(headers()?).build()?;
         for page in 1.. {
             let url = CommitsUrl {
                 page,
-                author,
+                author: BORS_AUTHOR,
                 since: self.since_date,
                 sha: self.most_recent_sha,
             }
@@ -138,21 +150,17 @@ impl CommitsQuery<'_> {
             let response: Response = client.get(&url).send()?;
 
             let action = parse_paged_elems(response, |elem: GithubCommitElem| {
-                let date = elem.date()?;
-                let sha = elem.sha.clone();
-                let summary = elem.commit.message;
-                let commit = Commit { sha, date, summary };
-                commits.push(commit);
-
-                Ok(if elem.sha == self.earliest_sha {
+                let found_last = elem.sha == self.earliest_sha;
+                if found_last {
                     eprintln!(
                         "ending github query because we found starting sha: {}",
                         elem.sha
                     );
-                    Loop::Break
-                } else {
-                    Loop::Next
-                })
+                }
+                let commit = elem.git_commit()?;
+                commits.push(commit);
+
+                Ok(if found_last { Loop::Break } else { Loop::Next })
             })?;
 
             if let Loop::Break = action {
@@ -254,9 +262,15 @@ mod tests {
     #[test]
     fn test_github() {
         let c = get_commit("25674202bb7415e0c0ecd07856749cfb7f591be6").unwrap();
+        let committer = Author {
+            name: String::from("bors"),
+            email: String::from("bors@rust-lang.org"),
+            date: GitDate::from_ymd_opt(2022, 5, 4).unwrap(),
+        };
         let expected_c = Commit { sha: "25674202bb7415e0c0ecd07856749cfb7f591be6".to_string(), 
                                 date: parse_to_naive_date("2022-05-04").unwrap(),
-                                summary: "Auto merge of #96695 - JohnTitor:rollup-oo4fc1h, r=JohnTitor\n\nRollup of 6 pull requests\n\nSuccessful merges:\n\n - #96597 (openbsd: unbreak build on native platform)\n - #96662 (Fix typo in lint levels doc)\n - #96668 (Fix flaky rustdoc-ui test because it did not replace time result)\n - #96679 (Quick fix for #96223.)\n - #96684 (Update `ProjectionElem::Downcast` documentation)\n - #96686 (Add some TAIT-related tests)\n\nFailed merges:\n\nr? `@ghost`\n`@rustbot` modify labels: rollup".to_string()
+                                summary: "Auto merge of #96695 - JohnTitor:rollup-oo4fc1h, r=JohnTitor\n\nRollup of 6 pull requests\n\nSuccessful merges:\n\n - #96597 (openbsd: unbreak build on native platform)\n - #96662 (Fix typo in lint levels doc)\n - #96668 (Fix flaky rustdoc-ui test because it did not replace time result)\n - #96679 (Quick fix for #96223.)\n - #96684 (Update `ProjectionElem::Downcast` documentation)\n - #96686 (Add some TAIT-related tests)\n\nFailed merges:\n\nr? `@ghost`\n`@rustbot` modify labels: rollup".to_string(),
+                                committer,
                             };
         assert_eq!(c, expected_c)
     }

src/main.rs

@@ -36,11 +36,21 @@ use crate::toolchains::{
     TestOutcome, Toolchain, ToolchainSpec, NIGHTLY_SERVER, YYYY_MM_DD,
 };
 
+const BORS_AUTHOR: &str = "bors";
+
 #[derive(Debug, Clone, PartialEq)]
 pub struct Commit {
     pub sha: String,
     pub date: GitDate,
     pub summary: String,
+    pub committer: Author,
+}
+
+#[derive(Debug, Clone, PartialEq)]
+pub struct Author {
+    pub name: String,
+    pub email: String,
+    pub date: GitDate,
 }
 
 /// The first commit which build artifacts are made available through the CI for
@@ -1121,10 +1131,25 @@ impl Config {
 
     fn bisect_ci_via(&self, start_sha: &str, end_ref: &str) -> anyhow::Result<BisectionResult> {
         let access = self.args.access.repo();
-        let end_sha = access.commit(end_ref)?.sha;
-        let commits = access.commits(start_sha, &end_sha)?;
+        let start = access.commit(start_sha)?;
+        let end = access.commit(end_ref)?;
+        let assert_by_bors = |c: &Commit| -> anyhow::Result<()> {
+            if c.committer.name != BORS_AUTHOR {
+                bail!(
+                    "Expected author {} to be {BORS_AUTHOR} for {}.\n \
+                     Make sure specified commits are on the master branch \
+                     and refer to a bors merge commit!",
+                    c.committer.name,
+                    c.sha
+                );
+            }
+            Ok(())
+        };
+        assert_by_bors(&start)?;
+        assert_by_bors(&end)?;
+        let commits = access.commits(start_sha, &end.sha)?;
 
-        assert_eq!(commits.last().expect("at least one commit").sha, end_sha);
+        assert_eq!(commits.last().expect("at least one commit").sha, end.sha);
 
         commits.iter().zip(commits.iter().skip(1)).all(|(a, b)| {
             let sorted_by_date = a.date <= b.date;
@@ -1146,7 +1171,7 @@ impl Config {
             )
         }
 
-        self.bisect_ci_in_commits(start_sha, &end_sha, commits)
+        self.bisect_ci_in_commits(start_sha, &end.sha, commits)
     }
 
     fn bisect_ci_in_commits(