From 342266853e08f23570ecac253819161cb6f0347f Mon Sep 17 00:00:00 2001 From: Casey Rodarmor Date: Wed, 29 Apr 2020 00:20:04 -0700 Subject: [PATCH] Improve bin/gen error messages Create an error enum with actual error messages. type: development --- Cargo.lock | 9 +--- bin/gen/Cargo.toml | 3 +- bin/gen/src/changelog.rs | 7 ++- bin/gen/src/command_ext.rs | 34 +++++++++++-- bin/gen/src/common.rs | 24 ++++++---- bin/gen/src/config.rs | 5 +- bin/gen/src/error.rs | 88 ++++++++++++++++++++++++++++++++++ bin/gen/src/exit_status_ext.rs | 14 ------ bin/gen/src/main.rs | 10 ++-- bin/gen/src/metadata.rs | 10 +++- bin/gen/src/opt.rs | 78 +++++++++++++----------------- bin/gen/src/project.rs | 23 ++++----- bin/gen/src/readme.rs | 2 +- bin/gen/src/subcommand.rs | 6 ++- bin/gen/src/template_ext.rs | 13 ++++- 15 files changed, 218 insertions(+), 108 deletions(-) create mode 100644 bin/gen/src/error.rs delete mode 100644 bin/gen/src/exit_status_ext.rs diff --git a/Cargo.lock b/Cargo.lock index 50ea487..b5723ae 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -27,12 +27,6 @@ dependencies = [ "winapi 0.3.8", ] -[[package]] -name = "anyhow" -version = "1.0.28" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "d9a60d744a80c30fcb657dfe2c1b22bcb3e814c1a1e3674f32bf5820b570fbff" - [[package]] name = "array-init" version = "0.0.4" @@ -364,18 +358,19 @@ checksum = "2fad85553e09a6f881f739c29f0b00b0f01357c743266d478b68951ce23285f3" name = "gen" version = "0.0.0" dependencies = [ - "anyhow", "askama", "cargo_toml", "chrono", "fehler", "git2", "globset", + "libc", "log", "pretty_env_logger", "regex", "serde", "serde_yaml", + "snafu", "structopt", "strum", "strum_macros", diff --git a/bin/gen/Cargo.toml b/bin/gen/Cargo.toml index 73ae8a9..b8f8647 100644 --- a/bin/gen/Cargo.toml +++ b/bin/gen/Cargo.toml @@ -6,17 +6,18 @@ edition = "2018" publish = false [dependencies] -anyhow = "1.0.28" askama = "0.9.0" cargo_toml = "0.8.0" chrono = "0.4.11" fehler = "1.0.0" git2 = "0.13.1" globset = "0.4.5" +libc = "0.2.69" log = "0.4.8" pretty_env_logger = "0.4.0" regex = "1.3.6" serde_yaml = "0.8.11" +snafu = "0.6.0" structopt = "0.3.12" strum = "0.18.0" strum_macros = "0.18.0" diff --git a/bin/gen/src/changelog.rs b/bin/gen/src/changelog.rs index b04b472..31ac17e 100644 --- a/bin/gen/src/changelog.rs +++ b/bin/gen/src/changelog.rs @@ -16,7 +16,7 @@ impl Changelog { loop { let summary_bytes = current .summary_bytes() - .ok_or_else(|| anyhow!("Commit had no summary"))?; + .ok_or_else(|| Error::CommitSummery { hash: current.id() })?; let summary = String::from_utf8_lossy(summary_bytes); @@ -47,7 +47,10 @@ impl Changelog { match current.parent_count() { 0 => break, 1 => current = current.parent(0)?, - _ => throw!(anyhow!("Commit had multiple parents!")), + other => throw!(Error::CommitParents { + hash: current.id(), + parents: other + }), } } diff --git a/bin/gen/src/command_ext.rs b/bin/gen/src/command_ext.rs index f7eda9b..0225a56 100644 --- a/bin/gen/src/command_ext.rs +++ b/bin/gen/src/command_ext.rs @@ -1,8 +1,12 @@ use crate::common::*; +#[allow(redundant_semicolons)] pub(crate) trait CommandExt { #[throws] fn out(&mut self) -> String; + + #[throws] + fn status_into_result(&mut self); } impl CommandExt for Command { @@ -13,12 +17,36 @@ impl CommandExt for Command { let output = self .stdout(Stdio::piped()) .stderr(Stdio::inherit()) - .output()?; + .output() + .context(error::CommandInvoke { + command: format!("{:?}", self), + })?; - output.status.into_result()?; + if !output.status.success() { + throw!(Error::CommandStatus { + command: format!("{:?}", self), + exit_status: output.status, + }); + } - let text = String::from_utf8(output.stdout)?; + let text = String::from_utf8(output.stdout).context(error::CommandDecode { + command: format!("{:?}", self), + })?; text } + + #[throws] + fn status_into_result(&mut self) { + let status = self.status().context(error::CommandInvoke { + command: format!("{:?}", self), + })?; + + if !status.success() { + throw!(Error::CommandStatus { + command: format!("{:?}", self), + exit_status: status + }); + } + } } diff --git a/bin/gen/src/common.rs b/bin/gen/src/common.rs index 5003c50..7d5a11e 100644 --- a/bin/gen/src/common.rs +++ b/bin/gen/src/common.rs @@ -4,36 +4,40 @@ pub(crate) use std::{ env, fmt::{self, Display, Formatter}, fs::{self, File}, + io, + ops::Deref, path::{Path, PathBuf}, - process::{Command, ExitStatus, Stdio}, + process::{self, Command, ExitStatus, Stdio}, str, }; -pub(crate) use anyhow::{anyhow, Error}; pub(crate) use askama::Template; pub(crate) use cargo_toml::Manifest; pub(crate) use chrono::{DateTime, NaiveDateTime, Utc}; pub(crate) use fehler::{throw, throws}; -pub(crate) use git2::{Commit, Repository}; +pub(crate) use git2::{Commit, Oid, Repository}; +pub(crate) use libc::EXIT_FAILURE; pub(crate) use log::info; pub(crate) use regex::Regex; pub(crate) use serde::{Deserialize, Serialize}; +pub(crate) use snafu::{ResultExt, Snafu}; +pub(crate) use std::string::FromUtf8Error; pub(crate) use structopt::StructOpt; pub(crate) use strum::VariantNames; pub(crate) use strum_macros::{EnumVariantNames, IntoStaticStr}; pub(crate) use url::Url; +// modules +pub(crate) use crate::error; + // traits -pub(crate) use crate::{ - command_ext::CommandExt, exit_status_ext::ExitStatusExt, row::Row, slug::Slug, - template_ext::TemplateExt, -}; +pub(crate) use crate::{command_ext::CommandExt, row::Row, slug::Slug, template_ext::TemplateExt}; // structs and enums pub(crate) use crate::{ - bin::Bin, changelog::Changelog, config::Config, entry::Entry, example::Example, faq::Faq, - faq_entry::FaqEntry, introduction::Introduction, kind::Kind, metadata::Metadata, opt::Opt, - package::Package, project::Project, readme::Readme, reference::Reference, + bin::Bin, changelog::Changelog, config::Config, entry::Entry, error::Error, example::Example, + faq::Faq, faq_entry::FaqEntry, introduction::Introduction, kind::Kind, metadata::Metadata, + opt::Opt, package::Package, project::Project, readme::Readme, reference::Reference, reference_section::ReferenceSection, release::Release, subcommand::Subcommand, summary::Summary, table::Table, }; diff --git a/bin/gen/src/config.rs b/bin/gen/src/config.rs index 734155c..0747f59 100644 --- a/bin/gen/src/config.rs +++ b/bin/gen/src/config.rs @@ -14,7 +14,8 @@ pub(crate) struct Config { impl Config { #[throws] pub(crate) fn load(root: &Path) -> Config { - let file = File::open(root.join(PATH))?; - serde_yaml::from_reader(file)? + let path = root.join(PATH); + let file = File::open(&path).context(error::Filesystem { path: &path })?; + serde_yaml::from_reader(file).context(error::ConfigDeserialize { path })? } } diff --git a/bin/gen/src/error.rs b/bin/gen/src/error.rs new file mode 100644 index 0000000..62dba36 --- /dev/null +++ b/bin/gen/src/error.rs @@ -0,0 +1,88 @@ +use crate::common::*; + +#[derive(Debug, Snafu)] +#[snafu(visibility(pub(crate)))] +pub(crate) enum Error { + #[snafu(display("Failed to deserialize `Cargo.toml`: {}", source))] + CargoToml { source: cargo_toml::Error }, + #[snafu(display("Failed to decode command `{}` output: {}", command, source))] + CommandDecode { + command: String, + source: FromUtf8Error, + }, + #[snafu(display("Failed to invoke command `{}` output: {}", command, source))] + CommandInvoke { command: String, source: io::Error }, + #[snafu(display("Command `{}` failed: {}", command, exit_status))] + CommandStatus { + command: String, + exit_status: ExitStatus, + }, + #[snafu(display( + "Failed to deserialize commit metadata: {}\n{}\n{}", + source, + hash, + message + ))] + CommitMetadataDeserialize { + hash: Oid, + message: String, + source: serde_yaml::Error, + }, + #[snafu(display("Commit missing metadata:\n{}\n{}", hash, message))] + CommitMetadataMissing { hash: Oid, message: String }, + #[snafu(display("Commit has `{}` parents: {}", hash, parents))] + CommitParents { hash: Oid, parents: usize }, + #[snafu(display("Commit has no summery: {}", hash))] + CommitSummery { hash: Oid }, + #[snafu(display("Failed to deserialize config from `{}`: {}", path.display(), source))] + ConfigDeserialize { + path: PathBuf, + source: serde_yaml::Error, + }, + #[snafu(display("Failed to get current dir: {}", source))] + CurrentDir { source: io::Error }, + #[snafu(display( + "Example commands `{}` don't match bin commands `{}`", + example.iter().map(|command| command.deref()).collect::>().join(","), + bin.iter().map(|command| command.deref()).collect::>().join(","), + ))] + ExampleCommands { + example: BTreeSet, + bin: BTreeSet, + }, + #[snafu(display("I/O error at `{}`: {}", path.display(), source))] + Filesystem { path: PathBuf, source: io::Error }, + #[snafu(display("Git error: {}", source))] + Git { source: git2::Error }, + #[snafu(display("Regex compilation error: {}", source))] + Regex { source: regex::Error }, + #[snafu(display("Failed to find repository from `{}`: {}", start_dir.display(), source))] + RepositoryDiscover { + start_dir: PathBuf, + source: git2::Error, + }, + #[snafu(display("Failed to create tempdir: {}", source))] + Tempdir { source: io::Error }, + #[snafu(display("Failed to render template: {}", source))] + TemplateRender { source: askama::Error }, + #[snafu(display("Failed to get workdir for repo at `{}`", repo.display()))] + Workdir { repo: PathBuf }, +} + +impl From for Error { + fn from(source: regex::Error) -> Self { + Self::Regex { source } + } +} + +impl From for Error { + fn from(source: git2::Error) -> Self { + Self::Git { source } + } +} + +impl From for Error { + fn from(source: cargo_toml::Error) -> Self { + Self::CargoToml { source } + } +} diff --git a/bin/gen/src/exit_status_ext.rs b/bin/gen/src/exit_status_ext.rs deleted file mode 100644 index 687ce5d..0000000 --- a/bin/gen/src/exit_status_ext.rs +++ /dev/null @@ -1,14 +0,0 @@ -use crate::common::*; - -pub(crate) trait ExitStatusExt { - fn into_result(self) -> anyhow::Result<()>; -} - -impl ExitStatusExt for ExitStatus { - #[throws] - fn into_result(self) { - if !self.success() { - throw!(anyhow!(self)); - } - } -} diff --git a/bin/gen/src/main.rs b/bin/gen/src/main.rs index c84d4e5..fc4eabd 100644 --- a/bin/gen/src/main.rs +++ b/bin/gen/src/main.rs @@ -9,8 +9,8 @@ mod command_ext; mod common; mod config; mod entry; +mod error; mod example; -mod exit_status_ext; mod faq; mod faq_entry; mod introduction; @@ -30,11 +30,11 @@ mod summary; mod table; mod template_ext; -#[throws] fn main() { pretty_env_logger::init(); - let project = Project::load()?; - - Opt::from_args().run(&project)?; + if let Err(error) = Opt::from_args().run() { + eprintln!("{}", error); + process::exit(EXIT_FAILURE); + } } diff --git a/bin/gen/src/metadata.rs b/bin/gen/src/metadata.rs index 31870df..47fbec5 100644 --- a/bin/gen/src/metadata.rs +++ b/bin/gen/src/metadata.rs @@ -19,11 +19,17 @@ impl Metadata { let blank = message .rfind(BLANK) - .ok_or_else(|| anyhow!("Commit message missing metadata: {}", message))?; + .ok_or_else(|| Error::CommitMetadataMissing { + hash: commit.id(), + message: message.to_string(), + })?; let yaml = &message[blank + BLANK.len()..]; - let metadata = serde_yaml::from_str(yaml)?; + let metadata = serde_yaml::from_str(yaml).context(error::CommitMetadataDeserialize { + hash: commit.id(), + message, + })?; metadata } diff --git a/bin/gen/src/opt.rs b/bin/gen/src/opt.rs index 9693d1e..76d7bd1 100644 --- a/bin/gen/src/opt.rs +++ b/bin/gen/src/opt.rs @@ -35,27 +35,29 @@ fn blank(path: impl AsRef, title: &str) { title ); - fs::write(path, text)?; + fs::write(&path, text).context(error::Filesystem { path })?; } #[throws] -fn clean_dir(dir: impl AsRef) { - let dir = dir.as_ref(); +fn clean_dir(path: impl AsRef) { + let path = path.as_ref(); - info!("Cleaning `{}`…", dir.display()); + info!("Cleaning `{}`…", path.display()); - if dir.is_dir() { - fs::remove_dir_all(dir)?; + if path.is_dir() { + fs::remove_dir_all(path).context(error::Filesystem { path: &path })?; } - fs::create_dir_all(dir)?; + fs::create_dir_all(path).context(error::Filesystem { path: &path })?; } impl Opt { #[throws] - pub(crate) fn run(self, project: &Project) { + pub(crate) fn run(self) { + let project = Project::load()?; + match self { - Self::Changelog => Self::changelog(project)?, + Self::Changelog => Self::changelog(&project)?, Self::CommitTemplate => { println!("{}", Metadata::default().to_string()); } @@ -64,16 +66,16 @@ impl Opt { println!("{}", kind) } } - Self::CompletionScripts => Self::completion_scripts(project)?, - Self::Readme => Self::readme(project)?, - Self::Book => Self::book(project)?, - Self::Man => Self::man(project)?, + Self::CompletionScripts => Self::completion_scripts(&project)?, + Self::Readme => Self::readme(&project)?, + Self::Book => Self::book(&project)?, + Self::Man => Self::man(&project)?, Self::All => { - Self::changelog(project)?; - Self::completion_scripts(project)?; - Self::readme(project)?; - Self::book(project)?; - Self::man(project)?; + Self::changelog(&project)?; + Self::completion_scripts(&project)?; + Self::readme(&project)?; + Self::book(&project)?; + Self::man(&project)?; } } } @@ -83,9 +85,9 @@ impl Opt { info!("Generating changelog…"); let changelog = Changelog::new(&project)?; - let dst = project.root.join("CHANGELOG.md"); + let path = project.root.join("CHANGELOG.md"); - fs::write(dst, changelog.to_string())?; + fs::write(&path, changelog.to_string()).context(error::Filesystem { path })?; } #[throws] @@ -104,20 +106,21 @@ impl Opt { "--dir", completions ) - .status()? - .into_result()?; + .status_into_result()? } #[throws] pub(crate) fn readme(project: &Project) { info!("Generating readme…"); + let template = project.root.join("bin/gen/templates/README.md"); let readme = Readme::load(&project.config, &template)?; let text = readme.render_newline()?; - fs::write(project.root.join("README.md"), text)?; + let path = project.root.join("README.md"); + fs::write(&path, text).context(error::Filesystem { path })?; } #[throws] @@ -137,35 +140,20 @@ impl Opt { let dst = commands.join(format!("{}.md", subcommand.slug())); - fs::write(dst, page)?; + fs::write(&dst, page).context(error::Filesystem { path: dst })?; } - let references = project.root.join("book/src/references/"); - clean_dir(&references)?; + clean_dir(&project.root.join("book/src/references/"))?; for section in &project.config.references { - let text = section.render_newline()?; - - let path = project.root.join("book/src").join(section.path()); - - fs::write(path, text)?; + section.render_to(project.root.join("book/src").join(section.path()))?; } - let faq = Faq::new(&project.config.faq); + Faq::new(&project.config.faq).render_to(project.root.join("book/src/faq.md"))?; - fs::write(project.root.join("book/src/faq.md"), faq.render_newline()?)?; + Summary::new(project).render_to(project.root.join("book/src/SUMMARY.md"))?; - let summary = Summary::new(project); - - let text = summary.render_newline()?; - - fs::write(project.root.join("book/src/SUMMARY.md"), text)?; - - let introduction = Introduction::new(&project.config); - - let text = introduction.render_newline()?; - - fs::write(project.root.join("book/src/introduction.md"), text)?; + Introduction::new(&project.config).render_to(project.root.join("book/src/introduction.md"))?; } #[throws] @@ -182,7 +170,7 @@ impl Opt { info!("Writing man page to `{}`", dst.display()); - fs::write(dst, man)?; + fs::write(&dst, man).context(error::Filesystem { path: dst })?; } } } diff --git a/bin/gen/src/project.rs b/bin/gen/src/project.rs index cd466c3..4f10f11 100644 --- a/bin/gen/src/project.rs +++ b/bin/gen/src/project.rs @@ -10,11 +10,15 @@ pub(crate) struct Project { impl Project { #[throws] pub(crate) fn load() -> Self { - let repo = Repository::discover(env::current_dir()?)?; + let start_dir = env::current_dir().context(error::CurrentDir)?; + + let repo = Repository::discover(&start_dir).context(error::RepositoryDiscover { start_dir })?; let root = repo .workdir() - .ok_or_else(|| anyhow!("Repository at `{}` had no workdir", repo.path().display()))? + .ok_or_else(|| Error::Workdir { + repo: repo.path().to_owned(), + })? .to_owned(); let config = Config::load(&root)?; @@ -34,17 +38,10 @@ impl Project { .collect::>(); if example_commands != bin_commands { - println!("Example commands:"); - for command in example_commands { - println!("{}", command); - } - - println!("…don't match bin commands:"); - for command in bin_commands { - println!("{}", command); - } - - throw!(anyhow!("")); + throw!(Error::ExampleCommands { + example: example_commands, + bin: bin_commands + }); } Project { diff --git a/bin/gen/src/readme.rs b/bin/gen/src/readme.rs index c4ecd91..a095640 100644 --- a/bin/gen/src/readme.rs +++ b/bin/gen/src/readme.rs @@ -12,7 +12,7 @@ const HEADING_PATTERN: &str = "(?m)^(?P#+) (?P.*)$"; impl Readme { #[throws] pub(crate) fn load(config: &Config, template: &Path) -> Readme { - let text = fs::read_to_string(template)?; + let text = fs::read_to_string(template).context(error::Filesystem { path: template })?; let header_re = Regex::new(HEADING_PATTERN)?; diff --git a/bin/gen/src/subcommand.rs b/bin/gen/src/subcommand.rs index 0cb5ce7..66b2b84 100644 --- a/bin/gen/src/subcommand.rs +++ b/bin/gen/src/subcommand.rs @@ -72,11 +72,13 @@ impl Subcommand { name, description ); - let tmp = tempfile::tempdir()?; + let tmp = tempfile::tempdir().context(error::Tempdir)?; let include_path = tmp.path().join("include"); - fs::write(&include_path, include)?; + fs::write(&include_path, include).context(error::Filesystem { + path: &include_path, + })?; let version = cmd!(&self.bin, "--version") .out()? diff --git a/bin/gen/src/template_ext.rs b/bin/gen/src/template_ext.rs index 8438a06..a56147b 100644 --- a/bin/gen/src/template_ext.rs +++ b/bin/gen/src/template_ext.rs @@ -3,12 +3,23 @@ use crate::common::*; pub(crate) trait TemplateExt { #[throws] fn render_newline(&self) -> String; + + #[throws] + fn render_to(&self, path: impl AsRef) { + let path = path.as_ref(); + let text = self.render_newline()?; + fs::write(&path, text).context(error::Filesystem { path })?; + } } impl TemplateExt for T { #[throws] fn render_newline(&self) -> String { - let mut text = self.render()?.trim().to_owned(); + let mut text = self + .render() + .context(error::TemplateRender)? + .trim() + .to_owned(); text.push('\n'); text }