From patchwork Thu Dec 13 02:43:51 2018 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Michael Ellerman X-Patchwork-Id: 1012480 Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@bilbo.ozlabs.org Received: from lists.ozlabs.org (lists.ozlabs.org [203.11.71.2]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits)) (No client certificate requested) by ozlabs.org (Postfix) with ESMTPS id 43FdQW1FYlz9s8r for ; Thu, 13 Dec 2018 13:48:03 +1100 (AEDT) Authentication-Results: ozlabs.org; dmarc=none (p=none dis=none) header.from=ellerman.id.au Received: from lists.ozlabs.org (lists.ozlabs.org [IPv6:2401:3900:2:1::3]) by lists.ozlabs.org (Postfix) with ESMTP id 43FdQV5jVWzDqHr for ; Thu, 13 Dec 2018 13:48:02 +1100 (AEDT) Authentication-Results: lists.ozlabs.org; dmarc=none (p=none dis=none) header.from=ellerman.id.au X-Original-To: snowpatch@lists.ozlabs.org Delivered-To: snowpatch@lists.ozlabs.org Received: from ozlabs.org (bilbo.ozlabs.org [IPv6:2401:3900:2:1::2]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits)) (No client certificate requested) by lists.ozlabs.org (Postfix) with ESMTPS id 43FdKp58CGzDr3R for ; Thu, 13 Dec 2018 13:43:58 +1100 (AEDT) Authentication-Results: lists.ozlabs.org; dmarc=none (p=none dis=none) header.from=ellerman.id.au Received: by ozlabs.org (Postfix, from userid 1034) id 43FdKp4DCzz9s8r; Thu, 13 Dec 2018 13:43:58 +1100 (AEDT) From: Michael Ellerman To: snowpatch@lists.ozlabs.org Date: Thu, 13 Dec 2018 13:43:51 +1100 Message-Id: <20181213024353.19725-1-mpe@ellerman.id.au> X-Mailer: git-send-email 2.17.2 X-Mailman-Approved-At: Thu, 13 Dec 2018 13:48:00 +1100 Subject: [snowpatch] [PATCH 1/3] Rework main() to return a Result() X-BeenThere: snowpatch@lists.ozlabs.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Continuous Integration for patch-based workflows List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , MIME-Version: 1.0 Errors-To: snowpatch-bounces+incoming=patchwork.ozlabs.org@lists.ozlabs.org Sender: "snowpatch" Turn main() into run() and have it return a Result(). Then in main() we print a message for the error case. We could just return Result() directly from main() but that prints the Err() with {:?} which is ugly. Reviewed-by: Andrew Donnellan --- src/main.rs | 19 +++++++++++++++---- 1 file changed, 15 insertions(+), 4 deletions(-) diff --git a/src/main.rs b/src/main.rs index 7a0debaebec4..2e022a3939e9 100644 --- a/src/main.rs +++ b/src/main.rs @@ -45,8 +45,10 @@ use env_logger::Builder; use log::LevelFilter; use std::env; +use std::error::Error; use std::fs; use std::path::Path; +use std::process; use std::string::String; use std::sync::Arc; use std::thread; @@ -282,7 +284,7 @@ fn test_patch( } #[cfg_attr(feature = "cargo-clippy", allow(cyclomatic_complexity))] -fn main() { +fn run() -> Result<(), Box> { let mut log_builder = Builder::new(); // By default, log at the "info" level for every module log_builder.filter(None, LevelFilter::Info); @@ -358,7 +360,7 @@ fn main() { test_patch(&settings, &client, &project, &mbox, true); } } - return; + return Ok(()); } if args.flag_series > 0 { @@ -385,7 +387,7 @@ fn main() { } } } - return; + return Ok(()); } // At this point, specifying a project is required @@ -396,7 +398,7 @@ fn main() { let patch = Path::new(&args.flag_mbox); test_patch(&settings, &client, &project, patch, true); - return; + return Ok(()); } /* @@ -491,4 +493,13 @@ fn main() { info!("Finished testing new revisions, sleeping."); thread::sleep(Duration::new(settings.patchwork.polling_interval * 60, 0)); } + + Ok(()) +} + +fn main() { + if let Err(e) = run() { + println!("Error: {}", e); + process::exit(1); + } } From patchwork Thu Dec 13 02:43:52 2018 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Michael Ellerman X-Patchwork-Id: 1012481 Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@bilbo.ozlabs.org Received: from lists.ozlabs.org (lists.ozlabs.org [203.11.71.2]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits)) (No client certificate requested) by ozlabs.org (Postfix) with ESMTPS id 43FdQZ2X9pz9s7T for ; Thu, 13 Dec 2018 13:48:06 +1100 (AEDT) Authentication-Results: ozlabs.org; dmarc=none (p=none dis=none) header.from=ellerman.id.au Received: from lists.ozlabs.org (lists.ozlabs.org [IPv6:2401:3900:2:1::3]) by lists.ozlabs.org (Postfix) with ESMTP id 43FdQZ0xjKzDqKD for ; Thu, 13 Dec 2018 13:48:06 +1100 (AEDT) Authentication-Results: lists.ozlabs.org; dmarc=none (p=none dis=none) header.from=ellerman.id.au X-Original-To: snowpatch@lists.ozlabs.org Delivered-To: snowpatch@lists.ozlabs.org Received: from ozlabs.org (bilbo.ozlabs.org [203.11.71.1]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits)) (No client certificate requested) by lists.ozlabs.org (Postfix) with ESMTPS id 43FdKq6L8czDqW2 for ; Thu, 13 Dec 2018 13:43:59 +1100 (AEDT) Authentication-Results: lists.ozlabs.org; dmarc=none (p=none dis=none) header.from=ellerman.id.au Received: by ozlabs.org (Postfix, from userid 1034) id 43FdKq2Q2Gz9s9G; Thu, 13 Dec 2018 13:43:59 +1100 (AEDT) From: Michael Ellerman To: snowpatch@lists.ozlabs.org Date: Thu, 13 Dec 2018 13:43:52 +1100 Message-Id: <20181213024353.19725-2-mpe@ellerman.id.au> X-Mailer: git-send-email 2.17.2 In-Reply-To: <20181213024353.19725-1-mpe@ellerman.id.au> References: <20181213024353.19725-1-mpe@ellerman.id.au> X-Mailman-Approved-At: Thu, 13 Dec 2018 13:48:00 +1100 Subject: [snowpatch] [PATCH 2/3] Use git2::Error explicitly in settings.rs X-BeenThere: snowpatch@lists.ozlabs.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Continuous Integration for patch-based workflows List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , MIME-Version: 1.0 Errors-To: snowpatch-bounces+incoming=patchwork.ozlabs.org@lists.ozlabs.org Sender: "snowpatch" So that we can use std:error:Error as Error. Reviewed-by: Andrew Donnellan --- src/settings.rs | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/src/settings.rs b/src/settings.rs index 241580b36e20..a5513728fbaa 100644 --- a/src/settings.rs +++ b/src/settings.rs @@ -18,7 +18,8 @@ use toml; use serde::de::{self, Deserialize, Deserializer, MapAccess, Visitor}; -use git2::{Error, Repository}; +use git2; +use git2::Repository; use std::collections::BTreeMap; use std::fmt; @@ -69,7 +70,7 @@ pub struct Project { } impl Project { - pub fn get_repo(&self) -> Result { + pub fn get_repo(&self) -> Result { Repository::open(&self.repository) } } From patchwork Thu Dec 13 02:43:53 2018 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Michael Ellerman X-Patchwork-Id: 1012482 Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@bilbo.ozlabs.org Received: from lists.ozlabs.org (lists.ozlabs.org [IPv6:2401:3900:2:1::3]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits)) (No client certificate requested) by ozlabs.org (Postfix) with ESMTPS id 43FdQf14Xcz9s7T for ; Thu, 13 Dec 2018 13:48:10 +1100 (AEDT) Authentication-Results: ozlabs.org; dmarc=none (p=none dis=none) header.from=ellerman.id.au Received: from lists.ozlabs.org (lists.ozlabs.org [IPv6:2401:3900:2:1::3]) by lists.ozlabs.org (Postfix) with ESMTP id 43FdQd6wm9zDqJW for ; Thu, 13 Dec 2018 13:48:09 +1100 (AEDT) Authentication-Results: lists.ozlabs.org; dmarc=none (p=none dis=none) header.from=ellerman.id.au X-Original-To: snowpatch@lists.ozlabs.org Delivered-To: snowpatch@lists.ozlabs.org Received: from ozlabs.org (bilbo.ozlabs.org [IPv6:2401:3900:2:1::2]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits)) (No client certificate requested) by lists.ozlabs.org (Postfix) with ESMTPS id 43FdKr5PY5zDqPW for ; Thu, 13 Dec 2018 13:44:00 +1100 (AEDT) Authentication-Results: lists.ozlabs.org; dmarc=none (p=none dis=none) header.from=ellerman.id.au Received: by ozlabs.org (Postfix, from userid 1034) id 43FdKr3nkmz9s8r; Thu, 13 Dec 2018 13:44:00 +1100 (AEDT) From: Michael Ellerman To: snowpatch@lists.ozlabs.org Date: Thu, 13 Dec 2018 13:43:53 +1100 Message-Id: <20181213024353.19725-3-mpe@ellerman.id.au> X-Mailer: git-send-email 2.17.2 In-Reply-To: <20181213024353.19725-1-mpe@ellerman.id.au> References: <20181213024353.19725-1-mpe@ellerman.id.au> X-Mailman-Approved-At: Thu, 13 Dec 2018 13:48:00 +1100 Subject: [snowpatch] [PATCH 3/3] Rewrite settings parsing to avoid panic X-BeenThere: snowpatch@lists.ozlabs.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Continuous Integration for patch-based workflows List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , MIME-Version: 1.0 Errors-To: snowpatch-bounces+incoming=patchwork.ozlabs.org@lists.ozlabs.org Sender: "snowpatch" Change parse() to return a Result, using the question mark operator to catch errors and return early. Use map_err() to turn the low-level errors into something human readable. We could define our own error type but that seems like overkill for this. Rework the tests to cope. For the two failing cases that's a bit ugly because we have to map the Ok() to Err() and vice versa. There are now no panic()s or unwrap()s in settings.rs! Error messages seen by the user look like, eg: Error: Unable to parse config file: missing field `git` Error: Unable to open config file: Permission denied (os error 13) Error: Unable to open config file: No such file or directory (os error 2) Reviewed-by: Andrew Donnellan --- src/main.rs | 2 +- src/settings.rs | 42 +++++++++++++++++++++--------------------- 2 files changed, 22 insertions(+), 22 deletions(-) diff --git a/src/main.rs b/src/main.rs index 2e022a3939e9..d2c24be3a59f 100644 --- a/src/main.rs +++ b/src/main.rs @@ -303,7 +303,7 @@ fn run() -> Result<(), Box> { .and_then(|d| d.version(Some(version)).deserialize()) .unwrap_or_else(|e| e.exit()); - let settings = settings::parse(&args.arg_config_file); + let settings = settings::parse(&args.arg_config_file)?; // The HTTP client we'll use to access the APIs // TODO: Handle https_proxy diff --git a/src/settings.rs b/src/settings.rs index a5513728fbaa..8e86b213a6bc 100644 --- a/src/settings.rs +++ b/src/settings.rs @@ -22,6 +22,7 @@ use git2; use git2::Repository; use std::collections::BTreeMap; +use std::error::Error; use std::fmt; use std::fs::File; use std::io::Read; @@ -186,23 +187,18 @@ pub struct Config { pub projects: BTreeMap, } -pub fn parse(path: &str) -> Config { +pub fn parse(path: &str) -> Result> { let mut toml_config = String::new(); - let mut file = File::open(&path).expect("Couldn't open config file, exiting."); + File::open(&path) + .map_err(|e| format!("Unable to open config file: {}", e))? + .read_to_string(&mut toml_config) + .map_err(|e| format!("Unable to read config file: {}", e))?; - file.read_to_string(&mut toml_config) - .unwrap_or_else(|err| panic!("Couldn't read config: {}", err)); + let config = toml::de::from_str::(&toml_config) + .map_err(|e| format!("Unable to parse config file: {}", e))?; - let toml_config = toml::de::from_str::(&toml_config); - - match toml_config { - Ok(config_inside) => config_inside, - Err(err) => { - error!("TOML error: {}", err); - panic!("Could not parse configuration file, exiting"); - } - } + Ok(config) } #[cfg(test)] @@ -210,19 +206,23 @@ mod test { use settings::*; #[test] - #[should_panic(expected = "Couldn't open config file")] - fn bad_path() { - parse("/nonexistent/config.file"); + fn bad_path() -> Result<(), &'static str> { + match parse("/nonexistent/config.file") { + Ok(_) => Err("Didn't fail opening non-existent file?"), + Err(_) => Ok(()), + } } #[test] - fn parse_example_openpower() { - parse("examples/openpower.toml"); + fn parse_example_openpower() -> Result<(), Box> { + parse("examples/openpower.toml").map(|_| ()) } #[test] - #[should_panic(expected = "Could not parse configuration file, exiting")] - fn parse_example_invalid() { - parse("examples/tests/invalid.toml"); + fn parse_example_invalid() -> Result<(), &'static str> { + match parse("examples/tests/invalid.toml") { + Ok(_) => Err("Didn't fail parsing invalid TOML?"), + Err(_) => Ok(()) + } } }