From 9d85f62835a41dcebef72b38820c49067fc41f75 Mon Sep 17 00:00:00 2001 From: Daiderd Jordan Date: Sun, 5 Aug 2018 19:56:11 +0200 Subject: [PATCH] build: use status enum from task instead of success bool This allows the commentposter to handle other build statuses like a timeout. The BuildResult struct still includes success and the commentposter can still handle messages in the old format for backwards compatibility. --- ofborg/src/message/buildresult.rs | 13 +- ofborg/src/tasks/build.rs | 42 ++++-- ofborg/src/tasks/githubcommentposter.rs | 161 ++++++++++++++++++++-- ofborg/src/tasks/log_message_collector.rs | 5 +- 4 files changed, 195 insertions(+), 26 deletions(-) diff --git a/ofborg/src/message/buildresult.rs b/ofborg/src/message/buildresult.rs index ad024b3..2eea547 100644 --- a/ofborg/src/message/buildresult.rs +++ b/ofborg/src/message/buildresult.rs @@ -1,5 +1,15 @@ use ofborg::message::{Pr, Repo}; + +#[derive(Serialize, Deserialize, Debug)] +pub enum BuildStatus { + Skipped, + Success, + Failure, + TimedOut, + UnexpectedError { err: String }, +} + #[derive(Serialize, Deserialize, Debug)] pub struct BuildResult { pub repo: Repo, @@ -8,7 +18,8 @@ pub struct BuildResult { pub output: Vec, pub attempt_id: String, pub request_id: Option, - pub success: Option, + pub success: Option, // replaced by status + pub status: Option, pub skipped_attrs: Option>, pub attempted_attrs: Option>, } diff --git a/ofborg/src/tasks/build.rs b/ofborg/src/tasks/build.rs index b181085..b84057b 100644 --- a/ofborg/src/tasks/build.rs +++ b/ofborg/src/tasks/build.rs @@ -7,7 +7,7 @@ use uuid::Uuid; use std::collections::VecDeque; use ofborg::checkout; use ofborg::message::buildjob; -use ofborg::message::buildresult; +use ofborg::message::buildresult::{BuildResult, BuildStatus}; use ofborg::message::buildlogmsg; use ofborg::nix; use ofborg::commentparser; @@ -112,7 +112,7 @@ impl<'a, 'b> JobActions<'a, 'b> { } pub fn merge_failed(&mut self) { - let msg = buildresult::BuildResult { + let msg = BuildResult { repo: self.job.repo.clone(), pr: self.job.pr.clone(), system: self.system.clone(), @@ -121,6 +121,7 @@ impl<'a, 'b> JobActions<'a, 'b> { request_id: Some(self.job.request_id.clone()), attempted_attrs: None, skipped_attrs: None, + status: Some(BuildStatus::Failure), success: Some(false), }; @@ -195,7 +196,7 @@ impl<'a, 'b> JobActions<'a, 'b> { pub fn build_not_attempted(&mut self, not_attempted_attrs: Vec, ) { - let msg = buildresult::BuildResult { + let msg = BuildResult { repo: self.job.repo.clone(), pr: self.job.pr.clone(), system: self.system.clone(), @@ -204,6 +205,7 @@ impl<'a, 'b> JobActions<'a, 'b> { request_id: Some(self.job.request_id.clone()), skipped_attrs: Some(not_attempted_attrs), attempted_attrs: None, + status: None, success: None, }; @@ -226,18 +228,23 @@ impl<'a, 'b> JobActions<'a, 'b> { self.tell(worker::Action::Ack); } - pub fn build_finished(&mut self, success: bool, + pub fn build_finished(&mut self, status: BuildStatus, attempted_attrs: Vec, not_attempted_attrs: Vec, ) { - let msg = buildresult::BuildResult { + let success = match status { + BuildStatus::Success => true, + _ => false, + }; + let msg = BuildResult { repo: self.job.repo.clone(), pr: self.job.pr.clone(), system: self.system.clone(), output: self.log_snippet(), attempt_id: self.attempt_id.clone(), request_id: Some(self.job.request_id.clone()), + status: Some(status), success: Some(success), attempted_attrs: Some(attempted_attrs), skipped_attrs: Some(not_attempted_attrs), @@ -373,21 +380,30 @@ impl notifyworker::SimpleNotifyWorker for BuildWorker { actions.log_line(&line); } - let success = match spawned.wait() { - Ok(status) => status.success(), - e => { - println!("Failed on the interior command: {:?}", e); - false + // TODO: this belongs in the nix module. + let status = match spawned.wait() { + Ok(s) => match s.code() { + Some(0) => BuildStatus::Success, + Some(100) => BuildStatus::Failure, // nix permanent failure + Some(101) => BuildStatus::TimedOut, // nix build timedout + Some(i) => BuildStatus::UnexpectedError { + err: format!("command failed with exit code {}", i), + }, + None => BuildStatus::UnexpectedError { + err: "unexpected build failure".into(), + }, + }, + e => BuildStatus::UnexpectedError { + err: format!("failed on interior command {:?}", e), } }; - println!("ok built ({:?}), building", success); + println!("ok built ({:?}), building", status); println!("Lines:\n-----8<-----"); actions.log_snippet().iter().inspect(|x| println!("{}", x)).last(); println!("----->8-----"); - - actions.build_finished(success, can_build, cannot_build_attrs); + actions.build_finished(status, can_build, cannot_build_attrs); println!("Done!"); } } diff --git a/ofborg/src/tasks/githubcommentposter.rs b/ofborg/src/tasks/githubcommentposter.rs index 27201b2..6449415 100644 --- a/ofborg/src/tasks/githubcommentposter.rs +++ b/ofborg/src/tasks/githubcommentposter.rs @@ -4,7 +4,7 @@ extern crate env_logger; use serde_json; use hubcaps; -use ofborg::message::buildresult::BuildResult; +use ofborg::message::buildresult::{BuildStatus, BuildResult}; use ofborg::worker; use amqp::protocol::basic::{Deliver, BasicProperties}; @@ -86,13 +86,27 @@ fn result_to_comment(result: &BuildResult) -> String { "".to_owned() }; + let status = match result.status { + None => { + // Fallback for old format. + match result.success { + None => &BuildStatus::Skipped, + Some(true) => &BuildStatus::Success, + Some(false) => &BuildStatus::Failure, + } + }, + Some(ref s) => s, + }; + reply.push(format!( "{} on {}{}", - (match result.success { - Some(true) => "Success", - Some(false) => "Failure", - None => "No attempt", - }), + (match *status { + BuildStatus::Skipped => "No attempt".into(), + BuildStatus::Success => "Success".into(), + BuildStatus::Failure => "Failure".into(), + BuildStatus::TimedOut => "Timed out, unknown build status".into(), + BuildStatus::UnexpectedError { ref err } => format!("Unexpected error: {}", err), + }), result.system, log_link )); @@ -185,7 +199,8 @@ mod tests { system: "x86_64-linux".to_owned(), attempted_attrs: Some(vec!["foo".to_owned()]), skipped_attrs: Some(vec!["bar".to_owned()]), - success: Some(true), + status: Some(BuildStatus::Success), + success: None, }; assert_eq!( @@ -247,7 +262,8 @@ patching script interpreter paths in /nix/store/pcja75y9isdvgz5i00pkrpif9rxzxc29 system: "x86_64-linux".to_owned(), attempted_attrs: Some(vec!["foo".to_owned()]), skipped_attrs: None, - success: Some(false), + status: Some(BuildStatus::Failure), + success: None, }; assert_eq!( @@ -276,6 +292,64 @@ patching script interpreter paths in /nix/store/pcja75y9isdvgz5i00pkrpif9rxzxc29 ); } + #[test] + pub fn test_timedout_build() { + let result = BuildResult { + repo: Repo { + clone_url: "https://github.com/nixos/nixpkgs.git".to_owned(), + full_name: "NixOS/nixpkgs".to_owned(), + owner: "NixOS".to_owned(), + name: "nixpkgs".to_owned(), + }, + pr: Pr { + head_sha: "abc123".to_owned(), + number: 2345, + target_branch: Some("master".to_owned()), + }, + output: vec![ + "make[2]: Entering directory '/private/tmp/nix-build-gdb-8.1.drv-0/gdb-8.1/readline'".to_owned(), + "make[2]: Nothing to be done for 'install'.".to_owned(), + "make[2]: Leaving directory '/private/tmp/nix-build-gdb-8.1.drv-0/gdb-8.1/readline'".to_owned(), + "make[1]: Nothing to be done for 'install-target'.".to_owned(), + "make[1]: Leaving directory '/private/tmp/nix-build-gdb-8.1.drv-0/gdb-8.1'".to_owned(), + "removed '/nix/store/pcja75y9isdvgz5i00pkrpif9rxzxc29-gdb-8.1/share/info/bfd.info'".to_owned(), + "post-installation fixup".to_owned(), + "building of '/nix/store/l1limh50lx2cx45yb2gqpv7k8xl1mik2-gdb-8.1.drv' timed out after 1 seconds".to_owned(), + "error: build of '/nix/store/l1limh50lx2cx45yb2gqpv7k8xl1mik2-gdb-8.1.drv' failed".to_owned(), + ], + attempt_id: "neatattemptid".to_owned(), + request_id: Some("bogus-request-id".to_owned()), + system: "x86_64-linux".to_owned(), + attempted_attrs: Some(vec!["foo".to_owned()]), + skipped_attrs: None, + status: Some(BuildStatus::TimedOut), + success: None, + }; + + assert_eq!( + &result_to_comment(&result), + "Timed out, unknown build status on x86_64-linux [(full log)](https://logs.nix.ci/?key=nixos/nixpkgs.2345&attempt_id=neatattemptid) + +Attempted: foo + +
Partial log (click to expand)

+ +``` +make[2]: Entering directory '/private/tmp/nix-build-gdb-8.1.drv-0/gdb-8.1/readline' +make[2]: Nothing to be done for 'install'. +make[2]: Leaving directory '/private/tmp/nix-build-gdb-8.1.drv-0/gdb-8.1/readline' +make[1]: Nothing to be done for 'install-target'. +make[1]: Leaving directory '/private/tmp/nix-build-gdb-8.1.drv-0/gdb-8.1' +removed '/nix/store/pcja75y9isdvgz5i00pkrpif9rxzxc29-gdb-8.1/share/info/bfd.info' +post-installation fixup +building of '/nix/store/l1limh50lx2cx45yb2gqpv7k8xl1mik2-gdb-8.1.drv' timed out after 1 seconds +error: build of '/nix/store/l1limh50lx2cx45yb2gqpv7k8xl1mik2-gdb-8.1.drv' failed +``` +

+ +" + ); + } #[test] pub fn test_passing_build_unspecified_attributes() { @@ -308,7 +382,8 @@ patching script interpreter paths in /nix/store/pcja75y9isdvgz5i00pkrpif9rxzxc29 system: "x86_64-linux".to_owned(), attempted_attrs: None, skipped_attrs: None, - success: Some(true), + status: Some(BuildStatus::Success), + success: None, }; assert_eq!( @@ -366,7 +441,8 @@ patching script interpreter paths in /nix/store/pcja75y9isdvgz5i00pkrpif9rxzxc29 system: "x86_64-linux".to_owned(), attempted_attrs: None, skipped_attrs: None, - success: Some(false), + status: Some(BuildStatus::Failure), + success: None, }; assert_eq!( @@ -413,6 +489,7 @@ patching script interpreter paths in /nix/store/pcja75y9isdvgz5i00pkrpif9rxzxc29 system: "x86_64-linux".to_owned(), attempted_attrs: None, skipped_attrs: Some(vec!["not-attempted".to_owned()]), + status: Some(BuildStatus::Skipped), success: None, }; @@ -453,6 +530,7 @@ foo system: "x86_64-linux".to_owned(), attempted_attrs: None, skipped_attrs: Some(vec!["not-attempted".to_owned()]), + status: Some(BuildStatus::Skipped), success: None, }; @@ -463,6 +541,69 @@ foo The following builds were skipped because they don't evaluate on x86_64-linux: not-attempted No partial log is available. +" + ); + } + + #[test] + pub fn test_no_status_compatibility() { + let result = BuildResult { + repo: Repo { + clone_url: "https://github.com/nixos/nixpkgs.git".to_owned(), + full_name: "NixOS/nixpkgs".to_owned(), + owner: "NixOS".to_owned(), + name: "nixpkgs".to_owned(), + }, + pr: Pr { + head_sha: "abc123".to_owned(), + number: 2345, + target_branch: Some("master".to_owned()), + }, + output: vec![ + "make[2]: Entering directory '/private/tmp/nix-build-gdb-8.1.drv-0/gdb-8.1/readline'".to_owned(), + "make[2]: Nothing to be done for 'install'.".to_owned(), + "make[2]: Leaving directory '/private/tmp/nix-build-gdb-8.1.drv-0/gdb-8.1/readline'".to_owned(), + "make[1]: Nothing to be done for 'install-target'.".to_owned(), + "make[1]: Leaving directory '/private/tmp/nix-build-gdb-8.1.drv-0/gdb-8.1'".to_owned(), + "removed '/nix/store/pcja75y9isdvgz5i00pkrpif9rxzxc29-gdb-8.1/share/info/bfd.info'".to_owned(), + "post-installation fixup".to_owned(), + "strip is /nix/store/5a88zk3jgimdmzg8rfhvm93kxib3njf9-cctools-binutils-darwin/bin/strip".to_owned(), + "patching script interpreter paths in /nix/store/pcja75y9isdvgz5i00pkrpif9rxzxc29-gdb-8.1".to_owned(), + "/nix/store/pcja75y9isdvgz5i00pkrpif9rxzxc29-gdb-8.1".to_owned(), + ], + attempt_id: "neatattemptid".to_owned(), + request_id: Some("bogus-request-id".to_owned()), + system: "x86_64-linux".to_owned(), + attempted_attrs: Some(vec!["foo".to_owned()]), + skipped_attrs: Some(vec!["bar".to_owned()]), + status: None, + success: Some(true), + }; + + assert_eq!( + &result_to_comment(&result), + "Success on x86_64-linux [(full log)](https://logs.nix.ci/?key=nixos/nixpkgs.2345&attempt_id=neatattemptid) + +Attempted: foo + +The following builds were skipped because they don't evaluate on x86_64-linux: bar + +
Partial log (click to expand)

+ +``` +make[2]: Entering directory '/private/tmp/nix-build-gdb-8.1.drv-0/gdb-8.1/readline' +make[2]: Nothing to be done for 'install'. +make[2]: Leaving directory '/private/tmp/nix-build-gdb-8.1.drv-0/gdb-8.1/readline' +make[1]: Nothing to be done for 'install-target'. +make[1]: Leaving directory '/private/tmp/nix-build-gdb-8.1.drv-0/gdb-8.1' +removed '/nix/store/pcja75y9isdvgz5i00pkrpif9rxzxc29-gdb-8.1/share/info/bfd.info' +post-installation fixup +strip is /nix/store/5a88zk3jgimdmzg8rfhvm93kxib3njf9-cctools-binutils-darwin/bin/strip +patching script interpreter paths in /nix/store/pcja75y9isdvgz5i00pkrpif9rxzxc29-gdb-8.1 +/nix/store/pcja75y9isdvgz5i00pkrpif9rxzxc29-gdb-8.1 +``` +

+ " ); } diff --git a/ofborg/src/tasks/log_message_collector.rs b/ofborg/src/tasks/log_message_collector.rs index 4dc2bb9..47ad006 100644 --- a/ofborg/src/tasks/log_message_collector.rs +++ b/ofborg/src/tasks/log_message_collector.rs @@ -9,7 +9,7 @@ use std::io::Write; use ofborg::writetoline::LineWriter; use ofborg::message::buildlogmsg::{BuildLogStart, BuildLogMsg}; -use ofborg::message::buildresult::BuildResult; +use ofborg::message::buildresult::{BuildStatus, BuildResult}; use ofborg::worker; use amqp::protocol::basic::{Deliver, BasicProperties}; @@ -445,6 +445,7 @@ mod tests { output: vec![], attempt_id: "attempt-id-foo".to_owned(), request_id: Some("bogus-request-id".to_owned()), + status: Some(BuildStatus::Success), success: Some(true), attempted_attrs: Some(vec!["foo".to_owned()]), skipped_attrs: Some(vec!["bar".to_owned()]), @@ -478,6 +479,6 @@ mod tests { let mut s = String::new(); pr.push("routing-key-foo/attempt-id-foo.result.json"); File::open(pr).unwrap().read_to_string(&mut s).unwrap(); - assert_eq!(&s, "{\"repo\":{\"owner\":\"NixOS\",\"name\":\"ofborg\",\"full_name\":\"NixOS/ofborg\",\"clone_url\":\"https://github.com/nixos/ofborg.git\"},\"pr\":{\"target_branch\":\"scratch\",\"number\":42,\"head_sha\":\"6dd9f0265d52b946dd13daf996f30b64e4edb446\"},\"system\":\"x86_64-linux\",\"output\":[],\"attempt_id\":\"attempt-id-foo\",\"request_id\":\"bogus-request-id\",\"success\":true,\"skipped_attrs\":[\"bar\"],\"attempted_attrs\":[\"foo\"]}"); + assert_eq!(&s, "{\"repo\":{\"owner\":\"NixOS\",\"name\":\"ofborg\",\"full_name\":\"NixOS/ofborg\",\"clone_url\":\"https://github.com/nixos/ofborg.git\"},\"pr\":{\"target_branch\":\"scratch\",\"number\":42,\"head_sha\":\"6dd9f0265d52b946dd13daf996f30b64e4edb446\"},\"system\":\"x86_64-linux\",\"output\":[],\"attempt_id\":\"attempt-id-foo\",\"request_id\":\"bogus-request-id\",\"success\":true,\"status\":\"Success\",\"skipped_attrs\":[\"bar\"],\"attempted_attrs\":[\"foo\"]}"); } }