diff --git a/ofborg/src/tasks/eval/nixpkgs.rs b/ofborg/src/tasks/eval/nixpkgs.rs index 6b88423..0e6ee46 100644 --- a/ofborg/src/tasks/eval/nixpkgs.rs +++ b/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, outpath_diff: Option, changed_paths: Option>, + touched_packages: Option>, } 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> { + 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 = 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 { + 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::>(), + ), + expect + ); + } +} diff --git a/ofborg/src/tasks/evaluate.rs b/ofborg/src/tasks/evaluate.rs index 60fb8bf..500458b 100644 --- a/ofborg/src/tasks/evaluate.rs +++ b/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 { cloner: checkout::CachedCloner, @@ -247,11 +243,6 @@ impl worker::SimpleWorker for EvaluationWorker 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 worker::SimpleWorker for EvaluationWorker ), ]; - 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 worker::SimpleWorker for EvaluationWorker let mut response: worker::Actions = vec![]; if eval_results { - let mut status = CommitStatus::new( - repo.statuses(), - 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 state: hubcaps::statuses::State; - let gist_url: Option; - - 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 = 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()), - ); + let ret = evaluation_strategy + .all_evaluations_passed(&Path::new(&refpath), &mut overall_status); + match ret { + Ok(builds) => { + for buildjob in builds { 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) => { - eval_results = false; - state = hubcaps::statuses::State::Failure; - gist_url = make_gist( - &gists, - "Meta Check", - 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); - } + 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 { - 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::>(), - ), - expect - ); - } -} - fn issue_is_wip(issue: &hubcaps::issues::Issue) -> bool { if issue.title.contains("[WIP]") { return true;