From c8ae1497317a8511c1f9eede46ebd8630bfdbf7c Mon Sep 17 00:00:00 2001 From: Graham Christensen Date: Thu, 21 Mar 2019 18:13:36 -0400 Subject: [PATCH 01/20] Shell: use rust from mozilla --- shell.nix | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/shell.nix b/shell.nix index 37ed31b..f492c59 100644 --- a/shell.nix +++ b/shell.nix @@ -77,8 +77,7 @@ let buildInputs = with pkgs; [ bash nix-prefetch-git - rust.rustc - rust.cargo + latest.rustChannels.stable.rust #rustfmt #carnix openssl.dev From 9c83a06ab18595539d4ac84021bc6a5ba863066c Mon Sep 17 00:00:00 2001 From: Graham Christensen Date: Thu, 21 Mar 2019 18:09:18 -0400 Subject: [PATCH 02/20] Rename mass-rebuilder things to evaluation --- ofborg/src/bin/mass-rebuilder.rs | 2 +- .../{massrebuildjob.rs => evaluationjob.rs} | 12 ++++------ ofborg/src/message/mod.rs | 2 +- .../tasks/{massrebuilder.rs => evaluate.rs} | 24 +++++++++---------- ofborg/src/tasks/evaluationfilter.rs | 6 ++--- ofborg/src/tasks/githubcommentfilter.rs | 4 ++-- ofborg/src/tasks/mod.rs | 2 +- 7 files changed, 24 insertions(+), 28 deletions(-) rename ofborg/src/message/{massrebuildjob.rs => evaluationjob.rs} (51%) rename ofborg/src/tasks/{massrebuilder.rs => evaluate.rs} (97%) diff --git a/ofborg/src/bin/mass-rebuilder.rs b/ofborg/src/bin/mass-rebuilder.rs index 277cb26..c6322c8 100644 --- a/ofborg/src/bin/mass-rebuilder.rs +++ b/ofborg/src/bin/mass-rebuilder.rs @@ -47,7 +47,7 @@ fn main() { session.open_channel(3).unwrap(), ); - let mrw = tasks::massrebuilder::MassRebuildWorker::new( + let mrw = tasks::evaluate::EvaluationWorker::new( cloner, &nix, cfg.github(), diff --git a/ofborg/src/message/massrebuildjob.rs b/ofborg/src/message/evaluationjob.rs similarity index 51% rename from ofborg/src/message/massrebuildjob.rs rename to ofborg/src/message/evaluationjob.rs index a80c16b..b16a52d 100644 --- a/ofborg/src/message/massrebuildjob.rs +++ b/ofborg/src/message/evaluationjob.rs @@ -2,12 +2,12 @@ use ofborg::message::{Pr, Repo}; use ofborg::worker; use serde_json; -pub fn from(data: &[u8]) -> Result { +pub fn from(data: &[u8]) -> Result { serde_json::from_slice(&data) } #[derive(Serialize, Deserialize, Debug)] -pub struct MassRebuildJob { +pub struct EvaluationJob { pub repo: Repo, pub pr: Pr, } @@ -15,15 +15,11 @@ pub struct MassRebuildJob { pub struct Actions {} impl Actions { - pub fn skip(&mut self, _job: &MassRebuildJob) -> worker::Actions { + pub fn skip(&mut self, _job: &EvaluationJob) -> worker::Actions { vec![worker::Action::Ack] } - pub fn done( - &mut self, - _job: &MassRebuildJob, - mut response: worker::Actions, - ) -> worker::Actions { + pub fn done(&mut self, _job: &EvaluationJob, mut response: worker::Actions) -> worker::Actions { response.push(worker::Action::Ack); response } diff --git a/ofborg/src/message/mod.rs b/ofborg/src/message/mod.rs index fde351f..03551cd 100644 --- a/ofborg/src/message/mod.rs +++ b/ofborg/src/message/mod.rs @@ -2,6 +2,6 @@ pub mod buildjob; pub mod buildlogmsg; pub mod buildresult; mod common; -pub mod massrebuildjob; +pub mod evaluationjob; pub use self::common::{Pr, Repo}; diff --git a/ofborg/src/tasks/massrebuilder.rs b/ofborg/src/tasks/evaluate.rs similarity index 97% rename from ofborg/src/tasks/massrebuilder.rs rename to ofborg/src/tasks/evaluate.rs index bc592f1..3c2ac9e 100644 --- a/ofborg/src/tasks/massrebuilder.rs +++ b/ofborg/src/tasks/evaluate.rs @@ -1,4 +1,4 @@ -/// This is what evaluates every pull-requests +/// This is what evaluates every pull-request extern crate amqp; extern crate env_logger; extern crate uuid; @@ -14,7 +14,7 @@ use ofborg::commentparser::Subset; use ofborg::commitstatus::CommitStatus; use ofborg::evalchecker::EvalChecker; use ofborg::files::file_to_str; -use ofborg::message::{buildjob, massrebuildjob}; +use ofborg::message::{buildjob, evaluationjob}; use ofborg::nix; use ofborg::outpathdiff::{OutPathDiff, OutPaths}; use ofborg::stats; @@ -31,7 +31,7 @@ use std::time::Instant; use tasks::eval; use uuid::Uuid; -pub struct MassRebuildWorker { +pub struct EvaluationWorker { cloner: checkout::CachedCloner, nix: nix::Nix, github: hubcaps::Github, @@ -41,7 +41,7 @@ pub struct MassRebuildWorker { tag_paths: HashMap>, } -impl MassRebuildWorker { +impl EvaluationWorker { pub fn new( cloner: checkout::CachedCloner, nix: &nix::Nix, @@ -50,8 +50,8 @@ impl MassRebuildWorker { identity: String, events: E, tag_paths: HashMap>, - ) -> MassRebuildWorker { - MassRebuildWorker { + ) -> EvaluationWorker { + EvaluationWorker { cloner, nix: nix.without_limited_supported_systems(), github, @@ -62,8 +62,8 @@ impl MassRebuildWorker { } } - fn actions(&self) -> massrebuildjob::Actions { - massrebuildjob::Actions {} + fn actions(&self) -> evaluationjob::Actions { + evaluationjob::Actions {} } fn tag_from_title(&self, issue: &hubcaps::issues::IssueRef) { @@ -91,8 +91,8 @@ impl MassRebuildWorker { } } -impl worker::SimpleWorker for MassRebuildWorker { - type J = massrebuildjob::MassRebuildJob; +impl worker::SimpleWorker for EvaluationWorker { + type J = evaluationjob::EvaluationJob; fn msg_to_job( &mut self, @@ -101,7 +101,7 @@ impl worker::SimpleWorker for MassRebuildWorker Result { self.events.notify(Event::JobReceived); - match massrebuildjob::from(body) { + match evaluationjob::from(body) { Ok(e) => { self.events.notify(Event::JobDecodeSuccess); Ok(e) @@ -118,7 +118,7 @@ impl worker::SimpleWorker for MassRebuildWorker worker::Actions { + fn consumer(&mut self, job: &evaluationjob::EvaluationJob) -> worker::Actions { let repo = self .github .repo(job.repo.owner.clone(), job.repo.name.clone()); diff --git a/ofborg/src/tasks/evaluationfilter.rs b/ofborg/src/tasks/evaluationfilter.rs index 4d300fd..26e0b00 100644 --- a/ofborg/src/tasks/evaluationfilter.rs +++ b/ofborg/src/tasks/evaluationfilter.rs @@ -6,7 +6,7 @@ use ofborg::ghevent; use serde_json; use amqp::protocol::basic::{BasicProperties, Deliver}; -use ofborg::message::{massrebuildjob, Pr, Repo}; +use ofborg::message::{evaluationjob, Pr, Repo}; use ofborg::worker; pub struct EvaluationFilterWorker { @@ -92,7 +92,7 @@ impl worker::SimpleWorker for EvaluationFilterWorker { target_branch: Some(job.pull_request.base.git_ref.clone()), }; - let msg = massrebuildjob::MassRebuildJob { + let msg = evaluationjob::EvaluationJob { repo: repo_msg.clone(), pr: pr_msg.clone(), }; @@ -128,7 +128,7 @@ mod tests { worker::publish_serde_action( None, Some("mass-rebuild-check-jobs".to_owned()), - &massrebuildjob::MassRebuildJob { + &evaluationjob::EvaluationJob { repo: Repo { clone_url: String::from("https://github.com/NixOS/nixpkgs.git"), full_name: String::from("NixOS/nixpkgs"), diff --git a/ofborg/src/tasks/githubcommentfilter.rs b/ofborg/src/tasks/githubcommentfilter.rs index cf3e5fd..850ac2f 100644 --- a/ofborg/src/tasks/githubcommentfilter.rs +++ b/ofborg/src/tasks/githubcommentfilter.rs @@ -10,7 +10,7 @@ use uuid::Uuid; use amqp::protocol::basic::{BasicProperties, Deliver}; use hubcaps; use ofborg::commentparser; -use ofborg::message::{buildjob, massrebuildjob, Pr, Repo}; +use ofborg::message::{buildjob, evaluationjob, Pr, Repo}; use ofborg::worker; pub struct GitHubCommentWorker { @@ -147,7 +147,7 @@ impl worker::SimpleWorker for GitHubCommentWorker { )); } commentparser::Instruction::Eval => { - let msg = massrebuildjob::MassRebuildJob { + let msg = evaluationjob::EvaluationJob { repo: repo_msg.clone(), pr: pr_msg.clone(), }; diff --git a/ofborg/src/tasks/mod.rs b/ofborg/src/tasks/mod.rs index d7ed9fb..5aab0fa 100644 --- a/ofborg/src/tasks/mod.rs +++ b/ofborg/src/tasks/mod.rs @@ -1,8 +1,8 @@ pub mod build; pub mod eval; +pub mod evaluate; pub mod evaluationfilter; pub mod githubcommentfilter; pub mod githubcommentposter; pub mod log_message_collector; -pub mod massrebuilder; pub mod statscollector; From 379761c03642ba9dcd8d4f5f9a3aa91bfba6cf1b Mon Sep 17 00:00:00 2001 From: Graham Christensen Date: Fri, 22 Mar 2019 13:19:25 -0400 Subject: [PATCH 03/20] eval: initialize nixpkgs and generic evaluation strategies, based on a trait developed in a different iteration --- ofborg/src/tasks/eval/generic.rs | 45 ++++++++++++++++++++++++++++++++ ofborg/src/tasks/eval/mod.rs | 31 ++++++++++++++++++++++ ofborg/src/tasks/eval/nixpkgs.rs | 45 ++++++++++++++++++++++++++++++++ 3 files changed, 121 insertions(+) create mode 100644 ofborg/src/tasks/eval/generic.rs create mode 100644 ofborg/src/tasks/eval/nixpkgs.rs diff --git a/ofborg/src/tasks/eval/generic.rs b/ofborg/src/tasks/eval/generic.rs new file mode 100644 index 0000000..f8462ae --- /dev/null +++ b/ofborg/src/tasks/eval/generic.rs @@ -0,0 +1,45 @@ +use ofborg::checkout::CachedProjectCo; +use ofborg::commitstatus::CommitStatus; +use ofborg::evalchecker::EvalChecker; +use ofborg::message::buildjob::BuildJob; +use std::path::Path; +use tasks::eval::{EvaluationStrategy, StepResult}; + +pub struct GenericStrategy {} +impl GenericStrategy { + pub fn new() -> GenericStrategy { + Self {} + } +} + +impl EvaluationStrategy for GenericStrategy { + fn pre_clone(&mut self) -> StepResult<()> { + Ok(()) + } + + fn on_target_branch(&mut self, _co: &Path, _status: &mut CommitStatus) -> StepResult<()> { + Ok(()) + } + + fn after_fetch(&mut self, _co: &CachedProjectCo) -> StepResult<()> { + Ok(()) + } + + fn merge_conflict(&mut self) {} + + fn after_merge(&mut self, _status: &mut CommitStatus) -> StepResult<()> { + Ok(()) + } + + fn evaluation_checks(&self) -> Vec { + vec![] + } + + fn all_evaluations_passed( + &mut self, + _co: &Path, + _status: &mut CommitStatus, + ) -> StepResult> { + Ok(vec![]) + } +} diff --git a/ofborg/src/tasks/eval/mod.rs b/ofborg/src/tasks/eval/mod.rs index 35a344b..74b7b2d 100644 --- a/ofborg/src/tasks/eval/mod.rs +++ b/ofborg/src/tasks/eval/mod.rs @@ -1,2 +1,33 @@ pub mod stdenvs; pub use self::stdenvs::Stdenvs; +mod nixpkgs; +pub use self::nixpkgs::NixpkgsStrategy; +mod generic; +pub use self::generic::GenericStrategy; +use ofborg::checkout::CachedProjectCo; +use ofborg::commitstatus::CommitStatus; +use ofborg::evalchecker::EvalChecker; +use ofborg::message::buildjob::BuildJob; +use std::path::Path; + +pub trait EvaluationStrategy { + fn pre_clone(&mut self) -> StepResult<()>; + fn on_target_branch(&mut self, co: &Path, status: &mut CommitStatus) -> StepResult<()>; + fn after_fetch(&mut self, co: &CachedProjectCo) -> StepResult<()>; + fn merge_conflict(&mut self); + fn after_merge(&mut self, status: &mut CommitStatus) -> StepResult<()>; + fn evaluation_checks(&self) -> Vec; + fn all_evaluations_passed( + &mut self, + co: &Path, + status: &mut CommitStatus, + ) -> StepResult>; +} + +type StepResult = Result; + +#[derive(Debug)] +pub enum Error { + Fail(String), + FailWithGist(String, String, String), +} diff --git a/ofborg/src/tasks/eval/nixpkgs.rs b/ofborg/src/tasks/eval/nixpkgs.rs new file mode 100644 index 0000000..cf30b9d --- /dev/null +++ b/ofborg/src/tasks/eval/nixpkgs.rs @@ -0,0 +1,45 @@ +use ofborg::checkout::CachedProjectCo; +use ofborg::commitstatus::CommitStatus; +use ofborg::evalchecker::EvalChecker; +use ofborg::message::buildjob::BuildJob; +use std::path::Path; +use tasks::eval::{EvaluationStrategy, StepResult}; + +pub struct NixpkgsStrategy {} +impl NixpkgsStrategy { + pub fn new() -> NixpkgsStrategy { + Self {} + } +} + +impl EvaluationStrategy for NixpkgsStrategy { + fn pre_clone(&mut self) -> StepResult<()> { + Ok(()) + } + + fn on_target_branch(&mut self, _co: &Path, _status: &mut CommitStatus) -> StepResult<()> { + Ok(()) + } + + fn after_fetch(&mut self, _co: &CachedProjectCo) -> StepResult<()> { + Ok(()) + } + + fn merge_conflict(&mut self) {} + + fn after_merge(&mut self, _status: &mut CommitStatus) -> StepResult<()> { + Ok(()) + } + + fn evaluation_checks(&self) -> Vec { + vec![] + } + + fn all_evaluations_passed( + &mut self, + _co: &Path, + _status: &mut CommitStatus, + ) -> StepResult> { + Ok(vec![]) + } +} From 4665ea2bbcecdd81f46c54f1c91a53bbf83a660f Mon Sep 17 00:00:00 2001 From: Graham Christensen Date: Fri, 22 Mar 2019 13:25:22 -0400 Subject: [PATCH 04/20] Teach EvaluationJob about is_nixpkgs --- ofborg/src/message/evaluationjob.rs | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/ofborg/src/message/evaluationjob.rs b/ofborg/src/message/evaluationjob.rs index b16a52d..0042acd 100644 --- a/ofborg/src/message/evaluationjob.rs +++ b/ofborg/src/message/evaluationjob.rs @@ -12,6 +12,12 @@ pub struct EvaluationJob { pub pr: Pr, } +impl EvaluationJob { + pub fn is_nixpkgs(&self) -> bool { + self.repo.name == "nixpkgs" + } +} + pub struct Actions {} impl Actions { From 721276fbd1f88031baa9afff3b1c112856a8f2e5 Mon Sep 17 00:00:00 2001 From: Graham Christensen Date: Fri, 22 Mar 2019 13:25:51 -0400 Subject: [PATCH 05/20] Load the evaluation strategy in evaluation --- ofborg/src/tasks/evaluate.rs | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/ofborg/src/tasks/evaluate.rs b/ofborg/src/tasks/evaluate.rs index 3c2ac9e..db1a914 100644 --- a/ofborg/src/tasks/evaluate.rs +++ b/ofborg/src/tasks/evaluate.rs @@ -129,6 +129,12 @@ impl worker::SimpleWorker for EvaluationWorker let issue: Issue; let auto_schedule_build_archs: Vec; + let mut evaluation_strategy: Box = if job.is_nixpkgs() { + Box::new(eval::NixpkgsStrategy::new()) + } else { + Box::new(eval::GenericStrategy::new()) + }; + match issue_ref.get() { Ok(iss) => { if iss.state == "closed" { From d9d3ac5fcb0a3eae9f2598f4fb80d6b8e2f7c1fd Mon Sep 17 00:00:00 2001 From: Graham Christensen Date: Fri, 22 Mar 2019 13:32:34 -0400 Subject: [PATCH 06/20] nixpkgs: pass in issue_ref --- ofborg/src/tasks/eval/nixpkgs.rs | 13 ++++++++----- ofborg/src/tasks/evaluate.rs | 2 +- 2 files changed, 9 insertions(+), 6 deletions(-) diff --git a/ofborg/src/tasks/eval/nixpkgs.rs b/ofborg/src/tasks/eval/nixpkgs.rs index cf30b9d..27498c9 100644 --- a/ofborg/src/tasks/eval/nixpkgs.rs +++ b/ofborg/src/tasks/eval/nixpkgs.rs @@ -1,3 +1,4 @@ +use hubcaps::issues::IssueRef; use ofborg::checkout::CachedProjectCo; use ofborg::commitstatus::CommitStatus; use ofborg::evalchecker::EvalChecker; @@ -5,14 +6,16 @@ use ofborg::message::buildjob::BuildJob; use std::path::Path; use tasks::eval::{EvaluationStrategy, StepResult}; -pub struct NixpkgsStrategy {} -impl NixpkgsStrategy { - pub fn new() -> NixpkgsStrategy { - Self {} +pub struct NixpkgsStrategy<'a> { + issue_ref: &'a IssueRef<'a>, +} +impl<'a> NixpkgsStrategy<'a> { + pub fn new(issue_ref: &'a IssueRef) -> NixpkgsStrategy<'a> { + Self { issue_ref } } } -impl EvaluationStrategy for NixpkgsStrategy { +impl<'a> EvaluationStrategy for NixpkgsStrategy<'a> { fn pre_clone(&mut self) -> StepResult<()> { Ok(()) } diff --git a/ofborg/src/tasks/evaluate.rs b/ofborg/src/tasks/evaluate.rs index db1a914..311fd68 100644 --- a/ofborg/src/tasks/evaluate.rs +++ b/ofborg/src/tasks/evaluate.rs @@ -130,7 +130,7 @@ impl worker::SimpleWorker for EvaluationWorker let auto_schedule_build_archs: Vec; let mut evaluation_strategy: Box = if job.is_nixpkgs() { - Box::new(eval::NixpkgsStrategy::new()) + Box::new(eval::NixpkgsStrategy::new(&issue_ref)) } else { Box::new(eval::GenericStrategy::new()) }; From 3b4117e6eb4cbc545494cbe224eb8188773044c0 Mon Sep 17 00:00:00 2001 From: Graham Christensen Date: Fri, 22 Mar 2019 13:46:52 -0400 Subject: [PATCH 07/20] StepResult: make public --- ofborg/src/tasks/eval/mod.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/ofborg/src/tasks/eval/mod.rs b/ofborg/src/tasks/eval/mod.rs index 74b7b2d..3379fb6 100644 --- a/ofborg/src/tasks/eval/mod.rs +++ b/ofborg/src/tasks/eval/mod.rs @@ -24,7 +24,7 @@ pub trait EvaluationStrategy { ) -> StepResult>; } -type StepResult = Result; +pub type StepResult = Result; #[derive(Debug)] pub enum Error { From 795ace48df17805901d42b78f65c93b522e434ba Mon Sep 17 00:00:00 2001 From: Graham Christensen Date: Fri, 22 Mar 2019 13:48:48 -0400 Subject: [PATCH 08/20] evaluator: give a fn to handle strategy errors --- ofborg/src/tasks/evaluate.rs | 22 ++++++++++++++++++++++ 1 file changed, 22 insertions(+) diff --git a/ofborg/src/tasks/evaluate.rs b/ofborg/src/tasks/evaluate.rs index 311fd68..45670e9 100644 --- a/ofborg/src/tasks/evaluate.rs +++ b/ofborg/src/tasks/evaluate.rs @@ -7,6 +7,7 @@ use crate::maintainers; use crate::maintainers::ImpactedMaintainers; use amqp::protocol::basic::{BasicProperties, Deliver}; use hubcaps; +use hubcaps::gists::Gists; use hubcaps::issues::Issue; use ofborg::acl::ACL; use ofborg::checkout; @@ -29,6 +30,7 @@ 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 { @@ -89,6 +91,26 @@ impl EvaluationWorker { update_labels(&issue, &tagger.tags_to_add(), &tagger.tags_to_remove()); } + + fn handle_strategy_err( + &self, + ret: StepResult<()>, + gists: &Gists<'_>, + status: &mut CommitStatus, + ) -> Result<(), ()> { + match ret { + Ok(()) => Ok(()), + Err(eval::Error::Fail(msg)) => { + status.set_with_description(&msg, hubcaps::statuses::State::Failure); + Err(()) + } + Err(eval::Error::FailWithGist(msg, filename, content)) => { + status.set_with_description(&msg, hubcaps::statuses::State::Failure); + status.set_url(make_gist(&gists, &filename, Some("".to_owned()), content)); + Err(()) + } + } + } } impl worker::SimpleWorker for EvaluationWorker { From 297dd02f622bfe95a6616afdae377166edfdaa35 Mon Sep 17 00:00:00 2001 From: Graham Christensen Date: Fri, 22 Mar 2019 13:49:04 -0400 Subject: [PATCH 09/20] evaluator: move out title-based tagging --- ofborg/src/tasks/eval/nixpkgs.rs | 19 ++++++++++++++++++- ofborg/src/tasks/evaluate.rs | 23 +++++++---------------- 2 files changed, 25 insertions(+), 17 deletions(-) diff --git a/ofborg/src/tasks/eval/nixpkgs.rs b/ofborg/src/tasks/eval/nixpkgs.rs index 27498c9..7fad199 100644 --- a/ofborg/src/tasks/eval/nixpkgs.rs +++ b/ofborg/src/tasks/eval/nixpkgs.rs @@ -3,8 +3,9 @@ use ofborg::checkout::CachedProjectCo; use ofborg::commitstatus::CommitStatus; use ofborg::evalchecker::EvalChecker; use ofborg::message::buildjob::BuildJob; +use ofborg::tasks::eval::{EvaluationStrategy, StepResult}; +use ofborg::tasks::evaluate::update_labels; use std::path::Path; -use tasks::eval::{EvaluationStrategy, StepResult}; pub struct NixpkgsStrategy<'a> { issue_ref: &'a IssueRef<'a>, @@ -13,10 +14,26 @@ impl<'a> NixpkgsStrategy<'a> { pub fn new(issue_ref: &'a IssueRef) -> NixpkgsStrategy<'a> { Self { issue_ref } } + + fn tag_from_title(&self) { + let darwin = self + .issue_ref + .get() + .map(|iss| { + iss.title.to_lowercase().contains("darwin") + || iss.title.to_lowercase().contains("macos") + }) + .unwrap_or(false); + + if darwin { + update_labels(&self.issue_ref, &[String::from("6.topic: darwin")], &[]); + } + } } impl<'a> EvaluationStrategy for NixpkgsStrategy<'a> { fn pre_clone(&mut self) -> StepResult<()> { + self.tag_from_title(); Ok(()) } diff --git a/ofborg/src/tasks/evaluate.rs b/ofborg/src/tasks/evaluate.rs index 45670e9..6f70fe6 100644 --- a/ofborg/src/tasks/evaluate.rs +++ b/ofborg/src/tasks/evaluate.rs @@ -68,20 +68,6 @@ impl EvaluationWorker { evaluationjob::Actions {} } - fn tag_from_title(&self, issue: &hubcaps::issues::IssueRef) { - let darwin = issue - .get() - .map(|iss| { - iss.title.to_lowercase().contains("darwin") - || iss.title.to_lowercase().contains("macos") - }) - .unwrap_or(false); - - if darwin { - update_labels(&issue, &[String::from("6.topic: darwin")], &[]); - } - } - fn tag_from_paths(&self, issue: &hubcaps::issues::IssueRef, paths: &[String]) { let mut tagger = PathsTagger::new(self.tag_paths.clone()); @@ -185,8 +171,6 @@ impl worker::SimpleWorker for EvaluationWorker } }; - self.tag_from_title(&issue_ref); - let mut overall_status = CommitStatus::new( repo.statuses(), job.pr.head_sha.clone(), @@ -197,6 +181,13 @@ impl worker::SimpleWorker for EvaluationWorker overall_status.set_with_description("Starting", hubcaps::statuses::State::Pending); + if self + .handle_strategy_err(evaluation_strategy.pre_clone(), &gists, &mut overall_status) + .is_err() + { + return self.actions().skip(&job); + } + let project = self .cloner .project(&job.repo.full_name, job.repo.clone_url.clone()); From 697ce8635a174a2bc2c15f2e298e58913925733b Mon Sep 17 00:00:00 2001 From: Graham Christensen Date: Fri, 22 Mar 2019 13:52:01 -0400 Subject: [PATCH 10/20] execute on-target-branch hook --- ofborg/src/tasks/evaluate.rs | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/ofborg/src/tasks/evaluate.rs b/ofborg/src/tasks/evaluate.rs index 6f70fe6..76b01a3 100644 --- a/ofborg/src/tasks/evaluate.rs +++ b/ofborg/src/tasks/evaluate.rs @@ -211,6 +211,17 @@ impl worker::SimpleWorker for EvaluationWorker info!("Checking out target branch {}", &target_branch); let refpath = co.checkout_origin_ref(target_branch.as_ref()).unwrap(); + if self + .handle_strategy_err( + evaluation_strategy.on_target_branch(&Path::new(&refpath), &mut overall_status), + &gists, + &mut overall_status, + ) + .is_err() + { + return self.actions().skip(&job); + } + overall_status.set_with_description( "Checking original stdenvs", hubcaps::statuses::State::Pending, From 14ff7b1e6257f6697a3372df6ecf4ce6e6849ff0 Mon Sep 17 00:00:00 2001 From: Graham Christensen Date: Fri, 22 Mar 2019 14:03:19 -0400 Subject: [PATCH 11/20] pass nix to nixpkgs --- ofborg/src/tasks/eval/nixpkgs.rs | 6 ++++-- ofborg/src/tasks/evaluate.rs | 2 +- 2 files changed, 5 insertions(+), 3 deletions(-) diff --git a/ofborg/src/tasks/eval/nixpkgs.rs b/ofborg/src/tasks/eval/nixpkgs.rs index 7fad199..1170953 100644 --- a/ofborg/src/tasks/eval/nixpkgs.rs +++ b/ofborg/src/tasks/eval/nixpkgs.rs @@ -3,16 +3,18 @@ use ofborg::checkout::CachedProjectCo; use ofborg::commitstatus::CommitStatus; use ofborg::evalchecker::EvalChecker; use ofborg::message::buildjob::BuildJob; +use ofborg::nix::Nix; use ofborg::tasks::eval::{EvaluationStrategy, StepResult}; use ofborg::tasks::evaluate::update_labels; use std::path::Path; pub struct NixpkgsStrategy<'a> { issue_ref: &'a IssueRef<'a>, + nix: Nix, } impl<'a> NixpkgsStrategy<'a> { - pub fn new(issue_ref: &'a IssueRef) -> NixpkgsStrategy<'a> { - Self { issue_ref } + pub fn new(issue_ref: &'a IssueRef, nix: Nix) -> NixpkgsStrategy<'a> { + Self { issue_ref, nix } } fn tag_from_title(&self) { diff --git a/ofborg/src/tasks/evaluate.rs b/ofborg/src/tasks/evaluate.rs index 76b01a3..77378d9 100644 --- a/ofborg/src/tasks/evaluate.rs +++ b/ofborg/src/tasks/evaluate.rs @@ -138,7 +138,7 @@ impl worker::SimpleWorker for EvaluationWorker let auto_schedule_build_archs: Vec; let mut evaluation_strategy: Box = if job.is_nixpkgs() { - Box::new(eval::NixpkgsStrategy::new(&issue_ref)) + Box::new(eval::NixpkgsStrategy::new(&issue_ref, self.nix.clone())) } else { Box::new(eval::GenericStrategy::new()) }; From 22fc680357155f92d5580697f3040cff89b4c237 Mon Sep 17 00:00:00 2001 From: Graham Christensen Date: Fri, 22 Mar 2019 14:06:57 -0400 Subject: [PATCH 12/20] call after-merge hook --- ofborg/src/tasks/evaluate.rs | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/ofborg/src/tasks/evaluate.rs b/ofborg/src/tasks/evaluate.rs index 77378d9..c8ed1f3 100644 --- a/ofborg/src/tasks/evaluate.rs +++ b/ofborg/src/tasks/evaluate.rs @@ -300,6 +300,17 @@ impl worker::SimpleWorker for EvaluationWorker update_labels(&issue_ref, &[], &["2.status: merge conflict".to_owned()]); } + if self + .handle_strategy_err( + evaluation_strategy.after_merge(&mut overall_status), + &gists, + &mut overall_status, + ) + .is_err() + { + return self.actions().skip(&job); + } + overall_status .set_with_description("Checking new stdenvs", hubcaps::statuses::State::Pending); From 2141f7352bd1cf00b0c6c25af148a387029663a0 Mon Sep 17 00:00:00 2001 From: Graham Christensen Date: Fri, 22 Mar 2019 14:19:47 -0400 Subject: [PATCH 13/20] Call all_evaluations_passed hook --- ofborg/src/tasks/evaluate.rs | 20 ++++++++++++++++++++ 1 file changed, 20 insertions(+) diff --git a/ofborg/src/tasks/evaluate.rs b/ofborg/src/tasks/evaluate.rs index c8ed1f3..91ddb7d 100644 --- a/ofborg/src/tasks/evaluate.rs +++ b/ofborg/src/tasks/evaluate.rs @@ -642,6 +642,26 @@ impl worker::SimpleWorker for EvaluationWorker &rebuild_tags.tags_to_remove(), ); + { + 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); + } + } + } + } + overall_status.set_with_description("^.^!", hubcaps::statuses::State::Success); } else { overall_status From f0b342e4d6778b5ed9c10a72c6978e9323b197c8 Mon Sep 17 00:00:00 2001 From: Graham Christensen Date: Fri, 22 Mar 2019 14:20:07 -0400 Subject: [PATCH 14/20] Pull out stdenv tagging --- ofborg/src/tasks/eval/nixpkgs.rs | 51 +++++++++++++++++++++++++++++--- ofborg/src/tasks/evaluate.rs | 27 +---------------- 2 files changed, 48 insertions(+), 30 deletions(-) diff --git a/ofborg/src/tasks/eval/nixpkgs.rs b/ofborg/src/tasks/eval/nixpkgs.rs index 1170953..7e3b23c 100644 --- a/ofborg/src/tasks/eval/nixpkgs.rs +++ b/ofborg/src/tasks/eval/nixpkgs.rs @@ -4,17 +4,23 @@ use ofborg::commitstatus::CommitStatus; use ofborg::evalchecker::EvalChecker; use ofborg::message::buildjob::BuildJob; use ofborg::nix::Nix; -use ofborg::tasks::eval::{EvaluationStrategy, StepResult}; +use ofborg::tagger::StdenvTagger; +use ofborg::tasks::eval::{stdenvs::Stdenvs, EvaluationStrategy, StepResult}; use ofborg::tasks::evaluate::update_labels; use std::path::Path; pub struct NixpkgsStrategy<'a> { issue_ref: &'a IssueRef<'a>, nix: Nix, + stdenv_diff: Option, } impl<'a> NixpkgsStrategy<'a> { pub fn new(issue_ref: &'a IssueRef, nix: Nix) -> NixpkgsStrategy<'a> { - Self { issue_ref, nix } + Self { + issue_ref, + nix, + stdenv_diff: None, + } } fn tag_from_title(&self) { @@ -31,6 +37,32 @@ impl<'a> NixpkgsStrategy<'a> { update_labels(&self.issue_ref, &[String::from("6.topic: darwin")], &[]); } } + + fn check_stdenvs_before(&mut self, dir: &Path) { + let mut stdenvs = Stdenvs::new(self.nix.clone(), dir.to_path_buf()); + stdenvs.identify_before(); + self.stdenv_diff = Some(stdenvs); + } + + fn check_stdenvs_after(&mut self) { + if let Some(ref mut stdenvs) = self.stdenv_diff { + stdenvs.identify_after(); + } + } + + fn update_stdenv_labels(&self) { + if let Some(ref stdenvs) = self.stdenv_diff { + let mut stdenvtagger = StdenvTagger::new(); + if !stdenvs.are_same() { + stdenvtagger.changed(stdenvs.changed()); + } + update_labels( + &self.issue_ref, + &stdenvtagger.tags_to_add(), + &stdenvtagger.tags_to_remove(), + ); + } + } } impl<'a> EvaluationStrategy for NixpkgsStrategy<'a> { @@ -39,7 +71,13 @@ impl<'a> EvaluationStrategy for NixpkgsStrategy<'a> { Ok(()) } - fn on_target_branch(&mut self, _co: &Path, _status: &mut CommitStatus) -> StepResult<()> { + fn on_target_branch(&mut self, dir: &Path, status: &mut CommitStatus) -> StepResult<()> { + status.set_with_description( + "Checking original stdenvs", + hubcaps::statuses::State::Pending, + ); + self.check_stdenvs_before(dir); + Ok(()) } @@ -49,7 +87,10 @@ impl<'a> EvaluationStrategy for NixpkgsStrategy<'a> { fn merge_conflict(&mut self) {} - fn after_merge(&mut self, _status: &mut CommitStatus) -> StepResult<()> { + fn after_merge(&mut self, status: &mut CommitStatus) -> StepResult<()> { + status.set_with_description("Checking new stdenvs", hubcaps::statuses::State::Pending); + self.check_stdenvs_after(); + Ok(()) } @@ -62,6 +103,8 @@ impl<'a> EvaluationStrategy for NixpkgsStrategy<'a> { _co: &Path, _status: &mut CommitStatus, ) -> StepResult> { + self.update_stdenv_labels(); + Ok(vec![]) } } diff --git a/ofborg/src/tasks/evaluate.rs b/ofborg/src/tasks/evaluate.rs index 91ddb7d..da2c151 100644 --- a/ofborg/src/tasks/evaluate.rs +++ b/ofborg/src/tasks/evaluate.rs @@ -21,9 +21,7 @@ use ofborg::outpathdiff::{OutPathDiff, OutPaths}; use ofborg::stats; use ofborg::stats::Event; use ofborg::systems; -use ofborg::tagger::{ - MaintainerPRTagger, PathsTagger, PkgsAddedRemovedTagger, RebuildTagger, StdenvTagger, -}; +use ofborg::tagger::{MaintainerPRTagger, PathsTagger, PkgsAddedRemovedTagger, RebuildTagger}; use ofborg::worker; use std::collections::HashMap; use std::path::Path; @@ -222,14 +220,6 @@ impl worker::SimpleWorker for EvaluationWorker return self.actions().skip(&job); } - overall_status.set_with_description( - "Checking original stdenvs", - hubcaps::statuses::State::Pending, - ); - - let mut stdenvs = eval::Stdenvs::new(self.nix.clone(), PathBuf::from(&refpath)); - stdenvs.identify_before(); - let mut rebuildsniff = OutPathDiff::new(self.nix.clone(), PathBuf::from(&refpath)); overall_status.set_with_description( @@ -311,11 +301,6 @@ impl worker::SimpleWorker for EvaluationWorker return self.actions().skip(&job); } - overall_status - .set_with_description("Checking new stdenvs", hubcaps::statuses::State::Pending); - - stdenvs.identify_after(); - overall_status .set_with_description("Checking new out paths", hubcaps::statuses::State::Pending); @@ -552,16 +537,6 @@ impl worker::SimpleWorker for EvaluationWorker hubcaps::statuses::State::Pending, ); - let mut stdenvtagger = StdenvTagger::new(); - if !stdenvs.are_same() { - stdenvtagger.changed(stdenvs.changed()); - } - update_labels( - &issue_ref, - &stdenvtagger.tags_to_add(), - &stdenvtagger.tags_to_remove(), - ); - if let Some((removed, added)) = rebuildsniff.package_diff() { let mut addremovetagger = PkgsAddedRemovedTagger::new(); addremovetagger.changed(&removed, &added); From f212c67945eafdf929eb4dd7113859ca1c0e2b57 Mon Sep 17 00:00:00 2001 From: Graham Christensen Date: Fri, 22 Mar 2019 15:24:45 -0400 Subject: [PATCH 15/20] Move output path based calculation --- ofborg/src/outpathdiff.rs | 6 +- ofborg/src/tagger.rs | 2 +- ofborg/src/tasks/eval/nixpkgs.rs | 237 ++++++++++++++++++++++++++++++- ofborg/src/tasks/evaluate.rs | 201 ++++---------------------- 4 files changed, 262 insertions(+), 184 deletions(-) diff --git a/ofborg/src/outpathdiff.rs b/ofborg/src/outpathdiff.rs index 8ec8703..dd0efa7 100644 --- a/ofborg/src/outpathdiff.rs +++ b/ofborg/src/outpathdiff.rs @@ -81,11 +81,11 @@ impl OutPathDiff { } } - pub fn calculate_rebuild(self) -> Option> { + pub fn calculate_rebuild(&self) -> Option> { let mut rebuild: Vec = vec![]; - if let Some(cur) = self.current { - if let Some(orig) = self.original { + if let Some(ref cur) = self.current { + if let Some(ref orig) = self.original { for key in cur.keys() { trace!("Checking out {:?}", key); if cur.get(key) != orig.get(key) { diff --git a/ofborg/src/tagger.rs b/ofborg/src/tagger.rs index 11535d1..cbb5b72 100644 --- a/ofborg/src/tagger.rs +++ b/ofborg/src/tagger.rs @@ -148,7 +148,7 @@ impl RebuildTagger { Default::default() } - pub fn parse_attrs(&mut self, attrs: Vec) { + pub fn parse_attrs(&mut self, attrs: &Vec) { let mut counter_darwin = 0; let mut counter_linux = 0; diff --git a/ofborg/src/tasks/eval/nixpkgs.rs b/ofborg/src/tasks/eval/nixpkgs.rs index 7e3b23c..6b88423 100644 --- a/ofborg/src/tasks/eval/nixpkgs.rs +++ b/ofborg/src/tasks/eval/nixpkgs.rs @@ -1,25 +1,60 @@ -use hubcaps::issues::IssueRef; +use crate::maintainers; +use crate::maintainers::ImpactedMaintainers; +use hubcaps::gists::Gists; +use hubcaps::issues::{Issue, IssueRef}; +use hubcaps::repositories::Repository; use ofborg::checkout::CachedProjectCo; 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::tagger::StdenvTagger; -use ofborg::tasks::eval::{stdenvs::Stdenvs, EvaluationStrategy, StepResult}; +use ofborg::outpathdiff::{OutPathDiff, PackageArch}; +use ofborg::tagger::{MaintainerPRTagger, PathsTagger, RebuildTagger}; +use ofborg::tagger::{PkgsAddedRemovedTagger, StdenvTagger}; +use ofborg::tasks::eval::{stdenvs::Stdenvs, Error, EvaluationStrategy, StepResult}; use ofborg::tasks::evaluate::update_labels; +use std::collections::HashMap; use std::path::Path; +use tasks::evaluate::make_gist; pub struct NixpkgsStrategy<'a> { + job: &'a EvaluationJob, + pull: &'a hubcaps::pulls::PullRequest<'a>, + issue: &'a Issue, issue_ref: &'a IssueRef<'a>, + repo: &'a Repository<'a>, + gists: &'a Gists<'a>, nix: Nix, + tag_paths: &'a HashMap>, stdenv_diff: Option, + outpath_diff: Option, + changed_paths: Option>, } impl<'a> NixpkgsStrategy<'a> { - pub fn new(issue_ref: &'a IssueRef, nix: Nix) -> NixpkgsStrategy<'a> { + pub fn new( + job: &'a EvaluationJob, + pull: &'a hubcaps::pulls::PullRequest, + issue: &'a Issue, + issue_ref: &'a IssueRef, + repo: &'a Repository, + gists: &'a Gists, + nix: Nix, + tag_paths: &'a HashMap>, + ) -> NixpkgsStrategy<'a> { Self { + job, + pull, + issue, issue_ref, + repo, + gists, nix, + tag_paths, stdenv_diff: None, + outpath_diff: None, + changed_paths: None, } } @@ -38,6 +73,22 @@ impl<'a> NixpkgsStrategy<'a> { } } + fn tag_from_paths(&self) { + if let Some(ref changed_paths) = self.changed_paths { + let mut tagger = PathsTagger::new(self.tag_paths.clone()); + + for path in changed_paths { + tagger.path_changed(&path); + } + + update_labels( + &self.issue_ref, + &tagger.tags_to_add(), + &tagger.tags_to_remove(), + ); + } + } + fn check_stdenvs_before(&mut self, dir: &Path) { let mut stdenvs = Stdenvs::new(self.nix.clone(), dir.to_path_buf()); stdenvs.identify_before(); @@ -63,6 +114,139 @@ impl<'a> NixpkgsStrategy<'a> { ); } } + + fn check_outpaths_before(&mut self, dir: &Path) -> StepResult<()> { + let mut rebuildsniff = OutPathDiff::new(self.nix.clone(), dir.to_path_buf()); + + if let Err(mut output) = rebuildsniff.find_before() { + /* + self.events + .notify(Event::TargetBranchFailsEvaluation(target_branch.clone())); + */ + + Err(Error::FailWithGist( + String::from("The branch this PR will merge in to does not evaluate, and so this PR cannot be checked."), + String::from("Output path comparison"), + file_to_str(&mut output), + )) + } else { + self.outpath_diff = Some(rebuildsniff); + Ok(()) + } + } + + fn check_outpaths_after(&mut self) -> StepResult<()> { + if let Some(ref mut rebuildsniff) = self.outpath_diff { + if let Err(mut output) = rebuildsniff.find_after() { + Err(Error::FailWithGist( + String::from("This PR breaks listing of package outputs after merging."), + String::from("Output path comparison"), + file_to_str(&mut output), + )) + } else { + Ok(()) + } + } else { + Err(Error::Fail(String::from( + "Ofborg BUG: No outpath diff! Please report!", + ))) + } + } + + fn update_new_package_labels(&self) { + if let Some(ref rebuildsniff) = self.outpath_diff { + if let Some((removed, added)) = rebuildsniff.package_diff() { + let mut addremovetagger = PkgsAddedRemovedTagger::new(); + addremovetagger.changed(&removed, &added); + update_labels( + &self.issue_ref, + &addremovetagger.tags_to_add(), + &addremovetagger.tags_to_remove(), + ); + } + } + } + + fn update_rebuild_labels(&self, dir: &Path, overall_status: &mut CommitStatus) { + if let Some(ref rebuildsniff) = self.outpath_diff { + let mut rebuild_tags = RebuildTagger::new(); + + if let Some(attrs) = rebuildsniff.calculate_rebuild() { + if !attrs.is_empty() { + overall_status.set_url(self.gist_changed_paths(&attrs)); + self.record_impacted_maintainers(&dir, &attrs); + } + + rebuild_tags.parse_attrs(&attrs); + } + + update_labels( + &self.issue_ref, + &rebuild_tags.tags_to_add(), + &rebuild_tags.tags_to_remove(), + ); + } + } + + fn gist_changed_paths(&self, attrs: &Vec) -> Option { + make_gist( + &self.gists, + "Changed Paths", + Some("".to_owned()), + attrs + .iter() + .map(|attr| format!("{}\t{}", &attr.architecture, &attr.package)) + .collect::>() + .join("\n"), + ) + } + + fn record_impacted_maintainers(&self, dir: &Path, attrs: &Vec) -> () { + let changed_attributes = attrs + .iter() + .map(|attr| attr.package.split('.').collect::>()) + .collect::>>(); + + if let Some(ref changed_paths) = self.changed_paths { + let m = ImpactedMaintainers::calculate( + &self.nix, + &dir.to_path_buf(), + &changed_paths, + &changed_attributes, + ); + + let gist_url = make_gist( + &self.gists, + "Potential Maintainers", + Some("".to_owned()), + match m { + Ok(ref maintainers) => format!("Maintainers:\n{}", maintainers), + Err(ref e) => format!("Ignorable calculation error:\n{:?}", e), + }, + ); + + let mut status = CommitStatus::new( + self.repo.statuses(), + self.job.pr.head_sha.clone(), + String::from("grahamcofborg-eval-check-maintainers"), + String::from("matching changed paths to changed attrs..."), + gist_url, + ); + status.set(hubcaps::statuses::State::Success); + + if let Ok(ref maint) = m { + request_reviews(&maint, &self.pull); + let mut maint_tagger = MaintainerPRTagger::new(); + maint_tagger + .record_maintainer(&self.issue.user.login, &maint.maintainers_by_package()); + update_labels( + &self.issue_ref, + &maint_tagger.tags_to_add(), + &maint_tagger.tags_to_remove(), + ); + } + } + } } impl<'a> EvaluationStrategy for NixpkgsStrategy<'a> { @@ -78,10 +262,22 @@ impl<'a> EvaluationStrategy for NixpkgsStrategy<'a> { ); self.check_stdenvs_before(dir); + status.set_with_description( + "Checking original out paths", + hubcaps::statuses::State::Pending, + ); + self.check_outpaths_before(dir)?; + Ok(()) } - fn after_fetch(&mut self, _co: &CachedProjectCo) -> StepResult<()> { + fn after_fetch(&mut self, co: &CachedProjectCo) -> StepResult<()> { + let changed_paths = co + .files_changed_from_head(&self.job.pr.head_sha) + .unwrap_or_else(|_| vec![]); + self.changed_paths = Some(changed_paths); + self.tag_from_paths(); + Ok(()) } @@ -91,6 +287,9 @@ impl<'a> EvaluationStrategy for NixpkgsStrategy<'a> { status.set_with_description("Checking new stdenvs", hubcaps::statuses::State::Pending); self.check_stdenvs_after(); + status.set_with_description("Checking new out paths", hubcaps::statuses::State::Pending); + self.check_outpaths_after()?; + Ok(()) } @@ -100,11 +299,35 @@ impl<'a> EvaluationStrategy for NixpkgsStrategy<'a> { fn all_evaluations_passed( &mut self, - _co: &Path, - _status: &mut CommitStatus, + dir: &Path, + status: &mut CommitStatus, ) -> StepResult> { self.update_stdenv_labels(); + status.set_with_description( + "Calculating Changed Outputs", + hubcaps::statuses::State::Pending, + ); + + self.update_new_package_labels(); + self.update_rebuild_labels(&dir, status); + Ok(vec![]) } } + +fn request_reviews(maint: &maintainers::ImpactedMaintainers, pull: &hubcaps::pulls::PullRequest) { + if maint.maintainers().len() < 10 { + for maintainer in maint.maintainers() { + if let Err(e) = + pull.review_requests() + .create(&hubcaps::review_requests::ReviewRequestOptions { + reviewers: vec![maintainer.clone()], + team_reviewers: vec![], + }) + { + println!("Failure requesting a review from {}: {:#?}", maintainer, e,); + } + } + } +} diff --git a/ofborg/src/tasks/evaluate.rs b/ofborg/src/tasks/evaluate.rs index da2c151..60fb8bf 100644 --- a/ofborg/src/tasks/evaluate.rs +++ b/ofborg/src/tasks/evaluate.rs @@ -2,9 +2,6 @@ extern crate amqp; extern crate env_logger; extern crate uuid; - -use crate::maintainers; -use crate::maintainers::ImpactedMaintainers; use amqp::protocol::basic::{BasicProperties, Deliver}; use hubcaps; use hubcaps::gists::Gists; @@ -17,11 +14,10 @@ use ofborg::evalchecker::EvalChecker; use ofborg::files::file_to_str; use ofborg::message::{buildjob, evaluationjob}; use ofborg::nix; -use ofborg::outpathdiff::{OutPathDiff, OutPaths}; +use ofborg::outpathdiff::OutPaths; use ofborg::stats; use ofborg::stats::Event; use ofborg::systems; -use ofborg::tagger::{MaintainerPRTagger, PathsTagger, PkgsAddedRemovedTagger, RebuildTagger}; use ofborg::worker; use std::collections::HashMap; use std::path::Path; @@ -66,16 +62,6 @@ impl EvaluationWorker { evaluationjob::Actions {} } - fn tag_from_paths(&self, issue: &hubcaps::issues::IssueRef, paths: &[String]) { - let mut tagger = PathsTagger::new(self.tag_paths.clone()); - - for path in paths { - tagger.path_changed(&path); - } - - update_labels(&issue, &tagger.tags_to_add(), &tagger.tags_to_remove()); - } - fn handle_strategy_err( &self, ret: StepResult<()>, @@ -135,12 +121,6 @@ impl worker::SimpleWorker for EvaluationWorker let issue: Issue; let auto_schedule_build_archs: Vec; - let mut evaluation_strategy: Box = if job.is_nixpkgs() { - Box::new(eval::NixpkgsStrategy::new(&issue_ref, self.nix.clone())) - } else { - Box::new(eval::GenericStrategy::new()) - }; - match issue_ref.get() { Ok(iss) => { if iss.state == "closed" { @@ -169,6 +149,21 @@ impl worker::SimpleWorker for EvaluationWorker } }; + let mut evaluation_strategy: Box = if job.is_nixpkgs() { + Box::new(eval::NixpkgsStrategy::new( + &job, + &pull, + &issue, + &issue_ref, + &repo, + &gists, + self.nix.clone(), + &self.tag_paths, + )) + } else { + Box::new(eval::GenericStrategy::new()) + }; + let mut overall_status = CommitStatus::new( repo.statuses(), job.pr.head_sha.clone(), @@ -220,32 +215,8 @@ impl worker::SimpleWorker for EvaluationWorker return self.actions().skip(&job); } - let mut rebuildsniff = OutPathDiff::new(self.nix.clone(), PathBuf::from(&refpath)); - - overall_status.set_with_description( - "Checking original out paths", - hubcaps::statuses::State::Pending, - ); - let target_branch_rebuild_sniff_start = Instant::now(); - if let Err(mut output) = rebuildsniff.find_before() { - overall_status.set_url(make_gist( - &gists, - "Output path comparison", - Some("".to_owned()), - file_to_str(&mut output), - )); - - self.events - .notify(Event::TargetBranchFailsEvaluation(target_branch.clone())); - overall_status.set_with_description( - format!("Target branch {} doesn't evaluate!", &target_branch).as_ref(), - hubcaps::statuses::State::Failure, - ); - - return self.actions().skip(&job); - } self.events.notify(Event::EvaluationDuration( target_branch.clone(), target_branch_rebuild_sniff_start.elapsed().as_secs(), @@ -265,16 +236,22 @@ impl worker::SimpleWorker for EvaluationWorker return self.actions().skip(&job); } + if self + .handle_strategy_err( + evaluation_strategy.after_fetch(&co), + &gists, + &mut overall_status, + ) + .is_err() + { + 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()]), ); - let changed_paths = co - .files_changed_from_head(&job.pr.head_sha) - .unwrap_or_else(|_| vec![]); - self.tag_from_paths(&issue_ref, &changed_paths); - overall_status.set_with_description("Merging PR", hubcaps::statuses::State::Pending); if co.merge_commit(job.pr.head_sha.as_ref()).is_err() { @@ -301,27 +278,6 @@ impl worker::SimpleWorker for EvaluationWorker return self.actions().skip(&job); } - overall_status - .set_with_description("Checking new out paths", hubcaps::statuses::State::Pending); - - if let Err(mut output) = rebuildsniff.find_after() { - overall_status.set_url(make_gist( - &gists, - "Output path comparison", - Some("".to_owned()), - file_to_str(&mut output), - )); - overall_status.set_with_description( - format!( - "Failed to enumerate outputs after merging to {}", - &target_branch - ) - .as_ref(), - hubcaps::statuses::State::Failure, - ); - return self.actions().skip(&job); - } - println!("Got path: {:?}, building", refpath); overall_status .set_with_description("Beginning Evaluations", hubcaps::statuses::State::Pending); @@ -532,91 +488,6 @@ impl worker::SimpleWorker for EvaluationWorker } if eval_results { - overall_status.set_with_description( - "Calculating Changed Outputs", - hubcaps::statuses::State::Pending, - ); - - if let Some((removed, added)) = rebuildsniff.package_diff() { - let mut addremovetagger = PkgsAddedRemovedTagger::new(); - addremovetagger.changed(&removed, &added); - update_labels( - &issue_ref, - &addremovetagger.tags_to_add(), - &addremovetagger.tags_to_remove(), - ); - } - - let mut rebuild_tags = RebuildTagger::new(); - if let Some(attrs) = rebuildsniff.calculate_rebuild() { - if !attrs.is_empty() { - let gist_url = make_gist( - &gists, - "Changed Paths", - Some("".to_owned()), - attrs - .iter() - .map(|attr| format!("{}\t{}", &attr.architecture, &attr.package)) - .collect::>() - .join("\n"), - ); - - overall_status.set_url(gist_url); - - let changed_attributes = attrs - .iter() - .map(|attr| attr.package.split('.').collect::>()) - .collect::>>(); - - let m = ImpactedMaintainers::calculate( - &self.nix, - &PathBuf::from(&refpath), - &changed_paths, - &changed_attributes, - ); - - let gist_url = make_gist( - &gists, - "Potential Maintainers", - Some("".to_owned()), - match m { - Ok(ref maintainers) => format!("Maintainers:\n{}", maintainers), - Err(ref e) => format!("Ignorable calculation error:\n{:?}", e), - }, - ); - - if let Ok(ref maint) = m { - request_reviews(&maint, &pull); - let mut maint_tagger = MaintainerPRTagger::new(); - maint_tagger - .record_maintainer(&issue.user.login, &maint.maintainers_by_package()); - update_labels( - &issue_ref, - &maint_tagger.tags_to_add(), - &maint_tagger.tags_to_remove(), - ); - } - - let mut status = CommitStatus::new( - repo.statuses(), - job.pr.head_sha.clone(), - String::from("grahamcofborg-eval-check-maintainers"), - String::from("matching changed paths to changed attrs..."), - gist_url, - ); - - status.set(hubcaps::statuses::State::Success); - } - - rebuild_tags.parse_attrs(attrs); - } - - update_labels( - &issue_ref, - &rebuild_tags.tags_to_add(), - &rebuild_tags.tags_to_remove(), - ); - { let ret = evaluation_strategy .all_evaluations_passed(&Path::new(&refpath), &mut overall_status); @@ -649,7 +520,7 @@ impl worker::SimpleWorker for EvaluationWorker } } -fn make_gist<'a>( +pub fn make_gist<'a>( gists: &hubcaps::gists::Gists<'a>, name: &str, description: Option, @@ -796,19 +667,3 @@ fn indicates_wip(text: &str) -> bool { false } - -fn request_reviews(maint: &maintainers::ImpactedMaintainers, pull: &hubcaps::pulls::PullRequest) { - if maint.maintainers().len() < 10 { - for maintainer in maint.maintainers() { - if let Err(e) = - pull.review_requests() - .create(&hubcaps::review_requests::ReviewRequestOptions { - reviewers: vec![maintainer.clone()], - team_reviewers: vec![], - }) - { - println!("Failure requesting a review from {}: {:#?}", maintainer, e,); - } - } - } -} From d25f91b88e7c935e75ebde358c73961622f49973 Mon Sep 17 00:00:00 2001 From: Graham Christensen Date: Fri, 22 Mar 2019 15:55:58 -0400 Subject: [PATCH 16/20] Move meta checks and build triggering --- ofborg/src/tasks/eval/nixpkgs.rs | 137 ++++++++++++++++++++++++- ofborg/src/tasks/evaluate.rs | 167 ++++--------------------------- 2 files changed, 152 insertions(+), 152 deletions(-) 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; From ed0882a975c3a12b900a48f324fe489a4d9e323d Mon Sep 17 00:00:00 2001 From: Graham Christensen Date: Fri, 22 Mar 2019 15:59:03 -0400 Subject: [PATCH 17/20] evaluation checks: move over --- ofborg/src/tasks/eval/nixpkgs.rs | 87 +++++++++++++++++++++++++++++- ofborg/src/tasks/evaluate.rs | 90 +------------------------------- 2 files changed, 88 insertions(+), 89 deletions(-) diff --git a/ofborg/src/tasks/eval/nixpkgs.rs b/ofborg/src/tasks/eval/nixpkgs.rs index 0e6ee46..7af97a9 100644 --- a/ofborg/src/tasks/eval/nixpkgs.rs +++ b/ofborg/src/tasks/eval/nixpkgs.rs @@ -10,6 +10,7 @@ use ofborg::evalchecker::EvalChecker; use ofborg::files::file_to_str; use ofborg::message::buildjob::BuildJob; use ofborg::message::evaluationjob::EvaluationJob; +use ofborg::nix; use ofborg::nix::Nix; use ofborg::outpathdiff::{OutPathDiff, OutPaths, PackageArch}; use ofborg::tagger::{MaintainerPRTagger, PathsTagger, RebuildTagger}; @@ -364,7 +365,91 @@ impl<'a> EvaluationStrategy for NixpkgsStrategy<'a> { } fn evaluation_checks(&self) -> Vec { - vec![] + vec![ + EvalChecker::new( + "package-list", + nix::Operation::QueryPackagesJSON, + vec![String::from("--file"), String::from(".")], + self.nix.clone(), + ), + EvalChecker::new( + "package-list-no-aliases", + nix::Operation::QueryPackagesJSON, + vec![ + String::from("--file"), + String::from("."), + String::from("--arg"), + String::from("config"), + String::from("{ allowAliases = false; }"), + ], + self.nix.clone(), + ), + EvalChecker::new( + "nixos-options", + nix::Operation::Instantiate, + vec![ + String::from("--arg"), + String::from("nixpkgs"), + String::from("{ outPath=./.; revCount=999999; shortRev=\"ofborg\"; }"), + String::from("./nixos/release.nix"), + String::from("-A"), + String::from("options"), + ], + self.nix.clone(), + ), + EvalChecker::new( + "nixos-manual", + nix::Operation::Instantiate, + vec![ + String::from("--arg"), + String::from("nixpkgs"), + String::from("{ outPath=./.; revCount=999999; shortRev=\"ofborg\"; }"), + String::from("./nixos/release.nix"), + String::from("-A"), + String::from("manual"), + ], + self.nix.clone(), + ), + EvalChecker::new( + "nixpkgs-manual", + nix::Operation::Instantiate, + vec![ + String::from("--arg"), + String::from("nixpkgs"), + String::from("{ outPath=./.; revCount=999999; shortRev=\"ofborg\"; }"), + String::from("./pkgs/top-level/release.nix"), + String::from("-A"), + String::from("manual"), + ], + self.nix.clone(), + ), + EvalChecker::new( + "nixpkgs-tarball", + nix::Operation::Instantiate, + vec![ + String::from("--arg"), + String::from("nixpkgs"), + String::from("{ outPath=./.; revCount=999999; shortRev=\"ofborg\"; }"), + String::from("./pkgs/top-level/release.nix"), + String::from("-A"), + String::from("tarball"), + ], + self.nix.clone(), + ), + EvalChecker::new( + "nixpkgs-unstable-jobset", + nix::Operation::Instantiate, + vec![ + String::from("--arg"), + String::from("nixpkgs"), + String::from("{ outPath=./.; revCount=999999; shortRev=\"ofborg\"; }"), + String::from("./pkgs/top-level/release.nix"), + String::from("-A"), + String::from("unstable"), + ], + self.nix.clone(), + ), + ] } fn all_evaluations_passed( diff --git a/ofborg/src/tasks/evaluate.rs b/ofborg/src/tasks/evaluate.rs index 500458b..d95a438 100644 --- a/ofborg/src/tasks/evaluate.rs +++ b/ofborg/src/tasks/evaluate.rs @@ -9,7 +9,6 @@ use hubcaps::issues::Issue; use ofborg::acl::ACL; use ofborg::checkout; use ofborg::commitstatus::CommitStatus; -use ofborg::evalchecker::EvalChecker; use ofborg::files::file_to_str; use ofborg::message::{buildjob, evaluationjob}; use ofborg::nix; @@ -273,93 +272,8 @@ impl worker::SimpleWorker for EvaluationWorker overall_status .set_with_description("Beginning Evaluations", hubcaps::statuses::State::Pending); - let eval_checks = vec![ - EvalChecker::new( - "package-list", - nix::Operation::QueryPackagesJSON, - vec![String::from("--file"), String::from(".")], - self.nix.clone(), - ), - EvalChecker::new( - "package-list-no-aliases", - nix::Operation::QueryPackagesJSON, - vec![ - String::from("--file"), - String::from("."), - String::from("--arg"), - String::from("config"), - String::from("{ allowAliases = false; }"), - ], - self.nix.clone(), - ), - EvalChecker::new( - "nixos-options", - nix::Operation::Instantiate, - vec![ - String::from("--arg"), - String::from("nixpkgs"), - String::from("{ outPath=./.; revCount=999999; shortRev=\"ofborg\"; }"), - String::from("./nixos/release.nix"), - String::from("-A"), - String::from("options"), - ], - self.nix.clone(), - ), - EvalChecker::new( - "nixos-manual", - nix::Operation::Instantiate, - vec![ - String::from("--arg"), - String::from("nixpkgs"), - String::from("{ outPath=./.; revCount=999999; shortRev=\"ofborg\"; }"), - String::from("./nixos/release.nix"), - String::from("-A"), - String::from("manual"), - ], - self.nix.clone(), - ), - EvalChecker::new( - "nixpkgs-manual", - nix::Operation::Instantiate, - vec![ - String::from("--arg"), - String::from("nixpkgs"), - String::from("{ outPath=./.; revCount=999999; shortRev=\"ofborg\"; }"), - String::from("./pkgs/top-level/release.nix"), - String::from("-A"), - String::from("manual"), - ], - self.nix.clone(), - ), - EvalChecker::new( - "nixpkgs-tarball", - nix::Operation::Instantiate, - vec![ - String::from("--arg"), - String::from("nixpkgs"), - String::from("{ outPath=./.; revCount=999999; shortRev=\"ofborg\"; }"), - String::from("./pkgs/top-level/release.nix"), - String::from("-A"), - String::from("tarball"), - ], - self.nix.clone(), - ), - EvalChecker::new( - "nixpkgs-unstable-jobset", - nix::Operation::Instantiate, - vec![ - String::from("--arg"), - String::from("nixpkgs"), - String::from("{ outPath=./.; revCount=999999; shortRev=\"ofborg\"; }"), - String::from("./pkgs/top-level/release.nix"), - String::from("-A"), - String::from("unstable"), - ], - self.nix.clone(), - ), - ]; - - let eval_results: bool = eval_checks + let eval_results: bool = evaluation_strategy + .evaluation_checks() .into_iter() .map(|check| { let mut status = CommitStatus::new( From 6cbaa5d5c6e4f005080393a5acce0d6e1bcd698e Mon Sep 17 00:00:00 2001 From: Graham Christensen Date: Fri, 22 Mar 2019 16:00:42 -0400 Subject: [PATCH 18/20] merge conflict labels: move --- ofborg/src/tasks/eval/nixpkgs.rs | 14 +++++++++++++- ofborg/src/tasks/evaluate.rs | 4 +--- 2 files changed, 14 insertions(+), 4 deletions(-) diff --git a/ofborg/src/tasks/eval/nixpkgs.rs b/ofborg/src/tasks/eval/nixpkgs.rs index 7af97a9..d1f8894 100644 --- a/ofborg/src/tasks/eval/nixpkgs.rs +++ b/ofborg/src/tasks/eval/nixpkgs.rs @@ -352,9 +352,21 @@ impl<'a> EvaluationStrategy for NixpkgsStrategy<'a> { Ok(()) } - fn merge_conflict(&mut self) {} + fn merge_conflict(&mut self) { + update_labels( + &self.issue_ref, + &["2.status: merge conflict".to_owned()], + &[], + ); + } fn after_merge(&mut self, status: &mut CommitStatus) -> StepResult<()> { + update_labels( + &self.issue_ref, + &[], + &["2.status: merge conflict".to_owned()], + ); + status.set_with_description("Checking new stdenvs", hubcaps::statuses::State::Pending); self.check_stdenvs_after(); diff --git a/ofborg/src/tasks/evaluate.rs b/ofborg/src/tasks/evaluate.rs index d95a438..b523418 100644 --- a/ofborg/src/tasks/evaluate.rs +++ b/ofborg/src/tasks/evaluate.rs @@ -250,11 +250,9 @@ impl worker::SimpleWorker for EvaluationWorker info!("Failed to merge {}", job.pr.head_sha); - update_labels(&issue_ref, &["2.status: merge conflict".to_owned()], &[]); + evaluaton_strategy.merge_conflict(); return self.actions().skip(&job); - } else { - update_labels(&issue_ref, &[], &["2.status: merge conflict".to_owned()]); } if self From 542e66fcf52ce20b980ec7181c7df13c29f0e608 Mon Sep 17 00:00:00 2001 From: Graham Christensen Date: Fri, 22 Mar 2019 17:26:43 -0400 Subject: [PATCH 19/20] Make clippy happy --- ofborg/src/tagger.rs | 2 +- ofborg/src/tasks/eval/generic.rs | 1 + ofborg/src/tasks/eval/nixpkgs.rs | 8 +++++--- ofborg/src/tasks/evaluate.rs | 2 +- 4 files changed, 8 insertions(+), 5 deletions(-) diff --git a/ofborg/src/tagger.rs b/ofborg/src/tagger.rs index cbb5b72..11535d1 100644 --- a/ofborg/src/tagger.rs +++ b/ofborg/src/tagger.rs @@ -148,7 +148,7 @@ impl RebuildTagger { Default::default() } - pub fn parse_attrs(&mut self, attrs: &Vec) { + pub fn parse_attrs(&mut self, attrs: Vec) { let mut counter_darwin = 0; let mut counter_linux = 0; diff --git a/ofborg/src/tasks/eval/generic.rs b/ofborg/src/tasks/eval/generic.rs index f8462ae..8f31ae8 100644 --- a/ofborg/src/tasks/eval/generic.rs +++ b/ofborg/src/tasks/eval/generic.rs @@ -5,6 +5,7 @@ use ofborg::message::buildjob::BuildJob; use std::path::Path; use tasks::eval::{EvaluationStrategy, StepResult}; +#[derive(Default)] pub struct GenericStrategy {} impl GenericStrategy { pub fn new() -> GenericStrategy { diff --git a/ofborg/src/tasks/eval/nixpkgs.rs b/ofborg/src/tasks/eval/nixpkgs.rs index d1f8894..72a9e72 100644 --- a/ofborg/src/tasks/eval/nixpkgs.rs +++ b/ofborg/src/tasks/eval/nixpkgs.rs @@ -36,7 +36,9 @@ pub struct NixpkgsStrategy<'a> { changed_paths: Option>, touched_packages: Option>, } + impl<'a> NixpkgsStrategy<'a> { + #[allow(clippy::too_many_arguments)] pub fn new( job: &'a EvaluationJob, pull: &'a hubcaps::pulls::PullRequest, @@ -182,7 +184,7 @@ impl<'a> NixpkgsStrategy<'a> { self.record_impacted_maintainers(&dir, &attrs); } - rebuild_tags.parse_attrs(&attrs); + rebuild_tags.parse_attrs(attrs.clone()); } update_labels( @@ -193,7 +195,7 @@ impl<'a> NixpkgsStrategy<'a> { } } - fn gist_changed_paths(&self, attrs: &Vec) -> Option { + fn gist_changed_paths(&self, attrs: &[PackageArch]) -> Option { make_gist( &self.gists, "Changed Paths", @@ -206,7 +208,7 @@ impl<'a> NixpkgsStrategy<'a> { ) } - fn record_impacted_maintainers(&self, dir: &Path, attrs: &Vec) -> () { + fn record_impacted_maintainers(&self, dir: &Path, attrs: &[PackageArch]) { let changed_attributes = attrs .iter() .map(|attr| attr.package.split('.').collect::>()) diff --git a/ofborg/src/tasks/evaluate.rs b/ofborg/src/tasks/evaluate.rs index b523418..6a82eb3 100644 --- a/ofborg/src/tasks/evaluate.rs +++ b/ofborg/src/tasks/evaluate.rs @@ -250,7 +250,7 @@ impl worker::SimpleWorker for EvaluationWorker info!("Failed to merge {}", job.pr.head_sha); - evaluaton_strategy.merge_conflict(); + evaluation_strategy.merge_conflict(); return self.actions().skip(&job); } From 010d85fe4d455b964186d06b3b7539b48435f397 Mon Sep 17 00:00:00 2001 From: Graham Christensen Date: Fri, 22 Mar 2019 17:40:10 -0400 Subject: [PATCH 20/20] trim-right -> trim-end --- ofborg/src/tasks/eval/stdenvs.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/ofborg/src/tasks/eval/stdenvs.rs b/ofborg/src/tasks/eval/stdenvs.rs index 8972561..b54cf7c 100644 --- a/ofborg/src/tasks/eval/stdenvs.rs +++ b/ofborg/src/tasks/eval/stdenvs.rs @@ -130,7 +130,7 @@ mod tests { let remote = env::var("NIX_REMOTE").unwrap_or("".to_owned()); let nix = nix::Nix::new(String::from("x86_64-linux"), remote, 1200, None); - let mut stdenv = Stdenvs::new(nix.clone(), PathBuf::from(nixpkgs.trim_right())); + let mut stdenv = Stdenvs::new(nix.clone(), PathBuf::from(nixpkgs.trim_end())); stdenv.identify(System::X8664Linux, StdenvFrom::Before); stdenv.identify(System::X8664Darwin, StdenvFrom::Before);