Move meta checks and build triggering

ofborg/src/tasks/eval/nixpkgs.rs

@@ -4,13 +4,14 @@ use hubcaps::gists::Gists;
 use hubcaps::issues::{Issue, IssueRef};
 use hubcaps::repositories::Repository;
 use ofborg::checkout::CachedProjectCo;
+use ofborg::commentparser::Subset;
 use ofborg::commitstatus::CommitStatus;
 use ofborg::evalchecker::EvalChecker;
 use ofborg::files::file_to_str;
 use ofborg::message::buildjob::BuildJob;
 use ofborg::message::evaluationjob::EvaluationJob;
 use ofborg::nix::Nix;
-use ofborg::outpathdiff::{OutPathDiff, PackageArch};
+use ofborg::outpathdiff::{OutPathDiff, OutPaths, PackageArch};
 use ofborg::tagger::{MaintainerPRTagger, PathsTagger, RebuildTagger};
 use ofborg::tagger::{PkgsAddedRemovedTagger, StdenvTagger};
 use ofborg::tasks::eval::{stdenvs::Stdenvs, Error, EvaluationStrategy, StepResult};
@@ -18,6 +19,7 @@ use ofborg::tasks::evaluate::update_labels;
 use std::collections::HashMap;
 use std::path::Path;
 use tasks::evaluate::make_gist;
+use uuid::Uuid;
 
 pub struct NixpkgsStrategy<'a> {
     job: &'a EvaluationJob,
@@ -31,6 +33,7 @@ pub struct NixpkgsStrategy<'a> {
     stdenv_diff: Option<Stdenvs>,
     outpath_diff: Option<OutPathDiff>,
     changed_paths: Option<Vec<String>>,
+    touched_packages: Option<Vec<String>>,
 }
 impl<'a> NixpkgsStrategy<'a> {
     pub fn new(
@@ -55,6 +58,7 @@ impl<'a> NixpkgsStrategy<'a> {
             stdenv_diff: None,
             outpath_diff: None,
             changed_paths: None,
+            touched_packages: None,
         }
     }
 
@@ -247,6 +251,67 @@ impl<'a> NixpkgsStrategy<'a> {
             }
         }
     }
+
+    fn check_meta_queue_builds(&self, dir: &Path) -> StepResult<Vec<BuildJob>> {
+        if let Some(ref possibly_touched_packages) = self.touched_packages {
+            let mut status = CommitStatus::new(
+                self.repo.statuses(),
+                self.job.pr.head_sha.clone(),
+                String::from("grahamcofborg-eval-check-meta"),
+                String::from("config.nix: checkMeta = true"),
+                None,
+            );
+            status.set(hubcaps::statuses::State::Pending);
+
+            let checker = OutPaths::new(self.nix.clone(), dir.to_path_buf(), true);
+            match checker.find() {
+                Ok(pkgs) => {
+                    let mut try_build: Vec<String> = pkgs
+                        .keys()
+                        .map(|pkgarch| pkgarch.package.clone())
+                        .filter(|pkg| possibly_touched_packages.contains(&pkg))
+                        .collect();
+                    try_build.sort();
+                    try_build.dedup();
+
+                    status.set_url(None);
+                    status.set(hubcaps::statuses::State::Success);
+
+                    if !try_build.is_empty() && try_build.len() <= 10 {
+                        // In the case of trying to merge master in to
+                        // a stable branch, we don't want to do this.
+                        // Therefore, only schedule builds if there
+                        // less than or exactly 10
+                        Ok(vec![BuildJob::new(
+                            self.job.repo.clone(),
+                            self.job.pr.clone(),
+                            Subset::Nixpkgs,
+                            try_build,
+                            None,
+                            None,
+                            format!("{}", Uuid::new_v4()),
+                        )])
+                    } else {
+                        Ok(vec![])
+                    }
+                }
+                Err(mut out) => {
+                    status.set_url(make_gist(
+                        &self.gists,
+                        "Meta Check",
+                        None,
+                        file_to_str(&mut out),
+                    ));
+                    status.set(hubcaps::statuses::State::Failure);
+                    Err(Error::Fail(String::from(
+                        "Failed to validate package metadata.",
+                    )))
+                }
+            }
+        } else {
+            Ok(vec![])
+        }
+    }
 }
 
 impl<'a> EvaluationStrategy for NixpkgsStrategy<'a> {
@@ -278,6 +343,11 @@ impl<'a> EvaluationStrategy for NixpkgsStrategy<'a> {
         self.changed_paths = Some(changed_paths);
         self.tag_from_paths();
 
+        self.touched_packages = Some(parse_commit_messages(
+            &co.commit_messages_from_head(&self.job.pr.head_sha)
+                .unwrap_or_else(|_| vec!["".to_owned()]),
+        ));
+
         Ok(())
     }
 
@@ -312,7 +382,7 @@ impl<'a> EvaluationStrategy for NixpkgsStrategy<'a> {
         self.update_new_package_labels();
         self.update_rebuild_labels(&dir, status);
 
-        Ok(vec![])
+        self.check_meta_queue_builds(&dir)
     }
 }
 
@@ -331,3 +401,66 @@ fn request_reviews(maint: &maintainers::ImpactedMaintainers, pull: &hubcaps::pul
         }
     }
 }
+
+fn parse_commit_messages(messages: &[String]) -> Vec<String> {
+    messages
+        .iter()
+        .filter_map(|line| {
+            // Convert "foo: some notes" in to "foo"
+            let parts: Vec<&str> = line.splitn(2, ':').collect();
+            if parts.len() == 2 {
+                Some(parts[0])
+            } else {
+                None
+            }
+        })
+        .flat_map(|line| {
+            let pkgs: Vec<&str> = line.split(',').collect();
+            pkgs
+        })
+        .map(|line| line.trim().to_owned())
+        .collect()
+}
+
+#[cfg(test)]
+mod tests {
+
+    use super::*;
+
+    #[test]
+    fn test_parse_commit_messages() {
+        let expect: Vec<&str> = vec![
+            "firefox{-esr", // don't support such fancy syntax
+            "}",            // Don't support such fancy syntax
+            "firefox",
+            "buildkite-agent",
+            "python.pkgs.ptyprocess",
+            "python.pkgs.ptyprocess",
+            "android-studio-preview",
+            "foo",
+            "bar",
+        ];
+        assert_eq!(
+            parse_commit_messages(
+                &"
+              firefox{-esr,}: fix failing build due to the google-api-key
+              Merge pull request #34483 from andir/dovecot-cve-2017-15132
+              firefox: enable official branding
+              Merge pull request #34442 from rnhmjoj/virtual
+              buildkite-agent: enable building on darwin
+              python.pkgs.ptyprocess: 0.5 -> 0.5.2
+              python.pkgs.ptyprocess: move expression
+              Merge pull request #34465 from steveeJ/steveej-attempt-qtile-bump-0.10.7
+              android-studio-preview: 3.1.0.8 -> 3.1.0.9
+              Merge pull request #34188 from dotlambda/home-assistant
+              Merge pull request #34414 from dotlambda/postfix
+              foo,bar: something here: yeah
+            "
+                .lines()
+                .map(|l| l.to_owned())
+                .collect::<Vec<String>>(),
+            ),
+            expect
+        );
+    }
+}

ofborg/src/tasks/evaluate.rs

@@ -8,24 +8,20 @@ use hubcaps::gists::Gists;
 use hubcaps::issues::Issue;
 use ofborg::acl::ACL;
 use ofborg::checkout;
-use ofborg::commentparser::Subset;
 use ofborg::commitstatus::CommitStatus;
 use ofborg::evalchecker::EvalChecker;
 use ofborg::files::file_to_str;
 use ofborg::message::{buildjob, evaluationjob};
 use ofborg::nix;
-use ofborg::outpathdiff::OutPaths;
 use ofborg::stats;
 use ofborg::stats::Event;
 use ofborg::systems;
 use ofborg::worker;
 use std::collections::HashMap;
 use std::path::Path;
-use std::path::PathBuf;
 use std::time::Instant;
 use tasks::eval;
 use tasks::eval::StepResult;
-use uuid::Uuid;
 
 pub struct EvaluationWorker<E> {
     cloner: checkout::CachedCloner,
@@ -247,11 +243,6 @@ impl<E: stats::SysEvents + 'static> worker::SimpleWorker for EvaluationWorker<E>
             return self.actions().skip(&job);
         }
 
-        let possibly_touched_packages = parse_commit_messages(
-            &co.commit_messages_from_head(&job.pr.head_sha)
-                .unwrap_or_else(|_| vec!["".to_owned()]),
-        );
-
         overall_status.set_with_description("Merging PR", hubcaps::statuses::State::Pending);
 
         if co.merge_commit(job.pr.head_sha.as_ref()).is_err() {
@@ -368,7 +359,7 @@ impl<E: stats::SysEvents + 'static> worker::SimpleWorker for EvaluationWorker<E>
             ),
         ];
 
-        let mut eval_results: bool = eval_checks
+        let eval_results: bool = eval_checks
             .into_iter()
             .map(|check| {
                 let mut status = CommitStatus::new(
@@ -413,97 +404,36 @@ impl<E: stats::SysEvents + 'static> worker::SimpleWorker for EvaluationWorker<E>
         let mut response: worker::Actions = vec![];
 
         if eval_results {
-            let mut status = CommitStatus::new(
+            let ret = evaluation_strategy
-                repo.statuses(),
+                .all_evaluations_passed(&Path::new(&refpath), &mut overall_status);
-                job.pr.head_sha.clone(),
+            match ret {
-                String::from("grahamcofborg-eval-check-meta"),
+                Ok(builds) => {
-                String::from("config.nix: checkMeta = true"),
+                    for buildjob in builds {
-                None,
-            );
-
-            status.set(hubcaps::statuses::State::Pending);
-
-            let state: hubcaps::statuses::State;
-            let gist_url: Option<String>;
-
-            let checker = OutPaths::new(self.nix.clone(), PathBuf::from(&refpath), true);
-            match checker.find() {
-                Ok(pkgs) => {
-                    state = hubcaps::statuses::State::Success;
-                    gist_url = None;
-
-                    let mut try_build: Vec<String> = pkgs
-                        .keys()
-                        .map(|pkgarch| pkgarch.package.clone())
-                        .filter(|pkg| possibly_touched_packages.contains(&pkg))
-                        .collect();
-                    try_build.sort();
-                    try_build.dedup();
-
-                    if !try_build.is_empty() && try_build.len() <= 10 {
-                        // In the case of trying to merge master in to
-                        // a stable branch, we don't want to do this.
-                        // Therefore, only schedule builds if there
-                        // less than or exactly 10
-                        let msg = buildjob::BuildJob::new(
-                            job.repo.clone(),
-                            job.pr.clone(),
-                            Subset::Nixpkgs,
-                            try_build,
-                            None,
-                            None,
-                            format!("{}", Uuid::new_v4()),
-                        );
                         for arch in auto_schedule_build_archs.iter() {
                             let (exchange, routingkey) = arch.as_build_destination();
-                            response.push(worker::publish_serde_action(exchange, routingkey, &msg));
+                            response.push(worker::publish_serde_action(
+                                exchange, routingkey, &buildjob,
+                            ));
                         }
                         response.push(worker::publish_serde_action(
                             Some("build-results".to_string()),
                             None,
                             &buildjob::QueuedBuildJobs {
-                                job: msg,
+                                job: buildjob,
                                 architectures: auto_schedule_build_archs
-                                    .into_iter()
+                                    .iter()
                                     .map(|arch| arch.to_string())
                                     .collect(),
                             },
                         ));
                     }
                 }
-                Err(mut out) => {
+                Err(e) => {
-                    eval_results = false;
+                    if self
-                    state = hubcaps::statuses::State::Failure;
+                        .handle_strategy_err(Err(e), &gists, &mut overall_status)
-                    gist_url = make_gist(
+                        .is_err()
-                        &gists,
+                    {
-                        "Meta Check",
+                        return self.actions().skip(&job);
-                        Some(format!("{:?}", state)),
-                        file_to_str(&mut out),
-                    );
-                }
-            }
-
-            status.set_url(gist_url);
-            status.set(state.clone());
-        }
-
-        if eval_results {
-            {
-                let ret = evaluation_strategy
-                    .all_evaluations_passed(&Path::new(&refpath), &mut overall_status);
-                match ret {
-                    Ok(builds) => {
-                        if builds.len() != 0 {
-                            panic!("we shouldn't be here yet!");
-                        }
-                    }
-                    Err(e) => {
-                        if self
-                            .handle_strategy_err(Err(e), &gists, &mut overall_status)
-                            .is_err()
-                        {
-                            return self.actions().skip(&job);
-                        }
                     }
                 }
             }
@@ -579,69 +509,6 @@ pub fn update_labels(issue: &hubcaps::issues::IssueRef, add: &[String], remove:
     }
 }
 
-fn parse_commit_messages(messages: &[String]) -> Vec<String> {
-    messages
-        .iter()
-        .filter_map(|line| {
-            // Convert "foo: some notes" in to "foo"
-            let parts: Vec<&str> = line.splitn(2, ':').collect();
-            if parts.len() == 2 {
-                Some(parts[0])
-            } else {
-                None
-            }
-        })
-        .flat_map(|line| {
-            let pkgs: Vec<&str> = line.split(',').collect();
-            pkgs
-        })
-        .map(|line| line.trim().to_owned())
-        .collect()
-}
-
-#[cfg(test)]
-mod tests {
-
-    use super::*;
-
-    #[test]
-    fn test_parse_commit_messages() {
-        let expect: Vec<&str> = vec![
-            "firefox{-esr", // don't support such fancy syntax
-            "}",            // Don't support such fancy syntax
-            "firefox",
-            "buildkite-agent",
-            "python.pkgs.ptyprocess",
-            "python.pkgs.ptyprocess",
-            "android-studio-preview",
-            "foo",
-            "bar",
-        ];
-        assert_eq!(
-            parse_commit_messages(
-                &"
-              firefox{-esr,}: fix failing build due to the google-api-key
-              Merge pull request #34483 from andir/dovecot-cve-2017-15132
-              firefox: enable official branding
-              Merge pull request #34442 from rnhmjoj/virtual
-              buildkite-agent: enable building on darwin
-              python.pkgs.ptyprocess: 0.5 -> 0.5.2
-              python.pkgs.ptyprocess: move expression
-              Merge pull request #34465 from steveeJ/steveej-attempt-qtile-bump-0.10.7
-              android-studio-preview: 3.1.0.8 -> 3.1.0.9
-              Merge pull request #34188 from dotlambda/home-assistant
-              Merge pull request #34414 from dotlambda/postfix
-              foo,bar: something here: yeah
-            "
-                .lines()
-                .map(|l| l.to_owned())
-                .collect::<Vec<String>>(),
-            ),
-            expect
-        );
-    }
-}
-
 fn issue_is_wip(issue: &hubcaps::issues::Issue) -> bool {
     if issue.title.contains("[WIP]") {
         return true;