Skip to content

Commit

Permalink
Add a logger and don't panic on all errors (#304)
Browse files Browse the repository at this point in the history
  • Loading branch information
calixteman authored and marco-c committed Aug 8, 2019
1 parent ae0ece5 commit 10a285d
Show file tree
Hide file tree
Showing 6 changed files with 275 additions and 21 deletions.
176 changes: 176 additions & 0 deletions Cargo.lock

Large diffs are not rendered by default.

2 changes: 2 additions & 0 deletions Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,8 @@ rustc-hash = "^1.0"
clap = "^2.32"
fomat-macros = "^0.3"
chrono = "^0.4"
log = "^0.4"
simplelog = "^0.6"


[dev-dependencies]
Expand Down
39 changes: 33 additions & 6 deletions src/gcov.rs
Original file line number Diff line number Diff line change
@@ -1,8 +1,28 @@
use semver::Version;
use std::env;
use std::fmt;
use std::path::PathBuf;
use std::process::Command;

#[derive(Debug)]
pub enum GcovError {
ProcessFailure,
Failure((String, String, String)),
}

impl fmt::Display for GcovError {
fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result {
match *self {
GcovError::ProcessFailure => write!(f, "Failed to execute gcov process"),
GcovError::Failure((ref path, ref stdout, ref stderr)) => {
writeln!(f, "gcov execution failed on {}", path)?;
writeln!(f, "gcov stdout: {}", stdout)?;
writeln!(f, "gcov stderr: {}", stderr)
}
}
}
}

fn get_gcov() -> String {
if let Ok(s) = env::var("GCOV") {
s
Expand All @@ -11,7 +31,7 @@ fn get_gcov() -> String {
}
}

pub fn run_gcov(gcno_path: &PathBuf, branch_enabled: bool, working_dir: &PathBuf) {
pub fn run_gcov(gcno_path: &PathBuf, branch_enabled: bool, working_dir: &PathBuf) -> Result<(), GcovError> {
let mut command = Command::new(&get_gcov());
let command = if branch_enabled {
command.arg("-b").arg("-c")
Expand All @@ -23,14 +43,21 @@ pub fn run_gcov(gcno_path: &PathBuf, branch_enabled: bool, working_dir: &PathBuf
.arg("-i") // Generate intermediate gcov format, faster to parse.
.current_dir(working_dir);

let output = status.output().expect("Failed to execute gcov process");
let output = if let Ok(output) = status.output() {
output
} else {
return Err(GcovError::ProcessFailure);
};

if !output.status.success() {
eprintln!("gcov stdout: {}", String::from_utf8_lossy(&output.stdout));
eprintln!("gcov stderr: {}", String::from_utf8_lossy(&output.stderr));

panic!("gcov execution failed on {}", gcno_path.display());
return Err(GcovError::Failure((
gcno_path.to_str().unwrap().to_string(),
String::from_utf8_lossy(&output.stdout).to_string(),
String::from_utf8_lossy(&output.stderr).to_string(),
)));
}

Ok(())
}

fn is_recent_version(gcov_output: &str) -> bool {
Expand Down
20 changes: 13 additions & 7 deletions src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,8 @@ extern crate crossbeam;
#[macro_use]
extern crate fomat_macros;
extern crate globset;
#[macro_use]
extern crate log;
extern crate semver;
extern crate smallvec;
extern crate tempfile;
Expand Down Expand Up @@ -148,9 +150,8 @@ macro_rules! try_parse {
match $v {
Ok(val) => val,
Err(err) => {
eprintln!("Error parsing file {}:", $f);
eprintln!("{}", err);
std::process::exit(1);
error!("Error parsing file {}: {}", $f, err);
continue;
}
}
};
Expand All @@ -176,7 +177,10 @@ pub fn consumer(
match work_item.item {
ItemType::Path((stem, gcno_path)) => {
// GCC
run_gcov(&gcno_path, branch_enabled, working_dir);
if let Err(e) = run_gcov(&gcno_path, branch_enabled, working_dir) {
error!("Error when running gcov: {}", e);
continue;
};
let gcov_path =
gcno_path.file_name().unwrap().to_str().unwrap().to_string() + ".gcov";
let gcov_path = working_dir.join(gcov_path);
Expand Down Expand Up @@ -231,13 +235,14 @@ pub fn consumer(
},
Err(e) => {
// Just print the error, don't panic and continue
eprintln!("Error in computing counters:\n{}", e);
error!("Error in computing counters: {}", e);
Vec::new()
}
}
}
ItemType::Content(_) => {
panic!("Invalid content type");
error!("Invalid content type");
continue;
}
}
}
Expand All @@ -250,7 +255,8 @@ pub fn consumer(
try_parse!(parse_jacoco_xml_report(buffer), work_item.name)
}
} else {
panic!("Invalid content type")
error!("Invalid content type");
continue;
}
}
};
Expand Down
42 changes: 42 additions & 0 deletions src/main.rs
Original file line number Diff line number Diff line change
Expand Up @@ -5,13 +5,18 @@ extern crate grcov;
extern crate num_cpus;
extern crate rustc_hash;
extern crate serde_json;
extern crate simplelog;
extern crate tempfile;

use clap::{App, Arg};
use crossbeam::crossbeam_channel::unbounded;
use log::error;
use rustc_hash::FxHashMap;
use serde_json::Value;
use simplelog::{Config, LevelFilter, TerminalMode, TermLogger, WriteLogger};
use std::fs::{self, File};
use std::ops::Deref;
use std::panic;
use std::path::PathBuf;
use std::sync::{Arc, Mutex};
use std::{process, thread};
Expand Down Expand Up @@ -155,6 +160,13 @@ fn main() {
.value_name("VCS BRANCH")
.takes_value(true))

.arg(Arg::with_name("log")
.help("Set the file where to log (or stderr or stdout). Defaults to 'stderr'")
.long("log")
.default_value("stderr")
.value_name("LOG")
.takes_value(true))

.get_matches();

let paths: Vec<_> = matches.values_of("paths").unwrap().collect();
Expand Down Expand Up @@ -187,6 +199,36 @@ fn main() {
let service_number = matches.value_of("service_number").unwrap_or("");
let service_job_number = matches.value_of("service_job_number").unwrap_or("");
let vcs_branch = matches.value_of("vcs_branch").unwrap_or("");
let log = matches.value_of("log").unwrap_or("");
match log {
"stdout" => {
let _ = TermLogger::init(LevelFilter::Error, Config::default(), TerminalMode::Stdout);
},
"stderr" => {
let _ = TermLogger::init(LevelFilter::Error, Config::default(), TerminalMode::Stderr);
},
log => {
if let Ok(file) = File::create(log) {
let _ = WriteLogger::init(LevelFilter::Error, Config::default(), file);
} else {
let _ = TermLogger::init(LevelFilter::Error, Config::default(), TerminalMode::Stderr);
error!("Enable to create log file: {}. Swtich to stderr", log);
}
}
};

panic::set_hook(Box::new(|panic_info| {
let (filename, line) =
panic_info.location().map(|loc| (loc.file(), loc.line()))
.unwrap_or(("<unknown>", 0));
let cause = panic_info.payload().downcast_ref::<String>().map(String::deref);
let cause = cause.unwrap_or_else(||
panic_info.payload().downcast_ref::<&str>().map(|s| *s)
.unwrap_or("<cause unknown>")
);
error!("A panic occurred at {}:{}: {}", filename, line, cause);
}));

let num_threads: usize = matches
.value_of("threads")
.unwrap()
Expand Down
17 changes: 9 additions & 8 deletions src/parser.rs
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ pub enum ParserError {
Io(io::Error),
Parse(String),
InvalidRecord(String),
InvalidData(String),
}

impl From<io::Error> for ParserError {
Expand All @@ -38,6 +39,7 @@ impl fmt::Display for ParserError {
ParserError::Io(ref err) => write!(f, "IO error: {}", err),
ParserError::Parse(ref s) => write!(f, "Record containing invalid integer: '{}'", s),
ParserError::InvalidRecord(ref s) => write!(f, "Invalid record: '{}'", s),
ParserError::InvalidData(ref s) => write!(f, "Invalid data: '{}'", s),
}
}
}
Expand Down Expand Up @@ -196,10 +198,11 @@ pub fn parse_lcov<T: Read>(
let mut f_splits = value.splitn(2, ',');
let executed = try_next!(f_splits, l) != "0";
let f_name = try_next!(f_splits, l);
let f = cur_functions
.get_mut(f_name)
.unwrap_or_else(|| panic!("FN record missing for function {}", f_name));
f.executed |= executed;
if let Some(f) = cur_functions.get_mut(f_name) {
f.executed |= executed;
} else {
return Err(ParserError::Parse(format!("FN record missing for function {}", f_name)));
}
}
"BRDA" => {
if branch_enabled {
Expand Down Expand Up @@ -228,6 +231,7 @@ pub fn parse_gcov(gcov_path: &Path) -> Result<Vec<(String, CovResult)>, ParserEr

let f = File::open(&gcov_path)
.unwrap_or_else(|_| panic!("Failed to open gcov file {}", gcov_path.display()));

let mut file = BufReader::new(&f);
let mut l = vec![];

Expand Down Expand Up @@ -504,10 +508,7 @@ fn parse_jacoco_report_package<T: Read>(

for (class, result) in &results_map {
if result.lines.is_empty() && result.branches.is_empty() {
panic!(
"Class {}/{} is not the top class in its file.",
package, class
);
return Err(ParserError::InvalidData(format!("Class {}/{} is not the top class in its file.", package, class)));
}
}

Expand Down

0 comments on commit 10a285d

Please sign in to comment.